Skip to content

Fix some methods in POTelSpan #3492

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

Merged
merged 5 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
- Utility function `is_auto_session_tracking_enabled()` has been removed. There is no public replacement. There is a private `_is_auto_session_tracking_enabled()` (if you absolutely need this function) It accepts a `scope` parameter instead of the previously used `hub` parameter.
- Utility function `is_auto_session_tracking_enabled_scope()` has been removed. There is no public replacement. There is a private `_is_auto_session_tracking_enabled()` (if you absolutely need this function)
- Setting `scope.level` has been removed. Use `scope.set_level` instead.
- `span.containing_transaction` has been removed. Use `span.root_span` instead.
- `continue_from_headers`, `continue_from_environ` and `from_traceparent` have been removed, please use top-level API `sentry_sdk.continue_trace` instead.`


### Deprecated
Expand Down
40 changes: 19 additions & 21 deletions sentry_sdk/integrations/grpc/aio/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from sentry_sdk.consts import OP
from sentry_sdk.integrations import DidNotEnable
from sentry_sdk.integrations.grpc.consts import SPAN_ORIGIN
from sentry_sdk.tracing import Transaction, TRANSACTION_SOURCE_CUSTOM
from sentry_sdk.tracing import TRANSACTION_SOURCE_CUSTOM
from sentry_sdk.utils import event_from_exception

from typing import TYPE_CHECKING
Expand Down Expand Up @@ -44,26 +44,24 @@ async def wrapped(request, context):
return await handler(request, context)

# What if the headers are empty?
transaction = Transaction.continue_from_headers(
dict(context.invocation_metadata()),
op=OP.GRPC_SERVER,
name=name,
source=TRANSACTION_SOURCE_CUSTOM,
origin=SPAN_ORIGIN,
)

with sentry_sdk.start_transaction(transaction=transaction):
try:
return await handler.unary_unary(request, context)
except AbortError:
raise
except Exception as exc:
event, hint = event_from_exception(
exc,
mechanism={"type": "grpc", "handled": False},
)
sentry_sdk.capture_event(event, hint=hint)
raise
with sentry_sdk.continue_trace(dict(context.invocation_metadata())):
with sentry_sdk.start_transaction(
op=OP.GRPC_SERVER,
name=name,
source=TRANSACTION_SOURCE_CUSTOM,
origin=SPAN_ORIGIN,
):
try:
return await handler.unary_unary(request, context)
except AbortError:
raise
except Exception as exc:
event, hint = event_from_exception(
exc,
mechanism={"type": "grpc", "handled": False},
)
sentry_sdk.capture_event(event, hint=hint)
raise

elif not handler.request_streaming and handler.response_streaming:
handler_factory = grpc.unary_stream_rpc_method_handler
Expand Down
26 changes: 12 additions & 14 deletions sentry_sdk/integrations/grpc/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from sentry_sdk.consts import OP
from sentry_sdk.integrations import DidNotEnable
from sentry_sdk.integrations.grpc.consts import SPAN_ORIGIN
from sentry_sdk.tracing import Transaction, TRANSACTION_SOURCE_CUSTOM
from sentry_sdk.tracing import TRANSACTION_SOURCE_CUSTOM

from typing import TYPE_CHECKING

Expand Down Expand Up @@ -38,19 +38,17 @@ def behavior(request, context):
if name:
metadata = dict(context.invocation_metadata())

transaction = Transaction.continue_from_headers(
metadata,
op=OP.GRPC_SERVER,
name=name,
source=TRANSACTION_SOURCE_CUSTOM,
origin=SPAN_ORIGIN,
)

with sentry_sdk.start_transaction(transaction=transaction):
try:
return handler.unary_unary(request, context)
except BaseException as e:
raise e
with sentry_sdk.continue_trace(metadata):
with sentry_sdk.start_transaction(
op=OP.GRPC_SERVER,
name=name,
source=TRANSACTION_SOURCE_CUSTOM,
origin=SPAN_ORIGIN,
):
try:
return handler.unary_unary(request, context)
except BaseException as e:
raise e
else:
return handler.unary_unary(request, context)

Expand Down
92 changes: 30 additions & 62 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from opentelemetry import trace as otel_trace, context
from opentelemetry.trace import format_trace_id, format_span_id
from opentelemetry.trace.status import StatusCode
from opentelemetry.sdk.trace import ReadableSpan

import sentry_sdk
from sentry_sdk.consts import SPANSTATUS, SPANDATA
Expand Down Expand Up @@ -37,7 +38,6 @@
R = TypeVar("R")

import sentry_sdk.profiler
from sentry_sdk.scope import Scope
from sentry_sdk._types import (
Event,
MeasurementUnit,
Expand Down Expand Up @@ -1198,7 +1198,6 @@ def __init__(
op=None, # type: Optional[str]
description=None, # type: Optional[str]
status=None, # type: Optional[str]
scope=None, # type: Optional[Scope]
start_timestamp=None, # type: Optional[Union[datetime, float]]
origin=None, # type: Optional[str]
name=None, # type: Optional[str]
Expand All @@ -1218,10 +1217,9 @@ def __init__(
# OTel timestamps have nanosecond precision
start_timestamp = convert_to_otel_timestamp(start_timestamp)

# XXX deal with _otel_span being a NonRecordingSpan
self._otel_span = tracer.start_span(
description or op or "", start_time=start_timestamp
) # XXX
)

self.origin = origin or DEFAULT_SPAN_ORIGIN
self.op = op
Expand Down Expand Up @@ -1267,12 +1265,18 @@ def __exit__(self, ty, value, tb):
# XXX set status to error if unset and an exception occurred?
context.detach(self._ctx_token)

def _get_attribute(self, name):
# type: (str) -> Optional[Any]
if not isinstance(self._otel_span, ReadableSpan):
return None
Comment on lines +1270 to +1271
Copy link
Contributor

Choose a reason for hiding this comment

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

We can have a decorator at some point that does this, we could reuse that everywhere for this is-recording-span check. Don't have to do it now, just a note for later when we're cleaning stuff up

return self._otel_span.attributes.get(name)

@property
def description(self):
# type: () -> Optional[str]
from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute

return self._otel_span.attributes.get(SentrySpanAttribute.DESCRIPTION)
return self._get_attribute(SentrySpanAttribute.DESCRIPTION)

@description.setter
def description(self, value):
Expand All @@ -1287,7 +1291,7 @@ def origin(self):
# type: () -> Optional[str]
from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute

return self._otel_span.attributes.get(SentrySpanAttribute.ORIGIN)
return self._get_attribute(SentrySpanAttribute.ORIGIN)

@origin.setter
def origin(self, value):
Expand All @@ -1299,7 +1303,7 @@ def origin(self, value):

@property
def containing_transaction(self):
# type: () -> Optional[Transaction]
# type: () -> Optional[POTelSpan]
"""
Get the transaction this span is a child of.

Expand All @@ -1311,16 +1315,6 @@ def containing_transaction(self):
)
return self.root_span

@containing_transaction.setter
def containing_transaction(self, value):
# type: (Span) -> None
"""
Set this span's transaction.
.. deprecated:: 3.0.0
Use :func:`root_span` instead.
"""
pass

@property
def root_span(self):
# type: () -> Optional[POTelSpan]
Expand All @@ -1329,21 +1323,22 @@ def root_span(self):
# not sure if there's a way to retrieve the parent with pure otel.
return None

@root_span.setter
def root_span(self, value):
pass

@property
def is_root_span(self):
if isinstance(self._otel_span, otel_trace.NonRecordingSpan):
return False

return self._otel_span.parent is None
# type: () -> bool
return (
isinstance(self._otel_span, ReadableSpan) and self._otel_span.parent is None
)

@property
def parent_span_id(self):
# type: () -> Optional[str]
return self._otel_span.parent if hasattr(self._otel_span, "parent") else None
if (
not isinstance(self._otel_span, ReadableSpan)
or self._otel_span.parent is None
):
return None
return format_span_id(self._otel_span.parent.span_id)

@property
def trace_id(self):
Expand All @@ -1370,7 +1365,7 @@ def op(self):
# type: () -> Optional[str]
from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute

return self._otel_span.attributes.get(SentrySpanAttribute.OP)
return self._get_attribute(SentrySpanAttribute.OP)

@op.setter
def op(self, value):
Expand All @@ -1385,7 +1380,7 @@ def name(self):
# type: () -> Optional[str]
from sentry_sdk.integrations.opentelemetry.consts import SentrySpanAttribute

return self._otel_span.attributes.get(SentrySpanAttribute.NAME)
return self._get_attribute(SentrySpanAttribute.NAME)

@name.setter
def name(self, value):
Expand All @@ -1408,6 +1403,9 @@ def source(self, value):
@property
def start_timestamp(self):
# type: () -> Optional[datetime]
if not isinstance(self._otel_span, ReadableSpan):
return None

start_time = self._otel_span.start_time
if start_time is None:
return None
Expand All @@ -1421,6 +1419,9 @@ def start_timestamp(self):
@property
def timestamp(self):
# type: () -> Optional[datetime]
if not isinstance(self._otel_span, ReadableSpan):
return None

end_time = self._otel_span.end_time
if end_time is None:
return None
Expand All @@ -1432,49 +1433,16 @@ def timestamp(self):
return convert_from_otel_timestamp(end_time)

def start_child(self, **kwargs):
# type: (str, **Any) -> POTelSpan
# type: (**Any) -> POTelSpan
kwargs.setdefault("sampled", self.sampled)

span = POTelSpan(**kwargs)
return span

@classmethod
def continue_from_environ(
cls,
environ, # type: Mapping[str, str]
**kwargs, # type: Any
):
# type: (...) -> POTelSpan
# XXX actually propagate
span = POTelSpan(**kwargs)
return span

@classmethod
def continue_from_headers(
cls,
headers, # type: Mapping[str, str]
**kwargs, # type: Any
):
# type: (...) -> POTelSpan
# XXX actually propagate
span = POTelSpan(**kwargs)
return span

def iter_headers(self):
# type: () -> Iterator[Tuple[str, str]]
pass

@classmethod
def from_traceparent(
cls,
traceparent, # type: Optional[str]
**kwargs, # type: Any
):
# type: (...) -> Optional[Transaction]
# XXX actually propagate
span = POTelSpan(**kwargs)
return span

def to_traceparent(self):
# type: () -> str
if self.sampled is True:
Expand Down
Loading