From 15b1b512b5c793c8789d287974818630e1751486 Mon Sep 17 00:00:00 2001 From: "Anoop Babu (anoobabu)" Date: Mon, 5 Dec 2022 20:56:33 -0500 Subject: [PATCH 1/8] Initial changes --- .../Resources/DockerResourceDetector.cs | 60 +++++++++++++++++-- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs index fb6d25f452..3debc14008 100644 --- a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs +++ b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs @@ -28,6 +28,8 @@ namespace OpenTelemetry.Extensions.Docker.Resources; public class DockerResourceDetector : IResourceDetector { private const string FILEPATH = "/proc/self/cgroup"; + private const string FILEPATHV2 = "/proc/self/mountinfo"; + private const string HOSTNAME = "hostname"; /// /// Detects the resource attributes from Docker. @@ -35,17 +37,16 @@ public class DockerResourceDetector : IResourceDetector /// Resource with key-value pairs of resource attributes. public Resource Detect() { - return this.BuildResource(FILEPATH); + return this.BuildResource(); } /// /// Builds the resource attributes from Container Id in file path. /// - /// File path where container id exists. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - internal Resource BuildResource(string path) + internal Resource BuildResource() { - var containerId = this.ExtractContainerId(path); + var containerId = this.ExtractContainerId(); if (string.IsNullOrEmpty(containerId)) { @@ -60,9 +61,27 @@ internal Resource BuildResource(string path) /// /// Extracts Container Id from path. /// + /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. + private string ExtractContainerId() + { + try + { + return this.ExtractContainerIdV1(FILEPATH) ?? this.ExtractContainerIdV2(FILEPATHV2); + } + catch (Exception ex) + { + DockerExtensionsEventSource.Log.ExtractResourceAttributesException($"{nameof(DockerResourceDetector)} : Failed to extract Container id from path", ex); + } + + return null; + } + + /// + /// Extracts Container Id from path using the cgroupv1 format. + /// /// cgroup path. /// Container Id, Null if not found or exception being thrown. - private string ExtractContainerId(string path) + private string ExtractContainerIdV1(string path) { try { @@ -88,6 +107,37 @@ private string ExtractContainerId(string path) return null; } + /// + /// Extracts Container Id from path using the cgroupv2 format. + /// + /// File path where container id exists. + /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. + private string ExtractContainerIdV2(string path) + { + try + { + if (!File.Exists(path)) + { + return null; + } + + foreach (string line in File.ReadLines(path)) + { + string containerId = line.Contains(HOSTNAME) ? this.GetIdFromLine(line) : null; + if (!string.IsNullOrEmpty(containerId)) + { + return containerId; + } + } + } + catch (Exception ex) + { + DockerExtensionsEventSource.Log.ExtractResourceAttributesException($"{nameof(DockerResourceDetector)} : Failed to extract Container id from path", ex); + } + + return null; + } + /// /// Gets the Container Id from the line after removing the prefix and suffix. /// From 4cb146d74b78666c633c7165d52351eb6fa6c19c Mon Sep 17 00:00:00 2001 From: "Anoop Babu (anoobabu)" Date: Wed, 14 Dec 2022 23:12:10 -0500 Subject: [PATCH 2/8] Adding cgroupv2 to test and updating the approach such that it checks for the value before hostname --- .../Resources/DockerResourceDetector.cs | 35 +++++++++++++++---- .../Resources/DockerResourceDetectorTests.cs | 14 ++++++++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs index 3debc14008..2689169ef6 100644 --- a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs +++ b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs @@ -37,16 +37,17 @@ public class DockerResourceDetector : IResourceDetector /// Resource with key-value pairs of resource attributes. public Resource Detect() { - return this.BuildResource(); + return this.BuildResource(FILEPATH); } /// /// Builds the resource attributes from Container Id in file path. /// + /// cgroup path. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - internal Resource BuildResource() + internal Resource BuildResource(string path) { - var containerId = this.ExtractContainerId(); + var containerId = this.ExtractContainerId(path); if (string.IsNullOrEmpty(containerId)) { @@ -61,12 +62,13 @@ internal Resource BuildResource() /// /// Extracts Container Id from path. /// + /// /// cgroup path. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - private string ExtractContainerId() + private string ExtractContainerId(string path) { try { - return this.ExtractContainerIdV1(FILEPATH) ?? this.ExtractContainerIdV2(FILEPATHV2); + return this.ExtractContainerIdV1(path) ?? this.ExtractContainerIdV2(path); } catch (Exception ex) { @@ -123,7 +125,7 @@ private string ExtractContainerIdV2(string path) foreach (string line in File.ReadLines(path)) { - string containerId = line.Contains(HOSTNAME) ? this.GetIdFromLine(line) : null; + string containerId = (!string.IsNullOrEmpty(line) && line.Contains(HOSTNAME)) ? this.GetIdFromLineV2(line) : null; if (!string.IsNullOrEmpty(containerId)) { return containerId; @@ -166,6 +168,27 @@ private string GetIdFromLine(string line) return containerId; } + private string GetIdFromLineV2(string line) + { + int hostnameIndex = line.LastIndexOf("/" + HOSTNAME); + if (hostnameIndex < 0) + { + return null; + } + + string earlierSection = line.Substring(0, hostnameIndex); + int startIndex = earlierSection.LastIndexOf('/'); + + string containerId = this.RemovePrefixAndSuffixIfneeded(earlierSection, startIndex, hostnameIndex); + + if (string.IsNullOrEmpty(containerId) || !EncodingUtils.IsValidHexString(containerId)) + { + return null; + } + + return containerId; + } + private string RemovePrefixAndSuffixIfneeded(string input, int startIndex, int endIndex) { startIndex = (startIndex == -1) ? 0 : startIndex + 1; diff --git a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs index 004410766e..5832bffe95 100644 --- a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs +++ b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs @@ -57,6 +57,14 @@ public class DockerResourceDetectorTests private const string CONTAINERID = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"; + // cgroupv2 line with container Id + private const string CGROUPLINEV2 = + "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356/hostname"; + + // Expected Container Id + private const string CONTAINERIDV2 = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"; + + [Fact] public void TestValidContainer() { @@ -85,6 +93,12 @@ public void TestValidContainer() tempFile.Write(CGROUPLINE); Assert.Equal(CONTAINERID, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); } + + using (TempFile tempFile = new TempFile()) + { + tempFile.Write(CGROUPLINEV2); + Assert.Equal(CONTAINERIDV2, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); + } } [Fact] From 94be6e9e7761652d9d57d6e4e08310c3ac59e9af Mon Sep 17 00:00:00 2001 From: "Anoop Babu (anoobabu)" Date: Mon, 19 Dec 2022 14:47:30 -0500 Subject: [PATCH 3/8] Respoding to Comments and build checks --- .../Resources/DockerResourceDetector.cs | 4 ++-- .../Resources/DockerResourceDetectorTests.cs | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs index 2689169ef6..5db389c4aa 100644 --- a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs +++ b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs @@ -43,7 +43,7 @@ public Resource Detect() /// /// Builds the resource attributes from Container Id in file path. /// - /// cgroup path. + /// File path where container id exists. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. internal Resource BuildResource(string path) { @@ -68,7 +68,7 @@ private string ExtractContainerId(string path) { try { - return this.ExtractContainerIdV1(path) ?? this.ExtractContainerIdV2(path); + return this.ExtractContainerIdV2(path) ?? this.ExtractContainerIdV1(path); } catch (Exception ex) { diff --git a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs index 5832bffe95..f32c0c76bb 100644 --- a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs +++ b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs @@ -64,7 +64,6 @@ public class DockerResourceDetectorTests // Expected Container Id private const string CONTAINERIDV2 = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"; - [Fact] public void TestValidContainer() { From fd0c8e4b8f4212a38d9b7976add771af6c7687ec Mon Sep 17 00:00:00 2001 From: "Anoop Babu (anoobabu)" Date: Thu, 22 Dec 2022 17:57:26 -0500 Subject: [PATCH 4/8] Adjusted build such that Detect checks both versions, and BuildResource assumes the version with default parameter --- .../Resources/DockerResourceDetector.cs | 25 ++++++++++++++----- .../Resources/DockerResourceDetectorTests.cs | 2 +- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs index 5db389c4aa..cf27d49354 100644 --- a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs +++ b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs @@ -37,17 +37,24 @@ public class DockerResourceDetector : IResourceDetector /// Resource with key-value pairs of resource attributes. public Resource Detect() { - return this.BuildResource(FILEPATH); + var cGroupBuild = this.BuildResource(FILEPATH, true); + if (cGroupBuild == Resource.Empty) + { + cGroupBuild = this.BuildResource(FILEPATHV2, false); + } + + return cGroupBuild; } /// /// Builds the resource attributes from Container Id in file path. /// /// File path where container id exists. + /// Default parameter signifying where cgroup is of version 1 or 2. Defaults to version 1. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - internal Resource BuildResource(string path) + internal Resource BuildResource(string path, bool isCGroupV1 = true) { - var containerId = this.ExtractContainerId(path); + var containerId = this.ExtractContainerId(path, isCGroupV1); if (string.IsNullOrEmpty(containerId)) { @@ -62,13 +69,19 @@ internal Resource BuildResource(string path) /// /// Extracts Container Id from path. /// - /// /// cgroup path. + /// cgroup path. + /// Default parameter signifying where cgroup is of version 1 or 2. Defaults to version 1. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - private string ExtractContainerId(string path) + private string ExtractContainerId(string path, bool isCGroupV1) { try { - return this.ExtractContainerIdV2(path) ?? this.ExtractContainerIdV1(path); + if (isCGroupV1) + { + return this.ExtractContainerIdV1(path); + } + + return this.ExtractContainerIdV2(path); } catch (Exception ex) { diff --git a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs index f32c0c76bb..4bba1dbd11 100644 --- a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs +++ b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs @@ -96,7 +96,7 @@ public void TestValidContainer() using (TempFile tempFile = new TempFile()) { tempFile.Write(CGROUPLINEV2); - Assert.Equal(CONTAINERIDV2, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); + Assert.Equal(CONTAINERIDV2, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath, false))); } } From 14ce9b1c13b258e87c10795e111d77d82fa0aecc Mon Sep 17 00:00:00 2001 From: "Anoop Babu (anoobabu)" Date: Thu, 22 Dec 2022 20:01:30 -0500 Subject: [PATCH 5/8] Adjusting BuildResource at suggestion --- .../Resources/DockerResourceDetector.cs | 35 ++++++++----------- .../Resources/DockerResourceDetectorTests.cs | 14 ++++---- 2 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs index cf27d49354..dc63d4522d 100644 --- a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs +++ b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs @@ -37,24 +37,23 @@ public class DockerResourceDetector : IResourceDetector /// Resource with key-value pairs of resource attributes. public Resource Detect() { - var cGroupBuild = this.BuildResource(FILEPATH, true); + var cGroupBuild = this.BuildResourceV1(FILEPATH); if (cGroupBuild == Resource.Empty) { - cGroupBuild = this.BuildResource(FILEPATHV2, false); + cGroupBuild = this.BuildResourceV2(FILEPATHV2); } return cGroupBuild; } /// - /// Builds the resource attributes from Container Id in file path. + /// Builds the resource attributes from Container Id in file path of cgroupv1. /// /// File path where container id exists. - /// Default parameter signifying where cgroup is of version 1 or 2. Defaults to version 1. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - internal Resource BuildResource(string path, bool isCGroupV1 = true) + internal Resource BuildResourceV1(string path) { - var containerId = this.ExtractContainerId(path, isCGroupV1); + var containerId = this.ExtractContainerIdV1(path); if (string.IsNullOrEmpty(containerId)) { @@ -67,28 +66,22 @@ internal Resource BuildResource(string path, bool isCGroupV1 = true) } /// - /// Extracts Container Id from path. + /// Builds the resource attributes from Container Id in file path of cgroupv2. /// - /// cgroup path. - /// Default parameter signifying where cgroup is of version 1 or 2. Defaults to version 1. + /// File path where container id exists. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - private string ExtractContainerId(string path, bool isCGroupV1) + internal Resource BuildResourceV2(string path) { - try - { - if (isCGroupV1) - { - return this.ExtractContainerIdV1(path); - } + var containerId = this.ExtractContainerIdV2(path); - return this.ExtractContainerIdV2(path); + if (string.IsNullOrEmpty(containerId)) + { + return Resource.Empty; } - catch (Exception ex) + else { - DockerExtensionsEventSource.Log.ExtractResourceAttributesException($"{nameof(DockerResourceDetector)} : Failed to extract Container id from path", ex); + return new Resource(new List>() { new KeyValuePair(DockerSemanticConventions.AttributeContainerID, containerId), }); } - - return null; } /// diff --git a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs index 4bba1dbd11..eaaf6384a2 100644 --- a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs +++ b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs @@ -72,31 +72,31 @@ public void TestValidContainer() using (TempFile tempFile = new TempFile()) { tempFile.Write(CGROUPLINEWITHPREFIX); - Assert.Equal(CONTAINERIDWITHPREFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); + Assert.Equal(CONTAINERIDWITHPREFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); } using (TempFile tempFile = new TempFile()) { tempFile.Write(CGROUPLINEWITHSUFFIX); - Assert.Equal(CONTAINERIDWITHSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); + Assert.Equal(CONTAINERIDWITHSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); } using (TempFile tempFile = new TempFile()) { tempFile.Write(CGROUPLINEWITHPREFIXandSUFFIX); - Assert.Equal(CONTAINERIDWITHPREFIXANDSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); + Assert.Equal(CONTAINERIDWITHPREFIXANDSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); } using (TempFile tempFile = new TempFile()) { tempFile.Write(CGROUPLINE); - Assert.Equal(CONTAINERID, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); + Assert.Equal(CONTAINERID, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); } using (TempFile tempFile = new TempFile()) { tempFile.Write(CGROUPLINEV2); - Assert.Equal(CONTAINERIDV2, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath, false))); + Assert.Equal(CONTAINERIDV2, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); } } @@ -109,11 +109,11 @@ public void TestInvalidContainer() using (TempFile tempFile = new TempFile()) { tempFile.Write(INVALIDCGROUPLINE); - Assert.Equal(dockerResourceDetector.BuildResource(tempFile.FilePath), Resource.Empty); + Assert.Equal(dockerResourceDetector.BuildResourceV1(tempFile.FilePath), Resource.Empty); } // test invalid file - Assert.Equal(dockerResourceDetector.BuildResource(Path.GetTempPath()), Resource.Empty); + Assert.Equal(dockerResourceDetector.BuildResourceV1(Path.GetTempPath()), Resource.Empty); } private string GetContainerId(Resource resource) From a3647b36fef5b79c7a6b81a16a3d8cb1a4381f40 Mon Sep 17 00:00:00 2001 From: Anoop Babu Date: Wed, 4 Jan 2023 16:56:49 -0500 Subject: [PATCH 6/8] Added same tests as the go version as well as their regex expression --- .../Resources/DockerResourceDetector.cs | 13 +-- .../Resources/DockerResourceDetectorTests.cs | 92 ++++++++++++++++++- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs index dc63d4522d..e57fb071be 100644 --- a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs +++ b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs @@ -17,6 +17,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Text.RegularExpressions; using OpenTelemetry.Extensions.Docker.Utils; using OpenTelemetry.Resources; @@ -176,17 +177,13 @@ private string GetIdFromLine(string line) private string GetIdFromLineV2(string line) { - int hostnameIndex = line.LastIndexOf("/" + HOSTNAME); - if (hostnameIndex < 0) + string containerId = null; + var match = Regex.Match(line, @".*/.+/([\w+-.]{64})/.*$"); + if (match.Success) { - return null; + containerId = match.Groups[1].Value; } - string earlierSection = line.Substring(0, hostnameIndex); - int startIndex = earlierSection.LastIndexOf('/'); - - string containerId = this.RemovePrefixAndSuffixIfneeded(earlierSection, startIndex, hostnameIndex); - if (string.IsNullOrEmpty(containerId) || !EncodingUtils.IsValidHexString(containerId)) { return null; diff --git a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs index eaaf6384a2..c54e72c66e 100644 --- a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs +++ b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs @@ -61,11 +61,45 @@ public class DockerResourceDetectorTests private const string CGROUPLINEV2 = "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356/hostname"; - // Expected Container Id + // cgroupv2 Expected Container Id private const string CONTAINERIDV2 = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"; + // hostname + private const string CGROUPLINEV2WITHHOSTNAME = + "473 456 254:1 /docker/containers/dc64b5743252dbaef6e30521c34d6bbd1620c8ce65bdb7bf9e7143b61bb5b183/hostname /etc/hostname rw,relatime - ext4 /dev/vda1 rw"; + + private const string CONTAINERIDV2WITHHOSTNAME = "dc64b5743252dbaef6e30521c34d6bbd1620c8ce65bdb7bf9e7143b61bb5b183"; + + // minikube containerd mountinfo + private const string CGROUPLINEV2WITHMINIKUBECONTAINERD = + "1537 1517 8:1 /var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3ff9320f4402ae6/hostname /etc/hostname rw,relatime - ext4 /dev/sda1 rw"; + + private const string CONTAINERIDV2WITHMINIKUBECONTAINERD = "fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3ff9320f4402ae6"; + + // minikube docker mountinfo + private const string CGROUPLINEV2WITHDOCKER = + "2327 2307 8:1 /var/lib/docker/containers/a1551a1d7e1881d6c18d2c9ec462cab6ad3666825f0adb2098e9d5b198fd7e19/hostname /etc/hostname rw,relatime - ext4 /dev/sda1 rw"; + + private const string CONTAINERIDV2WITHDOCKER = "a1551a1d7e1881d6c18d2c9ec462cab6ad3666825f0adb2098e9d5b198fd7e19"; + + // minikube docker mountinfo2 + private const string CGROUPLINEV2WITHDOCKER2 = + "929 920 254:1 /docker/volumes/minikube/_data/lib/docker/containers/0eaa6718003210b6520f7e82d14b4c8d4743057a958a503626240f8d1900bc33/hostname /etc/hostname rw,relatime - ext4 /dev/vda1 rw"; + + private const string CONTAINERIDV2WITHDOCKER2 = "0eaa6718003210b6520f7e82d14b4c8d4743057a958a503626240f8d1900bc33"; + + // podman mountinfo + private const string CGROUPLINEV2WITHPODMAN = + "1096 1088 0:104 /containers/overlay-containers/1a2de27e7157106568f7e081e42a8c14858c02bd9df30d6e352b298178b46809/userdata/hostname /etc/hostname rw,nosuid,nodev,relatime - tmpfs tmpfs rw,size=813800k,nr_inodes=203450,mode=700,uid=1000,gid=1000"; + + private const string CONTAINERIDV2WITHPODMAN = "1a2de27e7157106568f7e081e42a8c14858c02bd9df30d6e352b298178b46809"; + + // Invalid cgroup line (contains a z)" + private const string INVALIDHEXCGROUPLINEV2 = + "13:name=systemd:/var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3fz9320f4402ae6/hostname"; + [Fact] - public void TestValidContainer() + public void TestValidContainerV1() { var dockerResourceDetector = new DockerResourceDetector(); @@ -92,16 +126,52 @@ public void TestValidContainer() tempFile.Write(CGROUPLINE); Assert.Equal(CONTAINERID, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); } + } + + [Fact] + public void TestValidContainerV2() + { + var dockerResourceDetector = new DockerResourceDetector(); using (TempFile tempFile = new TempFile()) { tempFile.Write(CGROUPLINEV2); Assert.Equal(CONTAINERIDV2, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); } + + using (TempFile tempFile = new TempFile()) + { + tempFile.Write(CGROUPLINEV2WITHHOSTNAME); + Assert.Equal(CONTAINERIDV2WITHHOSTNAME, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); + } + + using (TempFile tempFile = new TempFile()) + { + tempFile.Write(CGROUPLINEV2WITHMINIKUBECONTAINERD); + Assert.Equal(CONTAINERIDV2WITHMINIKUBECONTAINERD, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); + } + + using (TempFile tempFile = new TempFile()) + { + tempFile.Write(CGROUPLINEV2WITHDOCKER); + Assert.Equal(CONTAINERIDV2WITHDOCKER, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); + } + + using (TempFile tempFile = new TempFile()) + { + tempFile.Write(CGROUPLINEV2WITHDOCKER2); + Assert.Equal(CONTAINERIDV2WITHDOCKER2, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); + } + + using (TempFile tempFile = new TempFile()) + { + tempFile.Write(CGROUPLINEV2WITHPODMAN); + Assert.Equal(CONTAINERIDV2WITHPODMAN, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); + } } [Fact] - public void TestInvalidContainer() + public void TestInvalidContainerV1() { var dockerResourceDetector = new DockerResourceDetector(); @@ -116,6 +186,22 @@ public void TestInvalidContainer() Assert.Equal(dockerResourceDetector.BuildResourceV1(Path.GetTempPath()), Resource.Empty); } + [Fact] + public void TestInvalidContainerV2() + { + var dockerResourceDetector = new DockerResourceDetector(); + + // test invalid containerId (non-hex) + using (TempFile tempFile = new TempFile()) + { + tempFile.Write(INVALIDHEXCGROUPLINEV2); + Assert.Equal(dockerResourceDetector.BuildResourceV1(tempFile.FilePath), Resource.Empty); + } + + // test invalid file + Assert.Equal(dockerResourceDetector.BuildResourceV1(Path.GetTempPath()), Resource.Empty); + } + private string GetContainerId(Resource resource) { var resourceAttributes = resource.Attributes.ToDictionary(x => x.Key, x => x.Value); From b5857f0f7f9b7305b78e4c2597bbc139830c9efe Mon Sep 17 00:00:00 2001 From: Anoop Babu Date: Thu, 5 Jan 2023 15:06:00 -0500 Subject: [PATCH 7/8] Addressing Comments by adding Enums and cleaning up tests --- .../Resources/DockerResourceDetector.cs | 99 +++---- .../Resources/DockerResourceDetectorTests.cs | 276 ++++++++---------- 2 files changed, 171 insertions(+), 204 deletions(-) diff --git a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs index e57fb071be..a589117d37 100644 --- a/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs +++ b/src/OpenTelemetry.Extensions.Docker/Resources/DockerResourceDetector.cs @@ -32,48 +32,46 @@ public class DockerResourceDetector : IResourceDetector private const string FILEPATHV2 = "/proc/self/mountinfo"; private const string HOSTNAME = "hostname"; + /// + /// CGroup Parse Versions. + /// + internal enum ParseMode + { + /// + /// Represents CGroupV1. + /// + V1, + + /// + /// Represents CGroupV2. + /// + V2, + } + /// /// Detects the resource attributes from Docker. /// /// Resource with key-value pairs of resource attributes. public Resource Detect() { - var cGroupBuild = this.BuildResourceV1(FILEPATH); + var cGroupBuild = this.BuildResource(FILEPATH, ParseMode.V1); if (cGroupBuild == Resource.Empty) { - cGroupBuild = this.BuildResourceV2(FILEPATHV2); + cGroupBuild = this.BuildResource(FILEPATHV2, ParseMode.V2); } return cGroupBuild; } /// - /// Builds the resource attributes from Container Id in file path of cgroupv1. + /// Builds the resource attributes from Container Id in file path. /// /// File path where container id exists. + /// CGroup Version of file to parse from. /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - internal Resource BuildResourceV1(string path) + internal Resource BuildResource(string path, ParseMode cgroupVersion) { - var containerId = this.ExtractContainerIdV1(path); - - if (string.IsNullOrEmpty(containerId)) - { - return Resource.Empty; - } - else - { - return new Resource(new List>() { new KeyValuePair(DockerSemanticConventions.AttributeContainerID, containerId), }); - } - } - - /// - /// Builds the resource attributes from Container Id in file path of cgroupv2. - /// - /// File path where container id exists. - /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - internal Resource BuildResourceV2(string path) - { - var containerId = this.ExtractContainerIdV2(path); + var containerId = this.ExtractContainerId(path, cgroupVersion); if (string.IsNullOrEmpty(containerId)) { @@ -89,8 +87,9 @@ internal Resource BuildResourceV2(string path) /// Extracts Container Id from path using the cgroupv1 format. /// /// cgroup path. + /// CGroup Version of file to parse from. /// Container Id, Null if not found or exception being thrown. - private string ExtractContainerIdV1(string path) + private string ExtractContainerId(string path, ParseMode cgroupVersion) { try { @@ -101,38 +100,19 @@ private string ExtractContainerIdV1(string path) foreach (string line in File.ReadLines(path)) { - string containerId = (!string.IsNullOrEmpty(line)) ? this.GetIdFromLine(line) : null; - if (!string.IsNullOrEmpty(containerId)) + string containerId = null; + if (!string.IsNullOrEmpty(line)) { - return containerId; + if (cgroupVersion == ParseMode.V1) + { + containerId = this.GetIdFromLineV1(line); + } + else if (cgroupVersion == ParseMode.V2 && line.Contains(HOSTNAME)) + { + containerId = this.GetIdFromLineV2(line); + } } - } - } - catch (Exception ex) - { - DockerExtensionsEventSource.Log.ExtractResourceAttributesException($"{nameof(DockerResourceDetector)} : Failed to extract Container id from path", ex); - } - - return null; - } - - /// - /// Extracts Container Id from path using the cgroupv2 format. - /// - /// File path where container id exists. - /// Returns Resource with list of key-value pairs of container resource attributes if container id exists else empty resource. - private string ExtractContainerIdV2(string path) - { - try - { - if (!File.Exists(path)) - { - return null; - } - foreach (string line in File.ReadLines(path)) - { - string containerId = (!string.IsNullOrEmpty(line) && line.Contains(HOSTNAME)) ? this.GetIdFromLineV2(line) : null; if (!string.IsNullOrEmpty(containerId)) { return containerId; @@ -148,11 +128,11 @@ private string ExtractContainerIdV2(string path) } /// - /// Gets the Container Id from the line after removing the prefix and suffix. + /// Gets the Container Id from the line after removing the prefix and suffix from the cgroupv1 format. /// /// line read from cgroup file. - /// Container Id. - private string GetIdFromLine(string line) + /// Container Id, Null if not found. + private string GetIdFromLineV1(string line) { // This cgroup output line should have the container id in it int lastSlashIndex = line.LastIndexOf('/'); @@ -175,6 +155,11 @@ private string GetIdFromLine(string line) return containerId; } + /// + /// Gets the Container Id from the line of the cgroupv2 format. + /// + /// line read from cgroup file. + /// Container Id, Null if not found. private string GetIdFromLineV2(string line) { string containerId = null; diff --git a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs index c54e72c66e..74b5b0f551 100644 --- a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs +++ b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs @@ -14,6 +14,7 @@ // limitations under the License. // +using System.Collections.Generic; using System.IO; using System.Linq; using OpenTelemetry.Extensions.Docker.Resources; @@ -24,182 +25,152 @@ namespace OpenTelemetry.Extensions.Docker.Tests; public class DockerResourceDetectorTests { - // Invalid cgroup line - private const string INVALIDCGROUPLINE = - "13:name=systemd:/podruntime/docker/kubepods/ac679f8a8319c8cf7d38e1adf263bc08d23zzzz"; - - // cgroup line with prefix - private const string CGROUPLINEWITHPREFIX = - "13:name=systemd:/podruntime/docker/kubepods/crio-e2cc29debdf85dde404998aa128997a819ff"; - - // Expected Container Id with prefix removed - private const string CONTAINERIDWITHPREFIXREMOVED = "e2cc29debdf85dde404998aa128997a819ff"; - - // cgroup line with suffix - private const string CGROUPLINEWITHSUFFIX = - "13:name=systemd:/podruntime/docker/kubepods/ac679f8a8319c8cf7d38e1adf263bc08d23.aaaa"; - - // Expected Container Id with suffix removed - private const string CONTAINERIDWITHSUFFIXREMOVED = "ac679f8a8319c8cf7d38e1adf263bc08d23"; - - // cgroup line with prefix and suffix - private const string CGROUPLINEWITHPREFIXandSUFFIX = - "13:name=systemd:/podruntime/docker/kubepods/crio-dc679f8a8319c8cf7d38e1adf263bc08d23.stuff"; - - // Expected Container Id with both prefix and suffix removed - private const string CONTAINERIDWITHPREFIXANDSUFFIXREMOVED = "dc679f8a8319c8cf7d38e1adf263bc08d23"; - - // cgroup line with container Id - private const string CGROUPLINE = - "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"; - - // Expected Container Id - private const string CONTAINERID = - "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"; - - // cgroupv2 line with container Id - private const string CGROUPLINEV2 = - "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356/hostname"; - - // cgroupv2 Expected Container Id - private const string CONTAINERIDV2 = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356"; - - // hostname - private const string CGROUPLINEV2WITHHOSTNAME = - "473 456 254:1 /docker/containers/dc64b5743252dbaef6e30521c34d6bbd1620c8ce65bdb7bf9e7143b61bb5b183/hostname /etc/hostname rw,relatime - ext4 /dev/vda1 rw"; - - private const string CONTAINERIDV2WITHHOSTNAME = "dc64b5743252dbaef6e30521c34d6bbd1620c8ce65bdb7bf9e7143b61bb5b183"; - - // minikube containerd mountinfo - private const string CGROUPLINEV2WITHMINIKUBECONTAINERD = - "1537 1517 8:1 /var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3ff9320f4402ae6/hostname /etc/hostname rw,relatime - ext4 /dev/sda1 rw"; - - private const string CONTAINERIDV2WITHMINIKUBECONTAINERD = "fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3ff9320f4402ae6"; - - // minikube docker mountinfo - private const string CGROUPLINEV2WITHDOCKER = - "2327 2307 8:1 /var/lib/docker/containers/a1551a1d7e1881d6c18d2c9ec462cab6ad3666825f0adb2098e9d5b198fd7e19/hostname /etc/hostname rw,relatime - ext4 /dev/sda1 rw"; - - private const string CONTAINERIDV2WITHDOCKER = "a1551a1d7e1881d6c18d2c9ec462cab6ad3666825f0adb2098e9d5b198fd7e19"; - - // minikube docker mountinfo2 - private const string CGROUPLINEV2WITHDOCKER2 = - "929 920 254:1 /docker/volumes/minikube/_data/lib/docker/containers/0eaa6718003210b6520f7e82d14b4c8d4743057a958a503626240f8d1900bc33/hostname /etc/hostname rw,relatime - ext4 /dev/vda1 rw"; - - private const string CONTAINERIDV2WITHDOCKER2 = "0eaa6718003210b6520f7e82d14b4c8d4743057a958a503626240f8d1900bc33"; - - // podman mountinfo - private const string CGROUPLINEV2WITHPODMAN = - "1096 1088 0:104 /containers/overlay-containers/1a2de27e7157106568f7e081e42a8c14858c02bd9df30d6e352b298178b46809/userdata/hostname /etc/hostname rw,nosuid,nodev,relatime - tmpfs tmpfs rw,size=813800k,nr_inodes=203450,mode=700,uid=1000,gid=1000"; - - private const string CONTAINERIDV2WITHPODMAN = "1a2de27e7157106568f7e081e42a8c14858c02bd9df30d6e352b298178b46809"; - - // Invalid cgroup line (contains a z)" - private const string INVALIDHEXCGROUPLINEV2 = - "13:name=systemd:/var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3fz9320f4402ae6/hostname"; - - [Fact] - public void TestValidContainerV1() + private readonly List testValidCasesV1 = new() { - var dockerResourceDetector = new DockerResourceDetector(); - - using (TempFile tempFile = new TempFile()) + new TestCase() { - tempFile.Write(CGROUPLINEWITHPREFIX); - Assert.Equal(CONTAINERIDWITHPREFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv1 with prefix", + Line = "13:name=systemd:/podruntime/docker/kubepods/crio-e2cc29debdf85dde404998aa128997a819ff", + ExpectedContainerId = "e2cc29debdf85dde404998aa128997a819ff", + CgroupVersion = DockerResourceDetector.ParseMode.V1, + }, + new TestCase() { - tempFile.Write(CGROUPLINEWITHSUFFIX); - Assert.Equal(CONTAINERIDWITHSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv1 with suffix", + Line = "13:name=systemd:/podruntime/docker/kubepods/ac679f8a8319c8cf7d38e1adf263bc08d23.aaaa", + ExpectedContainerId = "ac679f8a8319c8cf7d38e1adf263bc08d23", + CgroupVersion = DockerResourceDetector.ParseMode.V1, + }, + new TestCase() { - tempFile.Write(CGROUPLINEWITHPREFIXandSUFFIX); - Assert.Equal(CONTAINERIDWITHPREFIXANDSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv1 with prefix and suffix", + Line = "13:name=systemd:/podruntime/docker/kubepods/crio-dc679f8a8319c8cf7d38e1adf263bc08d23.stuff", + ExpectedContainerId = "dc679f8a8319c8cf7d38e1adf263bc08d23", + CgroupVersion = DockerResourceDetector.ParseMode.V1, + }, + new TestCase() { - tempFile.Write(CGROUPLINE); - Assert.Equal(CONTAINERID, this.GetContainerId(dockerResourceDetector.BuildResourceV1(tempFile.FilePath))); - } - } - - [Fact] - public void TestValidContainerV2() + Name = "cgroupv1 with container Id", + Line = "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356", + ExpectedContainerId = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356", + CgroupVersion = DockerResourceDetector.ParseMode.V1, + }, + }; + + private readonly List testValidCasesV2 = new() { - var dockerResourceDetector = new DockerResourceDetector(); - - using (TempFile tempFile = new TempFile()) + new TestCase() { - tempFile.Write(CGROUPLINEV2); - Assert.Equal(CONTAINERIDV2, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv2 with container Id", + Line = "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356/hostname", + ExpectedContainerId = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356", + CgroupVersion = DockerResourceDetector.ParseMode.V2, + }, + new TestCase() { - tempFile.Write(CGROUPLINEV2WITHHOSTNAME); - Assert.Equal(CONTAINERIDV2WITHHOSTNAME, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv2 with full line", + Line = "473 456 254:1 /docker/containers/dc64b5743252dbaef6e30521c34d6bbd1620c8ce65bdb7bf9e7143b61bb5b183/hostname /etc/hostname rw,relatime - ext4 /dev/vda1 rw", + ExpectedContainerId = "dc64b5743252dbaef6e30521c34d6bbd1620c8ce65bdb7bf9e7143b61bb5b183", + CgroupVersion = DockerResourceDetector.ParseMode.V2, + }, + new TestCase() { - tempFile.Write(CGROUPLINEV2WITHMINIKUBECONTAINERD); - Assert.Equal(CONTAINERIDV2WITHMINIKUBECONTAINERD, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv2 with minikube containerd mountinfo", + Line = "1537 1517 8:1 /var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3ff9320f4402ae6/hostname /etc/hostname rw,relatime - ext4 /dev/sda1 rw", + ExpectedContainerId = "fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3ff9320f4402ae6", + CgroupVersion = DockerResourceDetector.ParseMode.V2, + }, + new TestCase() { - tempFile.Write(CGROUPLINEV2WITHDOCKER); - Assert.Equal(CONTAINERIDV2WITHDOCKER, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv2 with minikube docker mountinfo", + Line = "2327 2307 8:1 /var/lib/docker/containers/a1551a1d7e1881d6c18d2c9ec462cab6ad3666825f0adb2098e9d5b198fd7e19/hostname /etc/hostname rw,relatime - ext4 /dev/sda1 rw", + ExpectedContainerId = "a1551a1d7e1881d6c18d2c9ec462cab6ad3666825f0adb2098e9d5b198fd7e19", + CgroupVersion = DockerResourceDetector.ParseMode.V2, + }, + new TestCase() { - tempFile.Write(CGROUPLINEV2WITHDOCKER2); - Assert.Equal(CONTAINERIDV2WITHDOCKER2, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); - } - - using (TempFile tempFile = new TempFile()) + Name = "cgroupv2 with minikube docker mountinfo2", + Line = "929 920 254:1 /docker/volumes/minikube/_data/lib/docker/containers/0eaa6718003210b6520f7e82d14b4c8d4743057a958a503626240f8d1900bc33/hostname /etc/hostname rw,relatime - ext4 /dev/vda1 rw", + ExpectedContainerId = "0eaa6718003210b6520f7e82d14b4c8d4743057a958a503626240f8d1900bc33", + CgroupVersion = DockerResourceDetector.ParseMode.V2, + }, + new TestCase() { - tempFile.Write(CGROUPLINEV2WITHPODMAN); - Assert.Equal(CONTAINERIDV2WITHPODMAN, this.GetContainerId(dockerResourceDetector.BuildResourceV2(tempFile.FilePath))); - } - } + Name = "cgroupv2 with podman mountinfo", + Line = "1096 1088 0:104 /containers/overlay-containers/1a2de27e7157106568f7e081e42a8c14858c02bd9df30d6e352b298178b46809/userdata/hostname /etc/hostname rw,nosuid,nodev,relatime - tmpfs tmpfs rw,size=813800k,nr_inodes=203450,mode=700,uid=1000,gid=1000", + ExpectedContainerId = "1a2de27e7157106568f7e081e42a8c14858c02bd9df30d6e352b298178b46809", + CgroupVersion = DockerResourceDetector.ParseMode.V2, + }, + }; + + private readonly List testInvalidCases = new() + { + new TestCase() + { + Name = "Invalid cgroupv1 line", + Line = "13:name=systemd:/podruntime/docker/kubepods/ac679f8a8319c8cf7d38e1adf263bc08d23zzzz", + CgroupVersion = DockerResourceDetector.ParseMode.V1, + }, + new TestCase() + { + Name = "Invalid hex cgroupv2 line (contains a z)", + Line = "13:name=systemd:/var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3fz9320f4402ae6/hostname", + CgroupVersion = DockerResourceDetector.ParseMode.V1, + }, + }; [Fact] - public void TestInvalidContainerV1() + public void TestValidContainer() { var dockerResourceDetector = new DockerResourceDetector(); + var allValidTestCases = this.testValidCasesV1.Concat(this.testValidCasesV2); - // test invalid containerId (non-hex) - using (TempFile tempFile = new TempFile()) + foreach (var testCase in allValidTestCases) { - tempFile.Write(INVALIDCGROUPLINE); - Assert.Equal(dockerResourceDetector.BuildResourceV1(tempFile.FilePath), Resource.Empty); + using TempFile tempFile = new TempFile(); + tempFile.Write(testCase.Line); + Assert.Equal( + testCase.ExpectedContainerId, + this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath, testCase.CgroupVersion))); } - - // test invalid file - Assert.Equal(dockerResourceDetector.BuildResourceV1(Path.GetTempPath()), Resource.Empty); } [Fact] - public void TestInvalidContainerV2() + public void TestInvalidContainer() { var dockerResourceDetector = new DockerResourceDetector(); - // test invalid containerId (non-hex) - using (TempFile tempFile = new TempFile()) + // Valid in cgroupv1 is not valid in cgroupv2 + foreach (var testCase in this.testValidCasesV1) + { + using TempFile tempFile = new TempFile(); + tempFile.Write(testCase.Line); + Assert.Equal( + dockerResourceDetector.BuildResource(tempFile.FilePath, DockerResourceDetector.ParseMode.V2), + Resource.Empty); + } + + // Valid in cgroupv1 is not valid in cgroupv1 + foreach (var testCase in this.testValidCasesV2) + { + using TempFile tempFile = new TempFile(); + tempFile.Write(testCase.Line); + Assert.Equal( + dockerResourceDetector.BuildResource(tempFile.FilePath, DockerResourceDetector.ParseMode.V1), + Resource.Empty); + } + + // test invalid cases + foreach (var testCase in this.testInvalidCases) { - tempFile.Write(INVALIDHEXCGROUPLINEV2); - Assert.Equal(dockerResourceDetector.BuildResourceV1(tempFile.FilePath), Resource.Empty); + using TempFile tempFile = new TempFile(); + tempFile.Write(testCase.Line); + Assert.Equal(dockerResourceDetector.BuildResource(tempFile.FilePath, testCase.CgroupVersion), Resource.Empty); } // test invalid file - Assert.Equal(dockerResourceDetector.BuildResourceV1(Path.GetTempPath()), Resource.Empty); + Assert.Equal(dockerResourceDetector.BuildResource(Path.GetTempPath(), DockerResourceDetector.ParseMode.V1), Resource.Empty); + Assert.Equal(dockerResourceDetector.BuildResource(Path.GetTempPath(), DockerResourceDetector.ParseMode.V2), Resource.Empty); } private string GetContainerId(Resource resource) @@ -207,4 +178,15 @@ private string GetContainerId(Resource resource) var resourceAttributes = resource.Attributes.ToDictionary(x => x.Key, x => x.Value); return resourceAttributes[DockerSemanticConventions.AttributeContainerID]?.ToString(); } + + private class TestCase + { + public string Name { get; set; } + + public string Line { get; set; } + + public string ExpectedContainerId { get; set; } + + public DockerResourceDetector.ParseMode CgroupVersion { get; set; } + } } From ba6b6ab619e9c9bcf960f22db4f46c14bece67e1 Mon Sep 17 00:00:00 2001 From: Anoop Babu Date: Fri, 6 Jan 2023 16:12:41 -0500 Subject: [PATCH 8/8] Fix parse mode version in test --- .../Resources/DockerResourceDetectorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs index 74b5b0f551..5c50045727 100644 --- a/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs +++ b/test/OpenTelemetry.Extensions.Docker.Tests/Resources/DockerResourceDetectorTests.cs @@ -115,7 +115,7 @@ public class DockerResourceDetectorTests { Name = "Invalid hex cgroupv2 line (contains a z)", Line = "13:name=systemd:/var/lib/containerd/io.containerd.grpc.v1.cri/sandboxes/fb5916a02feca96bdeecd8e062df9e5e51d6617c8214b5e1f3fz9320f4402ae6/hostname", - CgroupVersion = DockerResourceDetector.ParseMode.V1, + CgroupVersion = DockerResourceDetector.ParseMode.V2, }, };