-
Notifications
You must be signed in to change notification settings - Fork 93
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
Conversation
826715c
to
91ef94a
Compare
91ef94a
to
4e6ac2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems excellent!
src/OpenTelemetry.AutoInstrumentation/Configurations/ResourceDetector.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.AutoInstrumentation/Configurations/ConfigurationKeys.cs
Outdated
Show resolved
Hide resolved
Changes in this PR looks good to me. Once this conversation #2415 (comment) is sorted we could merge. |
There was a problem hiding this 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.
test/OpenTelemetry.AutoInstrumentation.Tests/Configurations/SettingsTests.cs
Outdated
Show resolved
Hide resolved
6e63924
to
34abca9
Compare
ad519b8
to
b38c268
Compare
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.WIP
For now, it is referencing obsolete Docker package, the new one will be released after merge:
open-telemetry/opentelemetry-dotnet-contrib#1132
DistributionStructure
andModuleTests
tests will be updated with new package