-
Notifications
You must be signed in to change notification settings - Fork 99
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
Env var instrumentation improvements #2088
Env var instrumentation improvements #2088
Conversation
Co-authored-by: Robert Pająk <pellared@hotmail.com>
🤔 @Kielek IIRC the conversation on the SIG with @carlosalberto was to follow Java in this regard, i.e.: use @carlosalberto what are your thoughts in this regard? Issue regarding bool env vars in this project: #2090 |
See: #2090 |
@pellared I'm aware of that but I'm under the impression that at the SIG meeting, we talked with Carlos about following the Java precedent in this case. |
Java is still a little different. We have not discussed |
@pellared neither I will fight for it :), but, I would prefer to commit the version that has the smaller chance of having to be reverted later. |
So, what are we going forward with in the end? 😬 |
src/OpenTelemetry.AutoInstrumentation.Native/bytecode_instrumentations.h
Outdated
Show resolved
Hide resolved
Even, I feel it is easier for customers to use _ENABLED in the environment variables. We could hold the merge of the PR till SIG meeting for a detailed discussion. |
Reasons why I lean towards
I think this approach has smaller chances in getting changed as it is based on OTel spec. That is why I would prefer merging the current version. |
src/OpenTelemetry.AutoInstrumentation.Native/bytecode_instrumentations.h
Show resolved
Hide resolved
src/OpenTelemetry.AutoInstrumentation/Configurations/ConfigurationKeys.cs
Outdated
Show resolved
Hide resolved
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.
LGTM
left very minor comments, could be addressed in a next PR. Nice work.
Why
Fixes #1633
What
Change the way to manage enabled instrumentations. The following environmental
variables:
OTEL_DOTNET_AUTO_TRACES_ENABLED_INSTRUMENTATIONS
,OTEL_DOTNET_AUTO_TRACES_DISABLED_INSTRUMENTATIONS
,OTEL_DOTNET_AUTO_METRICS_ENABLED_INSTRUMENTATIONS
,OTEL_DOTNET_AUTO_METRICS_DISABLED_INSTRUMENTATIONS
,OTEL_DOTNET_AUTO_LOGS_ENABLED_INSTRUMENTATIONS
,OTEL_DOTNET_AUTO_LOGS_DISABLED_INSTRUMENTATIONS
are replaced by:
OTEL_DOTNET_AUTO_INSTRUMENTATION_ENABLED
,OTEL_DOTNET_AUTO_TRACES_INSTRUMENTATION_ENABLED
,OTEL_DOTNET_AUTO_TRACES_{0}_INSTRUMENTATION_ENABLED
,OTEL_DOTNET_AUTO_METRICS_INSTRUMENTATION_ENABLED
,OTEL_DOTNET_AUTO_METRICS_{0}_INSTRUMENTATION_ENABLED
,OTEL_DOTNET_AUTO_LOGS_INSTRUMENTATION_ENABLED
,OTEL_DOTNET_AUTO_LOGS_{0}_INSTRUMENTATION_ENABLED
.Tests
CI + updated tests
Checklist
CHANGELOG.md
is updated.