Skip to content
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

POtel implementation base branch #3152

Draft
wants to merge 256 commits into
base: master
Choose a base branch
from
Draft

POtel implementation base branch #3152

wants to merge 256 commits into from

Conversation

sl0thentr0py
Copy link
Member

@sl0thentr0py sl0thentr0py commented Jun 10, 2024

Full state of CI: #3744

Contains:

Simple test

import sentry_sdk
from time import sleep

sentry_sdk.init(
    debug=True,
    traces_sample_rate=1.0,
    _experiments={"otel_powered_performance": True},
)

with sentry_sdk.start_span(description="sentry request"):
    sleep(0.1)
    with sentry_sdk.start_span(description="sentry db"):
        sleep(0.5)
        with sentry_sdk.start_span(description="sentry redis"):
            sleep(0.2)
    with sentry_sdk.start_span(description="sentry http"):
        sleep(1)

References

Misc

In OTel, this:

with tracer.start_as_current_span("parent") as parent:
    with tracer.start_span("child1"):
        pass
    with tracer.start_span("child2"):
        pass

is equivalent to

from opentelemetry import trace, context

parent = tracer.start_span("parent")

# Creates a Context object with parent set as current span
ctx = trace.set_span_in_context(parent)

# Set as the implicit current context
token = context.attach(ctx)

# Child will automatically be a child of parent
child1 = tracer.start_span("child1")
child1.end()

# Child will automatically be a child of parent
child2 = tracer.start_span("child2")
child2.end()

# Don't forget to detach or parent will remain the parent above this call stack
context.detach(token)
parent.end()

@sl0thentr0py sl0thentr0py requested a review from sentrivana June 10, 2024 19:00
@sl0thentr0py sl0thentr0py force-pushed the potel-base branch 2 times, most recently from f7f153c to 28effd6 Compare June 11, 2024 11:43
@sl0thentr0py sl0thentr0py force-pushed the potel-base branch 2 times, most recently from 16f9341 to 951477f Compare June 25, 2024 15:16
Copy link

codecov bot commented Jun 26, 2024

❌ 813 Tests Failed:

Tests completed Failed Passed Skipped
21714 813 20901 4573
View the top 3 failed test(s) by shortest run time
 tests.integrations.celery.integration_tests.test_celery_beat_cron_monitoring
Stack Traces | 0s run time
.../hostedtoolcache/Python/3.7.17.../x64/lib/python3.7/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1006: in _gcd_import
    ???
<frozen importlib._bootstrap>:983: in _find_and_load
    ???
<frozen importlib._bootstrap>:953: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
<frozen importlib._bootstrap>:1006: in _gcd_import
    ???
<frozen importlib._bootstrap>:983: in _find_and_load
    ???
<frozen importlib._bootstrap>:967: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:677: in _load_unlocked
    ???
<frozen importlib._bootstrap_external>:728: in exec_module
    ???
<frozen importlib._bootstrap>:219: in _call_with_frames_removed
    ???
.../celery/integration_tests/__init__.py:7: in <module>
    from celery.beat import Scheduler
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/beat.py:22: in <module>
    from . import __version__, platforms, signals
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/signals.py:14: in <module>
    from .utils.dispatch import Signal
.tox/py3.7-celery-v5.0.5/lib/python3.7.../celery/utils/__init__.py:16: in <module>
    from .nodenames import nodename, nodesplit, worker_direct
.tox/py3.7-celery-v5.0.5/lib/python3.7.../celery/utils/nodenames.py:6: in <module>
    from kombu.entity import Exchange, Queue
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/kombu/entity.py:7: in <module>
    from .serialization import prepare_accept_content
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/kombu/serialization.py:440: in <module>
    for ep, args in entrypoints('kombu.serializers'):  # pragma: no cover
.tox/py3.7-celery-v5.0.5/lib/python3.7.../kombu/utils/compat.py:82: in entrypoints
    for ep in importlib_metadata.entry_points().get(namespace, [])
E   AttributeError: 'EntryPoints' object has no attribute 'get'
 tests.integrations.celery.test_celery
Stack Traces | 0s run time
ImportError while importing test module '.../integrations/celery/test_celery.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/_pytest/python.py:617: in _importtestmodule
    mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/_pytest/pathlib.py:567: in import_path
    importlib.import_module(module_name)
.../hostedtoolcache/Python/3.7.17.../x64/lib/python3.7/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1006: in _gcd_import
    ???
<frozen importlib._bootstrap>:983: in _find_and_load
    ???
<frozen importlib._bootstrap>:967: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:677: in _load_unlocked
    ???
.tox/py3.7-celery-v5.0.5/lib/python3.7.../_pytest/assertion/rewrite.py:186: in exec_module
    exec(co, module.__dict__)
.../integrations/celery/test_celery.py:6: in <module>
    from celery import Celery, VERSION
