diff --git a/sentry_sdk/_types.py b/sentry_sdk/_types.py index 41828cb9e3..9625859307 100644 --- a/sentry_sdk/_types.py +++ b/sentry_sdk/_types.py @@ -262,5 +262,3 @@ class SDKInfo(TypedDict): ) HttpStatusCodeRange = Union[int, Container[int]] - - OtelExtractedSpanData = tuple[str, str, Optional[str], Optional[int], Optional[str]] diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 8f67b127df..98552fb565 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -674,6 +674,8 @@ class OP: HTTP_CLIENT = "http.client" HTTP_CLIENT_STREAM = "http.client.stream" HTTP_SERVER = "http.server" + HTTP = "http" + MESSAGE = "message" MIDDLEWARE_DJANGO = "middleware.django" MIDDLEWARE_LITESTAR = "middleware.litestar" MIDDLEWARE_LITESTAR_RECEIVE = "middleware.litestar.receive" @@ -705,6 +707,7 @@ class OP: QUEUE_TASK_HUEY = "queue.task.huey" QUEUE_SUBMIT_RAY = "queue.submit.ray" QUEUE_TASK_RAY = "queue.task.ray" + RPC = "rpc" SUBPROCESS = "subprocess" SUBPROCESS_WAIT = "subprocess.wait" SUBPROCESS_COMMUNICATE = "subprocess.communicate" diff --git a/sentry_sdk/opentelemetry/span_processor.py b/sentry_sdk/opentelemetry/span_processor.py index cc18cad827..16d2f9e6b3 100644 --- a/sentry_sdk/opentelemetry/span_processor.py +++ b/sentry_sdk/opentelemetry/span_processor.py @@ -219,10 +219,8 @@ def _root_span_to_transaction_event(self, span: ReadableSpan) -> Optional[Event] if profile_context: contexts["profile"] = profile_context - (_, description, _, http_status, _) = span_data - - if http_status: - contexts["response"] = {"status_code": http_status} + if span_data.http_status: + contexts["response"] = {"status_code": span_data.http_status} if span.resource.attributes: contexts[OTEL_SENTRY_CONTEXT] = {"resource": dict(span.resource.attributes)} @@ -230,7 +228,7 @@ def _root_span_to_transaction_event(self, span: ReadableSpan) -> Optional[Event] event.update( { "type": "transaction", - "transaction": transaction_name or description, + "transaction": transaction_name or span_data.description, "transaction_info": {"source": transaction_source or "custom"}, "contexts": contexts, } @@ -257,19 +255,21 @@ def _span_to_json(self, span: ReadableSpan) -> Optional[dict[str, Any]]: span_id = format_span_id(span.context.span_id) parent_span_id = format_span_id(span.parent.span_id) if span.parent else None - (op, description, status, _, origin) = extract_span_data(span) + span_data = extract_span_data(span) span_json.update( { "trace_id": trace_id, "span_id": span_id, - "op": op, - "description": description, - "status": status, - "origin": origin or DEFAULT_SPAN_ORIGIN, + "description": span_data.description, + "origin": span_data.origin or DEFAULT_SPAN_ORIGIN, } ) + if span_data.op: + span_json["op"] = span_data.op + if span_data.status: + span_json["status"] = span_data.status if parent_span_id: span_json["parent_span_id"] = parent_span_id diff --git a/sentry_sdk/opentelemetry/utils.py b/sentry_sdk/opentelemetry/utils.py index 114b1dfd36..15bf9a6083 100644 --- a/sentry_sdk/opentelemetry/utils.py +++ b/sentry_sdk/opentelemetry/utils.py @@ -1,6 +1,7 @@ from __future__ import annotations import re from datetime import datetime, timezone +from dataclasses import dataclass from urllib3.util import parse_url as urlparse from urllib.parse import quote, unquote @@ -30,8 +31,8 @@ from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: - from typing import Any, Optional, Mapping, Sequence, Union, Type, TypeVar - from sentry_sdk._types import OtelExtractedSpanData + from typing import Any, Optional, Union, Type, TypeVar + from opentelemetry.util.types import Attributes T = TypeVar("T") @@ -111,190 +112,157 @@ def extract_transaction_name_source( ) -def extract_span_data(span: ReadableSpan) -> OtelExtractedSpanData: - op = span.name - description = span.name - status, http_status = extract_span_status(span) - origin = None - if span.attributes is None: - return (op, description, status, http_status, origin) +@dataclass +class ExtractedSpanData: + description: str + op: Optional[str] = None + status: Optional[str] = None + http_status: Optional[int] = None + origin: Optional[str] = None + + +def extract_span_data(span: ReadableSpan) -> ExtractedSpanData: + """ + Try to populate sane values for op, description and statuses based on what we have. + The op and description mapping is fundamentally janky because otel only has a single `name`. + + Priority is given first to attributes explicitly defined by us via the SDK. + Otherwise we try to infer sane values from other attributes. + """ + op = get_typed_attribute(span.attributes, SentrySpanAttribute.OP, str) or infer_op( + span + ) - attribute_op = get_typed_attribute(span.attributes, SentrySpanAttribute.OP, str) - op = attribute_op or op description = ( get_typed_attribute(span.attributes, SentrySpanAttribute.DESCRIPTION, str) - or description + or get_typed_attribute(span.attributes, SentrySpanAttribute.NAME, str) + or infer_description(span) ) - origin = get_typed_attribute(span.attributes, SentrySpanAttribute.ORIGIN, str) - http_method = get_typed_attribute(span.attributes, SpanAttributes.HTTP_METHOD, str) - if http_method: - return span_data_for_http_method(span) - - db_query = span.attributes.get(SpanAttributes.DB_SYSTEM) - if db_query: - return span_data_for_db_query(span) - - rpc_service = span.attributes.get(SpanAttributes.RPC_SERVICE) - if rpc_service: - return ( - attribute_op or "rpc", - description, - status, - http_status, - origin, - ) - - messaging_system = span.attributes.get(SpanAttributes.MESSAGING_SYSTEM) - if messaging_system: - return ( - attribute_op or "message", - description, - status, - http_status, - origin, - ) - - faas_trigger = span.attributes.get(SpanAttributes.FAAS_TRIGGER) - if faas_trigger: - return (str(faas_trigger), description, status, http_status, origin) + origin = get_typed_attribute(span.attributes, SentrySpanAttribute.ORIGIN, str) - return (op, description, status, http_status, origin) + (status, http_status) = extract_span_status(span) + return ExtractedSpanData( + description=description or span.name, + op=op, + status=status, + http_status=http_status, + origin=origin, + ) -def span_data_for_http_method(span: ReadableSpan) -> OtelExtractedSpanData: - span_attributes = span.attributes or {} - op = get_typed_attribute(span_attributes, SentrySpanAttribute.OP, str) - if op is None: - op = "http" +def infer_op(span: ReadableSpan) -> Optional[str]: + """ + Try to infer op for the various types of instrumentation. + """ + if span.attributes is None: + return None + if SpanAttributes.HTTP_METHOD in span.attributes: if span.kind == SpanKind.SERVER: - op += ".server" + return OP.HTTP_SERVER elif span.kind == SpanKind.CLIENT: - op += ".client" + return OP.HTTP_CLIENT + else: + return OP.HTTP + elif SpanAttributes.DB_SYSTEM in span.attributes: + return OP.DB + elif SpanAttributes.RPC_SERVICE in span.attributes: + return OP.RPC + elif SpanAttributes.MESSAGING_SYSTEM in span.attributes: + return OP.MESSAGE + elif SpanAttributes.FAAS_TRIGGER in span.attributes: + return get_typed_attribute(span.attributes, SpanAttributes.FAAS_TRIGGER, str) + else: + return None - http_method = span_attributes.get(SpanAttributes.HTTP_METHOD) - route = span_attributes.get(SpanAttributes.HTTP_ROUTE) - target = span_attributes.get(SpanAttributes.HTTP_TARGET) - peer_name = span_attributes.get(SpanAttributes.NET_PEER_NAME) - # TODO-neel-potel remove description completely - description = get_typed_attribute( - span_attributes, SentrySpanAttribute.DESCRIPTION, str - ) or get_typed_attribute(span_attributes, SentrySpanAttribute.NAME, str) - if description is None: - description = f"{http_method}" +def infer_description(span: ReadableSpan) -> Optional[str]: + if span.attributes is None: + return None + + if SpanAttributes.HTTP_METHOD in span.attributes: + http_method = get_typed_attribute( + span.attributes, SpanAttributes.HTTP_METHOD, str + ) + route = get_typed_attribute(span.attributes, SpanAttributes.HTTP_ROUTE, str) + target = get_typed_attribute(span.attributes, SpanAttributes.HTTP_TARGET, str) + peer_name = get_typed_attribute( + span.attributes, SpanAttributes.NET_PEER_NAME, str + ) + url = get_typed_attribute(span.attributes, SpanAttributes.HTTP_URL, str) if route: - description = f"{http_method} {route}" + return f"{http_method} {route}" elif target: - description = f"{http_method} {target}" + return f"{http_method} {target}" elif peer_name: - description = f"{http_method} {peer_name}" + return f"{http_method} {peer_name}" + elif url: + parsed_url = urlparse(url) + url = "{}://{}{}".format( + parsed_url.scheme, parsed_url.netloc, parsed_url.path + ) + return f"{http_method} {url}" else: - url = span_attributes.get(SpanAttributes.HTTP_URL) - url = get_typed_attribute(span_attributes, SpanAttributes.HTTP_URL, str) - - if url: - parsed_url = urlparse(url) - url = "{}://{}{}".format( - parsed_url.scheme, parsed_url.netloc, parsed_url.path - ) - description = f"{http_method} {url}" - - status, http_status = extract_span_status(span) - - origin = get_typed_attribute(span_attributes, SentrySpanAttribute.ORIGIN, str) - - return (op, description, status, http_status, origin) - - -def span_data_for_db_query(span: ReadableSpan) -> OtelExtractedSpanData: - span_attributes = span.attributes or {} - - op = get_typed_attribute(span_attributes, SentrySpanAttribute.OP, str) or OP.DB - statement = get_typed_attribute(span_attributes, SpanAttributes.DB_STATEMENT, str) - - description = statement or span.name - origin = get_typed_attribute(span_attributes, SentrySpanAttribute.ORIGIN, str) - - return (op, description, None, None, origin) + return http_method + elif SpanAttributes.DB_SYSTEM in span.attributes: + return get_typed_attribute(span.attributes, SpanAttributes.DB_STATEMENT, str) + else: + return None def extract_span_status(span: ReadableSpan) -> tuple[Optional[str], Optional[int]]: - span_attributes = span.attributes or {} - status = span.status or None - - if status: - inferred_status, http_status = infer_status_from_attributes(span_attributes) - - if status.status_code == StatusCode.OK: - return (SPANSTATUS.OK, http_status) - elif status.status_code == StatusCode.ERROR: - if status.description is None: - if inferred_status: - return (inferred_status, http_status) - - if http_status is not None: - return (inferred_status, http_status) - - if ( - status.description is not None - and status.description in GRPC_ERROR_MAP.values() - ): - return (status.description, None) - else: - return (SPANSTATUS.UNKNOWN_ERROR, None) - - inferred_status, http_status = infer_status_from_attributes(span_attributes) - if inferred_status: - return (inferred_status, http_status) - - if status and status.status_code == StatusCode.UNSET: - return (None, None) + """ + Extract a reasonable Sentry SPANSTATUS and a HTTP status code from the otel span. + OKs are simply OKs. + ERRORs first try to map to HTTP/GRPC statuses via attributes otherwise fallback + on the description if it is a valid status for Sentry. + In the final UNSET case, we try to infer HTTP/GRPC. + """ + status = span.status + http_status = get_http_status_code(span.attributes) + final_status = None + + if status.status_code == StatusCode.OK: + final_status = SPANSTATUS.OK + elif status.status_code == StatusCode.ERROR: + inferred_status = infer_status_from_attributes(span.attributes, http_status) + + if inferred_status is not None: + final_status = inferred_status + elif ( + status.description is not None + and status.description in GRPC_ERROR_MAP.values() + ): + final_status = status.description + else: + final_status = SPANSTATUS.UNKNOWN_ERROR else: - return (SPANSTATUS.UNKNOWN_ERROR, None) + # UNSET case + final_status = infer_status_from_attributes(span.attributes, http_status) + + return (final_status, http_status) def infer_status_from_attributes( - span_attributes: Mapping[ - str, - str - | bool - | int - | float - | Sequence[str] - | Sequence[bool] - | Sequence[int] - | Sequence[float], - ], -) -> tuple[Optional[str], Optional[int]]: - http_status = get_http_status_code(span_attributes) + span_attributes: Attributes, http_status: Optional[int] +) -> Optional[str]: + if span_attributes is None: + return None if http_status: - return (get_span_status_from_http_code(http_status), http_status) + return get_span_status_from_http_code(http_status) grpc_status = span_attributes.get(SpanAttributes.RPC_GRPC_STATUS_CODE) if grpc_status: - return (GRPC_ERROR_MAP.get(str(grpc_status), SPANSTATUS.UNKNOWN_ERROR), None) - - return (None, None) - - -def get_http_status_code( - span_attributes: Mapping[ - str, - str - | bool - | int - | float - | Sequence[str] - | Sequence[bool] - | Sequence[int] - | Sequence[float], - ], -) -> Optional[int]: + return GRPC_ERROR_MAP.get(str(grpc_status), SPANSTATUS.UNKNOWN_ERROR) + + return None + + +def get_http_status_code(span_attributes: Attributes) -> Optional[int]: try: http_status = get_typed_attribute( span_attributes, SpanAttributes.HTTP_RESPONSE_STATUS_CODE, int @@ -329,7 +297,7 @@ def extract_span_attributes(span: ReadableSpan, namespace: str) -> dict[str, Any def get_trace_context( - span: ReadableSpan, span_data: Optional[OtelExtractedSpanData] = None + span: ReadableSpan, span_data: Optional[ExtractedSpanData] = None ) -> dict[str, Any]: if not span.context: return {} @@ -341,27 +309,23 @@ def get_trace_context( if span_data is None: span_data = extract_span_data(span) - (op, _, status, _, origin) = span_data - trace_context: dict[str, Any] = { "trace_id": trace_id, "span_id": span_id, "parent_span_id": parent_span_id, - "op": op, - "origin": origin or DEFAULT_SPAN_ORIGIN, + "origin": span_data.origin or DEFAULT_SPAN_ORIGIN, } - if status: - trace_context["status"] = status - + if span_data.op: + trace_context["op"] = span_data.op + if span_data.status: + trace_context["status"] = span_data.status if span.attributes: trace_context["data"] = dict(span.attributes) trace_state = get_trace_state(span) trace_context["dynamic_sampling_context"] = dsc_from_trace_state(trace_state) - # TODO-neel-potel profiler thread_id, thread_name - return trace_context @@ -491,12 +455,12 @@ def get_profile_context(span: ReadableSpan) -> Optional[dict[str, str]]: return {"profiler_id": profiler_id} -def get_typed_attribute( - attributes: Mapping[str, Any], key: str, type: Type[T] -) -> Optional[T]: +def get_typed_attribute(attributes: Attributes, key: str, type: Type[T]) -> Optional[T]: """ helper method to coerce types of attribute values """ + if attributes is None: + return None value = attributes.get(key) if value is not None and isinstance(value, type): return value diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index a2dc58ae7e..dc3d4bc37a 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -187,7 +187,7 @@ def __init__( # OTel timestamps have nanosecond precision start_timestamp = convert_to_otel_timestamp(start_timestamp) - span_name = name or description or op or DEFAULT_SPAN_NAME + span_name = name or description or DEFAULT_SPAN_NAME # Prepopulate some attrs so that they're accessible in traces_sampler attributes = attributes or {} diff --git a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py index 6d89d85c91..47131f2d3d 100644 --- a/tests/integrations/clickhouse_driver/test_clickhouse_driver.py +++ b/tests/integrations/clickhouse_driver/test_clickhouse_driver.py @@ -1,7 +1,7 @@ """ Tests need a local clickhouse instance running, this can best be done using ```sh -docker run -d -p 18123:8123 -p9000:9000 --name clickhouse-test --ulimit nofile=262144:262144 --rm clickhouse +docker run -d -e CLICKHOUSE_SKIP_USER_SETUP=1 -p 8123:8123 -p 9000:9000 --name clickhouse-test --ulimit nofile=262144:262144 --rm clickhouse ``` """ @@ -822,7 +822,7 @@ def test_clickhouse_dbapi_spans(sentry_init, capture_events, capture_envelopes) span.pop("span_id", None) span.pop("start_timestamp", None) span.pop("timestamp", None) - span.pop("status") + span.pop("status", None) assert event["spans"] == expected_spans diff --git a/tests/integrations/django/test_cache_module.py b/tests/integrations/django/test_cache_module.py index 2d8cc3d5d6..a8a6b745c8 100644 --- a/tests/integrations/django/test_cache_module.py +++ b/tests/integrations/django/test_cache_module.py @@ -536,12 +536,13 @@ def test_cache_spans_get_many( cache.get_many([f"S{id}", f"S{id+1}"]) (transaction,) = events + assert transaction["transaction"] == "caches" assert len(transaction["spans"]) == 7 assert ( render_span_tree(transaction) == f"""\ -- op="caches": description=null +- op=null: description=null - op="cache.get": description="S{id}, S{id+1}" - op="cache.get": description="S{id}" - op="cache.get": description="S{id+1}" @@ -579,12 +580,13 @@ def test_cache_spans_set_many( cache.get(f"S{id}") (transaction,) = events + assert transaction["transaction"] == "caches" assert len(transaction["spans"]) == 4 assert ( render_span_tree(transaction) == f"""\ -- op="caches": description=null +- op=null: description=null - op="cache.put": description="S{id}, S{id+1}" - op="cache.put": description="S{id}" - op="cache.put": description="S{id+1}" diff --git a/tests/integrations/redis/test_redis.py b/tests/integrations/redis/test_redis.py index 68c49ae90e..4faf4b7fa2 100644 --- a/tests/integrations/redis/test_redis.py +++ b/tests/integrations/redis/test_redis.py @@ -60,7 +60,7 @@ def test_redis_pipeline( events = capture_events() connection = FakeStrictRedis() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): pipeline = connection.pipeline(transaction=is_transaction) pipeline.get("foo") pipeline.set("bar", 1) @@ -94,16 +94,17 @@ def test_sensitive_data(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeStrictRedis() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): connection.get( "this is super secret" ) # because fakeredis does not support AUTH we use GET instead (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == """\ -- op="": description=null +- op=null: description=null - op="db.redis": description="GET [Filtered]"\ """ ) @@ -117,17 +118,18 @@ def test_pii_data_redacted(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeStrictRedis() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): connection.set("somekey1", "my secret string1") connection.set("somekey2", "my secret string2") connection.get("somekey2") connection.delete("somekey1", "somekey2") (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == """\ -- op="": description=null +- op=null: description=null - op="db.redis": description="SET 'somekey1' [Filtered]" - op="db.redis": description="SET 'somekey2' [Filtered]" - op="db.redis": description="GET 'somekey2'" @@ -145,17 +147,18 @@ def test_pii_data_sent(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeStrictRedis() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): connection.set("somekey1", "my secret string1") connection.set("somekey2", "my secret string2") connection.get("somekey2") connection.delete("somekey1", "somekey2") (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == """\ -- op="": description=null +- op=null: description=null - op="db.redis": description="SET 'somekey1' 'my secret string1'" - op="db.redis": description="SET 'somekey2' 'my secret string2'" - op="db.redis": description="GET 'somekey2'" @@ -173,17 +176,18 @@ def test_data_truncation(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeStrictRedis() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): long_string = "a" * 100000 connection.set("somekey1", long_string) short_string = "b" * 10 connection.set("somekey2", short_string) (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == f"""\ -- op="": description=null +- op=null: description=null - op="db.redis": description="SET 'somekey1' '{long_string[: 1024 - len("...") - len("SET 'somekey1' '")]}..." - op="db.redis": description="SET 'somekey2' 'bbbbbbbbbb'"\ """ # noqa: E221 @@ -199,17 +203,18 @@ def test_data_truncation_custom(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeStrictRedis() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): long_string = "a" * 100000 connection.set("somekey1", long_string) short_string = "b" * 10 connection.set("somekey2", short_string) (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == f"""\ -- op="": description=null +- op=null: description=null - op="db.redis": description="SET 'somekey1' '{long_string[: 30 - len("...") - len("SET 'somekey1' '")]}..." - op="db.redis": description="SET 'somekey2' '{short_string}'"\ """ # noqa: E221 @@ -268,7 +273,7 @@ def test_db_connection_attributes_client(sentry_init, capture_events): ) events = capture_events() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): connection = FakeStrictRedis(connection_pool=MOCK_CONNECTION_POOL) connection.get("foobar") @@ -290,7 +295,7 @@ def test_db_connection_attributes_pipeline(sentry_init, capture_events): ) events = capture_events() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): connection = FakeStrictRedis(connection_pool=MOCK_CONNECTION_POOL) pipeline = connection.pipeline(transaction=False) pipeline.get("foo") diff --git a/tests/integrations/redis/test_redis_cache_module.py b/tests/integrations/redis/test_redis_cache_module.py index 75f58d346d..ea597e0d92 100644 --- a/tests/integrations/redis/test_redis_cache_module.py +++ b/tests/integrations/redis/test_redis_cache_module.py @@ -28,10 +28,11 @@ def test_no_cache_basic(sentry_init, capture_events, render_span_tree): connection.get("mycachekey") (event,) = events + assert event["transaction"] == "cache" assert ( render_span_tree(event) == """\ -- op="cache": description=null +- op=null: description=null - op="db.redis": description="GET 'mycachekey'"\ """ ) @@ -57,11 +58,12 @@ def test_cache_basic(sentry_init, capture_events, render_span_tree): connection.mget("mycachekey1", "mycachekey2") (event,) = events + assert event["transaction"] == "cache" # no cache support for HGET command assert ( render_span_tree(event) == """\ -- op="cache": description=null +- op=null: description=null - op="db.redis": description="HGET 'mycachekey' [Filtered]" - op="cache.get": description="mycachekey" - op="db.redis": description="GET 'mycachekey'" @@ -94,10 +96,11 @@ def test_cache_keys(sentry_init, capture_events, render_span_tree): connection.get("bl") (event,) = events + assert event["transaction"] == "cache" assert ( render_span_tree(event) == """\ -- op="cache": description=null +- op=null: description=null - op="db.redis": description="GET 'somethingelse'" - op="cache.get": description="blub" - op="db.redis": description="GET 'blub'" @@ -126,6 +129,7 @@ def test_cache_data(sentry_init, capture_events): connection.get("mycachekey") (event,) = events + assert event["transaction"] == "cache" spans = sorted(event["spans"], key=lambda x: x["start_timestamp"]) assert len(spans) == 6 @@ -214,6 +218,7 @@ def test_cache_prefixes(sentry_init, capture_events): connection.mget(uuid.uuid4().bytes, "yes") (event,) = events + assert event["transaction"] == "cache" spans = sorted(event["spans"], key=lambda x: x["start_timestamp"]) assert len(spans) == 13 # 8 db spans + 5 cache spans diff --git a/tests/integrations/redis/test_redis_cache_module_async.py b/tests/integrations/redis/test_redis_cache_module_async.py index d4ce4936bb..cd1804a9a3 100644 --- a/tests/integrations/redis/test_redis_cache_module_async.py +++ b/tests/integrations/redis/test_redis_cache_module_async.py @@ -31,14 +31,15 @@ async def test_no_cache_basic(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeRedisAsync() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): await connection.get("myasynccachekey") (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == """\ -- op="": description=null +- op=null: description=null - op="db.redis": description="GET 'myasynccachekey'"\ """ ) @@ -57,14 +58,15 @@ async def test_cache_basic(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeRedisAsync() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): await connection.get("myasynccachekey") (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == """\ -- op="": description=null +- op=null: description=null - op="cache.get": description="myasynccachekey" - op="db.redis": description="GET 'myasynccachekey'"\ """ @@ -84,17 +86,18 @@ async def test_cache_keys(sentry_init, capture_events, render_span_tree): events = capture_events() connection = FakeRedisAsync() - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): await connection.get("asomethingelse") await connection.get("ablub") await connection.get("ablubkeything") await connection.get("abl") (event,) = events + assert event["transaction"] == "redis" assert ( render_span_tree(event) == """\ -- op="": description=null +- op=null: description=null - op="db.redis": description="GET 'asomethingelse'" - op="cache.get": description="ablub" - op="db.redis": description="GET 'ablub'" @@ -118,12 +121,13 @@ async def test_cache_data(sentry_init, capture_events): events = capture_events() connection = FakeRedisAsync(host="mycacheserver.io", port=6378) - with sentry_sdk.start_span(): + with sentry_sdk.start_span(name="redis"): await connection.get("myasynccachekey") await connection.set("myasynccachekey", "事实胜于雄辩") await connection.get("myasynccachekey") (event,) = events + assert event["transaction"] == "redis" spans = sorted(event["spans"], key=lambda x: x["start_timestamp"]) assert len(spans) == 6 diff --git a/tests/integrations/sqlalchemy/test_sqlalchemy.py b/tests/integrations/sqlalchemy/test_sqlalchemy.py index 999b17a19f..6e030f57a4 100644 --- a/tests/integrations/sqlalchemy/test_sqlalchemy.py +++ b/tests/integrations/sqlalchemy/test_sqlalchemy.py @@ -125,6 +125,7 @@ class Address(Base): session.query(Person).first() (event,) = events + assert event["transaction"] == "test_transaction" for span in event["spans"]: assert span["data"][SPANDATA.DB_SYSTEM] == "sqlite" @@ -135,7 +136,7 @@ class Address(Base): assert ( render_span_tree(event) == """\ -- op="test_transaction": description=null +- op=null: description=null - op="db": description="SAVEPOINT sa_savepoint_1" - op="db": description="SELECT person.id AS person_id, person.name AS person_name \\nFROM person\\n LIMIT ? OFFSET ?" - op="db": description="RELEASE SAVEPOINT sa_savepoint_1" diff --git a/tests/opentelemetry/test_potel.py b/tests/opentelemetry/test_potel.py index 341d6554a2..e99b530a71 100644 --- a/tests/opentelemetry/test_potel.py +++ b/tests/opentelemetry/test_potel.py @@ -40,7 +40,6 @@ def test_root_span_transaction_payload_started_with_otel_only(capture_envelopes) assert "trace_id" in trace_context assert "span_id" in trace_context assert trace_context["origin"] == "manual" - assert trace_context["op"] == "request" assert payload["spans"] == [] @@ -57,7 +56,6 @@ def test_child_span_payload_started_with_otel_only(capture_envelopes): payload = item.payload.json (span,) = payload["spans"] - assert span["op"] == "db" assert span["description"] == "db" assert span["origin"] == "manual" assert span["span_id"] is not None @@ -82,9 +80,9 @@ def test_children_span_nesting_started_with_otel_only(capture_envelopes): payload = item.payload.json (db_span, http_span, redis_span) = payload["spans"] - assert db_span["op"] == "db" - assert redis_span["op"] == "redis" - assert http_span["op"] == "http" + assert db_span["description"] == "db" + assert redis_span["description"] == "redis" + assert http_span["description"] == "http" assert db_span["trace_id"] == payload["contexts"]["trace"]["trace_id"] assert redis_span["trace_id"] == payload["contexts"]["trace"]["trace_id"] @@ -121,7 +119,6 @@ def test_root_span_transaction_payload_started_with_sentry_only(capture_envelope assert "trace_id" in trace_context assert "span_id" in trace_context assert trace_context["origin"] == "manual" - assert trace_context["op"] == "request" assert trace_context["status"] == "ok" assert payload["spans"] == [] @@ -139,7 +136,6 @@ def test_child_span_payload_started_with_sentry_only(capture_envelopes): payload = item.payload.json (span,) = payload["spans"] - assert span["op"] == "db" assert span["description"] == "db" assert span["origin"] == "manual" assert span["status"] == "ok" @@ -165,9 +161,9 @@ def test_children_span_nesting_started_with_sentry_only(capture_envelopes): payload = item.payload.json (db_span, http_span, redis_span) = payload["spans"] - assert db_span["op"] == "db" - assert redis_span["op"] == "redis" - assert http_span["op"] == "http" + assert db_span["description"] == "db" + assert redis_span["description"] == "redis" + assert http_span["description"] == "http" assert db_span["trace_id"] == payload["contexts"]["trace"]["trace_id"] assert redis_span["trace_id"] == payload["contexts"]["trace"]["trace_id"] @@ -193,9 +189,9 @@ def test_children_span_nesting_mixed(capture_envelopes): payload = item.payload.json (db_span, http_span, redis_span) = payload["spans"] - assert db_span["op"] == "db" - assert redis_span["op"] == "redis" - assert http_span["op"] == "http" + assert db_span["description"] == "db" + assert redis_span["description"] == "redis" + assert http_span["description"] == "http" assert db_span["trace_id"] == payload["contexts"]["trace"]["trace_id"] assert redis_span["trace_id"] == payload["contexts"]["trace"]["trace_id"] diff --git a/tests/opentelemetry/test_utils.py b/tests/opentelemetry/test_utils.py index a73efd9b3b..6313e6ccc1 100644 --- a/tests/opentelemetry/test_utils.py +++ b/tests/opentelemetry/test_utils.py @@ -7,8 +7,6 @@ from sentry_sdk.opentelemetry.utils import ( extract_span_data, extract_span_status, - span_data_for_db_query, - span_data_for_http_method, ) from sentry_sdk.utils import parse_version @@ -23,7 +21,7 @@ Status(StatusCode.UNSET), {}, { - "op": "OTel Span Blank", + "op": None, "description": "OTel Span Blank", "status": None, "http_status_code": None, @@ -80,13 +78,13 @@ def test_extract_span_data(name, status, attributes, expected): otel_span.status = Status(StatusCode.UNSET) otel_span.attributes = attributes - op, description, status, http_status_code, origin = extract_span_data(otel_span) + span_data = extract_span_data(otel_span) result = { - "op": op, - "description": description, - "status": status, - "http_status_code": http_status_code, - "origin": origin, + "op": span_data.op, + "description": span_data.description, + "status": span_data.status, + "http_status_code": span_data.http_status, + "origin": span_data.origin, } assert result == expected @@ -180,15 +178,14 @@ def test_span_data_for_http_method(kind, status, attributes, expected): otel_span.status = status otel_span.attributes = attributes - op, description, status, http_status_code, origin = span_data_for_http_method( - otel_span - ) + span_data = extract_span_data(otel_span) + result = { - "op": op, - "description": description, - "status": status, - "http_status_code": http_status_code, - "origin": origin, + "op": span_data.op, + "description": span_data.description, + "status": span_data.status, + "http_status_code": span_data.http_status, + "origin": span_data.origin, } assert result == expected @@ -197,52 +194,31 @@ def test_span_data_for_db_query(): otel_span = MagicMock() otel_span.name = "OTel Span" otel_span.attributes = {} + otel_span.status = Status(StatusCode.UNSET) - op, description, status, http_status, origin = span_data_for_db_query(otel_span) - assert op == "db" - assert description == "OTel Span" - assert status is None - assert http_status is None - assert origin is None + span_data = extract_span_data(otel_span) + assert span_data.op is None + assert span_data.description == "OTel Span" + assert span_data.status is None + assert span_data.http_status is None + assert span_data.origin is None - otel_span.attributes = {"db.statement": "SELECT * FROM table;"} + otel_span.attributes = { + "db.system": "mysql", + "db.statement": "SELECT * FROM table;", + } - op, description, status, http_status, origin = span_data_for_db_query(otel_span) - assert op == "db" - assert description == "SELECT * FROM table;" - assert status is None - assert http_status is None - assert origin is None + span_data = extract_span_data(otel_span) + assert span_data.op == "db" + assert span_data.description == "SELECT * FROM table;" + assert span_data.status is None + assert span_data.http_status is None + assert span_data.origin is None @pytest.mark.parametrize( "kind, status, attributes, expected", [ - ( - SpanKind.CLIENT, - None, # None means unknown error - { - "http.method": "POST", - "http.route": "/some/route", - }, - { - "status": "unknown_error", - "http_status_code": None, - }, - ), - ( - SpanKind.CLIENT, - None, - { - "http.method": "POST", - "http.route": "/some/route", - "http.status_code": 502, # Take this status in case of None status - }, - { - "status": "internal_error", - "http_status_code": 502, - }, - ), ( SpanKind.SERVER, Status(StatusCode.UNSET), @@ -268,23 +244,6 @@ def test_span_data_for_db_query(): "http_status_code": 502, }, ), - ( - SpanKind.SERVER, - None, - { - "http.method": "POST", - "http.route": "/some/route", - "http.status_code": 502, - "http.response.status_code": 503, # this takes precedence over deprecated http.status_code - }, - { - "status": "unavailable", - "http_status_code": 503, - # old otel versions won't take the new attribute into account - "status_old": "internal_error", - "http_status_code_old": 502, - }, - ), ( SpanKind.SERVER, Status(StatusCode.UNSET),