-
Notifications
You must be signed in to change notification settings - Fork 511
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
Extract origin from span attributes #3432
Merged
Merged
Conversation
This file contains 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
Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time. ❌ Failed Test Results:Completed 829 tests with View the full list of failed testspy3.10-potel
py3.11-aiohttp-latest
py3.11-aiohttp-v3.8
py3.11-anthropic-v0.16
py3.11-ariadne-v0.20
py3.11-boto3-latest
py3.11-boto3-v1.23
py3.11-clickhouse_driver-v0.2.0
py3.11-grpc-latest
py3.11-opentelemetry
py3.11-trytond-latest
py3.12-anthropic-latest
py3.12-anthropic-v0.25
py3.12-ariadne-latest
py3.12-boto3-latest
py3.12-boto3-v1.23
py3.12-grpc-latest
py3.12-opentelemetry
py3.7-aiohttp-v3.4
py3.7-anthropic-latest
py3.7-arq-v0.23
py3.7-bottle-latest
py3.7-gql-latest
py3.7-gql-v3.4
py3.7-grpc-v1.39
py3.7-opentelemetry
py3.8-aiohttp-latest
py3.8-ariadne-latest
py3.8-ariadne-v0.20
py3.8-grpc-latest
py3.8-litestar-v2.0
py3.8-potel
py3.8-trytond-latest
py3.9-aiohttp-v3.8
py3.9-cohere-latest
py3.9-cohere-v5
py3.9-grpc-v1.39
py3.9-httpx-latest
py3.9-opentelemetry
py3.9-pymongo-v3.12
|
sentrivana
force-pushed
the
ivana/potel/better-origin
branch
from
August 12, 2024 14:23
896fb6e
to
c5a7852
Compare
sentrivana
force-pushed
the
ivana/potel/better-origin
branch
from
August 12, 2024 14:25
c5a7852
to
4ee44f9
Compare
antonpirker
approved these changes
Aug 13, 2024
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.
looks good to me
sl0thentr0py
approved these changes
Aug 13, 2024
sentrivana
added a commit
that referenced
this pull request
Aug 26, 2024
* Skeletons for new components * Add simple scope management whenever a context is attached * create a new otel context `_SCOPES_KEY` that will hold a tuple of `(curent_scope, isolation_scope)` * the `current_scope` will always be forked (like on every span creation/context update in practice) * note that this is on `attach`, so not on all copy-on-write context object creation but only on apis such as [`trace.use_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L547) or [`tracer.start_as_current_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L329) * basically every otel `context` fork corresponds to our `current_scope` fork * the `isolation_scope` currently will not be forked * these will later be updated, for instance when we update our top level scope apis that fork isolation scope, that will also have a corresponding change in this `attach` function * Don't parse DSN twice * wip * Skeletons for new components * Skeletons for new components * Add simple scope management whenever a context is attached * create a new otel context `_SCOPES_KEY` that will hold a tuple of `(curent_scope, isolation_scope)` * the `current_scope` will always be forked (like on every span creation/context update in practice) * note that this is on `attach`, so not on all copy-on-write context object creation but only on apis such as [`trace.use_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L547) or [`tracer.start_as_current_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L329) * basically every otel `context` fork corresponds to our `current_scope` fork * the `isolation_scope` currently will not be forked * these will later be updated, for instance when we update our top level scope apis that fork isolation scope, that will also have a corresponding change in this `attach` function * mypy fixes * working span processor * lint * Port over op/description/status extraction * defaultdict * naive impl * wip * fix args * wip * remove extra docs * Add simple scope management whenever a context is attached (#3159) Add simple scope management whenever a context is attached * create a new otel context `_SCOPES_KEY` that will hold a tuple of `(curent_scope, isolation_scope)` * the `current_scope` will always be forked (like on every span creation/context update in practice) * note that this is on `attach`, so not on all copy-on-write context object creation but only on apis such as [`trace.use_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L547) or [`tracer.start_as_current_span`](https://github.com/open-telemetry/opentelemetry-python/blob/ba22b165471bde2037620f2c850ab648a849fbc0/opentelemetry-api/src/opentelemetry/trace/__init__.py#L329) * basically every otel `context` fork corresponds to our `current_scope` fork * the `isolation_scope` currently will not be forked * these will later be updated, for instance when we update our top level scope apis that fork isolation scope, that will also have a corresponding change in this `attach` function * Implement new POTel span processor (#3223) * only acts on `on_end` instead of both `on_start/on_end` as before * store children spans in a dict mapping `span_id -> children` * new dict only stores otel span objects and no sentry transaction/span objects so we save a bit of useless memory allocation * I'm not using our current `Transaction/Span` classes at all to build the event because when we add our APIs later, we'll need to rip these out and we also avoid having to deal with the `instrumenter` problem * if we get a root span (without parent), we recursively walk the dict and find the children and package up the transaction event and send it * I didn't do it like JS because I think this way is better * they [group an array of `finished_spans`](https://github.com/getsentry/sentry-javascript/blob/7e298036a21a5658f3eb9ba184165178c48d7ef8/packages/opentelemetry/src/spanExporter.ts#L132) every time a root span ends and I think this uses more cpu than what I did * and the dict like I used it doesn't take more space than the array either * if we get a span with a parent we just update the dict to find the span later * moved the common `is_sentry_span` logic to utils * Basic test cases for potel (#3286) * Proxy POTelSpan.set_data to underlying otel span attributes (#3297) * ref(tracing): Simplify backwards-compat code (#3379) With this change, we aim to simplify the backwards-compatibility code for POTel tracing. We do this as follows: - Remove `start_*` functions from `tracing` - Remove unused parameters from `tracing.POTelSpan.__init__`. - Make all parameters to `tracing.POTelSpan.__init__` kwarg-only. - Allow `tracing.POTelSpan.__init__` to accept arbitrary kwargs, which are all ignored, for compatibility with old `Span` interface. - Completely remove `start_inactive_span`, since inactive spans can be created by setting `active=False` when constructing a `POTelSpan`. * New Scope implementation based on OTel Context (#3389) * New `PotelScope` inherits from scope and reads the scope from the otel context key `SENTRY_SCOPES_KEY` * New `isolation_scope` and `new_scope` context managers just use the context manager forking and yield with the scopes living on the above context key * isolation scope forking is done with the `SENTRY_FORK_ISOLATION_SCOPE_KEY` boolean context key * Fix circular imports (#3431) * Random tweaks (#3437) * Origin improvements (#3432) * Tweak OTel timestamp utils (#3436) * Create spans on scope (#3442) * Fill out more property/method stubs (#3441) * Cleanup origin handling and defaults (#3445) * add note to migration guide * Attribute namespace for tags, measurements (#3448) --------- Co-authored-by: Neel Shah <neel.shah@sentry.io> Co-authored-by: Neel Shah <neelshah.sa@gmail.com> Co-authored-by: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com>
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.
The idea is that we set the origin as a span attribute here and then we can pick it up in the span processor, so that our instrumentation (i.e., us using
start_span
in an integration with origin provided) will still give us nice origins. Same with description.