opentelemetry-dotnet-contrib
opentelemetry-dotnet-contrib copied to clipboard
open-telemetry
Make DisableUrlQueryRedaction option in OpenTelemetry.Instrumentation.Http public
Component
OpenTelemetry.Instrumentation.Http
Package Version
| Package Name | Version |
|---|---|
| OpenTelemetry.Api | 1.9.0 |
| OpenTelemetry | 1.9.0 |
Runtime Version
net8.0
Description
After update URLs in traces became redacted. There is a property to disable it, but it's internal.
internal bool DisableUrlQueryRedaction { get; set; }
Why is this property internal? If I can disable it through environment variable, I should be able to disable it through code. Please make it public and set the default value however you like. You're forcing developers to use reflection, this is bad design decision.
You also hide the fact itself that it can be disabled, no one expect hidden internal property on options configuration object.
Steps to Reproduce
No response
Expected Result
No response
Actual Result
No response
Additional Context
The ugly but acceptable workaround is to disable access checks. Add it to your project file:
<ItemGroup>
<PackageReference Include="IgnoresAccessChecksToGenerator" Version="0.7.0" PrivateAssets="All" />
<InternalsAssemblyName Include="OpenTelemetry.Instrumentation.Http" />
<InternalsAssemblyName Include="OpenTelemetry.Instrumentation.AspNetCore" />
</ItemGroup>
After that you'll be able to change these properties. Also the same problem is with AspNetCoreInstrumentation, don't forget to disable it there too.
It uses https://github.com/aelij/IgnoresAccessChecksToGenerator, give the author some love.
Hi @ImoutoChan
The current feature is still Experimental which is why it's behind the OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_DISABLE_URL_QUERY_REDACTION environment variable.
We're waiting for the OpenTelemetry Community to define how to handle this feature. That conversation is still ongoing: https://github.com/open-telemetry/semantic-conventions/pull/961
That conversation is still ongoing
I understand that some features are still experimetal, but then you should provide them as opt-in, not as opt-out. Right now this experimental feature affects all users who updated their packages to the latest non-preview version.
As @ImoutoChan says, this could have been made trivial to opt-in OR opt-out, but that wasn't done.
This is a breaking change, with no semVer bump. Also exposing the option for people who use Builder and options function semantics would have been the right thing to do, especially since so many tutorials/docs use that convention for configuration.
I have no problem with redaction existing in the current form and being enabled by default, but I do want to be able to deal with it in a natural way for the language/framework that I'm using. For .Net that means IConfiguration and options classes not only environment variables.
The fact that the feature is experimental is immaterial. It has changed the observable behaviour of the library. If it was intended to be hidden and only used by people choosing to use experimental features then it needed to be opt-in not opt-out.
If you wanted to you could use the new experimental attribute https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-12.0/experimental-attribute and ifdef obsolete's for down level tfm's forcing people to see and disable the warnings.
How long do we have to wait for the semantic convention changes to be approved or rejected? the issue has gone stale several times already.
Hi @ImoutoChan The current feature is still Experimental which is why it's behind the
OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_DISABLE_URL_QUERY_REDACTIONenvironment variable.We're waiting for the OpenTelemetry Community to define how to handle this feature. That conversation is still ongoing: open-telemetry/semantic-conventions#961
The linked PR was closed and replaced by one that is more selective. What further decisions by the OpenTelemetry Community are being waited on in order to determine how this redaction should be configured?
I don't seem to be able to disable the query param redaction with OTEL_DOTNET_EXPERIMENTAL_HTTPCLIENT_DISABLE_URL_QUERY_REDACTION set to true. Why is an experimental feature opt-out? That doesn't make sense to me.
Redaction feature is not experimental. Experimental is only setting allowing disabling it. As long as it is experimental, it cannot be make public. We need Minimal, Reproducible Example to understand where you have an issue with env. vars. It is well covered by the tests, and all of them are passing.
