Skip to content

Conversation

alok87
Copy link

@alok87 alok87 commented Aug 22, 2025

  • Updated containerIDRegex to support both Docker and Kubernetes cgroup formats
  • Modified getContainerID() to properly extract container ID from regex capture groups
  • Fixes issue where EKS detector fails with 'cannot read containerID from file /proc/self/cgroup'
  • Now supports cgroup paths like /kubepods/burstable/pod/

Error I had encountered

W0821 12:39:45.604746     127 telemetry.go:115] EKS detector failed: getContainerID() error: cannot read containerID from file /proc/self/cgroup

- Updated containerIDRegex to support both Docker and Kubernetes cgroup formats
- Modified getContainerID() to properly extract container ID from regex capture groups
- Fixes issue where EKS detector fails with 'cannot read containerID from file /proc/self/cgroup'
- Now supports cgroup paths like /kubepods/burstable/pod<pod-id>/<container-id>
@alok87 alok87 requested a review from a team as a code owner August 22, 2025 04:42
@github-actions github-actions bot requested a review from pyohannes August 22, 2025 04:42
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misses unit tests.

Copy link

codecov bot commented Aug 25, 2025

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.9%. Comparing base (6a3469c) to head (9eb130b).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
detectors/aws/eks/detector.go 0.0% 8 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7786     +/-   ##
=======================================
- Coverage   78.9%   78.9%   -0.1%     
=======================================
  Files        186     186             
  Lines      14819   14824      +5     
=======================================
- Hits       11705   11699      -6     
- Misses      2776    2785      +9     
- Partials     338     340      +2     
Files with missing lines Coverage Δ
detectors/aws/eks/detector.go 38.8% <0.0%> (-2.3%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @pellared mentioned, this needs tests.


// is this going to stop working with 1.20 when Docker is deprecated?
var containerIDRegex = regexp.MustCompile(`^.*/docker/(.+)$`)
// Updated regex to support both Docker and Kubernetes/containerd cgroup formats
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments shouldn't be about things that have been updated (GIT has history for that), but actual issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants