Defensively import and vendor experimental components #41245
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lzchen
requested review from
jeremydvoss,
hectorhdzg,
rads-1996,
Karlie-777,
MSNev and
JacksonWeber
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR defensively imports and vendors experimental components to prevent crashes when upstream dependencies change. Key changes include:
- Modifying tests to conditionally patch experimental modules via sys.modules.
- Updating dependency versions and import logic in setup.py and _configure.py.
- Vendoring experimental OpenTelemetry code into _utils/instrumentation.py for added resilience.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/monitor/azure-monitor-opentelemetry/tests/test_configure.py | Updated test patches to use sys.modules for experimental imports testing against breaking changes. |
| sdk/monitor/azure-monitor-opentelemetry/setup.py | Upgraded dependency versions for Azure Monitor components and instrumentation libraries. |
| sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_utils/instrumentation.py | Added vendored experimental code with proper documentation. |
| sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py | Adjusted defensive import logic with try-except blocks for both tracing and logging setups. |
| sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md | Documented the changes in the changelog to reflect updated component handling and dependency upgrades. |
Comment on lines
+162
to
+167
| "Exception occured when importing Azure SDK Tracing. Please upgrade to the latest OpenTelemetry version: %s.", | ||
| ex, | ||
| ) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| _logger.warning( # pylint: disable=do-not-log-exceptions-if-not-debug | ||
| "Exception occured when setting Azure SDK Tracing: %s.", |
There was a problem hiding this comment.
Typo in error message: 'occured' should be spelled as 'occurred'.
Suggested change
| "Exception occured when importing Azure SDK Tracing. Please upgrade to the latest OpenTelemetry version: %s.", | |
| ex, | |
| ) | |
| except Exception as ex: # pylint: disable=broad-except | |
| _logger.warning( # pylint: disable=do-not-log-exceptions-if-not-debug | |
| "Exception occured when setting Azure SDK Tracing: %s.", | |
| "Exception occurred when importing Azure SDK Tracing. Please upgrade to the latest OpenTelemetry version: %s.", | |
| ex, | |
| ) | |
| except Exception as ex: # pylint: disable=broad-except | |
| _logger.warning( # pylint: disable=do-not-log-exceptions-if-not-debug | |
| "Exception occurred when setting Azure SDK Tracing: %s.", |
Copilot uses AI. Check for mistakes.
| # This could possibly be due to breaking change in upstream OpenTelemetry | ||
| # Advise user to upgrade to latest OpenTelemetry version | ||
| _logger.warning( # pylint: disable=do-not-log-exceptions-if-not-debug | ||
| "Exception occured when setting up Logging. Please upgrade to the latest OpenTelemetry version: %s.", |
There was a problem hiding this comment.
Typo in error message: 'occured' should be spelled as 'occurred'.
Suggested change
| "Exception occured when setting up Logging. Please upgrade to the latest OpenTelemetry version: %s.", | |
| "Exception occurred when setting up Logging. Please upgrade to the latest OpenTelemetry version: %s.", |
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Conditionally import experimental components to ensure distro does not crash on mismatched dependencies.
This could happen when there are breaking changes in upstream OpenTelemetry or azure.core