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(detector-container)!: change implementation to DetectorSync interface #2334

Merged

Conversation

david-luna
Copy link
Contributor

@david-luna david-luna commented Jul 12, 2024

Context

Another PR moving away from the deprecated Detector interface in order to prepare the codebase for a fix for #2320. This time is for Container detector.

Not doing all detectors at once:

My reasons to doing this change:

  • SDK handles both interfaces using detectResources from @opentelemetry/resources package. Ref
  • it is unlikely to be used oustide the SDK or auto-instrumentations

Short description of the changes

  • change detect method to be sync
  • update tests

Copy link

codecov bot commented Jul 12, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.44%. Comparing base (dfb2dff) to head (19a81e7).
Report is 205 commits behind head on main.

Files Patch % Lines
...ector-container/src/detectors/ContainerDetector.ts 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2334      +/-   ##
==========================================
- Coverage   90.97%   90.44%   -0.53%     
==========================================
  Files         146      150       +4     
  Lines        7492     7403      -89     
  Branches     1502     1535      +33     
==========================================
- Hits         6816     6696     -120     
- Misses        676      707      +31     
Files Coverage Δ
...ector-container/src/detectors/ContainerDetector.ts 95.08% <85.71%> (+2.22%) ⬆️

... and 67 files with indirect coverage changes

@david-luna david-luna self-assigned this Aug 1, 2024
@david-luna david-luna changed the title refactor(detector-container): change implementation to DetectorSync interface feat(detector-container)!: change implementation to DetectorSync interface Aug 1, 2024
@pichlermarc
Copy link
Member

pichlermarc commented Aug 7, 2024

FYI @abhee11(component owner)

@david-luna david-luna merged commit 18a5731 into open-telemetry:main Aug 7, 2024
21 checks passed
@david-luna david-luna deleted the dluna/container-detector-sync branch August 7, 2024 13:57
@dyladan dyladan mentioned this pull request Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants