-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(azure-monitor-opentelemetry): configure_azure_monitor args were overwritten #44131
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,7 +164,7 @@ def _default_sampling_ratio(configurations): | |
| default_value, | ||
| e, | ||
| ) | ||
| configurations[SAMPLING_TRACES_PER_SECOND_ARG] = default_value | ||
| configurations.setdefault(SAMPLING_TRACES_PER_SECOND_ARG, default_value) | ||
|
|
||
| # Handle fixed percentage sampler | ||
| elif sampler_type == FIXED_PERCENTAGE_SAMPLER: | ||
|
|
@@ -183,11 +183,11 @@ def _default_sampling_ratio(configurations): | |
| default_value, | ||
| e, | ||
| ) | ||
| configurations[SAMPLING_RATIO_ARG] = default_value | ||
| configurations.setdefault(SAMPLING_RATIO_ARG, default_value) | ||
|
||
|
|
||
| # Handle all other cases (no sampler type specified or unsupported sampler type) | ||
| else: | ||
| configurations[SAMPLING_RATIO_ARG] = default_value | ||
| configurations.setdefault(SAMPLING_RATIO_ARG, default_value) | ||
| if sampler_type is not None: | ||
| _logger.error( # pylint: disable=C | ||
| "Invalid argument for the sampler to be used for tracing. " | ||
|
Comment on lines
192
to
193
|
||
|
|
||
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.
The error message logged here states "Defaulting to %s" (where %s is
default_value), but with the change to usesetdefault(), the actual behavior may not match this message. If the user provided a value viaconfigure_azure_monitor()kwargs, that user-provided value will be preserved rather than defaulting todefault_value. The log message should be updated to reflect this, or the value that will actually be used should be logged.Consider logging the actual value that will be used after calling
setdefault():