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

Fix EnableTracing option conflict with TracesSampleRate #2368

Merged
merged 5 commits into from
May 14, 2023

Conversation

mattjohnsonpint
Copy link
Contributor

Fixes #2366 by having TracesSampleRate and EnableTracing properties have simple get/set implementations (except for some existing validation). This issue was mainly seen during configuration binding in ASP.NET Core applications, and then - only in the case where EnableTracing was set true and TracesSampleRate was unset.

Unfortunately, the only way to implement the correct fix was to make TracesSampleRate a nullable double?. Previously, it was a non-nullable double. (The default value is now null instead of 0.0.) Normally I'd consider that to be a breaking change that could not be done without a major version bump. But in this case, it's unlikely to affect anyone, because this property is typical set, but never retrieved externally.

There are no behavioral changes. Both values work as they were originally intended.

I also improved the xmldocs, and fixed the exception type thrown when an invalid sample rate is set. (ArgumentOutOfRangeException, is more appropriate than InvalidOperationException for that).

@mattjohnsonpint mattjohnsonpint merged commit 4e947d8 into main May 14, 2023
15 checks passed
@mattjohnsonpint mattjohnsonpint deleted the fix/tracing-options branch May 14, 2023 04:15
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.

Conflict between EnableTracing and TracesSampleRate
3 participants