ref(otel): Use new scopes API#2865
Conversation
9fc643b to
275f1d4
Compare
…ction' into ivana/2.0/otel-new-scopes
Adds the instrumenter argument to sentry_sdk.start_transaction. Should unblock #2865.
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Looks good mostly, please see the question I left
| def on_start(self, otel_span, parent_context=None): | ||
| # type: (OTelSpan, Optional[SpanContext]) -> None | ||
| hub = Hub.current | ||
| if not hub: |
There was a problem hiding this comment.
Does the dsn check make this check redundant? Trying to understand why we are removing this
There was a problem hiding this comment.
What would be the post-hub world equivalent of this check? It seems to me like the purpose of the hub in this function was basically two things: accessing the client and making sure the client has a DSN, and using the hub directly when starting the Sentry span (which we don't want to do anymore in favor of using the top-level API).
There was a problem hiding this comment.
As far as I remember those checks where here to just make sure we have a Sentry setup with and DSN where we can send data to.
So I think it is OK to remove the hub check and just check for a valid DSN further down.
| def on_start(self, otel_span, parent_context=None): | ||
| # type: (OTelSpan, Optional[SpanContext]) -> None | ||
| hub = Hub.current | ||
| if not hub: |
There was a problem hiding this comment.
As far as I remember those checks where here to just make sure we have a Sentry setup with and DSN where we can send data to.
So I think it is OK to remove the hub check and just check for a valid DSN further down.
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
I also approve, thank you both for addressing my question
3 participants
Speedrunning through the remaining integrations 🏃🏻♀️
Relates #2817
General Notes
Thank you for contributing to
sentry-python!Please add tests to validate your changes, and lint your code using
tox -e linters.Running the test suite on your PR might require maintainer approval. Some tests (AWS Lambda) additionally require a maintainer to add a special label to run and will fail if the label is not present.
For maintainers
Sensitive test suites require maintainer review to ensure that tests do not compromise our secrets. This review must be repeated after any code revisions.
Before running sensitive test suites, please carefully check the PR. Then, apply the
Trigger: tests using secretslabel. The label will be removed after any code changes to enforce our policy requiring maintainers to review all code revisions before running sensitive tests.