-
Notifications
You must be signed in to change notification settings - Fork 3.2k
add log exporter and metric reader params to configure azure monitor #44367
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?
Conversation
|
Thank you for your contribution @eavanvalkenburg! We will review the pull request and get back to you soon. |
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.
Pull request overview
This PR adds extensibility to the configure_azure_monitor function by introducing two new parameters: log_processors and metric_readers. These allow users to provide custom log processors and metric readers alongside Azure Monitor's default processors/readers, following the existing pattern established by the span_processors parameter.
Key changes:
- Added
LOG_PROCESSORS_ARGandMETRIC_READERS_ARGconstants - Implemented default configuration handlers for the new parameters
- Integrated custom log processors and metric readers into the setup pipeline
- Added test coverage for configuration storage and defaults
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
_constants.py |
Adds two new constant definitions for the log_processors and metric_readers arguments |
_utils/configurations.py |
Imports new constants and adds default configuration functions for log_processors and metric_readers |
_configure.py |
Imports new constants, updates docstring, adds logic to include custom log_processors in logger provider and custom metric_readers in meter provider |
test_configurations.py |
Adds test assertions for log_processors and metric_readers in configuration tests, includes whitespace cleanup |
test_configure.py |
Code formatting improvements (indentation, quote consistency) for existing logging tests |
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py
Outdated
Show resolved
Hide resolved
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_utils/configurations.py
Show resolved
Hide resolved
| for log_processor in configurations[LOG_PROCESSORS_ARG]: | ||
| logger_provider.add_log_record_processor(log_processor) # type: ignore |
Copilot
AI
Dec 10, 2025
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.
Missing test coverage for custom log processors. While test_configurations.py verifies that log_processors are stored in configurations, there are no tests in test_configure.py that verify custom log processors are actually added to the logger provider (similar to how test_setup_tracing verifies custom span processors on line 364). Add a test that verifies logger_provider.add_log_record_processor() is called with the custom log processors.
| if configurations.get(METRIC_READERS_ARG): | ||
| readers = [reader] + configurations[METRIC_READERS_ARG] # type: ignore | ||
| else: | ||
| readers = [reader] | ||
| meter_provider = MeterProvider( | ||
| metric_readers=[reader], | ||
| metric_readers=readers, |
Copilot
AI
Dec 10, 2025
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.
Missing test coverage for custom metric readers. While test_configurations.py verifies that metric_readers are stored in configurations, there are no tests in test_configure.py that verify custom metric readers are actually passed to the MeterProvider (similar to how test_setup_tracing verifies custom span processors on line 364). Add a test that verifies MeterProvider() is called with both the default reader and custom metric readers in the metric_readers parameter.
| :keyword list[~opentelemetry.sdk._logs.LogRecordProcessor] log_processors: List of `LogRecordProcessor` objects | ||
| to process every log prior to exporting. Will be run sequentially. | ||
| :keyword list[~opentelemetry.sdk.metrics.MetricReader] metric_readers: List of `MetricReader` objects | ||
| to process every metric prior to exporting. Will be run sequentially. |
Copilot
AI
Dec 10, 2025
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.
Misleading documentation: The description for metric_readers states they "process every metric prior to exporting" and "will be run sequentially", which is inaccurate. Unlike SpanProcessors and LogRecordProcessors, MetricReaders are responsible for pulling/reading metrics from the SDK and pushing them to exporters, not processing them sequentially. Consider revising to: "List of MetricReader objects to read and export metrics. Each reader can have its own exporter and collection interval."
| to process every metric prior to exporting. Will be run sequentially. | |
| to read and export metrics. Each reader can have its own exporter and collection interval. |
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
5c6a692 to
3060a62
Compare
|
@eavanvalkenburg Thank you for your contribution. Do you mind if I push some changes to this PR? |
326b6df to
c50f19d
Compare
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
This adds two additional params as kwargs to the
configure_azure_monitorfunction.One for log_processors, one for metric_readers, then parses them the same way as the span_processors.
Closes: #44366
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines