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

feat: Add support for capturing container id from AWS ECS. #2481

Merged
merged 9 commits into from
Jun 12, 2024

Conversation

jaffinito
Copy link
Member

@jaffinito jaffinito commented May 14, 2024

On hold while we wait for more details about the missing linkage with infra.

Description

  • Updates GetDockerVendorInfo to check for both the v4 and v3 ECS metadata endpoint URI environment variables and if present parse out the docker from JSON obtain from the endpoint.
  • Includes unit tests for both v3 and v4.

Notes:

  • Matches docker id captured by the ECS and Docker AWS integrations.
  • Environment variables are only read if the CGroup v1 and v2 checks fail (always with ECS Fargate).
  • Environment variables are only present in ECS (non-fargate and fargate). No additional web calls will be made outside of that environment.
  • Metadata v4 is checked first and if present, v3 will not be used. v3 is a fallback since it will eventually disappear.
  • No integration tests since that would either require running in ECS Fargate or faking the environment variables with a fake locally hosted endpoint. The unit test does basically the same thing without the extra effort.

Once/If this is approved, I will update the agent specs with our implementation.

Author Checklist

  • Unit tests, Integration tests, and Unbounded tests completed
  • Performance testing completed with satisfactory results (if required)

Reviewer Checklist

  • Perform code review
  • Pull request was adequately tested (new/existing tests, performance tests)

@nr-ahemsath nr-ahemsath changed the title Add support for capturnig docker id from AWS ECS Fargate. Add support for capturing docker id from AWS ECS Fargate. May 14, 2024
Copy link
Member

@chynesNR chynesNR left a comment

Choose a reason for hiding this comment

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

Looks good!

@jaffinito jaffinito marked this pull request as draft May 21, 2024 17:36
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 70.58824% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 80.56%. Comparing base (0be7ccd) to head (fe2a7d1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2481   +/-   ##
=======================================
  Coverage   80.55%   80.56%           
=======================================
  Files         459      459           
  Lines       28759    28802   +43     
  Branches     3152     3159    +7     
=======================================
+ Hits        23168    23203   +35     
- Misses       4812     4817    +5     
- Partials      779      782    +3     
Flag Coverage Δ
Agent 81.50% <70.58%> (-0.01%) ⬇️
Profiler 72.15% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...gent/NewRelic/Agent/Core/Utilization/VendorInfo.cs 82.47% <70.58%> (-1.00%) ⬇️

... and 1 file with indirect coverage changes

@jaffinito jaffinito changed the title Add support for capturing docker id from AWS ECS Fargate. Add support for capturing docker id from AWS ECS. Jun 3, 2024
@jaffinito jaffinito marked this pull request as ready for review June 3, 2024 15:26
Copy link
Member

@tippmar-nr tippmar-nr left a comment

Choose a reason for hiding this comment

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

Nice work!

Copy link
Member

@nr-ahemsath nr-ahemsath left a comment

Choose a reason for hiding this comment

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

Makes sense to me. 👍

@tippmar-nr tippmar-nr changed the title Add support for capturing docker id from AWS ECS. feat: Add support for capturing docker id from AWS ECS. Jun 11, 2024
@tippmar-nr tippmar-nr changed the title feat: Add support for capturing docker id from AWS ECS. feat: Add support for capturing container id from AWS ECS. Jun 11, 2024
@jaffinito jaffinito merged commit c018b8a into main Jun 12, 2024
100 checks passed
@jaffinito jaffinito deleted the feature/fargate-docker-id branch June 12, 2024 17:29
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.

None yet

6 participants