E   ImportError: cannot import name 'Celery' from 'celery' (.../sentry-python/sentry-python/.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/__init__.py)
 tests.integrations.celery.test_update_celery_task_headers
Stack Traces | 0s run time
.../integrations/celery/beat.py:28: in <module>
    from celery import Task, Celery  # type: ignore
E   ImportError: cannot import name 'Task' from 'celery' (.../sentry-python/sentry-python/.tox/py3.7-celery-v5.0.5/lib/python3.7.../site-packages/celery/__init__.py)

During handling of the above exception, another exception occurred:
.../integrations/celery/test_update_celery_task_headers.py:7: in <module>
    from sentry_sdk.integrations.celery import _update_celery_task_headers
.../integrations/celery/__init__.py:9: in <module>
    from sentry_sdk.integrations.celery.beat import (
.../integrations/celery/beat.py:37: in <module>
    raise DidNotEnable("Celery not installed")
E   sentry_sdk.integrations.DidNotEnable: Celery not installed

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@sl0thentr0py sl0thentr0py changed the title Skeletons for new POTEL components New POTEL base branch Jul 9, 2024
@sl0thentr0py sl0thentr0py changed the title New POTEL base branch potel implementation base branch Jul 9, 2024
@antonpirker antonpirker changed the title potel implementation base branch POtel implementation base branch Aug 5, 2024
szokeasaurusrex and others added 17 commits August 7, 2024 14:42
Also, remove any tests for `sentry_sdk.configure_scope`.

Since Strawberry's deprecated [Sentry tracing extensions](https://strawberry.rocks/docs/extensions/sentry-tracing) import `sentry_sdk.configure_scope`, importing `strawberry.extensions.tracing.SentryTracingExtension` (or `SentryTracingExtensionSync`) will result in an unhandled exception. Therefore, these imports, and any functionality associated with them, have also been removed. This itself is not a breaking change, as it is necessitated by the removal of `sentry_sdk.configure_scope`.

BREAKING CHANGE: Remove `sentry_sdk.configure_scope`.

Closes: #3402
Also, remove any tests that test `sentry_sdk.push_scope`.

BREAKING CHANGE: Remove `sentry_sdk.push_scope`.

Closes #3403
This change is a prerequisite for #3404.

BREAKING CHANGE: Remove `sentry_sdk.transport.HttpTransport`'s `hub_cls` attribute.
* Removed hub based functions from sessions implementation
* Removed scope manager
* Removed hub from tracing
* Removed hub from apidocs
* Updated migration guide
* Updated migration guide
* 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>
* Implement new continue_trace and make WSGI work

The new `continue_trace` API will no longer return a `Transaction` entity.
Instead, it will simply update the propagation context and run as a
contextmanager.

(TODO) It will set a remote span on the OTEL context so that it can be picked up by `start_span` later.
* fix: Fix mypy "indented block" error

Fix this [mypy error](https://github.com/getsentry/sentry-python/actions/runs/10390581522/job/28771379618):

```
sentry_sdk/integrations/opentelemetry/potel_span_processor.py:149: error: expected an indented block after function definition on line 31  [syntax]
```

Honestly not sure why this change fixes the problem, maybe there is some bug in `mypy`.

* fix: Fix other mypy syntax failures

After fixing the previous mypy failure, mypy discovered more syntax problems, which this commit fixes.

* fix: Correct typing in `potel_span_processor`

Fixing the original mypy error broke the typing; this change fixes the typing.
sl0thentr0py and others added 30 commits January 16, 2025 15:28
* `POTelSpan` no longer exists
* Old `Span/Transaction` both point to new `Span`
* `NoOpSpan` is still around to enable future potential errors-only
stuff

closes #3968
mypy was complaining too so just removed it
Co-authored-by: Andrew Liu <159852527+aliu39@users.noreply.github.com>
Co-authored-by: Marcelo Galigniana <marcelogaligniana@gmail.com>
Also make sure, that the span status is not set as a tag on the span.
SDKs should not set tags at all by default (only users are allowed to
set tags)

Fixes #4065
On potel-base, we got rid of the `maybe_create_breadcrumbs` function in
favor of creating breadcrumbs manually, so the new breadcrumb level
logic needs to go directly in the affected integrations.

Closes #4066
Oversight from #4090 --
forgot to update the breadcrumb level created by the async httpx client

Makes all Network tests green.
If the SDK uses a specific sample rate for a trace, it needs to update
it in the DSC for downstream SDKs.

On an incoming trace, the DSC's sample_rate is updated if:
- an explicit sampling decision is forced, e.g. startTransaction(sampled: true)
- the tracesSampler is invoked
- the tracesSampleRate is used

Closes #4028

---------

Co-authored-by: Anton Pirker <anton.pirker@sentry.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants