-
Notifications
You must be signed in to change notification settings - Fork 614
feat(span-first): Support before_send_span
#6239
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
Changes from all commits
d997009
7448f04
661d15e
58caff9
a61397f
3af5562
f7178a0
4339f3f
0c165af
2243832
4c26439
0d543b9
b5061e9
280a120
8aca36a
9221b12
e65b1c2
177cc5b
d0e102a
be1f8d2
5fa6c96
3c2cc83
b386fe9
cc76bbb
7436a3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -34,7 +34,7 @@ | |||||
| from sentry_sdk.scrubber import EventScrubber | ||||||
| from sentry_sdk.serializer import serialize | ||||||
| from sentry_sdk.sessions import SessionFlusher | ||||||
| from sentry_sdk.traces import SpanStatus | ||||||
| from sentry_sdk.traces import SpanStatus, StreamedSpan | ||||||
| from sentry_sdk.tracing import trace | ||||||
| from sentry_sdk.tracing_utils import has_span_streaming_enabled | ||||||
| from sentry_sdk.transport import ( | ||||||
|
|
@@ -51,7 +51,8 @@ | |||||
| env_to_bool, | ||||||
| format_timestamp, | ||||||
| get_before_send_log, | ||||||
| get_before_send_metric, | ||||||
|
Check warning on line 54 in sentry_sdk/client.py
|
||||||
| get_before_send_span, | ||||||
| get_default_release, | ||||||
| get_sdk_name, | ||||||
| get_type_name, | ||||||
|
|
@@ -1169,34 +1170,72 @@ | |||||
| ty: str, | ||||||
| scope: "Scope", | ||||||
| ) -> None: | ||||||
| # Capture attributes-based telemetry (logs, metrics, spansV2) | ||||||
| """ | ||||||
| Capture attributes-based telemetry (logs, metrics, streamed spans). | ||||||
|
|
||||||
| Apply any attributes set on the scope to it, and run the user's | ||||||
| before_send_{telemetry} on it, if applicable. | ||||||
| """ | ||||||
| if telemetry is None: | ||||||
| return | ||||||
|
|
||||||
| scope.apply_to_telemetry(telemetry) | ||||||
|
|
||||||
| before_send = None | ||||||
|
|
||||||
| if ty == "log": | ||||||
| before_send = get_before_send_log(self.options) | ||||||
| serialized = telemetry | ||||||
|
|
||||||
| elif ty == "metric": | ||||||
| before_send = get_before_send_metric(self.options) | ||||||
| serialized = telemetry | ||||||
|
|
||||||
| elif ty == "span": | ||||||
| before_send = get_before_send_span(self.options) | ||||||
| serialized = telemetry._to_json() # type: ignore[union-attr] | ||||||
|
|
||||||
| if before_send is not None: | ||||||
| telemetry = before_send(telemetry, {}) # type: ignore | ||||||
| serialized = before_send(serialized, {}) # type: ignore[arg-type] | ||||||
|
sentry-warden[bot] marked this conversation as resolved.
|
||||||
|
|
||||||
| if ty in ("log", "metric"): | ||||||
| # Logs and metrics can be dropped in their respective | ||||||
| # before_send, so if we get None, don't queue them for sending. | ||||||
| if serialized is None: | ||||||
| return | ||||||
|
|
||||||
| elif ty == "span" and isinstance(telemetry, StreamedSpan): | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||||||
| # Spans can't be dropped in before_send_span by design. They can | ||||||
| # be altered though (e.g. to sanitize). Only allow changes to | ||||||
| # name and attributes. | ||||||
| if isinstance(serialized, dict) and "name" in serialized: | ||||||
| telemetry.name = serialized["name"] # type: ignore[typeddict-item] | ||||||
| telemetry._attributes = {} | ||||||
| for k, v in (serialized.get("attributes") or {}).items(): | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to my comment above, you can remove the
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same concern as above -- there's a case where the two are not equivalent and I'd opt for the more paranoid version |
||||||
| telemetry.set_attribute(k, v) | ||||||
|
|
||||||
|
sentrivana marked this conversation as resolved.
|
||||||
| else: | ||||||
| logger.debug( | ||||||
| "[Tracing] Invalid return value from before_send_span. Keeping original span." | ||||||
| ) | ||||||
|
|
||||||
| if telemetry is None: | ||||||
| return | ||||||
| serialized = telemetry._to_json() | ||||||
|
|
||||||
| batcher = None | ||||||
| if ty == "log": | ||||||
| batcher = self.log_batcher | ||||||
|
|
||||||
| elif ty == "metric": | ||||||
| batcher = self.metrics_batcher | ||||||
|
|
||||||
| elif ty == "span": | ||||||
| # We need a reference to the segment span in the batcher to populate | ||||||
| # the dynamic sampling context (DSC) | ||||||
| serialized["_segment_span"] = telemetry._segment # type: ignore | ||||||
| batcher = self.span_batcher | ||||||
|
|
||||||
| if batcher is not None: | ||||||
| batcher.add(telemetry) # type: ignore | ||||||
| batcher.add(serialized) # type: ignore | ||||||
|
|
||||||
| def _capture_log(self, log: "Optional[Log]", scope: "Scope") -> None: | ||||||
| self._capture_telemetry(log, "log", scope) | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1079,19 +1079,19 @@ class Person(Base): | |
|
|
||
| class fake_record_sql_queries: # noqa: N801 | ||
| def __init__(self, *args, **kwargs): | ||
| with record_sql_queries_supporting_streaming( | ||
| self._ctx_mgr = record_sql_queries_supporting_streaming( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Had to change this test because it was failing -- it was closing the span too early. |
||
| *args, **kwargs | ||
| ) as span: | ||
| self.span = span | ||
| ) | ||
|
|
||
| def __enter__(self): | ||
| self.span = self._ctx_mgr.__enter__() | ||
| self.span._start_timestamp = datetime(2024, 1, 1, microsecond=0) | ||
| self.span._end_timestamp = datetime(2024, 1, 1, microsecond=101000) | ||
|
|
||
| def __enter__(self): | ||
| return self.span | ||
|
|
||
| def __exit__(self, type, value, traceback): | ||
| pass | ||
| self.span._end_timestamp = None | ||
| self._ctx_mgr.__exit__(type, value, traceback) | ||
|
|
||
| with mock.patch( | ||
| "sentry_sdk.integrations.sqlalchemy.record_sql_queries_supporting_streaming", | ||
|
|
||
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.
This can be made slightly more concise by doing the following:
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.
If
item["attributes"]exists and isNone, thenitem.get("attributes", {})would returnNoneand the subsequent.values()call would fail.FWIW I don't think
attributescan beNoneat this point, but wrote it that way just to be extra safe.