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

Container resource detectors + manage resource detectors #2415

Merged
merged 15 commits into from
Apr 13, 2023

Conversation

Kielek
Copy link
Contributor

@Kielek Kielek commented Apr 6, 2023

Why

Fixes #2414

What

Container resource detectors + manage resource detectors
by

  • OTEL_DOTNET_AUTO_RESOURCE_DETECTOR_ENABLED
  • OTEL_DOTNET_AUTO_{0}_RESOURCE_DETECTOR_ENABLED.

Available resource detectors:

  • ENVIRONMENTALVARIABLES
  • TELEMETRYSDK
  • CONTAINER

Tests

CI

Checklist

  • CHANGELOG.md is updated.
  • Documentation is updated.
  • New features are covered by tests. - unit test created. Lack of integration test for Container

WIP

For now, it is referencing obsolete Docker package, the new one will be released after merge:
open-telemetry/opentelemetry-dotnet-contrib#1132
DistributionStructure and ModuleTests tests will be updated with new package

@github-actions github-actions bot requested a review from theletterf April 6, 2023 10:41
@Kielek Kielek force-pushed the resource-detectors branch 2 times, most recently from 826715c to 91ef94a Compare April 6, 2023 10:58
Copy link
Contributor

@RassK RassK left a comment

Choose a reason for hiding this comment

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

Seems excellent!

@Kielek Kielek marked this pull request as ready for review April 7, 2023 06:00
@Kielek Kielek requested a review from a team April 7, 2023 06:00
@Kielek Kielek changed the title [Draft] Container resource detectors + manage resource detectors Container resource detectors + manage resource detectors Apr 7, 2023
CHANGELOG.md Outdated Show resolved Hide resolved
@rajkumar-rangaraj
Copy link
Contributor

Changes in this PR looks good to me. Once this conversation #2415 (comment) is sorted we could merge.

docs/config.md Outdated Show resolved Hide resolved
Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

There are few minor suggestion in the reviews, but, seems ready to merge after the spec Q is clarified.

docs/config.md Outdated Show resolved Hide resolved
docs/config.md Outdated Show resolved Hide resolved
docs/config.md Outdated Show resolved Hide resolved
@Kielek Kielek enabled auto-merge (squash) April 13, 2023 06:18
@Kielek Kielek merged commit 10d1be5 into open-telemetry:main Apr 13, 2023
@Kielek Kielek deleted the resource-detectors branch May 25, 2023 08:36
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.

Docker resource detector
8 participants