Skip to content

Commit

Permalink
Ensure that parsed container ID is 64 chars. (#8206)
Browse files Browse the repository at this point in the history
Resolves #7437.

A few caveats about this. The TL;DR on #7437 is that a non-containerized
process was reporting a `container.id` attribute. The submitter narrowed
it down and I was able to confirm with the test case in this PR.

I hunted for other means for code to determine if it's containerized
with the idea to not even do the parsing if not containerized, but I
couldn't find anything useful. In fact, most approaches of detecting
containerization at all do involve parsing cgroups. Wacky.

So I attempted to verify that container IDs should always be 64
characters. I found:
* podman - docs
[here](https://docs.podman.io/en/latest/markdown/podman-container-inspect.1.html)
"Container ID (full 64-char hash)"
* docker - UID generator source
[here](https://github.com/moby/moby/blob/634a848b8e3bdd8aed834559f3b2e0dfc7f5ae3a/pkg/stringid/stringid.go#L36)
shows 32 bytes (and even guards against fully numeric!)
* lxc [man page
](https://linuxcontainers.org/lxc/manpages/man1/lxc-info.1.html)says
"container identifier format is an alphanumeric string". If this maps
into cgroups (no idea!), it would have already been broken in some cases
because we enforce hex.

I'm a little concerned about this approach because the [otel
spec](https://github.com/open-telemetry/opentelemetry-specification/blob/94c9c75c4f82fbda08bddff02ce9a0c582fdd55c/specification/resource/semantic_conventions/container.md)
suggests that "The UUID might be abbreviated.", but it's
unclear/non-specific about the circumstances that might cause this.

Open to hearing about why the approach presented here is a bad idea. 🙃
  • Loading branch information
breedx-splk committed Apr 4, 2023
1 parent 04f2e3e commit 32b6bd2
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private static Optional<String> getIdFromLine(String line) {
containerId = lastSection.substring(startIdx, endIdx);
}

if (OtelEncodingUtils.isValidBase16String(containerId) && !containerId.isEmpty()) {
if (OtelEncodingUtils.isValidBase16String(containerId) && (containerId.length() == 64)) {
return Optional.of(containerId);
} else {
return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,27 +67,39 @@ void validCgroupLines(String line, String expectedContainerId) throws IOExceptio
assertThat(result.orElse("fail")).isEqualTo(expectedContainerId);
}

// See https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/7437
@Test
void hostStyleCgroupFile() throws IOException {
String line = "1:name=systemd:/user.slice/user-0.slice/session-31207.scope";
when(filesystem.isReadable(V1_CGROUP_PATH)).thenReturn(true);
when(filesystem.lines(V1_CGROUP_PATH)).thenReturn(Stream.of(line));

CgroupV1ContainerIdExtractor extractor = new CgroupV1ContainerIdExtractor(filesystem);
Optional<String> result = extractor.extractContainerId();
assertThat(result).isEmpty();
}

static Stream<Arguments> validLines() {
return Stream.of(
// with suffix
arguments(
"13:name=systemd:/podruntime/docker/kubepods/ac679f8a8319c8cf7d38e1adf263bc08d23.aaaa",
"ac679f8a8319c8cf7d38e1adf263bc08d23"),
"13:name=systemd:/podruntime/docker/kubepods/ac679f8a8319c8cf7d38e1adf263bc08d231f2ff81abda3915f6e8ba4d64156a.aaaa",
"ac679f8a8319c8cf7d38e1adf263bc08d231f2ff81abda3915f6e8ba4d64156a"),
// with prefix and suffix
arguments(
"13:name=systemd:/podruntime/docker/kubepods/crio-dc679f8a8319c8cf7d38e1adf263bc08d23.stuff",
"dc679f8a8319c8cf7d38e1adf263bc08d23"),
"13:name=systemd:/podruntime/docker/kubepods/crio-dc679f8a8319c8cf7d38e1adf263bc08d234f0749ea715fb6ca3bb259db69956.stuff",
"dc679f8a8319c8cf7d38e1adf263bc08d234f0749ea715fb6ca3bb259db69956"),
// just container id
arguments(
"13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356",
"d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"),
// with prefix
arguments(
"//\n"
+ "1:name=systemd:/podruntime/docker/kubepods/docker-dc579f8a8319c8cf7d38e1adf263bc08d23"
+ "2:name=systemd:/podruntime/docker/kubepods/docker-dc579f8a8319c8cf7d38e1adf263bc08d23"
+ "3:name=systemd:/podruntime/docker/kubepods/docker-dc579f8a8319c8cf7d38e1adf263bc08d23",
"dc579f8a8319c8cf7d38e1adf263bc08d23"),
+ "1:name=systemd:/podruntime/docker/kubepods/docker-dc579f8a8319c8cf7d38e1adf263bc08d230600179b07acfd7eaf9646778dc31"
+ "2:name=systemd:/podruntime/docker/kubepods/docker-dc579f8a8319c8cf7d38e1adf263bc08d230600179b07acfd7eaf9646778dc31"
+ "3:name=systemd:/podruntime/docker/kubepods/docker-dc579f8a8319c8cf7d38e1adf263bc08d230600179b07acfd7eaf9646778dc31",
"dc579f8a8319c8cf7d38e1adf263bc08d230600179b07acfd7eaf9646778dc31"),
// with two dashes in prefix
arguments(
"11:perf_event:/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod4415fd05_2c0f_4533_909b_f2180dca8d7c.slice/cri-containerd-713a77a26fe2a38ebebd5709604a048c3d380db1eb16aa43aca0b2499e54733c.scope",
Expand Down

0 comments on commit 32b6bd2

Please sign in to comment.