-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Update exporter to OpenTelemetry api/sdk v1.15.0 #27913
Conversation
Thank you for your contribution jabbera! We will review the pull request and get back to you soon. |
I didn't test that this still actually works, but at least it imports. This is really annoying to have broken. I get that this is beta but I'm hoping for a quick turnaround on this. |
38846f4
to
271b7de
Compare
Builds all pass! Followed the same format as this PR: #25659 |
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.
Thanks for fixing this!
@lzchen or @jeremydvoss: can one of you take a look? Monitor pipelines failing due to the severity import path change.
...monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/logs/_exporter.py
Outdated
Show resolved
Hide resolved
me too... |
@pvaneck Anything we can do to move this along? I don't think I can explain the pain this is causing our CI environment:-) |
Looks good to me. Not sure why the CI checks aren't reporting back. Mind doing a rebase or push another commit to retry the checks? |
Hey @jabbera, could you update the PR detaisl to say 1.15.0 instead of 1.0.15? Otherwise, looks good. Approved. |
@jeremydvoss rebased and comment adjusted, |
Thanks @jabbera. Most likely a new patch release will be cut tomorrow after some additional verification by @jeremydvoss. |
Hey @jabbera. I am looking into whether we can do a release before January. It's a bit difficult since many people are on vacation. It may be possible today, but I can't make promises yet. I will keep you posted. In the meantime, can you use OTel 1.14? |
@jeremydvoss Not easily. We depend on this package from about 50 pipelines. Each would have to be updated and the pinned dependency list is ugly because we use a bunch of otel contrib packages that don't have pinned dependencies |
There's another issue: when using the fixed version of the exporter with OTel 1.15, I consistently get the following warning: I've determined that no commit in exporter 1.0.0b11 has caused this. It's too early to know. But it seems to be an issue with how opentelemetry-instrumentation-flask==0.36b0 or opentelemetry-instrumentation-wsgi==0.36b0 use Werkzeug. I've confirmed the fixed version of the exporter does still send telemetry correctly. However, since the new version needs to be pinned to 1.15, we'll be encouraging people to use 1.15 seems to have some issue that needs to be addressed. |
I'll see what I can do. |
I am not able to release the exporter as is because of the memory issue with OTel 1.15. Instead, we can pin the exporter to 1.12<=x<=1.14 before the module path was changed. That way, we can avoid the memory allocation issue as well as the severity import breaking change. |
Pinning below 1.15 seems to break a "frozen requirement": |
Description
Opentelemetry 1.15.0 made breaking changes. See: #27900 and open-telemetry/opentelemetry-python#3038
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines