-
Notifications
You must be signed in to change notification settings - Fork 301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added CGroupv2 support into Docker Extensions #839
Merged
Merged
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
15b1b51
Initial changes
anoopbabu29 4cb146d
Adding cgroupv2 to test and updating the approach such that it checks…
anoopbabu29 cb8f573
Merge branch 'main' of https://github.com/anoopbabu29/opentelemetry-d…
anoopbabu29 7ff1941
Merge branch 'open-telemetry:main' into cgroupv2-sup
anoopbabu29 aad5630
Merge branch 'cgroupv2-sup' of https://github.com/anoopbabu29/opentel…
anoopbabu29 94be6e9
Respoding to Comments and build checks
anoopbabu29 856fed7
Merge branch 'main' into cgroupv2-sup
anoopbabu29 fd0c8e4
Adjusted build such that Detect checks both versions, and BuildResour…
anoopbabu29 5ba7994
Merge branch 'cgroupv2-sup' of https://github.com/anoopbabu29/opentel…
anoopbabu29 78a5a62
Merge branch 'main' into cgroupv2-sup
anoopbabu29 14ce9b1
Adjusting BuildResource at suggestion
anoopbabu29 492552d
Merge branch 'cgroupv2-sup' of https://github.com/anoopbabu29/opentel…
anoopbabu29 a3647b3
Added same tests as the go version as well as their regex expression
anoopbabu29 9e05aba
Merge branch 'main' into cgroupv2-sup
anoopbabu29 b5857f0
Addressing Comments by adding Enums and cleaning up tests
anoopbabu29 7b7e580
Merge branch 'main' into cgroupv2-sup
anoopbabu29 ba6b6ab
Fix parse mode version in test
anoopbabu29 d043c33
Merge branch 'main' into cgroupv2-sup
utpilla File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
// limitations under the License. | ||
// </copyright> | ||
|
||
using System.Collections.Generic; | ||
using System.IO; | ||
using System.Linq; | ||
using OpenTelemetry.Extensions.Docker.Resources; | ||
|
@@ -24,88 +25,168 @@ 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"; | ||
private readonly List<TestCase> testValidCasesV1 = new() | ||
{ | ||
new TestCase() | ||
{ | ||
Name = "cgroupv1 with prefix", | ||
Line = "13:name=systemd:/podruntime/docker/kubepods/crio-e2cc29debdf85dde404998aa128997a819ff", | ||
ExpectedContainerId = "e2cc29debdf85dde404998aa128997a819ff", | ||
CgroupVersion = DockerResourceDetector.ParseMode.V1, | ||
}, | ||
new TestCase() | ||
{ | ||
Name = "cgroupv1 with suffix", | ||
Line = "13:name=systemd:/podruntime/docker/kubepods/ac679f8a8319c8cf7d38e1adf263bc08d23.aaaa", | ||
ExpectedContainerId = "ac679f8a8319c8cf7d38e1adf263bc08d23", | ||
CgroupVersion = DockerResourceDetector.ParseMode.V1, | ||
}, | ||
new TestCase() | ||
{ | ||
Name = "cgroupv1 with prefix and suffix", | ||
Line = "13:name=systemd:/podruntime/docker/kubepods/crio-dc679f8a8319c8cf7d38e1adf263bc08d23.stuff", | ||
ExpectedContainerId = "dc679f8a8319c8cf7d38e1adf263bc08d23", | ||
CgroupVersion = DockerResourceDetector.ParseMode.V1, | ||
}, | ||
new TestCase() | ||
{ | ||
Name = "cgroupv1 with container Id", | ||
Line = "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356", | ||
ExpectedContainerId = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356", | ||
CgroupVersion = DockerResourceDetector.ParseMode.V1, | ||
}, | ||
}; | ||
|
||
private readonly List<TestCase> testValidCasesV2 = new() | ||
{ | ||
new TestCase() | ||
{ | ||
Name = "cgroupv2 with container Id", | ||
Line = "13:name=systemd:/pod/d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356/hostname", | ||
ExpectedContainerId = "d86d75589bf6cc254f3e2cc29debdf85dde404998aa128997a819ff991827356", | ||
CgroupVersion = DockerResourceDetector.ParseMode.V2, | ||
}, | ||
new TestCase() | ||
{ | ||
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() | ||
{ | ||
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() | ||
{ | ||
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() | ||
{ | ||
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() | ||
{ | ||
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<TestCase> 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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't it test for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, let me fix that. |
||
}, | ||
}; | ||
|
||
[Fact] | ||
public void TestValidContainer() | ||
{ | ||
var dockerResourceDetector = new DockerResourceDetector(); | ||
var allValidTestCases = this.testValidCasesV1.Concat(this.testValidCasesV2); | ||
|
||
using (TempFile tempFile = new TempFile()) | ||
foreach (var testCase in allValidTestCases) | ||
{ | ||
tempFile.Write(CGROUPLINEWITHPREFIX); | ||
Assert.Equal(CONTAINERIDWITHPREFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); | ||
using TempFile tempFile = new TempFile(); | ||
tempFile.Write(testCase.Line); | ||
Assert.Equal( | ||
testCase.ExpectedContainerId, | ||
this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath, testCase.CgroupVersion))); | ||
} | ||
} | ||
|
||
using (TempFile tempFile = new TempFile()) | ||
{ | ||
tempFile.Write(CGROUPLINEWITHSUFFIX); | ||
Assert.Equal(CONTAINERIDWITHSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); | ||
} | ||
[Fact] | ||
public void TestInvalidContainer() | ||
{ | ||
var dockerResourceDetector = new DockerResourceDetector(); | ||
|
||
using (TempFile tempFile = new TempFile()) | ||
// Valid in cgroupv1 is not valid in cgroupv2 | ||
foreach (var testCase in this.testValidCasesV1) | ||
{ | ||
tempFile.Write(CGROUPLINEWITHPREFIXandSUFFIX); | ||
Assert.Equal(CONTAINERIDWITHPREFIXANDSUFFIXREMOVED, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); | ||
using TempFile tempFile = new TempFile(); | ||
tempFile.Write(testCase.Line); | ||
Assert.Equal( | ||
dockerResourceDetector.BuildResource(tempFile.FilePath, DockerResourceDetector.ParseMode.V2), | ||
Resource.Empty); | ||
} | ||
|
||
using (TempFile tempFile = new TempFile()) | ||
// Valid in cgroupv1 is not valid in cgroupv1 | ||
foreach (var testCase in this.testValidCasesV2) | ||
{ | ||
tempFile.Write(CGROUPLINE); | ||
Assert.Equal(CONTAINERID, this.GetContainerId(dockerResourceDetector.BuildResource(tempFile.FilePath))); | ||
using TempFile tempFile = new TempFile(); | ||
tempFile.Write(testCase.Line); | ||
Assert.Equal( | ||
dockerResourceDetector.BuildResource(tempFile.FilePath, DockerResourceDetector.ParseMode.V1), | ||
Resource.Empty); | ||
} | ||
} | ||
|
||
[Fact] | ||
public void TestInvalidContainer() | ||
{ | ||
var dockerResourceDetector = new DockerResourceDetector(); | ||
|
||
// test invalid containerId (non-hex) | ||
using (TempFile tempFile = new TempFile()) | ||
// test invalid cases | ||
foreach (var testCase in this.testInvalidCases) | ||
{ | ||
tempFile.Write(INVALIDCGROUPLINE); | ||
Assert.Equal(dockerResourceDetector.BuildResource(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.BuildResource(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) | ||
{ | ||
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; } | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any usage of
FILEPATHV2
in detector. I'd guess, thatExtractContainerIdV2
should use it, instead ofFILEPATH
. Proper fix for it will also solve problem mentioned by @alexeypukhov , as that functions should check different file.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change has been made to check both versions in the Detect method and assuming the version being used in the BuildResource method. This way, the problem mentioned by @alexeypukhov would be solved as you mentioned since no container id would be able to be found when checking for cgroupv1 if cgroupv2 is being used.