Skip to content
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
145 changes: 116 additions & 29 deletions sentry_sdk/integrations/strawberry.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
from sentry_sdk.integrations import DidNotEnable, Integration, _check_minimum_version
from sentry_sdk.integrations.logging import ignore_logger
from sentry_sdk.scope import should_send_default_pii
from sentry_sdk.tracing import TransactionSource
from sentry_sdk.traces import SegmentSource
from sentry_sdk.tracing import Span, TransactionSource
from sentry_sdk.tracing_utils import StreamedSpan, has_span_streaming_enabled
Comment thread
ericapisani marked this conversation as resolved.
from sentry_sdk.utils import (
capture_internal_exceptions,
ensure_integration_enabled,
Expand Down Expand Up @@ -183,50 +185,111 @@
event_processor = _make_request_event_processor(self.execution_context)
scope.add_event_processor(event_processor)

graphql_span = sentry_sdk.start_span(
op=op,
name=description,
origin=StrawberryIntegration.origin,
)
graphql_span.__enter__()
client = sentry_sdk.get_client()
is_span_streaming_enabled = has_span_streaming_enabled(client.options)
if is_span_streaming_enabled:
additional_attributes: "dict[str, Any]" = {}

if should_send_default_pii():
additional_attributes["graphql.document"] = self.execution_context.query

if operation_name:
additional_attributes["graphql.operation.name"] = operation_name
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The operation_name may be None which is why we need this conditional here.


graphql_span = sentry_sdk.traces.start_span(
name=description,
attributes={
"sentry.origin": StrawberryIntegration.origin,
"sentry.op": op,
"graphql.operation.type": operation_type,
**additional_attributes,
},
)
else:
graphql_span = sentry_sdk.start_span(
op=op,
name=description,
origin=StrawberryIntegration.origin,
)
graphql_span.__enter__()

if type(graphql_span) is Span:
if should_send_default_pii():
graphql_span.set_data("graphql.document", self.execution_context.query)

graphql_span.set_data("graphql.operation.type", operation_type)
graphql_span.set_data("graphql.operation.name", operation_name)
if should_send_default_pii():
graphql_span.set_data("graphql.document", self.execution_context.query)
graphql_span.set_data("graphql.resource_name", self._resource_name)
graphql_span.set_data("graphql.operation.type", operation_type)
graphql_span.set_data("graphql.operation.name", operation_name)
# This attribute is being removed in streamed spans
graphql_span.set_data("graphql.resource_name", self._resource_name)

yield

transaction = graphql_span.containing_transaction
if transaction and self.execution_context.operation_name:
transaction.name = self.execution_context.operation_name
transaction.source = TransactionSource.COMPONENT
transaction.op = op
if type(graphql_span) is StreamedSpan:
if self.execution_context.operation_name:
segment = graphql_span._segment
segment.set_attribute("sentry.span.source", SegmentSource.COMPONENT)
segment.set_attribute("sentry.op", op)
segment.name = self.execution_context.operation_name
elif isinstance(graphql_span, Span):
transaction = graphql_span.containing_transaction
if transaction and self.execution_context.operation_name:
transaction.name = self.execution_context.operation_name
transaction.source = TransactionSource.COMPONENT
transaction.op = op

graphql_span.__exit__(None, None, None)

def on_validate(self) -> "Generator[None, None, None]":
validation_span = sentry_sdk.start_span(
op=OP.GRAPHQL_VALIDATE,
name="validation",
origin=StrawberryIntegration.origin,
)
client = sentry_sdk.get_client()
is_span_streaming_enabled = has_span_streaming_enabled(client.options)

if is_span_streaming_enabled:
validation_span = sentry_sdk.traces.start_span(
name="validation",
attributes={
"sentry.op": OP.GRAPHQL_VALIDATE,
"sentry.origin": StrawberryIntegration.origin,
},
)
else:
validation_span = sentry_sdk.start_span(
op=OP.GRAPHQL_VALIDATE,
name="validation",
origin=StrawberryIntegration.origin,
)

yield

validation_span.finish()
if isinstance(validation_span, StreamedSpan):
validation_span.end()
else:
validation_span.finish()
Comment thread
ericapisani marked this conversation as resolved.

def on_parse(self) -> "Generator[None, None, None]":
parsing_span = sentry_sdk.start_span(
op=OP.GRAPHQL_PARSE,
name="parsing",
origin=StrawberryIntegration.origin,
)
client = sentry_sdk.get_client()
is_span_streaming_enabled = has_span_streaming_enabled(client.options)

if is_span_streaming_enabled:
parsing_span = sentry_sdk.traces.start_span(
name="parsing",
attributes={
"sentry.op": OP.GRAPHQL_PARSE,
"sentry.origin": StrawberryIntegration.origin,
},
)
else:
parsing_span = sentry_sdk.start_span(
op=OP.GRAPHQL_PARSE,
name="parsing",
origin=StrawberryIntegration.origin,
)

Check warning on line 286 in sentry_sdk/integrations/strawberry.py

View check run for this annotation

@sentry/warden / warden: find-bugs

StreamedSpan scope leak in on_validate and on_parse on exception paths

In the streaming path, `sentry_sdk.traces.start_span()` immediately sets the span as the active scope span via `_start()` in `__init__`, but `end()` is only called after the bare `yield` with no try/finally guard. If an exception is thrown into the generator (e.g. via `generator.throw()` or `generator.close()`), `end()` is never called and the scope is left with the validation/parse span still active — causing any subsequent spans to be mis-parented. The non-streaming path never set these spans on scope at all (no `__enter__()` call), so this scope-leak is new behavior introduced by the streaming path. The `resolve` streaming path correctly uses a `with` statement for the same reason.
yield

parsing_span.finish()
if isinstance(parsing_span, StreamedSpan):
parsing_span.end()
else:
parsing_span.finish()
Comment thread
ericapisani marked this conversation as resolved.

def should_skip_tracing(
self,
Expand Down Expand Up @@ -263,6 +326,18 @@

field_path = "{}.{}".format(info.parent_type, info.field_name)

client = sentry_sdk.get_client()
is_span_streaming_enabled = has_span_streaming_enabled(client.options)
if is_span_streaming_enabled:
with sentry_sdk.traces.start_span(
name=f"resolving {field_path}",
attributes={
"sentry.origin": StrawberryIntegration.origin,
"sentry.op": OP.GRAPHQL_RESOLVE,
},
):
return await self._resolve(_next, root, info, *args, **kwargs)

Check warning on line 340 in sentry_sdk/integrations/strawberry.py

View check run for this annotation

@sentry/warden / warden: code-review

Resolver span drops GraphQL field attributes in streaming mode

The streaming path for both `async def resolve` and `def resolve` creates a resolver span with only `sentry.origin` and `sentry.op`, omitting `graphql.field_name`, `graphql.parent_type`, `graphql.field_path`, and `graphql.path` that the non-streaming path sets — add these as attributes on the streaming span to preserve parity.
with sentry_sdk.start_span(
op=OP.GRAPHQL_RESOLVE,
name="resolving {}".format(field_path),
Expand Down Expand Up @@ -290,6 +365,18 @@

field_path = "{}.{}".format(info.parent_type, info.field_name)

client = sentry_sdk.get_client()
is_span_streaming_enabled = has_span_streaming_enabled(client.options)
if is_span_streaming_enabled:
with sentry_sdk.traces.start_span(
name=f"resolving {field_path}",
attributes={
"sentry.origin": StrawberryIntegration.origin,
"sentry.op": OP.GRAPHQL_RESOLVE,
},
):
return _next(root, info, *args, **kwargs)

Comment thread
ericapisani marked this conversation as resolved.
with sentry_sdk.start_span(
op=OP.GRAPHQL_RESOLVE,
name="resolving {}".format(field_path),
Expand Down
Loading
Loading