Skip to content

Distro 1.6.7 Hotfix: Adjust to breaking change in opentelemetry-instrumentation#40463

Merged
lmolkova merged 7 commits intoAzure:mainfrom
jeremydvoss:otel-conflict-break
Apr 10, 2025
Merged

Distro 1.6.7 Hotfix: Adjust to breaking change in opentelemetry-instrumentation#40463
lmolkova merged 7 commits intoAzure:mainfrom
jeremydvoss:otel-conflict-break

Conversation

@jeremydvoss
Copy link
Copy Markdown
Member

Description

Breaking change detected by pipeline.

Changing configure_azure_monitor to use dependency conflict check inside of instrumentor.instrument.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@github-actions github-actions Bot added the Monitor - Distro Monitor OpenTelemetry Distro label Apr 10, 2025
@jeremydvoss jeremydvoss marked this pull request as ready for review April 10, 2025 18:17
Copilot AI review requested due to automatic review settings April 10, 2025 18:17
@jeremydvoss jeremydvoss requested a review from lzchen as a code owner April 10, 2025 18:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py Outdated
…metry/_configure.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@azure-sdk
Copy link
Copy Markdown
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Copy Markdown
Member

@MSNev MSNev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM based on everyone updating to the newer version, just have the question about how this is going to work if someone has this version but an older version of otel installed.

Or do we just say not supported, upgrade everything?

@jeremydvoss jeremydvoss enabled auto-merge (squash) April 10, 2025 19:42
@lmolkova lmolkova disabled auto-merge April 10, 2025 19:58
@lmolkova lmolkova enabled auto-merge (squash) April 10, 2025 19:58
@lmolkova lmolkova disabled auto-merge April 10, 2025 20:06
@lmolkova lmolkova merged commit 8b96068 into Azure:main Apr 10, 2025
24 checks passed
_logger.debug("Instrumentation skipped for library %s", entry_point.name)
continue
try:
# Check if dependent libraries/version are installed
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm doing something wrong, the removal of this logic causes some messy log messages in our containers - e.g. tracebacks starting like:

Exception occurred when instrumenting: django.
Traceback (most recent call last):
  File "/app/.venv/lib/python3.12/site-packages/azure/monitor/opentelemetry/_configure.py", line 222, in _setup_instrumentations
    instrumentor: BaseInstrumentor = entry_point.load()
                                     ^^^^^^^^^^^^^^^^^^

for all the libraries that aren't installed. Do we now have to explicitly disable these when calling configure_azure_monitor? Are the defaults documented anywhere?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dylwil3 please create a new issue (so it won't get overlooked). Thanks!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure - here it is: #40517

cRui861 pushed a commit that referenced this pull request May 14, 2025
…umentation (#40463)

* Fix breaking change re: dep conflict checks

* tests pass

* clean up

* changelog and min version

* Update sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_configure.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* lint: unused imports

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Monitor - Distro Monitor OpenTelemetry Distro

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants