Skip to content

ref(otel): Use new scopes API #2865

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.

Sign up for GitHub

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

Merged
sentrivana merged 5 commits into from
Mar 28, 2024
Merged

ref(otel): Use new scopes API #2865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
275f1d4
ref(otel): Use new API in span processor
sentrivana Mar 25, 2024
01c1475
ref: Correct `api.start_transaction` method signature
szokeasaurusrex Mar 26, 2024
1113950
Merge remote-tracking branch 'origin/szokeasaurusrex/fix-start-transa…
sentrivana Mar 26, 2024
c122ec1
Merge branch 'sentry-sdk-2.0' into ivana/2.0/otel-new-scopes
sentrivana Mar 28, 2024
6cdc8c8
Merge branch 'sentry-sdk-2.0' into ivana/2.0/otel-new-scopes
sentrivana Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 18 additions & 20 deletions sentry_sdk/integrations/opentelemetry/span_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
INVALID_SPAN_ID,
INVALID_TRACE_ID,
)
from sentry_sdk import get_client, start_transaction
from sentry_sdk.consts import INSTRUMENTER
from sentry_sdk.hub import Hub
from sentry_sdk.integrations.opentelemetry.consts import (
SENTRY_BAGGAGE_KEY,
SENTRY_TRACE_KEY,
Expand All @@ -40,11 +40,9 @@

def link_trace_context_to_error_event(event, otel_span_map):
# type: (Event, Dict[str, Union[Transaction, SentrySpan]]) -> Event
hub = Hub.current
if not hub:
return event
client = get_client()

if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL:
if client.options["instrumenter"] != INSTRUMENTER.OTEL:
return event

if hasattr(event, "type") and event["type"] == "transaction":
Expand Down Expand Up @@ -116,25 +114,23 @@ def _prune_old_spans(self):

def on_start(self, otel_span, parent_context=None):
# type: (OTelSpan, Optional[SpanContext]) -> None
hub = Hub.current
if not hub:

@szokeasaurusrex szokeasaurusrex Mar 28, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does the dsn check make this check redundant? Trying to understand why we are removing this

@sentrivana sentrivana Mar 28, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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).

@antonpirker antonpirker Mar 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

return
client = get_client()

if not hub.client or (hub.client and not hub.client.dsn):
if not client.dsn:
return

try:
_ = Dsn(hub.client.dsn or "")
_ = Dsn(client.dsn)
except Exception:
return

if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL:
if client.options["instrumenter"] != INSTRUMENTER.OTEL:
return

if not otel_span.get_span_context().is_valid:
return

if self._is_sentry_span(hub, otel_span):
if self._is_sentry_span(otel_span):
return

trace_data = self._get_trace_data(otel_span, parent_context)
Expand All @@ -155,7 +151,7 @@ def on_start(self, otel_span, parent_context=None):
instrumenter=INSTRUMENTER.OTEL,
)
else:
sentry_span = hub.start_transaction(
sentry_span = start_transaction(
name=otel_span.name,
span_id=trace_data["span_id"],
parent_span_id=parent_span_id,
Expand All @@ -179,11 +175,9 @@ def on_start(self, otel_span, parent_context=None):

def on_end(self, otel_span):
# type: (OTelSpan) -> None
hub = Hub.current
if not hub:
return
client = get_client()

if hub.client and hub.client.options["instrumenter"] != INSTRUMENTER.OTEL:
if client.options["instrumenter"] != INSTRUMENTER.OTEL:
return

span_context = otel_span.get_span_context()
Expand Down Expand Up @@ -219,14 +213,18 @@ def on_end(self, otel_span):
self.open_spans.setdefault(span_start_in_minutes, set()).discard(span_id)
self._prune_old_spans()

def _is_sentry_span(self, hub, otel_span):
# type: (Hub, OTelSpan) -> bool
def _is_sentry_span(self, otel_span):
# type: (OTelSpan) -> bool
"""
Break infinite loop:
HTTP requests to Sentry are caught by OTel and send again to Sentry.
"""
otel_span_url = otel_span.attributes.get(SpanAttributes.HTTP_URL, None)
dsn_url = hub.client and Dsn(hub.client.dsn or "").netloc

dsn_url = None
client = get_client()
if client.dsn:
dsn_url = Dsn(client.dsn).netloc

if otel_span_url and dsn_url in otel_span_url:
return True
Expand Down
Loading