Skip to content
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

Fix container parsing for k8s cgroup #1487

Merged
merged 3 commits into from
May 3, 2021

Conversation

delner
Copy link
Contributor

@delner delner commented Apr 28, 2021

Adds support for:

  • k8s burstable containers (drops .slice and .scope from IDs)
  • Gnome VTE cgroup
  • More defensive behavior against empty cgroup paths.

Also includes some minor refactoring for tests and a slightly more permissive expectation (so it doesn't break with pry.)

@delner delner added bug Involves a bug core Involves Datadog core libraries labels Apr 28, 2021
@delner delner added this to the 0.49.0 milestone Apr 28, 2021
@delner delner self-assigned this Apr 28, 2021
@delner delner requested a review from a team April 28, 2021 04:36
@delner
Copy link
Contributor Author

delner commented Apr 28, 2021

Before merging this, I want to investigate a report for Fargate 1.4... if there's a change to be made, it'd be easier to add it to this PR.

@codecov-commenter
Copy link

codecov-commenter commented Apr 28, 2021

Codecov Report

Merging #1487 (e8226ce) into master (480ebe2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1487   +/-   ##
=======================================
  Coverage   98.23%   98.24%           
=======================================
  Files         863      863           
  Lines       41582    41681   +99     
=======================================
+ Hits        40849    40948   +99     
  Misses        733      733           
Impacted Files Coverage Δ
lib/ddtrace/runtime/container.rb 93.02% <100.00%> (+0.52%) ⬆️
spec/ddtrace/runtime/cgroup_spec.rb 100.00% <100.00%> (ø)
spec/ddtrace/runtime/container_spec.rb 100.00% <100.00%> (ø)
spec/support/container_helpers.rb 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 480ebe2...e8226ce. Read the comment docs.

@ivoanjo
Copy link
Member

ivoanjo commented Apr 28, 2021

@delner not sure if it's helpful, but I have a repo https://github.com/DataDog/profile-ruby-app-on-amazon-ecs-experiment (internal link, sorry!) with some scaffolding to deploy stuff to ecs/eks, it may come in handy.

@delner delner force-pushed the fix/container_parsing_k8s_cgroups branch from 8ac0f05 to e8226ce Compare May 3, 2021 20:40
@delner
Copy link
Contributor Author

delner commented May 3, 2021

Okay, fixed one more scenario regarding Fargate 1.4+ cgroups. This should be ready for review.

@delner delner merged commit c518e9e into master May 3, 2021
@delner delner deleted the fix/container_parsing_k8s_cgroups branch May 3, 2021 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Involves a bug core Involves Datadog core libraries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error while parsing container info. Cause: undefined method `[]' for nil:NilClass
4 participants