From 5cc1bea3fb8fbbc807e8c0ee1653c975f7f8d032 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Thu, 30 Jul 2026 15:07:40 -0700 Subject: [PATCH 1/6] feat(tracing): Enable Sentry span streaming mode Turn on `trace_lifecycle="stream"` so spans are sent to Sentry in batches as they finish, instead of buffering a whole trace in memory and shipping it as a single transaction event. This removes the 1000-span-per-transaction cap, keeps memory flat in long-running consumers, and preserves spans that already finished when a process is killed mid-trace (e.g. an OOM-kill). Stream mode disables the legacy tracing API, and it fails silently: `start_span()`, `start_transaction()` and `update_current_span()` become no-ops, and `get_current_span()` / `scope.span` / `scope.transaction` return None. Every call site therefore has to move to `sentry_sdk.traces` in the same change or tracing goes dark with no error. `op` becomes the `sentry.op` attribute, `description` becomes the span name, and `set_data`/`set_tag` on a span become `set_attribute`. Scope tags and scope attributes reach disjoint destinations: tags land on error events only, attributes on spans only. Query-path tags are converted to attributes so they stay queryable on trace data; the consumer and error-reporting paths keep `set_tag`, since they have no active span and would otherwise lose the data entirely. Three things beyond the mechanical translation: - Several spans passed raw SQL as `description`, which becomes the span name -- the grouping key -- under stream mode. Give them stable names and move the SQL to a `db.query.text` attribute. - `metrics.incr`/`metrics.timing` no longer exist in sentry-sdk 2.66.1, so SentryMetricsBackend was a latent AttributeError. Map them to `count` and `distribution`. - Fix three pre-existing Mapping-vs-dict type errors in ClickhouseConnectPool. They were previously unreported because the mypy pre-commit hook only checks changed files, and this change touches that file. Drop the on-demand profiler: it drives `Transaction._profile`, which has no stream-mode equivalent. Note that `SENTRY_TRACE_SAMPLE_RATE` is 0 in every checked-in settings file, so this emits nothing until a deployment sets it. The CLI-init transaction previously forced `sampled=True`; stream mode has no equivalent, so it now obeys the sample rate like every other span. Co-Authored-By: Claude Opus 5 (1M context) --- .pre-commit-config.yaml | 4 +- docs/source/profiler.rst | 39 ++-------- sentry-options/schemas/snuba/schema.json | 5 -- snuba/cli/__init__.py | 12 +++- snuba/cli/consumer.py | 2 + snuba/clickhouse/connect.py | 72 ++++++++++++++----- snuba/clickhouse/native.py | 17 +++-- snuba/datasets/configuration/json_schema.py | 8 +-- snuba/datasets/configuration/loader.py | 10 +-- snuba/datasets/entities/factory.py | 4 +- snuba/datasets/factory.py | 4 +- snuba/datasets/plans/entity_processing.py | 14 ++-- snuba/datasets/plans/entity_validation.py | 4 +- snuba/datasets/plans/storage_processing.py | 6 +- snuba/datasets/storages/factory.py | 4 +- snuba/environment.py | 23 +++--- .../pipeline/composite_storage_processing.py | 7 +- snuba/pipeline/processors.py | 6 +- snuba/pipeline/stages/query_execution.py | 28 +++++--- snuba/query/allocation_policies/__init__.py | 20 ++++-- snuba/query/mql/parser.py | 26 ++++--- snuba/query/snql/parser.py | 16 ++--- snuba/querylog/__init__.py | 11 +-- snuba/replacers/projects_query_flags.py | 34 ++++++--- snuba/request/validation.py | 11 +-- snuba/utils/metrics/backends/sentry.py | 18 +++-- snuba/utils/metrics/util.py | 31 ++++++-- snuba/utils/profiler.py | 65 ----------------- snuba/web/db_query.py | 40 ++++++----- snuba/web/query.py | 4 +- snuba/web/rpc/__init__.py | 25 ++++--- .../routing_strategies/outcomes_based.py | 10 +-- .../routing_strategies/storage_routing.py | 28 +++++--- snuba/web/rpc/v1/endpoint_get_trace.py | 17 ++--- snuba/web/views.py | 66 ++++++++--------- 35 files changed, 371 insertions(+), 320 deletions(-) delete mode 100644 snuba/utils/profiler.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index eaf9805cacf..7849f1b3733 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,9 @@ repos: name: validate-configs-syntax entry: env SNUBA_SETTINGS=test python3 -m snuba.validate_configs language: python - additional_dependencies: [ 'fastjsonschema', 'pyyaml', 'sentry_sdk' ] + # sentry_sdk must be >=2.62 for `sentry_sdk.traces` (span streaming API). + # Keep the floor in sync with pyproject.toml. + additional_dependencies: [ 'fastjsonschema', 'pyyaml', 'sentry_sdk>=2.66.1' ] pass_filenames: false files: 'snuba/datasets/configuration/.*' - id: cargo-fmt diff --git a/docs/source/profiler.rst b/docs/source/profiler.rst index a4a4c25a6d6..97053b46d09 100644 --- a/docs/source/profiler.rst +++ b/docs/source/profiler.rst @@ -1,41 +1,10 @@ Profiling ========= -Snuba has two ways of using `Sentry's own profiling -`_: Profiles captured as part of -regular transactions like API calls, and "on-demand" profiles. - -Regular transaction profiling -============================= +Snuba uses `Sentry's own profiling +`_ to capture profiles as part of +regular tracing. Only enabled for a few deployments selectively via environment variables. Snuba admin is sampled at 100%, and some low-scale consumers also have a sample rate -set. - -On-demand profiling -=================== - -For consumers who have ``SNUBA_PROFILES_SAMPLE_RATE`` set to a nonzero value, -it is possible to capture a profile of the main thread for a fixed duration of -time via runtime config. - -For example, if the deployment ``snuba-querylog`` misbehaves, you can pick a -specific pod by ``pod_name`` like -``snuba-querylog-consumer-production-6d95f9c8d9-4cqht`` and enable profiling -just for that one pod. - -Since profiling has unknown amount of overhead, pick only a few pods at a time -to minimize the risk and size of a backlog. - -Once you have that ``pod_name``, set the runtime config -``ondemand_profiler_hostnames`` to that value, without quotes. Separate -multiple values by comma. - -To stop profiling and send the profile to Sentry, unset the value. - -You should see a warning message in Sentry saying "Starting ondemand profile -for ", and once you unset runtime config, a new profile with a -transaction name of "ondemand profile: " - -Profiles are capped to 30 second runtime, so if the runtime config is set -forever, there will be profiles sent to Sentry every 30 seconds. +set via ``SNUBA_PROFILES_SAMPLE_RATE``. diff --git a/sentry-options/schemas/snuba/schema.json b/sentry-options/schemas/snuba/schema.json index 16b888513b2..d1febf3d5e6 100644 --- a/sentry-options/schemas/snuba/schema.json +++ b/sentry-options/schemas/snuba/schema.json @@ -561,11 +561,6 @@ "default": false, "description": "When true, the ClickHouse read path uses the clickhouse-connect (HTTP) driver instead of the native protocol. Lets the driver migration roll out and roll back without a deploy. Migrated from the runtime config of the same name." }, - "ondemand_profiler_hostnames": { - "type": "string", - "default": "", - "description": "Comma-separated list of hostnames for which the on-demand profiler should capture a profile." - }, "max_spans_per_transaction": { "type": "integer", "default": 2000, diff --git a/snuba/cli/__init__.py b/snuba/cli/__init__.py index c7214c0d5e8..37a283ff4bf 100644 --- a/snuba/cli/__init__.py +++ b/snuba/cli/__init__.py @@ -6,8 +6,8 @@ from typing import Any import click -import sentry_sdk import structlog +from sentry_sdk import traces from snuba.core import initialize from snuba.environment import metrics as environment_metrics @@ -46,7 +46,15 @@ def get_command(self, ctx: Any, name: str) -> click.Command: # by default. To mimic this behavior we have to do that here # since all of our infrastructure depends on this being the # case - with sentry_sdk.start_transaction(op="snuba_init", name=f"[cli init] {name}", sampled=True): + # `parent_span=None` forces a service span (the stream-mode equivalent of + # a transaction). Note there is no stream-mode equivalent of the old + # `sampled=True`, so this now obeys SENTRY_TRACE_SAMPLE_RATE like every + # other span; the `snuba_init_time` metric below is unaffected. + with traces.start_span( + name=f"[cli init] {name}", + attributes={"sentry.op": "snuba_init"}, + parent_span=None, + ): actual_command_name = name.replace("-", "_") ns: dict[str, click.Command] = {} # NOTE: WE initialize snuba before we compile the command code diff --git a/snuba/cli/consumer.py b/snuba/cli/consumer.py index 66f95851d7a..6f0ba5cfd92 100644 --- a/snuba/cli/consumer.py +++ b/snuba/cli/consumer.py @@ -204,6 +204,8 @@ def consumer( logger.info("Consumer Starting") storage_key = StorageKey(storage_name) + # Deliberately a tag, not an attribute: consumers have no active span, and + # scope tags (unlike scope attributes) are what reach error events. sentry_sdk.set_tag("storage", storage_name) logger.info("Checking Clickhouse connections...") diff --git a/snuba/clickhouse/connect.py b/snuba/clickhouse/connect.py index 9f1e1422438..d97d8e7098f 100644 --- a/snuba/clickhouse/connect.py +++ b/snuba/clickhouse/connect.py @@ -14,6 +14,7 @@ from clickhouse_connect.driver.client import Client from clickhouse_connect.driver.exceptions import ClickHouseError, OperationalError from clickhouse_connect.driver.httputil import get_pool_manager +from sentry_sdk import traces from snuba import environment, settings, state from snuba.clickhouse.errors import ClickhouseError @@ -30,6 +31,19 @@ metrics = MetricsWrapper(environment.metrics, "clickhouse.connect") + +def _driver_params(params: Params) -> Sequence[Any] | dict[str, Any] | None: + """Narrow ``Params`` to what clickhouse-connect accepts. + + ``Params`` allows any ``Mapping``, but the driver's signature takes an + invariant ``dict``. Falsy params (including an empty sequence) mean "no + parameters" here, matching the previous ``params if params else None``. + """ + if not params: + return None + return dict(params) if isinstance(params, Mapping) else params + + # Stand-in for "no read timeout" on the HTTP path. The native driver maps a # profile with no timeout (``None``) to an unbounded socket, but clickhouse-connect # cannot safely take ``None`` (its progress-interval computation does arithmetic @@ -178,8 +192,8 @@ def _build_query_settings( settings: Mapping[str, Any] | None, query_id: str | None, capture_trace: bool, - ) -> Mapping[str, Any] | None: - query_settings = dict(settings) if settings else {} + ) -> dict[str, Any] | None: + query_settings: dict[str, Any] = dict(settings) if settings else {} if query_id is not None: query_settings["query_id"] = query_id if capture_trace: @@ -207,13 +221,20 @@ def _execute_once( client = self._get_client() query_settings = self._build_query_settings(settings, query_id, capture_trace) - with sentry_sdk.start_span(description=query, op="db.clickhouse") as span: - span.set_data(sentry_sdk.consts.SPANDATA.DB_SYSTEM, "clickhouse") - span.set_data("query_id", query_id) - span.set_data("settings", query_settings) + with traces.start_span( + name="clickhouse query", + attributes={ + "sentry.op": "db.clickhouse", + sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", + sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, + }, + ) as span: + if query_id is not None: + span.set_attribute("query_id", query_id) + span.set_attribute("settings", json.dumps(query_settings, default=repr)) query_result = client.query( query, - parameters=params if params else None, + parameters=_driver_params(params), settings=query_settings, column_oriented=columnar, ) @@ -292,12 +313,19 @@ def execute_with_totals( if query_id is not None: json_settings["query_id"] = query_id - with sentry_sdk.start_span(description=query, op="db.clickhouse") as span: - span.set_data(sentry_sdk.consts.SPANDATA.DB_SYSTEM, "clickhouse") - span.set_data("query_id", query_id) + with traces.start_span( + name="clickhouse query", + attributes={ + "sentry.op": "db.clickhouse", + sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", + sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, + }, + ) as span: + if query_id is not None: + span.set_attribute("query_id", query_id) raw = client.raw_query( query, - parameters=params if params else None, + parameters=_driver_params(params), settings=json_settings, fmt="JSONCompact", ) @@ -445,11 +473,15 @@ def insert( with self._translate_clickhouse_errors(): client = self._get_client() - with sentry_sdk.start_span( - description=f"INSERT INTO {table}", op="db.clickhouse" + with traces.start_span( + name=f"INSERT INTO {table}", + attributes={ + "sentry.op": "db.clickhouse", + sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", + }, ) as span: - span.set_data(sentry_sdk.consts.SPANDATA.DB_SYSTEM, "clickhouse") - span.set_data("query_id", query_id) + if query_id is not None: + span.set_attribute("query_id", query_id) client.insert( table, matrix, @@ -506,8 +538,14 @@ def execute_explain(self, query: str) -> ClickhouseResult: """ with self._translate_clickhouse_errors(): client = self._get_client() - with sentry_sdk.start_span(description=query, op="db.clickhouse") as span: - span.set_data(sentry_sdk.consts.SPANDATA.DB_SYSTEM, "clickhouse") + with traces.start_span( + name="clickhouse query", + attributes={ + "sentry.op": "db.clickhouse", + sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", + sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, + }, + ): output = client.command(query) return self._explain_result(output) diff --git a/snuba/clickhouse/native.py b/snuba/clickhouse/native.py index 9e53fbd7e2f..5bafad91dc3 100644 --- a/snuba/clickhouse/native.py +++ b/snuba/clickhouse/native.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import logging import queue import re @@ -20,6 +21,7 @@ import sentry_sdk from clickhouse_driver import Client, errors from dateutil.tz import tz +from sentry_sdk import traces from sentry_sdk.integrations.logging import ignore_logger from snuba import environment, settings @@ -279,10 +281,17 @@ def execute( ) def query_execute(conn: Client = conn, settings: Any = settings) -> Any: - with sentry_sdk.start_span(description=query, op="db.clickhouse") as span: - span.set_data(sentry_sdk.consts.SPANDATA.DB_SYSTEM, "clickhouse") - span.set_data("query_id", query_id) - span.set_data("settings", settings) + with traces.start_span( + name="clickhouse query", + attributes={ + "sentry.op": "db.clickhouse", + sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", + sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, + }, + ) as span: + if query_id is not None: + span.set_attribute("query_id", query_id) + span.set_attribute("settings", json.dumps(settings, default=repr)) return conn.execute( query, params=params, diff --git a/snuba/datasets/configuration/json_schema.py b/snuba/datasets/configuration/json_schema.py index 949c343d12c..a6d106902ac 100644 --- a/snuba/datasets/configuration/json_schema.py +++ b/snuba/datasets/configuration/json_schema.py @@ -4,7 +4,7 @@ from typing import Any import fastjsonschema -import sentry_sdk +from sentry_sdk import traces from snuba import settings @@ -864,17 +864,17 @@ def registered_class_array_schema( } if settings.VALIDATE_DATASET_YAMLS_ON_STARTUP: - with sentry_sdk.start_span(op="compile", description="Storage Validators"): + with traces.start_span(name="Storage Validators", attributes={"sentry.op": "compile"}): STORAGE_VALIDATORS = { "readable_storage": fastjsonschema.compile(V1_READABLE_STORAGE_SCHEMA), "writable_storage": fastjsonschema.compile(V1_WRITABLE_STORAGE_SCHEMA), "cdc_storage": fastjsonschema.compile(V1_CDC_STORAGE_SCHEMA), } - with sentry_sdk.start_span(op="compile", description="Entity Validators"): + with traces.start_span(name="Entity Validators", attributes={"sentry.op": "compile"}): ENTITY_VALIDATORS = {"entity": fastjsonschema.compile(V1_ENTITY_SCHEMA)} - with sentry_sdk.start_span(op="compile", description="Dataset Validators"): + with traces.start_span(name="Dataset Validators", attributes={"sentry.op": "compile"}): DATASET_VALIDATORS = {"dataset": fastjsonschema.compile(V1_DATASET_SCHEMA)} else: STORAGE_VALIDATORS = {} diff --git a/snuba/datasets/configuration/loader.py b/snuba/datasets/configuration/loader.py index b55c479db58..a7d9362ba1c 100644 --- a/snuba/datasets/configuration/loader.py +++ b/snuba/datasets/configuration/loader.py @@ -2,7 +2,7 @@ from typing import Any -import sentry_sdk +from sentry_sdk import traces from yaml import safe_load from snuba import settings @@ -13,12 +13,14 @@ def load_configuration_data(path: str, validators: dict[str, Any]) -> dict[str, Loads a configuration file from the given path Returns an untyped dict of dicts """ - with sentry_sdk.start_span(op="load_and_validate") as span: - span.set_tag("file", path) + with traces.start_span( + name="load_and_validate", + attributes={"sentry.op": "load_and_validate", "file": path}, + ) as span: with open(path) as file: config = safe_load(file) assert isinstance(config, dict) if settings.VALIDATE_DATASET_YAMLS_ON_STARTUP: validators[config["kind"]](config) - span.description = config["name"] + span.name = config["name"] return config diff --git a/snuba/datasets/entities/factory.py b/snuba/datasets/entities/factory.py index 17dd457a62a..532279aead9 100644 --- a/snuba/datasets/entities/factory.py +++ b/snuba/datasets/entities/factory.py @@ -3,7 +3,7 @@ from collections.abc import Sequence from glob import glob -import sentry_sdk +from sentry_sdk import traces from snuba import settings from snuba.datasets.configuration.entity_builder import build_entity_from_config @@ -18,7 +18,7 @@ class _EntityFactory(ConfigComponentFactory[Entity, EntityKey]): def __init__(self) -> None: - with sentry_sdk.start_span(op="initialize", description="Entity Factory"): + with traces.start_span(name="Entity Factory", attributes={"sentry.op": "initialize"}): initialize_storage_factory() self._entity_map: dict[EntityKey, PluggableEntity] = {} self._name_map: dict[type[Entity], EntityKey] = {} diff --git a/snuba/datasets/factory.py b/snuba/datasets/factory.py index 2a9dfe4305d..60ce2f335e4 100644 --- a/snuba/datasets/factory.py +++ b/snuba/datasets/factory.py @@ -2,7 +2,7 @@ from glob import glob -import sentry_sdk +from sentry_sdk import traces from snuba import settings from snuba.datasets.configuration.dataset_builder import build_dataset_from_config @@ -16,7 +16,7 @@ class _DatasetFactory(ConfigComponentFactory[Dataset, str]): def __init__(self) -> None: - with sentry_sdk.start_span(op="initialize", description="Dataset Factory"): + with traces.start_span(name="Dataset Factory", attributes={"sentry.op": "initialize"}): initialize_entity_factory() self._dataset_map: dict[str, Dataset] = {} self._name_map: dict[type[Dataset], str] = {} diff --git a/snuba/datasets/plans/entity_processing.py b/snuba/datasets/plans/entity_processing.py index 3d3066f107e..1d6c8df0346 100644 --- a/snuba/datasets/plans/entity_processing.py +++ b/snuba/datasets/plans/entity_processing.py @@ -3,7 +3,7 @@ from collections.abc import Sequence from typing import cast -import sentry_sdk +from sentry_sdk import traces from snuba.clickhouse.query import Query from snuba.clusters.cluster import ClickhouseCluster @@ -57,8 +57,8 @@ def __init__( self.__partition_key_column_name = partition_key_column_name def get_storage(self, query: LogicalQuery, settings: QuerySettings) -> EntityStorageConnection: - with sentry_sdk.start_span( - op="build_plan.storage_query_plan_builder", description="select_storage" + with traces.start_span( + name="select_storage", attributes={"sentry.op": "build_plan.storage_query_plan_builder"} ): return self.__selector.select_storage(query, settings, self.__storages) @@ -66,8 +66,8 @@ def get_cluster( self, storage: ReadableStorage, query: LogicalQuery, settings: QuerySettings ) -> ClickhouseCluster: if is_storage_set_sliced(storage.get_storage_set_key()): - with sentry_sdk.start_span( - op="build_plan.sliced_storage", description="select_storage" + with traces.start_span( + name="select_storage", attributes={"sentry.op": "build_plan.sliced_storage"} ): assert self.__partition_key_column_name is not None, ( "partition key column name must be defined for a sliced storage" @@ -91,8 +91,8 @@ def translate_query_and_apply_mappers( check_storage_readiness(storage) - with sentry_sdk.start_span( - op="build_plan.storage_query_plan_builder", description="translate" + with traces.start_span( + name="translate", attributes={"sentry.op": "build_plan.storage_query_plan_builder"} ): # The QueryTranslator class should be instantiated once for each call to # translate_query_and_apply_mappers to avoid cache conflicts. diff --git a/snuba/datasets/plans/entity_validation.py b/snuba/datasets/plans/entity_validation.py index bfb6ab64443..3035f3dd98b 100644 --- a/snuba/datasets/plans/entity_validation.py +++ b/snuba/datasets/plans/entity_validation.py @@ -1,6 +1,6 @@ from __future__ import annotations -import sentry_sdk +from sentry_sdk import traces from snuba.datasets.entities.factory import get_entity from snuba.query import Query @@ -69,7 +69,7 @@ def run_entity_validators( """ for validator_func in VALIDATORS: description = getattr(validator_func, "__name__", "custom") - with sentry_sdk.start_span(op="validator", description=description): + with traces.start_span(name=description, attributes={"sentry.op": "validator"}): if settings and settings.get_dry_run(): with explain_meta.with_query_differ("entity_validator", description, query): validator_func(query) diff --git a/snuba/datasets/plans/storage_processing.py b/snuba/datasets/plans/storage_processing.py index adc971af2bb..26e164deda4 100644 --- a/snuba/datasets/plans/storage_processing.py +++ b/snuba/datasets/plans/storage_processing.py @@ -3,7 +3,7 @@ from collections.abc import Sequence from typing import TypeVar -import sentry_sdk +from sentry_sdk import traces from snuba import settings as snuba_settings from snuba.clickhouse.query import Query @@ -143,7 +143,9 @@ def apply_storage_processors( ) assert isinstance(query_plan.query, Query) for processor in query_plan.db_query_processors: - with sentry_sdk.start_span(description=type(processor).__name__, op="processor"): + with traces.start_span( + name=type(processor).__name__, attributes={"sentry.op": "processor"} + ): if settings.get_dry_run(): with explain_meta.with_query_differ( "storage_processor", type(processor).__name__, query_plan.query diff --git a/snuba/datasets/storages/factory.py b/snuba/datasets/storages/factory.py index 0c111acc568..aebaf7d7441 100644 --- a/snuba/datasets/storages/factory.py +++ b/snuba/datasets/storages/factory.py @@ -4,7 +4,7 @@ from collections.abc import MutableSequence, Sequence from glob import glob -import sentry_sdk +from sentry_sdk import traces from snuba import settings from snuba.datasets.cdc.cdcstorage import CdcStorage @@ -18,7 +18,7 @@ class _StorageFactory(ConfigComponentFactory[Storage, StorageKey]): def __init__(self) -> None: - with sentry_sdk.start_span(op="initialize", description="Storage Factory"): + with traces.start_span(name="Storage Factory", attributes={"sentry.op": "initialize"}): self._config_built_storages: dict[StorageKey, Storage] = {} self._all_storages: dict[StorageKey, Storage] = {} self.__initialize() diff --git a/snuba/environment.py b/snuba/environment.py index 08c7cdad584..08bc342bce5 100644 --- a/snuba/environment.py +++ b/snuba/environment.py @@ -181,25 +181,22 @@ def setup_sentry() -> None: release=os.getenv("SNUBA_RELEASE"), traces_sample_rate=settings.SENTRY_TRACE_SAMPLE_RATE, profiles_sample_rate=settings.SNUBA_PROFILES_SAMPLE_RATE, - _experiments={ - # Turns on the metrics module - "enable_metrics": True, - # Enables sending of code locations for metrics - "metric_code_locations": True, - }, + # Send spans to Sentry in batches as they finish, instead of buffering a + # whole trace in memory and shipping it as a single transaction event. + # This removes the 1000-span-per-transaction cap, keeps memory flat in + # long-running consumers, and preserves the spans that already finished + # when a process is killed mid-trace (e.g. an OOM-kill). + # + # NOTE: this disables the legacy tracing API. `sentry_sdk.start_span()`, + # `start_transaction()`, `update_current_span()` and `scope.span` all + # silently become no-ops, so instrumentation must use `sentry_sdk.traces`. + trace_lifecycle="stream", ) from snuba.state.sentry_options import init_options init_options() - from snuba.utils.profiler import run_ondemand_profiler - - if settings.SENTRY_DSN is not None: - # Do not run ondemand profiler in tests, it interferes with mocked - # `time.sleep()` and assertions on that mock. - run_ondemand_profiler() - metrics = create_metrics( "snuba", diff --git a/snuba/pipeline/composite_storage_processing.py b/snuba/pipeline/composite_storage_processing.py index f11f1989d73..e8341ededbc 100644 --- a/snuba/pipeline/composite_storage_processing.py +++ b/snuba/pipeline/composite_storage_processing.py @@ -3,7 +3,7 @@ from collections.abc import Mapping, Sequence from typing import NamedTuple -import sentry_sdk +from sentry_sdk import traces from snuba.clickhouse.query import Query as ClickhouseQuery from snuba.clusters.storage_sets import StorageSetKey, is_valid_storage_set_combination @@ -269,8 +269,9 @@ def __process_simple_query( processors: Sequence[ClickhouseQueryProcessor], ) -> None: for clickhouse_processor in processors: - with sentry_sdk.start_span( - description=type(clickhouse_processor).__name__, op="processor" + with traces.start_span( + name=type(clickhouse_processor).__name__, + attributes={"sentry.op": "processor"}, ): clickhouse_processor.process_query(clickhouse_query, self.__settings) diff --git a/snuba/pipeline/processors.py b/snuba/pipeline/processors.py index c78b977f3fb..939c088c38a 100644 --- a/snuba/pipeline/processors.py +++ b/snuba/pipeline/processors.py @@ -1,4 +1,4 @@ -import sentry_sdk +from sentry_sdk import traces from snuba.datasets.entities.factory import get_entity from snuba.query.logical import EntityQuery @@ -14,7 +14,9 @@ def execute_entity_processors(query: EntityQuery, settings: QuerySettings) -> No entity = get_entity(query.get_from_clause().key) for processor in entity.get_query_processors(): - with sentry_sdk.start_span(description=type(processor).__name__, op="processor"): + with traces.start_span( + name=type(processor).__name__, attributes={"sentry.op": "processor"} + ): if settings.get_dry_run(): with explain_meta.with_query_differ( "entity_processor", type(processor).__name__, query diff --git a/snuba/pipeline/stages/query_execution.py b/snuba/pipeline/stages/query_execution.py index aa0acbc1dc2..e01a0da4519 100644 --- a/snuba/pipeline/stages/query_execution.py +++ b/snuba/pipeline/stages/query_execution.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import logging import textwrap from collections import defaultdict @@ -9,6 +10,7 @@ from typing import Any import sentry_sdk +from sentry_sdk import traces from snuba import environment from snuba import settings as snuba_settings @@ -105,9 +107,11 @@ def _dry_run_query_runner( clickhouse_query: ClickhouseQuery | CompositeQuery[Table], cluster_name: str, ) -> QueryResult: - with sentry_sdk.start_span(description="dryrun_create_query", op="function") as span: + with traces.start_span( + name="dryrun_create_query", attributes={"sentry.op": "function"} + ) as span: formatted_query = format_query(clickhouse_query) - span.set_data("query", formatted_query.structured()) + span.set_attribute("query", json.dumps(formatted_query.structured(), default=repr)) return QueryResult( {"data": [], "meta": []}, @@ -189,19 +193,19 @@ def _format_storage_query_and_run( visitor = TablesCollector() visitor.visit(from_clause) table_names = ",".join(sorted(visitor.get_tables())) - with sentry_sdk.start_span(description="create_query", op="function") as span: + with traces.start_span(name="create_query", attributes={"sentry.op": "function"}) as span: _apply_turbo_sampling_if_needed(clickhouse_query, query_settings) formatted_query = format_query(clickhouse_query) formatted_sql = formatted_query.get_sql() query_size_bytes = len(formatted_sql.encode("utf-8")) - span.set_data( + span.set_attribute( "query", textwrap.wrap(formatted_sql, 100, break_long_words=False) ) # To avoid the query being truncated - span.set_data("table", table_names) - span.set_data("query_size_bytes", query_size_bytes) - sentry_sdk.set_tag("query_size_group", get_query_size_group(query_size_bytes)) + span.set_attribute("table", table_names) + span.set_attribute("query_size_bytes", query_size_bytes) + sentry_sdk.set_attribute("query_size_group", get_query_size_group(query_size_bytes)) metrics.increment( "execute", tags={ @@ -251,8 +255,14 @@ def _format_storage_query_and_run( experiments=clickhouse_query.get_experiments(), ), ) from cause - with sentry_sdk.start_span(description=formatted_sql, op="function") as span: - span.set_tag("table", table_names) + with traces.start_span( + name="execute_query", + attributes={ + "sentry.op": "function", + sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: formatted_sql, + "table": table_names, + }, + ) as span: def execute() -> QueryResult: try: diff --git a/snuba/query/allocation_policies/__init__.py b/snuba/query/allocation_policies/__init__.py index 725b2fc6c5e..b3c47c05dea 100644 --- a/snuba/query/allocation_policies/__init__.py +++ b/snuba/query/allocation_policies/__init__.py @@ -1,13 +1,14 @@ from __future__ import annotations +import json import os from abc import ABC, abstractmethod from dataclasses import asdict, dataclass, field from enum import Enum from typing import Any, cast -import sentry_sdk from redis.exceptions import TimeoutError as RedisTimeoutError +from sentry_sdk import traces from snuba import environment, settings from snuba.configs.configuration import ( @@ -451,11 +452,12 @@ def _get_default_config_definitions(self) -> list[Configuration]: def get_quota_allowance( self, tenant_ids: dict[str, str | int], query_id: str ) -> QuotaAllowance: - with sentry_sdk.start_span( - op="allocation_policy.get_quota_allowance", name=self.__class__.__name__ + with traces.start_span( + name=self.__class__.__name__, + attributes={"sentry.op": "allocation_policy.get_quota_allowance"}, ) as span: for t, tid in tenant_ids.items(): - span.set_data(f"tenant_ids.{t}", str(tid)) + span.set_attribute(f"tenant_ids.{t}", str(tid)) try: if not self.is_active: allowance = QuotaAllowance( @@ -515,7 +517,7 @@ def get_quota_allowance( "max_threads": str(allowance.max_threads), }, ) - span.set_data("db_request_throttled", True) + span.set_attribute("db_request_throttled", True) if not self.is_enforced: allowance = QuotaAllowance( can_run=True, @@ -531,7 +533,13 @@ def get_quota_allowance( # make sure we always know which storage key we rejected a query from allowance.explanation["storage_key"] = self._resource_identifier.value for k, v in allowance.to_dict().items(): - span.set_data(f"quota_allowance.{k}", v) + # Span attributes only accept scalars; stringify the nested + # values (e.g. `explanation`) rather than letting the SDK + # safe_repr() them into something unreadable. + span.set_attribute( + f"quota_allowance.{k}", + v if isinstance(v, (str, int, float, bool)) else json.dumps(v, default=repr), + ) return allowance @abstractmethod diff --git a/snuba/query/mql/parser.py b/snuba/query/mql/parser.py index a1dc2ed3802..f289341395e 100644 --- a/snuba/query/mql/parser.py +++ b/snuba/query/mql/parser.py @@ -5,9 +5,9 @@ from dataclasses import dataclass, replace from typing import Any -import sentry_sdk from parsimonious.exceptions import IncompleteParseError from parsimonious.nodes import Node, NodeVisitor +from sentry_sdk import traces from snuba_sdk import BooleanCondition, Condition from snuba_sdk.metrics_visitors import AGGREGATE_ALIAS from snuba_sdk.mql.mql import MQL_GRAMMAR @@ -1265,7 +1265,7 @@ def parse_mql_query( # NOTE (volo): The anonymizer that runs after this function call chokes on # OR and AND clauses with multiple parameters so we have to treeify them # before we run the anonymizer and the rest of the post processors - with sentry_sdk.start_span(op="processor", description="treeify_conditions"): + with traces.start_span(name="treeify_conditions", attributes={"sentry.op": "processor"}): _post_process(query, [_treeify_or_and_conditions], settings) res = PostProcessAndValidateMQLQuery().execute( @@ -1295,13 +1295,17 @@ def _process_data( ) -> LogicalQuery: mql_str, dataset, mql_context_dict, settings = pipe_input.data - with sentry_sdk.start_span(op="parser", description="parse_mql_query_initial"): + with traces.start_span(name="parse_mql_query_initial", attributes={"sentry.op": "parser"}): query = parse_mql_query_body(mql_str, dataset) - with sentry_sdk.start_span(op="parser", description="populate_query_from_mql_context"): + with traces.start_span( + name="populate_query_from_mql_context", attributes={"sentry.op": "parser"} + ): query, mql_context = populate_query_from_mql_context(query, mql_context_dict) - with sentry_sdk.start_span(op="processor", description="resolve_indexer_mappings"): + with traces.start_span( + name="resolve_indexer_mappings", attributes={"sentry.op": "processor"} + ): resolve_mappings(query, mql_context.indexer_mappings, dataset) if settings and settings.get_dry_run(): @@ -1331,7 +1335,7 @@ def _process_data( ], ) -> LogicalQuery: query, settings, custom_processing = pipe_input.data - with sentry_sdk.start_span(op="processor", description="post_processors"): + with traces.start_span(name="post_processors", attributes={"sentry.op": "processor"}): _post_process( query, MQL_POST_PROCESSORS, @@ -1339,23 +1343,25 @@ def _process_data( ) # Filter in select optimizer - with sentry_sdk.start_span(op="processor", description="filter_in_select_optimize"): + with traces.start_span( + name="filter_in_select_optimize", attributes={"sentry.op": "processor"} + ): if settings is None: FilterInSelectOptimizer().process_query(query, HTTPQuerySettings()) else: FilterInSelectOptimizer().process_query(query, settings) # Custom processing to tweak the AST before validation - with sentry_sdk.start_span(op="processor", description="custom_processing"): + with traces.start_span(name="custom_processing", attributes={"sentry.op": "processor"}): if custom_processing is not None: _post_process(query, custom_processing, settings) # Time based processing - with sentry_sdk.start_span(op="processor", description="time_based_processing"): + with traces.start_span(name="time_based_processing", attributes={"sentry.op": "processor"}): _post_process(query, [_replace_time_condition], settings) # Validating - with sentry_sdk.start_span(op="validate", description="expression_validators"): + with traces.start_span(name="expression_validators", attributes={"sentry.op": "validate"}): _post_process(query, VALIDATORS) return query diff --git a/snuba/query/snql/parser.py b/snuba/query/snql/parser.py index e5134645bd5..72c07acfb73 100644 --- a/snuba/query/snql/parser.py +++ b/snuba/query/snql/parser.py @@ -11,10 +11,10 @@ cast, ) -import sentry_sdk from parsimonious.exceptions import IncompleteParseError from parsimonious.grammar import Grammar from parsimonious.nodes import Node, NodeVisitor +from sentry_sdk import traces from snuba.clickhouse.columns import Array, ColumnSet from snuba.clickhouse.query_dsl.accessors import get_time_range_expressions @@ -1400,7 +1400,7 @@ def _post_process( # custom processors can be partials instead of functions but partials don't # have the __name__ attribute set automatically (and we don't set it manually) description = getattr(func, "__name__", "custom") - with sentry_sdk.start_span(op="processor", description=description): + with traces.start_span(name=description, attributes={"sentry.op": "processor"}): if settings and settings.get_dry_run(): with explain_meta.with_query_differ("snql_parsing", description, query): func(query) @@ -1440,7 +1440,7 @@ def parse_snql_query( custom_processing: CustomProcessors | None = None, settings: QuerySettings | None = None, ) -> CompositeQuery[LogicalDataSource] | LogicalQuery: - with sentry_sdk.start_span(op="parser", description="parse_snql_query_initial"): + with traces.start_span(name="parse_snql_query_initial", attributes={"sentry.op": "parser"}): query = parse_snql_query_initial(body) if settings and settings.get_dry_run(): @@ -1450,7 +1450,7 @@ def parse_snql_query( # NOTE (volo): The anonymizer that runs after this function call chokes on # OR and AND clauses with multiple parameters so we have to treeify them # before we run the anonymizer and the rest of the post processors - with sentry_sdk.start_span(op="processor", description="treeify_conditions"): + with traces.start_span(name="treeify_conditions", attributes={"sentry.op": "processor"}): _post_process(query, [_treeify_or_and_conditions], settings) timer = Timer("snql_pipeline") @@ -1495,22 +1495,22 @@ def _process_data( query, dataset, custom_processing = pipe_input.data settings = pipe_input.query_settings - with sentry_sdk.start_span(op="processor", description="post_processors"): + with traces.start_span(name="post_processors", attributes={"sentry.op": "processor"}): _post_process( query, POST_PROCESSORS, settings, ) # Custom processing to tweak the AST before validation - with sentry_sdk.start_span(op="processor", description="custom_processing"): + with traces.start_span(name="custom_processing", attributes={"sentry.op": "processor"}): if custom_processing is not None: _post_process(query, custom_processing, settings) # Time based processing - with sentry_sdk.start_span(op="processor", description="time_based_processing"): + with traces.start_span(name="time_based_processing", attributes={"sentry.op": "processor"}): _post_process(query, [_replace_time_condition], settings) _post_process(query, [_select_entity_for_dataset(dataset)], settings) # Validating - with sentry_sdk.start_span(op="validate", description="expression_validators"): + with traces.start_span(name="expression_validators", attributes={"sentry.op": "validate"}): _post_process(query, VALIDATORS) return query diff --git a/snuba/querylog/__init__.py b/snuba/querylog/__init__.py index 295a2a3e81c..5df27116d57 100644 --- a/snuba/querylog/__init__.py +++ b/snuba/querylog/__init__.py @@ -8,6 +8,7 @@ import sentry_sdk from sentry_kafka_schemas.schema_types import snuba_queries_v1 +from sentry_sdk import traces from usageaccountant import UsageUnit from snuba import environment, settings, state @@ -222,19 +223,19 @@ def _add_tags( experiments: Mapping[str, Any] | None = None, metadata: SnubaQueryMetadata | None = None, ) -> None: - if sentry_sdk.get_current_span(): + if traces.get_current_span() is not None: duration_group = timer.get_duration_group() - sentry_sdk.set_tag("duration_group", duration_group) + sentry_sdk.set_attribute("duration_group", duration_group) if duration_group == ">30s": - sentry_sdk.set_tag("timeout", "too_long") + sentry_sdk.set_attribute("timeout", "too_long") if experiments is not None: for name, value in experiments.items(): - sentry_sdk.set_tag(name, str(value)) + sentry_sdk.set_attribute(name, str(value)) if metadata is not None: for query_data in metadata.query_list: max_threads = query_data.stats.get("max_threads") if max_threads is not None: - sentry_sdk.set_tag("max_threads", max_threads) + sentry_sdk.set_attribute("max_threads", max_threads) break diff --git a/snuba/replacers/projects_query_flags.py b/snuba/replacers/projects_query_flags.py index a4f1060bc6a..7e18a74be5f 100644 --- a/snuba/replacers/projects_query_flags.py +++ b/snuba/replacers/projects_query_flags.py @@ -9,6 +9,7 @@ import sentry_sdk from redis.cluster import ClusterPipeline as StrictClusterPipeline +from sentry_sdk import traces from snuba import settings from snuba.processor import ReplacementType @@ -127,23 +128,36 @@ def load_from_redis( try: with redis_client.pipeline() as p: - with sentry_sdk.start_span(op="function", description="build_redis_pipeline"): + with traces.start_span( + name="build_redis_pipeline", attributes={"sentry.op": "function"} + ): cls._query_redis(s_project_ids, state_name, p) - with sentry_sdk.start_span( - op="function", description="execute_redis_pipeline" + with traces.start_span( + name="execute_redis_pipeline", attributes={"sentry.op": "function"} ) as span: results = p.execute() # getting size of str(results) since sys.getsizeof() doesn't count recursively - span.set_tag("results_size", sys.getsizeof(str(results))) + span.set_attribute("results_size", sys.getsizeof(str(results))) - with sentry_sdk.start_span(op="function", description="process_redis_results") as span: + with traces.start_span( + name="process_redis_results", attributes={"sentry.op": "function"} + ) as span: flags = cls._process_redis_results(results, len(s_project_ids)) - span.set_tag("projects", s_project_ids) - span.set_tag("exclude_groups", flags.group_ids_to_exclude) - span.set_tag("len(exclude_groups)", len(flags.group_ids_to_exclude)) - span.set_tag("latest_replacement_time", flags.latest_replacement_time) - span.set_tag("replacement_types", flags.replacement_types) + # Span attributes only accept scalars and homogeneous lists, so + # the sets/datetime here have to be converted explicitly. + span.set_attribute("projects", sorted(s_project_ids)) + span.set_attribute("exclude_groups", sorted(flags.group_ids_to_exclude)) + span.set_attribute("len(exclude_groups)", len(flags.group_ids_to_exclude)) + span.set_attribute( + "latest_replacement_time", + ( + flags.latest_replacement_time.isoformat() + if flags.latest_replacement_time is not None + else "" + ), + ) + span.set_attribute("replacement_types", sorted(flags.replacement_types)) return flags except Exception as e: diff --git a/snuba/request/validation.py b/snuba/request/validation.py index 75c89e1412d..003ace062c7 100644 --- a/snuba/request/validation.py +++ b/snuba/request/validation.py @@ -7,6 +7,7 @@ from typing import Any, Protocol import sentry_sdk +from sentry_sdk import traces from snuba import environment, settings from snuba.attribution import get_app_id @@ -109,7 +110,7 @@ def build_request( referrer: str, custom_processing: CustomProcessors | None = None, ) -> Request: - with sentry_sdk.start_span(description="build_request", op="validate") as span: + with traces.start_span(name="build_request", attributes={"sentry.op": "validate"}) as span: try: dataset_name = get_dataset_name(dataset) if get_mapped_option( @@ -151,11 +152,11 @@ def build_request( ) raise exception - span.set_data( + span.set_attribute( "snuba_query_parsed", repr(query).split("\n"), ) - span.set_data( + span.set_attribute( "snuba_query_raw", textwrap.wrap(repr(request.original_body), 100, break_long_words=False), ) @@ -235,11 +236,11 @@ def _build_request( ) -> Request: org_ids = get_object_ids_in_query_ast(query, "org_id") if org_ids is not None and len(org_ids) == 1: - sentry_sdk.set_tag("snuba_org_id", org_ids.pop()) + sentry_sdk.set_attribute("snuba_org_id", org_ids.pop()) query_project_id = _get_project_id(query) if query_project_id: - sentry_sdk.set_tag("snuba_project_id", query_project_id) + sentry_sdk.set_attribute("snuba_project_id", query_project_id) attribution_info = _get_attribution_info(request_parts, referrer, query_project_id) diff --git a/snuba/utils/metrics/backends/sentry.py b/snuba/utils/metrics/backends/sentry.py index dc144e1401b..3c82cb944bd 100644 --- a/snuba/utils/metrics/backends/sentry.py +++ b/snuba/utils/metrics/backends/sentry.py @@ -1,11 +1,18 @@ from __future__ import annotations +from typing import Any + from sentry_sdk import metrics from snuba.utils.metrics.backends.abstract import MetricsBackend from snuba.utils.metrics.types import Tags +def _attributes(tags: Tags | None) -> dict[str, Any] | None: + """The SDK takes an invariant ``dict``; ``Tags`` is a ``Mapping``.""" + return dict(tags) if tags is not None else None + + class SentryMetricsBackend(MetricsBackend): """ A metrics backend that records metrics to Sentry. @@ -21,7 +28,7 @@ def increment( tags: Tags | None = None, unit: str | None = None, ) -> None: - metrics.incr(name, value, unit or "none", tags) + metrics.count(name, value, unit or "none", _attributes(tags)) def gauge( self, @@ -30,7 +37,7 @@ def gauge( tags: Tags | None = None, unit: str | None = None, ) -> None: - metrics.gauge(name, value, unit or "none", tags) + metrics.gauge(name, value, unit or "none", _attributes(tags)) def timing( self, @@ -39,8 +46,9 @@ def timing( tags: Tags | None = None, unit: str | None = None, ) -> None: - # The Sentry SDK has strict typing on the unit, so it doesn't allow passing arbitrary units - metrics.timing(name, value, unit or "millisecond", tags) # type: ignore[arg-type] + # The SDK dropped the dedicated `timing` API; a timing is a distribution + # measured in milliseconds. + metrics.distribution(name, value, unit or "millisecond", _attributes(tags)) def distribution( self, @@ -49,7 +57,7 @@ def distribution( tags: Tags | None = None, unit: str | None = None, ) -> None: - metrics.distribution(name, value, unit or "none", tags) + metrics.distribution(name, value, unit or "none", _attributes(tags)) def events( self, diff --git a/snuba/utils/metrics/util.py b/snuba/utils/metrics/util.py index 06a4eae1345..dafc6215558 100644 --- a/snuba/utils/metrics/util.py +++ b/snuba/utils/metrics/util.py @@ -1,15 +1,20 @@ +from __future__ import annotations + import _strptime # NOQA fixes _strptime deferred import issue import inspect -from functools import wraps -from typing import Any, TypeVar, cast from collections.abc import Callable, Mapping +from functools import wraps +from typing import TYPE_CHECKING, Any, TypeVar, cast -import sentry_sdk +from sentry_sdk import traces from snuba import settings from snuba.utils.metrics import MetricsBackend from snuba.utils.metrics.types import Tags +if TYPE_CHECKING: + from sentry_sdk._types import Attributes + def create_metrics( prefix: str, @@ -71,14 +76,28 @@ def with_span(op: str = "function") -> Callable[[F], F]: def decorator(func: F) -> F: frame_info = inspect.stack()[1] - filename = frame_info.filename + # Built once at decoration time rather than on every call. + attributes: Attributes = {"sentry.op": op, "filename": frame_info.filename} @wraps(func) def wrapper(*args: Any, **kwargs: Any) -> Any: - with sentry_sdk.start_span(description=func.__name__, op=op) as span: - span.set_data("filename", filename) + with traces.start_span(name=func.__name__, attributes=attributes): return func(*args, **kwargs) return cast(F, wrapper) return decorator + + +def set_current_span_attributes(attributes: Mapping[str, Any]) -> None: + """Set attributes on the currently active span, if there is one. + + The stream-mode replacement for ``sentry_sdk.update_current_span()``, which + is a no-op once ``trace_lifecycle="stream"`` is enabled. There is no active + span when the surrounding code runs outside a traced request (e.g. on a + consumer path), or when the trace was not sampled. + """ + span = traces.get_current_span() + if span is None: + return + span.set_attributes(dict(attributes)) diff --git a/snuba/utils/profiler.py b/snuba/utils/profiler.py deleted file mode 100644 index 7e856dd9623..00000000000 --- a/snuba/utils/profiler.py +++ /dev/null @@ -1,65 +0,0 @@ -import logging -import socket -import threading -import time - -import sentry_sdk -from sentry_sdk.tracing import NoOpSpan, Transaction - -from snuba.state.sentry_options import get_option - -logger = logging.getLogger(__name__) - - -def run_ondemand_profiler() -> None: - thread = threading.Thread(target=_profiler_main, name="snuba ondemand profiler", daemon=True) - thread.start() - - -def _profiler_main() -> None: - current_transaction = None - own_hostname = socket.gethostname() - - while True: - queried_hostnames = (get_option("ondemand_profiler_hostnames", "")).split(",") - - if own_hostname in queried_hostnames and current_transaction is None: - # Log an error to Sentry on purpose, if the pod slows down it - # should be obvious why. - logger.warning("starting ondemand profile for %s", own_hostname) - - with sentry_sdk.Hub.main: - open_transaction: Transaction | NoOpSpan | None = sentry_sdk.start_transaction( - name=f"ondemand profile: {own_hostname}", sampled=True - ) - assert isinstance(open_transaction, Transaction) - assert open_transaction._profile is not None - open_transaction._profile.sampled = True - if open_transaction._profile.scheduler is None: - logger.warning( - "unable to start ondemand profile, need to turn on profiling globally" - ) - return - - # Set main thread as active thread -- this current thread is - # fairly uninteresting to look at. - # The profile contains all threads anyway. - open_transaction._profile.active_thread_id = threading.main_thread().ident - - open_transaction.__enter__() - transaction_start = time.time() - current_transaction = open_transaction, transaction_start - - continue - - if current_transaction is not None: - open_transaction, transaction_start = current_transaction - if own_hostname not in queried_hostnames or time.time() - transaction_start >= 30: - logger.warning("stopping ondemand profile for %s", own_hostname) - with sentry_sdk.Hub.main: - open_transaction.__exit__(None, None, None) - current_transaction = None - - continue - - time.sleep(5) diff --git a/snuba/web/db_query.py b/snuba/web/db_query.py index a8281cfe857..94ff16af193 100644 --- a/snuba/web/db_query.py +++ b/snuba/web/db_query.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import logging import random import uuid @@ -15,6 +16,7 @@ from clickhouse_driver.errors import ErrorCodes from sentry_kafka_schemas.schema_types import snuba_queries_v1 from sentry_options import OptionValue +from sentry_sdk import traces from sentry_sdk.api import configure_scope from snuba import environment, settings @@ -64,7 +66,7 @@ from snuba.util import force_bytes from snuba.utils.codecs import ExceptionAwareCodec from snuba.utils.metrics.timer import Timer -from snuba.utils.metrics.util import with_span +from snuba.utils.metrics.util import set_current_span_attributes, with_span from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.utils.serializable_exception import ( SerializableException, @@ -323,7 +325,7 @@ def execute_query_with_readthrough_caching( ): query_id = f"randomized-{uuid.uuid4().hex}" clickhouse_query_settings["query_id"] = query_id - sentry_sdk.update_current_span(attributes={"query_id": query_id}) + set_current_span_attributes({"query_id": query_id}) return execute_query( clickhouse_query, query_settings, @@ -337,7 +339,7 @@ def execute_query_with_readthrough_caching( clickhouse_query_settings["query_id"] = f"randomized-{uuid.uuid4().hex}" - sentry_sdk.update_current_span(attributes={"query_id": query_id}) + set_current_span_attributes({"query_id": query_id}) def record_cache_hit_type(hit_type: int) -> None: span_tag = "cache_miss" @@ -349,8 +351,7 @@ def record_cache_hit_type(hit_type: int) -> None: span_tag = "cache_wait" elif hit_type == SIMPLE_READTHROUGH: stats["cache_hit_simple"] = 1 - sentry_sdk.set_tag("cache_status", span_tag) - sentry_sdk.update_current_span(attributes={"cache_status": span_tag}) + sentry_sdk.set_attribute("cache_status", span_tag) cache_partition = _get_cache_partition(reader) metrics.increment( @@ -537,9 +538,9 @@ def _raw_query( elif isinstance(cause, (TimeoutError, ExecutionTimeoutError)): status = QueryStatus.TIMEOUT - with configure_scope() as scope: - if scope.span: - sentry_sdk.set_tag("slo_status", request_status.status.value) + # The old `if scope.span:` guard is gone: in stream mode `scope.span` is + # always None, which would have silently dropped this attribute. + sentry_sdk.set_attribute("slo_status", request_status.status.value) stats = update_with_status( status=status or QueryStatus.ERROR, @@ -867,16 +868,21 @@ def _apply_allocation_policies_quota( rejection_quota_and_policy = None throttle_quota_and_policy = None min_threads_across_policies = MAX_THRESHOLD - with sentry_sdk.start_span( - op="allocation_policy", description="_apply_allocation_policies_quota" + with traces.start_span( + name="_apply_allocation_policies_quota", + attributes={"sentry.op": "allocation_policy"}, ) as span: for allocation_policy in allocation_policies: allowance = allocation_policy.get_quota_allowance(attribution_info.tenant_ids, query_id) can_run &= allowance.can_run quota_allowances[allocation_policy.class_name()] = allowance - span.set_data( + # QuotaAllowance is not a valid attribute value; serialize it rather + # than letting the SDK safe_repr() it. + span.set_attribute( "quota_allowance", - quota_allowances[allocation_policy.class_name()], + json.dumps( + quota_allowances[allocation_policy.class_name()].to_dict(), default=repr + ), ) if allowance.is_throttled and allowance.max_threads < min_threads_across_policies: throttle_quota_and_policy = _QuotaAndPolicy( @@ -919,8 +925,8 @@ def _apply_allocation_policies_quota( if rejection_quota_and_policy is not None else "unknown" ) - span.set_data("policy", rejecting_policy) - span.set_data("action", "rejected") + span.set_attribute("policy", rejecting_policy) + span.set_attribute("action", "rejected") metrics.increment( "rejected_query", tags={ @@ -932,8 +938,8 @@ def _apply_allocation_policies_quota( if throttle_quota_and_policy is not None: throttling_policy = throttle_quota_and_policy.policy.class_name() - span.set_data("policy", throttling_policy) - span.set_data("action", "throttled") + span.set_attribute("policy", throttling_policy) + span.set_attribute("action", "throttled") metrics.increment( "throttled_query", tags={ @@ -947,5 +953,5 @@ def _apply_allocation_policies_quota( tags={"storage_key": allocation_policies[0].resource_identifier.value}, ) max_threads = min_threads_across_policies - span.set_data("max_threads", max_threads) + span.set_attribute("max_threads", max_threads) query_settings.set_resource_quota(ResourceQuota(max_threads=max_threads)) diff --git a/snuba/web/query.py b/snuba/web/query.py index eb1426eeb99..40058cc379a 100644 --- a/snuba/web/query.py +++ b/snuba/web/query.py @@ -3,7 +3,7 @@ import logging from typing import Any -import sentry_sdk +from sentry_sdk import traces from snuba import environment, settings from snuba.datasets.dataset import Dataset @@ -140,7 +140,7 @@ def parse_and_run_query( referrer (str): legacy param, you probably don't need to provide this. It should be in the tenant_ids of the body """ - with sentry_sdk.start_span(description="build_schema", op="validate"): + with traces.start_span(name="build_schema", attributes={"sentry.op": "validate"}): schema = RequestSchema.build(HTTPQuerySettings, is_mql) # NOTE(Volo): dataset is not necessary for queries because storages can be queried directly diff --git a/snuba/web/rpc/__init__.py b/snuba/web/rpc/__init__.py index a08168ff442..a8eb32917a3 100644 --- a/snuba/web/rpc/__init__.py +++ b/snuba/web/rpc/__init__.py @@ -12,6 +12,7 @@ from sentry_protos.snuba.v1.downsampled_storage_pb2 import DownsampledStorageConfig from sentry_protos.snuba.v1.error_pb2 import Error as ErrorProto from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta, TraceItemType +from sentry_sdk import traces from snuba import environment from snuba.query.allocation_policies import AllocationPolicyViolations @@ -84,19 +85,19 @@ def _flush_logs() -> None: def _set_rpc_error_tags(in_msg: ProtobufMessage) -> None: - sentry_sdk.set_tag("source", "rpc_api") + sentry_sdk.set_attribute("source", "rpc_api") - # Extract and tag fields from meta if available + # Extract and annotate fields from meta if available if hasattr(in_msg, "meta"): meta = in_msg.meta if hasattr(meta, "referrer") and meta.referrer: - sentry_sdk.set_tag("referrer", meta.referrer) + sentry_sdk.set_attribute("referrer", meta.referrer) if hasattr(meta, "organization_id") and meta.organization_id: - sentry_sdk.set_tag("organization_id", str(meta.organization_id)) + sentry_sdk.set_attribute("organization_id", str(meta.organization_id)) if hasattr(meta, "request_id") and meta.request_id: - sentry_sdk.set_tag("request_id", str(meta.request_id)) + sentry_sdk.set_attribute("request_id", str(meta.request_id)) class TraceItemDataResolver(Generic[Tin, Tout], metaclass=RegisteredClass): @@ -202,10 +203,9 @@ def _uses_storage_routing(self, in_msg: Tin) -> bool: @final def execute(self, in_msg: Tin) -> Tout: scope = sentry_sdk.get_current_scope() + # In stream mode this renames the service span (the segment) directly, + # which is what the old `span.description = ...` line did separately. scope.set_transaction_name(self.config_key()) - span = scope.span - if span is not None: - span.description = self.config_key() self.routing_context = RoutingContext( timer=self._timer, in_msg=in_msg, @@ -218,8 +218,13 @@ def execute(self, in_msg: Tin) -> Tout: meta = getattr(in_msg, "meta", RequestMeta()) try: if self.routing_decision.can_run: - with sentry_sdk.start_span(op="execute") as span: - span.set_data("selected_tier", self.routing_decision.tier) + with traces.start_span( + name="execute", + attributes={ + "sentry.op": "execute", + "selected_tier": self.routing_decision.tier.name, + }, + ): out = self._execute(in_msg) else: raise RPCAllocationPolicyException( diff --git a/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py b/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py index 728dd7892a0..386b8cfd541 100644 --- a/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py +++ b/snuba/web/rpc/storage_routing/routing_strategies/outcomes_based.py @@ -2,7 +2,6 @@ from datetime import UTC, datetime, timedelta from typing import cast -import sentry_sdk from google.protobuf.json_format import MessageToDict from sentry_protos.snuba.v1.endpoint_get_traces_pb2 import GetTracesRequest from sentry_protos.snuba.v1.endpoint_time_series_pb2 import TimeSeriesRequest @@ -25,6 +24,7 @@ from snuba.query.query_settings import OutcomesQuerySettings from snuba.request import Request as SnubaRequest from snuba.state.sentry_options import get_mapped_option, get_option +from snuba.utils.metrics.util import set_current_span_attributes from snuba.web.query import run_query from snuba.web.rpc.common.common import ( timestamp_in_range_condition, @@ -246,8 +246,8 @@ def _update_routing_decision( ): routing_decision.tier = Tier.TIER_8 - sentry_sdk.update_current_span( - attributes={ + set_current_span_attributes( + { "downsampling_mode": ( "highest_accuracy" if self._is_highest_accuracy_mode(in_msg_meta) else "normal" ), @@ -297,8 +297,8 @@ def _update_routing_decision( elif ingested_items > max_items_before_downsampling * 100: routing_decision.tier = Tier.TIER_512 - sentry_sdk.update_current_span( - attributes={ + set_current_span_attributes( + { "ingested_items": ingested_items, "max_items_before_downsampling": max_items_before_downsampling, "tier": routing_decision.tier.name, diff --git a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py index ba5fc237f49..0ff4d8be088 100644 --- a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py +++ b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import os from abc import ABC from collections.abc import Callable @@ -22,6 +23,7 @@ from sentry_protos.snuba.v1.endpoint_time_series_pb2 import TimeSeriesRequest from sentry_protos.snuba.v1.endpoint_trace_item_table_pb2 import TraceItemTableRequest from sentry_protos.snuba.v1.request_common_pb2 import RequestMeta +from sentry_sdk import traces from snuba import environment, settings from snuba.configs.configuration import ( @@ -50,6 +52,7 @@ from snuba.state import record_query from snuba.state.sentry_options import get_mapped_option, get_option from snuba.utils.metrics.timer import Timer +from snuba.utils.metrics.util import set_current_span_attributes from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.utils.registered_class import import_submodules_in_directory from snuba.web import QueryException, QueryResult @@ -210,7 +213,10 @@ def get_stats_dict( def _construct_hacky_querylog_payload( strategy: BaseRoutingStrategy, routing_decision: RoutingDecision ) -> snuba_queries_v1.Querylog: - cur_span = sentry_sdk.get_current_span() + # Read the trace id off the propagation context rather than the active span: + # it is populated whether or not a span is active and whether or not the + # trace was sampled, so the querylog always gets a real id. + trace_id = sentry_sdk.get_current_scope().get_active_propagation_context().trace_id assert routing_decision.routing_context is not None query_result = routing_decision.routing_context.query_result or QueryResult( {}, {"stats": {}, "sql": "", "experiments": {}} @@ -247,7 +253,7 @@ def _construct_hacky_querylog_payload( "end_timestamp": in_message_meta.end_timestamp.seconds, "stats": get_stats_dict(routing_decision), "status": "0", - "trace_id": cur_span.trace_id if cur_span else "", + "trace_id": trace_id or "", "profile": { "time_range": None, "table": "eap_items", @@ -408,7 +414,7 @@ def _record_value_in_span_and_DD( "value": value, "tags": tags, } - sentry_sdk.update_current_span(attributes={name: value}) + set_current_span_attributes({name: value}) def _update_routing_decision( self, @@ -459,17 +465,19 @@ def _get_recommendations_from_allocation_policies( recommendations: dict[str, QuotaAllowance] = {} for allocation_policy in self.get_allocation_policies(): allocation_policy_name = allocation_policy.class_name() - with sentry_sdk.start_span( - op="allocation_policy.get_quota_allowance", - description=allocation_policy_name, + with traces.start_span( + name=allocation_policy_name, + attributes={"sentry.op": "allocation_policy.get_quota_allowance"}, ) as span: recommendations[allocation_policy_name] = allocation_policy.get_quota_allowance( routing_context.tenant_ids, routing_context.query_id, ) - span.set_data( + # QuotaAllowance is not a valid attribute value; serialize it + # rather than letting the SDK safe_repr() it. + span.set_attribute( f"{allocation_policy_name}_quota_allowance", - recommendations[allocation_policy_name], + json.dumps(recommendations[allocation_policy_name].to_dict(), default=repr), ) return recommendations @@ -481,7 +489,7 @@ def get_routing_decision(self, routing_context: RoutingContext) -> RoutingDecisi default_tier = self._get_default_routing_decision_tier() - with sentry_sdk.start_span(op="decide_tier") as span: + with traces.start_span(name="decide_tier", attributes={"sentry.op": "decide_tier"}) as span: try: routing_context.timer.mark(_START_ESTIMATION_MARK) @@ -541,7 +549,7 @@ def get_routing_decision(self, routing_context: RoutingContext) -> RoutingDecisi if settings.RAISE_ON_ROUTING_STRATEGY_FAILURES: raise e - span.set_data("decided_tier", routing_decision.tier) + span.set_attribute("decided_tier", routing_decision.tier.name) return routing_decision @final diff --git a/snuba/web/rpc/v1/endpoint_get_trace.py b/snuba/web/rpc/v1/endpoint_get_trace.py index 58c504707cd..035b4a91fb9 100644 --- a/snuba/web/rpc/v1/endpoint_get_trace.py +++ b/snuba/web/rpc/v1/endpoint_get_trace.py @@ -23,6 +23,7 @@ ComparisonFilter, TraceItemFilter, ) +from sentry_sdk import traces from snuba.attribution.appid import AppID from snuba.attribution.attribution_info import AttributionInfo @@ -347,9 +348,9 @@ def _build_query( if random.random() < _get_apply_final_rollout_percentage(): query.set_final(True) - span = sentry_sdk.get_current_span() - if span: - span.set_data("is_final", query.get_final()) + span = traces.get_current_span() + if span is not None: + span.set_attribute("is_final", query.get_final()) treeify_or_and_conditions(query) @@ -494,7 +495,7 @@ def _process_results( # First pass: parse rows and build attribute dicts parsed_rows: list[tuple[str, Timestamp, dict[str, GetTraceResponse.Item.Attribute]]] = [] - with sentry_sdk.start_span(op="function", description="add_attributes"): + with traces.start_span(name="add_attributes", attributes={"sentry.op": "function"}): for row in data: id = row.pop("id") ts = row.pop("timestamp") @@ -564,7 +565,7 @@ def add_attribute( # Second pass: sort attributes and assemble items items: list[GetTraceResponse.Item] = [] - with sentry_sdk.start_span(op="function", description="sort_attributes"): + with traces.start_span(name="sort_attributes", attributes={"sentry.op": "function"}): for id, timestamp, attributes in parsed_rows: item = GetTraceResponse.Item( id=id, @@ -576,9 +577,9 @@ def add_attribute( ) items.append(item) - current_span = sentry_sdk.get_current_span() + current_span = traces.get_current_span() if current_span is not None: - current_span.set_data("rows_processed", len(parsed_rows)) + current_span.set_attribute("rows_processed", len(parsed_rows)) return ProcessedResults( items=items, @@ -663,7 +664,7 @@ def _execute(self, in_msg: GetTraceRequest) -> GetTraceResponse: page_token = EndpointGetTracePageToken(i, last_seen_timestamp_precise, last_seen_id) break - with sentry_sdk.start_span(op="function", description="assemble_response"): + with traces.start_span(name="assemble_response", attributes={"sentry.op": "function"}): response_meta = extract_response_meta( in_msg.meta.request_id, in_msg.meta.debug, diff --git a/snuba/web/views.py b/snuba/web/views.py index 5598dd4d256..b950e035ac5 100644 --- a/snuba/web/views.py +++ b/snuba/web/views.py @@ -28,6 +28,7 @@ from flask import request as http_request from flask_compress import Compress from sentry_protos.snuba.v1.error_pb2 import Error as ErrorProto +from sentry_sdk import traces from werkzeug import Response as WerkzeugResponse from werkzeug.exceptions import InternalServerError @@ -96,14 +97,14 @@ def truncate_dataset(dataset: Dataset) -> None: def _add_compression_attrs(response: Response) -> Response: - # TODO: update to attributes once we update the sdk - # leaving these as tags so I can aggregate by them in the meantime - sentry_sdk.set_tag("snuba.req_accept_encoding", http_request.headers.get("Accept-Encoding")) - sentry_sdk.set_tag("snuba.resp_encoding", response.headers.get("Content-Encoding")) - sentry_sdk.set_tag("snuba.resp_mime_type", response.mimetype) - span = sentry_sdk.get_current_span() - if span is not None: - span.set_data("snuba.resp_content_length", response.content_length) + sentry_sdk.set_attribute( + "snuba.req_accept_encoding", http_request.headers.get("Accept-Encoding") or "" + ) + sentry_sdk.set_attribute("snuba.resp_encoding", response.headers.get("Content-Encoding") or "") + sentry_sdk.set_attribute("snuba.resp_mime_type", response.mimetype or "") + span = traces.get_current_span() + if span is not None and response.content_length is not None: + span.set_attribute("snuba.resp_content_length", response.content_length) return response @@ -232,7 +233,7 @@ def health() -> Response: def parse_request_body(http_request: Request) -> dict[str, Any]: - with sentry_sdk.start_span(description="parse_request_body", op="parse"): + with traces.start_span(name="parse_request_body", attributes={"sentry.op": "parse"}): metrics.timing("http_request_body_length", len(http_request.data)) try: body = json.loads(http_request.data) @@ -243,49 +244,50 @@ def parse_request_body(http_request: Request) -> dict[str, Any]: def _trace_transaction(dataset_name: str) -> None: - span = sentry_sdk.get_current_span() - if span: - span.set_tag("dataset", dataset_name) - span.set_tag("referrer", http_request.referrer) - - scope = sentry_sdk.get_current_scope() - if scope.transaction: - scope.set_transaction_name( - f"{scope.transaction.name}__{dataset_name}__{http_request.referrer}" - ) - scope.transaction.set_tag("dataset", dataset_name) - scope.transaction.set_tag("referrer", http_request.referrer) + # Set on the scope so every span in this request inherits them, including + # the service span the WSGI integration opened before we got here. + sentry_sdk.set_attribute("dataset", dataset_name) + sentry_sdk.set_attribute("referrer", http_request.referrer or "") + + # `scope.transaction` is None in stream mode, so we can't read the current + # name back off it. Rebuild it from the same source the Flask integration + # uses (the endpoint, its default transaction_style) and rename the segment + # through the scope, which does work in stream mode. + endpoint = http_request.url_rule.endpoint if http_request.url_rule else "unknown" + sentry_sdk.get_current_scope().set_transaction_name( + f"{endpoint}__{dataset_name}__{http_request.referrer}" + ) def _set_snql_api_error_tags(body: dict[str, Any], http_referrer: str | None) -> None: - """Set Sentry tags for SnQL API error tracking. + """Set Sentry scope attributes for SnQL API error tracking. - Tags all errors in the SnQL API with: + Annotates all errors in the SnQL API with: - source: snql_api - referrer: from HTTP header or request body - - tenant_ids: as context and individual tags + - tenant_ids: as context and individual attributes This function is wrapped in a try-except to ensure that any failure - in setting tags does not crash the API request. + in setting attributes does not crash the API request. """ try: - sentry_sdk.set_tag("source", "snql_api") + sentry_sdk.set_attribute("source", "snql_api") - # Extract and tag referrer + # Extract and annotate referrer referrer = http_referrer or body.get("tenant_ids", {}).get("referrer", "") - sentry_sdk.set_tag("referrer", referrer) + sentry_sdk.set_attribute("referrer", referrer) # Extract and set tenant_ids as context for better error tracking tenant_ids = body.get("tenant_ids", {}) if tenant_ids: sentry_sdk.set_context("tenant_ids", tenant_ids) - # Also set individual tenant_id tags for easier filtering + # Also set individual tenant_id attributes for easier filtering for key, value in tenant_ids.items(): - if key != "referrer": # Skip referrer as it's already a tag - sentry_sdk.set_tag(f"tenant_id.{key}", str(value)) + if key != "referrer": # Skip referrer as it's already set above + sentry_sdk.set_attribute(f"tenant_id.{key}", str(value)) except Exception as e: # Log the error but don't let it crash the API request - logger.warning("Failed to set Sentry tags for SnQL API", exc_info=e) + logger.warning("Failed to set Sentry attributes for SnQL API", exc_info=e) @application.route("/query", methods=["GET", "POST"]) From f20c2d83784c7d6bf23a0e6f67e48316b70fb9a3 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Fri, 31 Jul 2026 15:37:55 -0700 Subject: [PATCH 2/6] fix(tracing): Address PR review feedback for span streaming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dual-write scope tags and attributes on API error-enrichment paths so values still land on Sentry issues during the tags→attributes transition. Centralize the stream-mode op key as SENTRY_OP, harden querylog trace_id lookup, clarify clickhouse-connect param narrowing, and always set insert query_id on spans. --- .pre-commit-config.yaml | 1 - snuba/cli/__init__.py | 3 +- snuba/clickhouse/connect.py | 23 ++++++++---- snuba/clickhouse/native.py | 3 +- snuba/datasets/configuration/json_schema.py | 7 ++-- snuba/datasets/configuration/loader.py | 3 +- snuba/datasets/entities/factory.py | 3 +- snuba/datasets/factory.py | 3 +- snuba/datasets/plans/entity_processing.py | 7 ++-- snuba/datasets/plans/entity_validation.py | 3 +- snuba/datasets/plans/storage_processing.py | 5 ++- snuba/datasets/storages/factory.py | 3 +- .../pipeline/composite_storage_processing.py | 3 +- snuba/pipeline/processors.py | 5 ++- snuba/pipeline/stages/query_execution.py | 9 +++-- snuba/query/allocation_policies/__init__.py | 3 +- snuba/query/mql/parser.py | 19 +++++----- snuba/query/snql/parser.py | 15 ++++---- snuba/replacers/projects_query_flags.py | 7 ++-- snuba/request/validation.py | 8 +++-- snuba/utils/metrics/util.py | 8 ++--- snuba/utils/sentry.py | 20 +++++++++++ snuba/web/db_query.py | 3 +- snuba/web/query.py | 3 +- snuba/web/rpc/__init__.py | 13 ++++--- .../routing_strategies/storage_routing.py | 8 +++-- snuba/web/rpc/v1/endpoint_get_trace.py | 7 ++-- snuba/web/views.py | 36 +++++++++++-------- 28 files changed, 142 insertions(+), 89 deletions(-) create mode 100644 snuba/utils/sentry.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7849f1b3733..3c571cc4538 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -38,7 +38,6 @@ repos: entry: env SNUBA_SETTINGS=test python3 -m snuba.validate_configs language: python # sentry_sdk must be >=2.62 for `sentry_sdk.traces` (span streaming API). - # Keep the floor in sync with pyproject.toml. additional_dependencies: [ 'fastjsonschema', 'pyyaml', 'sentry_sdk>=2.66.1' ] pass_filenames: false files: 'snuba/datasets/configuration/.*' diff --git a/snuba/cli/__init__.py b/snuba/cli/__init__.py index 37a283ff4bf..5e456b0ff56 100644 --- a/snuba/cli/__init__.py +++ b/snuba/cli/__init__.py @@ -13,6 +13,7 @@ from snuba.environment import metrics as environment_metrics from snuba.environment import setup_logging, setup_sentry from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import SENTRY_OP setup_sentry() @@ -52,7 +53,7 @@ def get_command(self, ctx: Any, name: str) -> click.Command: # other span; the `snuba_init_time` metric below is unaffected. with traces.start_span( name=f"[cli init] {name}", - attributes={"sentry.op": "snuba_init"}, + attributes={SENTRY_OP: "snuba_init"}, parent_span=None, ): actual_command_name = name.replace("-", "_") diff --git a/snuba/clickhouse/connect.py b/snuba/clickhouse/connect.py index d97d8e7098f..31c59f2bf5f 100644 --- a/snuba/clickhouse/connect.py +++ b/snuba/clickhouse/connect.py @@ -26,6 +26,7 @@ ) from snuba.reader import unwrap_nullable_type from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import SENTRY_OP logger = logging.getLogger("snuba.clickhouse.connect") @@ -38,10 +39,13 @@ def _driver_params(params: Params) -> Sequence[Any] | dict[str, Any] | None: ``Params`` allows any ``Mapping``, but the driver's signature takes an invariant ``dict``. Falsy params (including an empty sequence) mean "no parameters" here, matching the previous ``params if params else None``. + Sequence params are passed through; mappings are copied to a plain ``dict``. """ if not params: return None - return dict(params) if isinstance(params, Mapping) else params + if isinstance(params, Mapping): + return dict(params) + return params # Stand-in for "no read timeout" on the HTTP path. The native driver maps a @@ -224,7 +228,7 @@ def _execute_once( with traces.start_span( name="clickhouse query", attributes={ - "sentry.op": "db.clickhouse", + SENTRY_OP: "db.clickhouse", sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, }, @@ -316,7 +320,7 @@ def execute_with_totals( with traces.start_span( name="clickhouse query", attributes={ - "sentry.op": "db.clickhouse", + SENTRY_OP: "db.clickhouse", sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, }, @@ -473,15 +477,20 @@ def insert( with self._translate_clickhouse_errors(): client = self._get_client() + # No DB_QUERY_TEXT here: insert goes through client.insert with a + # row matrix, not a SQL string. with traces.start_span( name=f"INSERT INTO {table}", attributes={ - "sentry.op": "db.clickhouse", + SENTRY_OP: "db.clickhouse", sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", }, ) as span: - if query_id is not None: - span.set_attribute("query_id", query_id) + # Always set so a missing id is obvious when inspecting the span. + span.set_attribute( + "query_id", + query_id if query_id is not None else "unknown-query-id", + ) client.insert( table, matrix, @@ -541,7 +550,7 @@ def execute_explain(self, query: str) -> ClickhouseResult: with traces.start_span( name="clickhouse query", attributes={ - "sentry.op": "db.clickhouse", + SENTRY_OP: "db.clickhouse", sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, }, diff --git a/snuba/clickhouse/native.py b/snuba/clickhouse/native.py index 5bafad91dc3..d1ec3bf034b 100644 --- a/snuba/clickhouse/native.py +++ b/snuba/clickhouse/native.py @@ -31,6 +31,7 @@ from snuba.state.sentry_options import get_option from snuba.utils.metrics.gauge import ThreadSafeGauge from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import SENTRY_OP ignore_logger("clickhouse_driver.connection") @@ -284,7 +285,7 @@ def query_execute(conn: Client = conn, settings: Any = settings) -> Any: with traces.start_span( name="clickhouse query", attributes={ - "sentry.op": "db.clickhouse", + SENTRY_OP: "db.clickhouse", sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: query, }, diff --git a/snuba/datasets/configuration/json_schema.py b/snuba/datasets/configuration/json_schema.py index a6d106902ac..d305f68c93c 100644 --- a/snuba/datasets/configuration/json_schema.py +++ b/snuba/datasets/configuration/json_schema.py @@ -7,6 +7,7 @@ from sentry_sdk import traces from snuba import settings +from snuba.utils.sentry import SENTRY_OP # Snubadocs are automatically generated from this file. When adding new schemas or individual keys, # please ensure you add a description key in the same level and succinctly describe the property. @@ -864,17 +865,17 @@ def registered_class_array_schema( } if settings.VALIDATE_DATASET_YAMLS_ON_STARTUP: - with traces.start_span(name="Storage Validators", attributes={"sentry.op": "compile"}): + with traces.start_span(name="Storage Validators", attributes={SENTRY_OP: "compile"}): STORAGE_VALIDATORS = { "readable_storage": fastjsonschema.compile(V1_READABLE_STORAGE_SCHEMA), "writable_storage": fastjsonschema.compile(V1_WRITABLE_STORAGE_SCHEMA), "cdc_storage": fastjsonschema.compile(V1_CDC_STORAGE_SCHEMA), } - with traces.start_span(name="Entity Validators", attributes={"sentry.op": "compile"}): + with traces.start_span(name="Entity Validators", attributes={SENTRY_OP: "compile"}): ENTITY_VALIDATORS = {"entity": fastjsonschema.compile(V1_ENTITY_SCHEMA)} - with traces.start_span(name="Dataset Validators", attributes={"sentry.op": "compile"}): + with traces.start_span(name="Dataset Validators", attributes={SENTRY_OP: "compile"}): DATASET_VALIDATORS = {"dataset": fastjsonschema.compile(V1_DATASET_SCHEMA)} else: STORAGE_VALIDATORS = {} diff --git a/snuba/datasets/configuration/loader.py b/snuba/datasets/configuration/loader.py index a7d9362ba1c..94cf7955abd 100644 --- a/snuba/datasets/configuration/loader.py +++ b/snuba/datasets/configuration/loader.py @@ -6,6 +6,7 @@ from yaml import safe_load from snuba import settings +from snuba.utils.sentry import SENTRY_OP def load_configuration_data(path: str, validators: dict[str, Any]) -> dict[str, Any]: @@ -15,7 +16,7 @@ def load_configuration_data(path: str, validators: dict[str, Any]) -> dict[str, """ with traces.start_span( name="load_and_validate", - attributes={"sentry.op": "load_and_validate", "file": path}, + attributes={SENTRY_OP: "load_and_validate", "file": path}, ) as span: with open(path) as file: config = safe_load(file) diff --git a/snuba/datasets/entities/factory.py b/snuba/datasets/entities/factory.py index 532279aead9..c2519f1c0ca 100644 --- a/snuba/datasets/entities/factory.py +++ b/snuba/datasets/entities/factory.py @@ -13,12 +13,13 @@ from snuba.datasets.storages.factory import initialize_storage_factory from snuba.datasets.table_storage import TableWriter from snuba.utils.config_component_factory import ConfigComponentFactory +from snuba.utils.sentry import SENTRY_OP from snuba.utils.serializable_exception import SerializableException class _EntityFactory(ConfigComponentFactory[Entity, EntityKey]): def __init__(self) -> None: - with traces.start_span(name="Entity Factory", attributes={"sentry.op": "initialize"}): + with traces.start_span(name="Entity Factory", attributes={SENTRY_OP: "initialize"}): initialize_storage_factory() self._entity_map: dict[EntityKey, PluggableEntity] = {} self._name_map: dict[type[Entity], EntityKey] = {} diff --git a/snuba/datasets/factory.py b/snuba/datasets/factory.py index 60ce2f335e4..2f5244e1d34 100644 --- a/snuba/datasets/factory.py +++ b/snuba/datasets/factory.py @@ -11,12 +11,13 @@ from snuba.datasets.pluggable_dataset import PluggableDataset from snuba.utils.config_component_factory import ConfigComponentFactory from snuba.utils.metrics.util import with_span +from snuba.utils.sentry import SENTRY_OP from snuba.utils.serializable_exception import SerializableException class _DatasetFactory(ConfigComponentFactory[Dataset, str]): def __init__(self) -> None: - with traces.start_span(name="Dataset Factory", attributes={"sentry.op": "initialize"}): + with traces.start_span(name="Dataset Factory", attributes={SENTRY_OP: "initialize"}): initialize_entity_factory() self._dataset_map: dict[str, Dataset] = {} self._name_map: dict[type[Dataset], str] = {} diff --git a/snuba/datasets/plans/entity_processing.py b/snuba/datasets/plans/entity_processing.py index 1d6c8df0346..3692718b363 100644 --- a/snuba/datasets/plans/entity_processing.py +++ b/snuba/datasets/plans/entity_processing.py @@ -26,6 +26,7 @@ from snuba.query.processors.physical import ClickhouseQueryProcessor from snuba.query.query_settings import QuerySettings from snuba.state import explain_meta +from snuba.utils.sentry import SENTRY_OP class EntityProcessingExecutor: @@ -58,7 +59,7 @@ def __init__( def get_storage(self, query: LogicalQuery, settings: QuerySettings) -> EntityStorageConnection: with traces.start_span( - name="select_storage", attributes={"sentry.op": "build_plan.storage_query_plan_builder"} + name="select_storage", attributes={SENTRY_OP: "build_plan.storage_query_plan_builder"} ): return self.__selector.select_storage(query, settings, self.__storages) @@ -67,7 +68,7 @@ def get_cluster( ) -> ClickhouseCluster: if is_storage_set_sliced(storage.get_storage_set_key()): with traces.start_span( - name="select_storage", attributes={"sentry.op": "build_plan.sliced_storage"} + name="select_storage", attributes={SENTRY_OP: "build_plan.sliced_storage"} ): assert self.__partition_key_column_name is not None, ( "partition key column name must be defined for a sliced storage" @@ -92,7 +93,7 @@ def translate_query_and_apply_mappers( check_storage_readiness(storage) with traces.start_span( - name="translate", attributes={"sentry.op": "build_plan.storage_query_plan_builder"} + name="translate", attributes={SENTRY_OP: "build_plan.storage_query_plan_builder"} ): # The QueryTranslator class should be instantiated once for each call to # translate_query_and_apply_mappers to avoid cache conflicts. diff --git a/snuba/datasets/plans/entity_validation.py b/snuba/datasets/plans/entity_validation.py index 3035f3dd98b..7ae9aa1877f 100644 --- a/snuba/datasets/plans/entity_validation.py +++ b/snuba/datasets/plans/entity_validation.py @@ -12,6 +12,7 @@ from snuba.query.parser.validation.functions import FunctionCallsValidator from snuba.query.query_settings import QuerySettings from snuba.state import explain_meta +from snuba.utils.sentry import SENTRY_OP EXPRESSION_VALIDATORS = [FunctionCallsValidator()] @@ -69,7 +70,7 @@ def run_entity_validators( """ for validator_func in VALIDATORS: description = getattr(validator_func, "__name__", "custom") - with traces.start_span(name=description, attributes={"sentry.op": "validator"}): + with traces.start_span(name=description, attributes={SENTRY_OP: "validator"}): if settings and settings.get_dry_run(): with explain_meta.with_query_differ("entity_validator", description, query): validator_func(query) diff --git a/snuba/datasets/plans/storage_processing.py b/snuba/datasets/plans/storage_processing.py index 26e164deda4..3684c4b131f 100644 --- a/snuba/datasets/plans/storage_processing.py +++ b/snuba/datasets/plans/storage_processing.py @@ -39,6 +39,7 @@ from snuba.query.query_settings import QuerySettings from snuba.state import explain_meta from snuba.utils.metrics.util import with_span +from snuba.utils.sentry import SENTRY_OP TQuery = TypeVar("TQuery", bound=AbstractQuery) @@ -143,9 +144,7 @@ def apply_storage_processors( ) assert isinstance(query_plan.query, Query) for processor in query_plan.db_query_processors: - with traces.start_span( - name=type(processor).__name__, attributes={"sentry.op": "processor"} - ): + with traces.start_span(name=type(processor).__name__, attributes={SENTRY_OP: "processor"}): if settings.get_dry_run(): with explain_meta.with_query_differ( "storage_processor", type(processor).__name__, query_plan.query diff --git a/snuba/datasets/storages/factory.py b/snuba/datasets/storages/factory.py index aebaf7d7441..0d316592402 100644 --- a/snuba/datasets/storages/factory.py +++ b/snuba/datasets/storages/factory.py @@ -14,11 +14,12 @@ from snuba.datasets.storages.storage_key import StorageKey from snuba.datasets.storages.validator import StorageValidator from snuba.utils.config_component_factory import ConfigComponentFactory +from snuba.utils.sentry import SENTRY_OP class _StorageFactory(ConfigComponentFactory[Storage, StorageKey]): def __init__(self) -> None: - with traces.start_span(name="Storage Factory", attributes={"sentry.op": "initialize"}): + with traces.start_span(name="Storage Factory", attributes={SENTRY_OP: "initialize"}): self._config_built_storages: dict[StorageKey, Storage] = {} self._all_storages: dict[StorageKey, Storage] = {} self.__initialize() diff --git a/snuba/pipeline/composite_storage_processing.py b/snuba/pipeline/composite_storage_processing.py index e8341ededbc..7851b9c30fb 100644 --- a/snuba/pipeline/composite_storage_processing.py +++ b/snuba/pipeline/composite_storage_processing.py @@ -22,6 +22,7 @@ from snuba.query.processors.physical import ClickhouseQueryProcessor from snuba.query.query_settings import QuerySettings from snuba.state import explain_meta +from snuba.utils.sentry import SENTRY_OP def apply_composite_storage_processors( @@ -271,7 +272,7 @@ def __process_simple_query( for clickhouse_processor in processors: with traces.start_span( name=type(clickhouse_processor).__name__, - attributes={"sentry.op": "processor"}, + attributes={SENTRY_OP: "processor"}, ): clickhouse_processor.process_query(clickhouse_query, self.__settings) diff --git a/snuba/pipeline/processors.py b/snuba/pipeline/processors.py index 939c088c38a..5dca4664695 100644 --- a/snuba/pipeline/processors.py +++ b/snuba/pipeline/processors.py @@ -4,6 +4,7 @@ from snuba.query.logical import EntityQuery from snuba.query.query_settings import QuerySettings from snuba.state import explain_meta +from snuba.utils.sentry import SENTRY_OP def execute_entity_processors(query: EntityQuery, settings: QuerySettings) -> None: @@ -14,9 +15,7 @@ def execute_entity_processors(query: EntityQuery, settings: QuerySettings) -> No entity = get_entity(query.get_from_clause().key) for processor in entity.get_query_processors(): - with traces.start_span( - name=type(processor).__name__, attributes={"sentry.op": "processor"} - ): + with traces.start_span(name=type(processor).__name__, attributes={SENTRY_OP: "processor"}): if settings.get_dry_run(): with explain_meta.with_query_differ( "entity_processor", type(processor).__name__, query diff --git a/snuba/pipeline/stages/query_execution.py b/snuba/pipeline/stages/query_execution.py index e01a0da4519..500f09c9035 100644 --- a/snuba/pipeline/stages/query_execution.py +++ b/snuba/pipeline/stages/query_execution.py @@ -37,6 +37,7 @@ from snuba.utils.metrics.gauge import Gauge from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import SENTRY_OP from snuba.web import ( QueryException, QueryExtraData, @@ -107,9 +108,7 @@ def _dry_run_query_runner( clickhouse_query: ClickhouseQuery | CompositeQuery[Table], cluster_name: str, ) -> QueryResult: - with traces.start_span( - name="dryrun_create_query", attributes={"sentry.op": "function"} - ) as span: + with traces.start_span(name="dryrun_create_query", attributes={SENTRY_OP: "function"}) as span: formatted_query = format_query(clickhouse_query) span.set_attribute("query", json.dumps(formatted_query.structured(), default=repr)) @@ -193,7 +192,7 @@ def _format_storage_query_and_run( visitor = TablesCollector() visitor.visit(from_clause) table_names = ",".join(sorted(visitor.get_tables())) - with traces.start_span(name="create_query", attributes={"sentry.op": "function"}) as span: + with traces.start_span(name="create_query", attributes={SENTRY_OP: "function"}) as span: _apply_turbo_sampling_if_needed(clickhouse_query, query_settings) formatted_query = format_query(clickhouse_query) @@ -258,7 +257,7 @@ def _format_storage_query_and_run( with traces.start_span( name="execute_query", attributes={ - "sentry.op": "function", + SENTRY_OP: "function", sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: formatted_sql, "table": table_names, }, diff --git a/snuba/query/allocation_policies/__init__.py b/snuba/query/allocation_policies/__init__.py index b3c47c05dea..0503ea82e66 100644 --- a/snuba/query/allocation_policies/__init__.py +++ b/snuba/query/allocation_policies/__init__.py @@ -21,6 +21,7 @@ from snuba.datasets.storages.storage_key import StorageKey from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.utils.registered_class import import_submodules_in_directory +from snuba.utils.sentry import SENTRY_OP from snuba.utils.serializable_exception import JsonSerializable, SerializableException from snuba.web import QueryResult @@ -454,7 +455,7 @@ def get_quota_allowance( ) -> QuotaAllowance: with traces.start_span( name=self.__class__.__name__, - attributes={"sentry.op": "allocation_policy.get_quota_allowance"}, + attributes={SENTRY_OP: "allocation_policy.get_quota_allowance"}, ) as span: for t, tid in tenant_ids.items(): span.set_attribute(f"tenant_ids.{t}", str(tid)) diff --git a/snuba/query/mql/parser.py b/snuba/query/mql/parser.py index f289341395e..944a912be7a 100644 --- a/snuba/query/mql/parser.py +++ b/snuba/query/mql/parser.py @@ -72,6 +72,7 @@ ) from snuba.state import explain_meta from snuba.utils.metrics.timer import Timer +from snuba.utils.sentry import SENTRY_OP # The parser returns a bunch of different types, so create a single aggregate type to # capture everything. @@ -1265,7 +1266,7 @@ def parse_mql_query( # NOTE (volo): The anonymizer that runs after this function call chokes on # OR and AND clauses with multiple parameters so we have to treeify them # before we run the anonymizer and the rest of the post processors - with traces.start_span(name="treeify_conditions", attributes={"sentry.op": "processor"}): + with traces.start_span(name="treeify_conditions", attributes={SENTRY_OP: "processor"}): _post_process(query, [_treeify_or_and_conditions], settings) res = PostProcessAndValidateMQLQuery().execute( @@ -1295,16 +1296,16 @@ def _process_data( ) -> LogicalQuery: mql_str, dataset, mql_context_dict, settings = pipe_input.data - with traces.start_span(name="parse_mql_query_initial", attributes={"sentry.op": "parser"}): + with traces.start_span(name="parse_mql_query_initial", attributes={SENTRY_OP: "parser"}): query = parse_mql_query_body(mql_str, dataset) with traces.start_span( - name="populate_query_from_mql_context", attributes={"sentry.op": "parser"} + name="populate_query_from_mql_context", attributes={SENTRY_OP: "parser"} ): query, mql_context = populate_query_from_mql_context(query, mql_context_dict) with traces.start_span( - name="resolve_indexer_mappings", attributes={"sentry.op": "processor"} + name="resolve_indexer_mappings", attributes={SENTRY_OP: "processor"} ): resolve_mappings(query, mql_context.indexer_mappings, dataset) @@ -1335,7 +1336,7 @@ def _process_data( ], ) -> LogicalQuery: query, settings, custom_processing = pipe_input.data - with traces.start_span(name="post_processors", attributes={"sentry.op": "processor"}): + with traces.start_span(name="post_processors", attributes={SENTRY_OP: "processor"}): _post_process( query, MQL_POST_PROCESSORS, @@ -1344,7 +1345,7 @@ def _process_data( # Filter in select optimizer with traces.start_span( - name="filter_in_select_optimize", attributes={"sentry.op": "processor"} + name="filter_in_select_optimize", attributes={SENTRY_OP: "processor"} ): if settings is None: FilterInSelectOptimizer().process_query(query, HTTPQuerySettings()) @@ -1352,16 +1353,16 @@ def _process_data( FilterInSelectOptimizer().process_query(query, settings) # Custom processing to tweak the AST before validation - with traces.start_span(name="custom_processing", attributes={"sentry.op": "processor"}): + with traces.start_span(name="custom_processing", attributes={SENTRY_OP: "processor"}): if custom_processing is not None: _post_process(query, custom_processing, settings) # Time based processing - with traces.start_span(name="time_based_processing", attributes={"sentry.op": "processor"}): + with traces.start_span(name="time_based_processing", attributes={SENTRY_OP: "processor"}): _post_process(query, [_replace_time_condition], settings) # Validating - with traces.start_span(name="expression_validators", attributes={"sentry.op": "validate"}): + with traces.start_span(name="expression_validators", attributes={SENTRY_OP: "validate"}): _post_process(query, VALIDATORS) return query diff --git a/snuba/query/snql/parser.py b/snuba/query/snql/parser.py index 72c07acfb73..d4fbceb8ceb 100644 --- a/snuba/query/snql/parser.py +++ b/snuba/query/snql/parser.py @@ -103,6 +103,7 @@ from snuba.state.sentry_options import get_option from snuba.util import parse_datetime from snuba.utils.metrics.timer import Timer +from snuba.utils.sentry import SENTRY_OP MAX_LIMIT = 10000 @@ -1400,7 +1401,7 @@ def _post_process( # custom processors can be partials instead of functions but partials don't # have the __name__ attribute set automatically (and we don't set it manually) description = getattr(func, "__name__", "custom") - with traces.start_span(name=description, attributes={"sentry.op": "processor"}): + with traces.start_span(name=description, attributes={SENTRY_OP: "processor"}): if settings and settings.get_dry_run(): with explain_meta.with_query_differ("snql_parsing", description, query): func(query) @@ -1440,7 +1441,7 @@ def parse_snql_query( custom_processing: CustomProcessors | None = None, settings: QuerySettings | None = None, ) -> CompositeQuery[LogicalDataSource] | LogicalQuery: - with traces.start_span(name="parse_snql_query_initial", attributes={"sentry.op": "parser"}): + with traces.start_span(name="parse_snql_query_initial", attributes={SENTRY_OP: "parser"}): query = parse_snql_query_initial(body) if settings and settings.get_dry_run(): @@ -1450,7 +1451,7 @@ def parse_snql_query( # NOTE (volo): The anonymizer that runs after this function call chokes on # OR and AND clauses with multiple parameters so we have to treeify them # before we run the anonymizer and the rest of the post processors - with traces.start_span(name="treeify_conditions", attributes={"sentry.op": "processor"}): + with traces.start_span(name="treeify_conditions", attributes={SENTRY_OP: "processor"}): _post_process(query, [_treeify_or_and_conditions], settings) timer = Timer("snql_pipeline") @@ -1495,22 +1496,22 @@ def _process_data( query, dataset, custom_processing = pipe_input.data settings = pipe_input.query_settings - with traces.start_span(name="post_processors", attributes={"sentry.op": "processor"}): + with traces.start_span(name="post_processors", attributes={SENTRY_OP: "processor"}): _post_process( query, POST_PROCESSORS, settings, ) # Custom processing to tweak the AST before validation - with traces.start_span(name="custom_processing", attributes={"sentry.op": "processor"}): + with traces.start_span(name="custom_processing", attributes={SENTRY_OP: "processor"}): if custom_processing is not None: _post_process(query, custom_processing, settings) # Time based processing - with traces.start_span(name="time_based_processing", attributes={"sentry.op": "processor"}): + with traces.start_span(name="time_based_processing", attributes={SENTRY_OP: "processor"}): _post_process(query, [_replace_time_condition], settings) _post_process(query, [_select_entity_for_dataset(dataset)], settings) # Validating - with traces.start_span(name="expression_validators", attributes={"sentry.op": "validate"}): + with traces.start_span(name="expression_validators", attributes={SENTRY_OP: "validate"}): _post_process(query, VALIDATORS) return query diff --git a/snuba/replacers/projects_query_flags.py b/snuba/replacers/projects_query_flags.py index 7e18a74be5f..bea1fadfd58 100644 --- a/snuba/replacers/projects_query_flags.py +++ b/snuba/replacers/projects_query_flags.py @@ -16,6 +16,7 @@ from snuba.redis import RedisClientKey, get_redis_client from snuba.replacers.replacer_processor import ReplacerState from snuba.state.sentry_options import get_option +from snuba.utils.sentry import SENTRY_OP redis_client = get_redis_client(RedisClientKey.REPLACEMENTS_STORE) @@ -129,19 +130,19 @@ def load_from_redis( try: with redis_client.pipeline() as p: with traces.start_span( - name="build_redis_pipeline", attributes={"sentry.op": "function"} + name="build_redis_pipeline", attributes={SENTRY_OP: "function"} ): cls._query_redis(s_project_ids, state_name, p) with traces.start_span( - name="execute_redis_pipeline", attributes={"sentry.op": "function"} + name="execute_redis_pipeline", attributes={SENTRY_OP: "function"} ) as span: results = p.execute() # getting size of str(results) since sys.getsizeof() doesn't count recursively span.set_attribute("results_size", sys.getsizeof(str(results))) with traces.start_span( - name="process_redis_results", attributes={"sentry.op": "function"} + name="process_redis_results", attributes={SENTRY_OP: "function"} ) as span: flags = cls._process_redis_results(results, len(s_project_ids)) # Span attributes only accept scalars and homogeneous lists, so diff --git a/snuba/request/validation.py b/snuba/request/validation.py index 003ace062c7..6f313dbd4fd 100644 --- a/snuba/request/validation.py +++ b/snuba/request/validation.py @@ -35,6 +35,7 @@ from snuba.state.sentry_options import get_mapped_option, get_option from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import SENTRY_OP, set_tag_and_attribute metrics = MetricsWrapper(environment.metrics, "snuba.validation") @@ -110,7 +111,7 @@ def build_request( referrer: str, custom_processing: CustomProcessors | None = None, ) -> Request: - with traces.start_span(name="build_request", attributes={"sentry.op": "validate"}) as span: + with traces.start_span(name="build_request", attributes={SENTRY_OP: "validate"}) as span: try: dataset_name = get_dataset_name(dataset) if get_mapped_option( @@ -236,11 +237,12 @@ def _build_request( ) -> Request: org_ids = get_object_ids_in_query_ast(query, "org_id") if org_ids is not None and len(org_ids) == 1: - sentry_sdk.set_attribute("snuba_org_id", org_ids.pop()) + # Dual-write tags + attributes while telemetry is mid-transition. + set_tag_and_attribute("snuba_org_id", org_ids.pop()) query_project_id = _get_project_id(query) if query_project_id: - sentry_sdk.set_attribute("snuba_project_id", query_project_id) + set_tag_and_attribute("snuba_project_id", query_project_id) attribution_info = _get_attribution_info(request_parts, referrer, query_project_id) diff --git a/snuba/utils/metrics/util.py b/snuba/utils/metrics/util.py index dafc6215558..982f398040e 100644 --- a/snuba/utils/metrics/util.py +++ b/snuba/utils/metrics/util.py @@ -4,16 +4,14 @@ import inspect from collections.abc import Callable, Mapping from functools import wraps -from typing import TYPE_CHECKING, Any, TypeVar, cast +from typing import Any, TypeVar, cast from sentry_sdk import traces from snuba import settings from snuba.utils.metrics import MetricsBackend from snuba.utils.metrics.types import Tags - -if TYPE_CHECKING: - from sentry_sdk._types import Attributes +from snuba.utils.sentry import SENTRY_OP def create_metrics( @@ -77,7 +75,7 @@ def with_span(op: str = "function") -> Callable[[F], F]: def decorator(func: F) -> F: frame_info = inspect.stack()[1] # Built once at decoration time rather than on every call. - attributes: Attributes = {"sentry.op": op, "filename": frame_info.filename} + attributes: dict[str, Any] = {SENTRY_OP: op, "filename": frame_info.filename} @wraps(func) def wrapper(*args: Any, **kwargs: Any) -> Any: diff --git a/snuba/utils/sentry.py b/snuba/utils/sentry.py new file mode 100644 index 00000000000..399aa8a18ff --- /dev/null +++ b/snuba/utils/sentry.py @@ -0,0 +1,20 @@ +from __future__ import annotations + +from typing import Any + +import sentry_sdk + +# Stream-mode span op lives as an attribute rather than a dedicated field. +# Centralize the key so call sites stay consistent. +SENTRY_OP = "sentry.op" + + +def set_tag_and_attribute(key: str, value: Any) -> None: + """Write both a scope tag and a scope attribute. + + During the transition to attribute-based telemetry, tags still land on + error events while attributes land on spans/logs/metrics. Write both so + the value is available on errors and on streamed spans. + """ + sentry_sdk.set_tag(key, value) + sentry_sdk.set_attribute(key, value) diff --git a/snuba/web/db_query.py b/snuba/web/db_query.py index 94ff16af193..123a6da419d 100644 --- a/snuba/web/db_query.py +++ b/snuba/web/db_query.py @@ -68,6 +68,7 @@ from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.util import set_current_span_attributes, with_span from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import SENTRY_OP from snuba.utils.serializable_exception import ( SerializableException, SerializableExceptionDict, @@ -870,7 +871,7 @@ def _apply_allocation_policies_quota( min_threads_across_policies = MAX_THRESHOLD with traces.start_span( name="_apply_allocation_policies_quota", - attributes={"sentry.op": "allocation_policy"}, + attributes={SENTRY_OP: "allocation_policy"}, ) as span: for allocation_policy in allocation_policies: allowance = allocation_policy.get_quota_allowance(attribution_info.tenant_ids, query_id) diff --git a/snuba/web/query.py b/snuba/web/query.py index 40058cc379a..9de4b8a8374 100644 --- a/snuba/web/query.py +++ b/snuba/web/query.py @@ -26,6 +26,7 @@ from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.util import with_span from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import SENTRY_OP from snuba.web import QueryException, QueryExtraData, QueryResult logger = logging.getLogger("snuba.query") @@ -140,7 +141,7 @@ def parse_and_run_query( referrer (str): legacy param, you probably don't need to provide this. It should be in the tenant_ids of the body """ - with traces.start_span(name="build_schema", attributes={"sentry.op": "validate"}): + with traces.start_span(name="build_schema", attributes={SENTRY_OP: "validate"}): schema = RequestSchema.build(HTTPQuerySettings, is_mql) # NOTE(Volo): dataset is not necessary for queries because storages can be queried directly diff --git a/snuba/web/rpc/__init__.py b/snuba/web/rpc/__init__.py index a8eb32917a3..8849c790695 100644 --- a/snuba/web/rpc/__init__.py +++ b/snuba/web/rpc/__init__.py @@ -25,6 +25,7 @@ RegisteredClass, import_submodules_in_directory, ) +from snuba.utils.sentry import SENTRY_OP, set_tag_and_attribute from snuba.web import QueryException from snuba.web.rpc.common.common import Tin, Tout from snuba.web.rpc.common.exceptions import ( @@ -85,19 +86,21 @@ def _flush_logs() -> None: def _set_rpc_error_tags(in_msg: ProtobufMessage) -> None: - sentry_sdk.set_attribute("source", "rpc_api") + # Dual-write tags + attributes while telemetry is mid-transition: tags land + # on error events, attributes on streamed spans. + set_tag_and_attribute("source", "rpc_api") # Extract and annotate fields from meta if available if hasattr(in_msg, "meta"): meta = in_msg.meta if hasattr(meta, "referrer") and meta.referrer: - sentry_sdk.set_attribute("referrer", meta.referrer) + set_tag_and_attribute("referrer", meta.referrer) if hasattr(meta, "organization_id") and meta.organization_id: - sentry_sdk.set_attribute("organization_id", str(meta.organization_id)) + set_tag_and_attribute("organization_id", str(meta.organization_id)) if hasattr(meta, "request_id") and meta.request_id: - sentry_sdk.set_attribute("request_id", str(meta.request_id)) + set_tag_and_attribute("request_id", str(meta.request_id)) class TraceItemDataResolver(Generic[Tin, Tout], metaclass=RegisteredClass): @@ -221,7 +224,7 @@ def execute(self, in_msg: Tin) -> Tout: with traces.start_span( name="execute", attributes={ - "sentry.op": "execute", + SENTRY_OP: "execute", "selected_tier": self.routing_decision.tier.name, }, ): diff --git a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py index 0ff4d8be088..eaa49b245c8 100644 --- a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py +++ b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py @@ -55,6 +55,7 @@ from snuba.utils.metrics.util import set_current_span_attributes from snuba.utils.metrics.wrapper import MetricsWrapper from snuba.utils.registered_class import import_submodules_in_directory +from snuba.utils.sentry import SENTRY_OP from snuba.web import QueryException, QueryResult from snuba.web.rpc.common.exceptions import RPCAllocationPolicyException from snuba.web.rpc.storage_routing.common import extract_message_meta @@ -216,7 +217,8 @@ def _construct_hacky_querylog_payload( # Read the trace id off the propagation context rather than the active span: # it is populated whether or not a span is active and whether or not the # trace was sampled, so the querylog always gets a real id. - trace_id = sentry_sdk.get_current_scope().get_active_propagation_context().trace_id + propagation_context = sentry_sdk.get_current_scope().get_active_propagation_context() + trace_id = propagation_context.trace_id if propagation_context is not None else "" assert routing_decision.routing_context is not None query_result = routing_decision.routing_context.query_result or QueryResult( {}, {"stats": {}, "sql": "", "experiments": {}} @@ -467,7 +469,7 @@ def _get_recommendations_from_allocation_policies( allocation_policy_name = allocation_policy.class_name() with traces.start_span( name=allocation_policy_name, - attributes={"sentry.op": "allocation_policy.get_quota_allowance"}, + attributes={SENTRY_OP: "allocation_policy.get_quota_allowance"}, ) as span: recommendations[allocation_policy_name] = allocation_policy.get_quota_allowance( routing_context.tenant_ids, @@ -489,7 +491,7 @@ def get_routing_decision(self, routing_context: RoutingContext) -> RoutingDecisi default_tier = self._get_default_routing_decision_tier() - with traces.start_span(name="decide_tier", attributes={"sentry.op": "decide_tier"}) as span: + with traces.start_span(name="decide_tier", attributes={SENTRY_OP: "decide_tier"}) as span: try: routing_context.timer.mark(_START_ESTIMATION_MARK) diff --git a/snuba/web/rpc/v1/endpoint_get_trace.py b/snuba/web/rpc/v1/endpoint_get_trace.py index 035b4a91fb9..ace4b97a819 100644 --- a/snuba/web/rpc/v1/endpoint_get_trace.py +++ b/snuba/web/rpc/v1/endpoint_get_trace.py @@ -48,6 +48,7 @@ ) from snuba.state.sentry_options import get_option from snuba.utils.metrics.util import with_span +from snuba.utils.sentry import SENTRY_OP from snuba.web.query import run_query from snuba.web.rpc import RPCEndpoint from snuba.web.rpc.common.common import ( @@ -495,7 +496,7 @@ def _process_results( # First pass: parse rows and build attribute dicts parsed_rows: list[tuple[str, Timestamp, dict[str, GetTraceResponse.Item.Attribute]]] = [] - with traces.start_span(name="add_attributes", attributes={"sentry.op": "function"}): + with traces.start_span(name="add_attributes", attributes={SENTRY_OP: "function"}): for row in data: id = row.pop("id") ts = row.pop("timestamp") @@ -565,7 +566,7 @@ def add_attribute( # Second pass: sort attributes and assemble items items: list[GetTraceResponse.Item] = [] - with traces.start_span(name="sort_attributes", attributes={"sentry.op": "function"}): + with traces.start_span(name="sort_attributes", attributes={SENTRY_OP: "function"}): for id, timestamp, attributes in parsed_rows: item = GetTraceResponse.Item( id=id, @@ -664,7 +665,7 @@ def _execute(self, in_msg: GetTraceRequest) -> GetTraceResponse: page_token = EndpointGetTracePageToken(i, last_seen_timestamp_precise, last_seen_id) break - with traces.start_span(name="assemble_response", attributes={"sentry.op": "function"}): + with traces.start_span(name="assemble_response", attributes={SENTRY_OP: "function"}): response_meta = extract_response_meta( in_msg.meta.request_id, in_msg.meta.debug, diff --git a/snuba/web/views.py b/snuba/web/views.py index b950e035ac5..bec4ead8d21 100644 --- a/snuba/web/views.py +++ b/snuba/web/views.py @@ -61,6 +61,7 @@ ) from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.util import with_span +from snuba.utils.sentry import SENTRY_OP, set_tag_and_attribute from snuba.web import QueryException, QueryTooLongException from snuba.web.bulk_delete_query import delete_from_storage as bulk_delete_from_storage from snuba.web.constants import get_http_status_for_clickhouse_error @@ -97,11 +98,11 @@ def truncate_dataset(dataset: Dataset) -> None: def _add_compression_attrs(response: Response) -> Response: - sentry_sdk.set_attribute( + set_tag_and_attribute( "snuba.req_accept_encoding", http_request.headers.get("Accept-Encoding") or "" ) - sentry_sdk.set_attribute("snuba.resp_encoding", response.headers.get("Content-Encoding") or "") - sentry_sdk.set_attribute("snuba.resp_mime_type", response.mimetype or "") + set_tag_and_attribute("snuba.resp_encoding", response.headers.get("Content-Encoding") or "") + set_tag_and_attribute("snuba.resp_mime_type", response.mimetype or "") span = traces.get_current_span() if span is not None and response.content_length is not None: span.set_attribute("snuba.resp_content_length", response.content_length) @@ -233,7 +234,7 @@ def health() -> Response: def parse_request_body(http_request: Request) -> dict[str, Any]: - with traces.start_span(name="parse_request_body", attributes={"sentry.op": "parse"}): + with traces.start_span(name="parse_request_body", attributes={SENTRY_OP: "parse"}): metrics.timing("http_request_body_length", len(http_request.data)) try: body = json.loads(http_request.data) @@ -245,9 +246,11 @@ def parse_request_body(http_request: Request) -> dict[str, Any]: def _trace_transaction(dataset_name: str) -> None: # Set on the scope so every span in this request inherits them, including - # the service span the WSGI integration opened before we got here. - sentry_sdk.set_attribute("dataset", dataset_name) - sentry_sdk.set_attribute("referrer", http_request.referrer or "") + # the service span the WSGI integration opened before we got here. Dual-write + # tags + attributes while telemetry is mid-transition: tags land on error + # events, attributes on streamed spans. + set_tag_and_attribute("dataset", dataset_name) + set_tag_and_attribute("referrer", http_request.referrer or "") # `scope.transaction` is None in stream mode, so we can't read the current # name back off it. Rebuild it from the same source the Flask integration @@ -260,34 +263,37 @@ def _trace_transaction(dataset_name: str) -> None: def _set_snql_api_error_tags(body: dict[str, Any], http_referrer: str | None) -> None: - """Set Sentry scope attributes for SnQL API error tracking. + """Set Sentry scope tags and attributes for SnQL API error tracking. Annotates all errors in the SnQL API with: - source: snql_api - referrer: from HTTP header or request body - - tenant_ids: as context and individual attributes + - tenant_ids: as context and individual tags/attributes + + Dual-writes tags and attributes while telemetry is mid-transition: tags + land on error events, attributes on streamed spans. This function is wrapped in a try-except to ensure that any failure - in setting attributes does not crash the API request. + in setting tags/attributes does not crash the API request. """ try: - sentry_sdk.set_attribute("source", "snql_api") + set_tag_and_attribute("source", "snql_api") # Extract and annotate referrer referrer = http_referrer or body.get("tenant_ids", {}).get("referrer", "") - sentry_sdk.set_attribute("referrer", referrer) + set_tag_and_attribute("referrer", referrer) # Extract and set tenant_ids as context for better error tracking tenant_ids = body.get("tenant_ids", {}) if tenant_ids: sentry_sdk.set_context("tenant_ids", tenant_ids) - # Also set individual tenant_id attributes for easier filtering + # Also set individual tenant_id tags/attributes for easier filtering for key, value in tenant_ids.items(): if key != "referrer": # Skip referrer as it's already set above - sentry_sdk.set_attribute(f"tenant_id.{key}", str(value)) + set_tag_and_attribute(f"tenant_id.{key}", str(value)) except Exception as e: # Log the error but don't let it crash the API request - logger.warning("Failed to set Sentry attributes for SnQL API", exc_info=e) + logger.warning("Failed to set Sentry tags/attributes for SnQL API", exc_info=e) @application.route("/query", methods=["GET", "POST"]) From 2087ec8a66e8f113e97824ab8a92739e429a7a13 Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 2 Aug 2026 11:30:05 -0700 Subject: [PATCH 3/6] ref(tracing): Set db.query.text on the clickhouse-connect insert span client.insert takes a row matrix rather than SQL, but it still builds and sends "INSERT INTO () FORMAT Native" on the wire, so the statement was available all along. Reconstruct it for db.query.text instead of leaving the attribute unset. This also brings the two pools in line: ClickhousePool.insert routes through execute() with an explicit statement and already populated the attribute, so only the HTTP path was missing it. Row data stays out of the attribute since it is unbounded and may hold PII. --- snuba/clickhouse/connect.py | 21 +++++++++++++++++++-- tests/clickhouse/test_connect.py | 10 +++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/snuba/clickhouse/connect.py b/snuba/clickhouse/connect.py index 31c59f2bf5f..53abfbdd2fa 100644 --- a/snuba/clickhouse/connect.py +++ b/snuba/clickhouse/connect.py @@ -11,6 +11,7 @@ import clickhouse_connect import sentry_sdk from clickhouse_connect import common as clickhouse_connect_common +from clickhouse_connect.driver.binding import quote_identifier from clickhouse_connect.driver.client import Client from clickhouse_connect.driver.exceptions import ClickHouseError, OperationalError from clickhouse_connect.driver.httputil import get_pool_manager @@ -48,6 +49,21 @@ def _driver_params(params: Params) -> Sequence[Any] | dict[str, Any] | None: return params +def _insert_statement(table: str, column_names: Sequence[str]) -> str: + """Rebuild the INSERT statement clickhouse-connect sends for ``client.insert``. + + ``client.insert`` takes a row matrix rather than SQL, but it still builds and + sends ``INSERT INTO
() FORMAT Native`` on the wire. Mirror it so + ``db.query.text`` is populated on this path too, matching the native pool, + where ``insert`` routes through ``execute`` with an explicit statement. + + The row data is deliberately excluded: it is unbounded in size and may hold + PII. + """ + columns = ", ".join(quote_identifier(name) for name in column_names) + return f"INSERT INTO {table} ({columns}) FORMAT Native" + + # Stand-in for "no read timeout" on the HTTP path. The native driver maps a # profile with no timeout (``None``) to an unbounded socket, but clickhouse-connect # cannot safely take ``None`` (its progress-interval computation does arithmetic @@ -477,13 +493,14 @@ def insert( with self._translate_clickhouse_errors(): client = self._get_client() - # No DB_QUERY_TEXT here: insert goes through client.insert with a - # row matrix, not a SQL string. with traces.start_span( name=f"INSERT INTO {table}", attributes={ SENTRY_OP: "db.clickhouse", sentry_sdk.consts.SPANDATA.DB_SYSTEM: "clickhouse", + sentry_sdk.consts.SPANDATA.DB_QUERY_TEXT: _insert_statement( + table, column_names + ), }, ) as span: # Always set so a missing id is obvious when inspecting the span. diff --git a/tests/clickhouse/test_connect.py b/tests/clickhouse/test_connect.py index 1d32c57bcd9..d5cc14e8f44 100644 --- a/tests/clickhouse/test_connect.py +++ b/tests/clickhouse/test_connect.py @@ -6,7 +6,7 @@ import pytest -from snuba.clickhouse.connect import ClickhouseConnectPool +from snuba.clickhouse.connect import ClickhouseConnectPool, _insert_statement from snuba.clickhouse.errors import ClickhouseError from snuba.clickhouse.formatter.nodes import FormattedQuery from snuba.clusters.cluster import ClickhouseClientSettings @@ -132,6 +132,14 @@ def test_insert_multiple_rows_build_a_matrix() -> None: ] +def test_insert_statement_mirrors_what_the_driver_sends() -> None: + # client.insert takes a row matrix, but sends this statement on the wire. + assert ( + _insert_statement("migrations_local", ["group", "migration_id"]) + == "INSERT INTO migrations_local (`group`, `migration_id`) FORMAT Native" + ) + + def test_insert_empty_rows_short_circuits() -> None: client = mock.Mock() From 1ac2e24d673ed7460c0d9d63a3ff040f1d3bf62f Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 2 Aug 2026 12:09:02 -0700 Subject: [PATCH 4/6] ref(tracing): Dual-write remaining scope tags on the query path Bugbot caught five scope-level set_tag call sites the sweep converted to set_attribute only: slo_status, cache_status, query_size_group, and the querylog duration_group/timeout/experiments/max_threads block. Scope tags and attributes reach disjoint destinations, so attribute-only means these stop appearing on Sentry issues. The API error-enrichment paths already dual-write via set_tag_and_attribute; these were missed. Route them through the same helper so no bare sentry_sdk.set_attribute is left outside it. Verified against a capturing transport that the value lands on both the error event tags and the streamed span attributes. Also normalize the referrer once in _trace_transaction so the transaction name matches the referrer attribute instead of interpolating a literal "None" when the Referer header is absent. --- snuba/pipeline/stages/query_execution.py | 4 ++-- snuba/querylog/__init__.py | 10 +++++----- snuba/web/db_query.py | 7 +++---- snuba/web/views.py | 9 +++++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/snuba/pipeline/stages/query_execution.py b/snuba/pipeline/stages/query_execution.py index 500f09c9035..b7f07556ce2 100644 --- a/snuba/pipeline/stages/query_execution.py +++ b/snuba/pipeline/stages/query_execution.py @@ -37,7 +37,7 @@ from snuba.utils.metrics.gauge import Gauge from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper -from snuba.utils.sentry import SENTRY_OP +from snuba.utils.sentry import SENTRY_OP, set_tag_and_attribute from snuba.web import ( QueryException, QueryExtraData, @@ -204,7 +204,7 @@ def _format_storage_query_and_run( ) # To avoid the query being truncated span.set_attribute("table", table_names) span.set_attribute("query_size_bytes", query_size_bytes) - sentry_sdk.set_attribute("query_size_group", get_query_size_group(query_size_bytes)) + set_tag_and_attribute("query_size_group", get_query_size_group(query_size_bytes)) metrics.increment( "execute", tags={ diff --git a/snuba/querylog/__init__.py b/snuba/querylog/__init__.py index 5df27116d57..661017e6410 100644 --- a/snuba/querylog/__init__.py +++ b/snuba/querylog/__init__.py @@ -6,7 +6,6 @@ from typing import Any from uuid import UUID -import sentry_sdk from sentry_kafka_schemas.schema_types import snuba_queries_v1 from sentry_sdk import traces from usageaccountant import UsageUnit @@ -25,6 +24,7 @@ from snuba.state.sentry_options import get_option from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.wrapper import MetricsWrapper +from snuba.utils.sentry import set_tag_and_attribute from snuba.web import QueryException, QueryResult metrics = MetricsWrapper(environment.metrics, "api") @@ -225,17 +225,17 @@ def _add_tags( ) -> None: if traces.get_current_span() is not None: duration_group = timer.get_duration_group() - sentry_sdk.set_attribute("duration_group", duration_group) + set_tag_and_attribute("duration_group", duration_group) if duration_group == ">30s": - sentry_sdk.set_attribute("timeout", "too_long") + set_tag_and_attribute("timeout", "too_long") if experiments is not None: for name, value in experiments.items(): - sentry_sdk.set_attribute(name, str(value)) + set_tag_and_attribute(name, str(value)) if metadata is not None: for query_data in metadata.query_list: max_threads = query_data.stats.get("max_threads") if max_threads is not None: - sentry_sdk.set_attribute("max_threads", max_threads) + set_tag_and_attribute("max_threads", max_threads) break diff --git a/snuba/web/db_query.py b/snuba/web/db_query.py index 123a6da419d..40fc40cd65c 100644 --- a/snuba/web/db_query.py +++ b/snuba/web/db_query.py @@ -12,7 +12,6 @@ from typing import Any, cast import rapidjson -import sentry_sdk from clickhouse_driver.errors import ErrorCodes from sentry_kafka_schemas.schema_types import snuba_queries_v1 from sentry_options import OptionValue @@ -68,7 +67,7 @@ from snuba.utils.metrics.timer import Timer from snuba.utils.metrics.util import set_current_span_attributes, with_span from snuba.utils.metrics.wrapper import MetricsWrapper -from snuba.utils.sentry import SENTRY_OP +from snuba.utils.sentry import SENTRY_OP, set_tag_and_attribute from snuba.utils.serializable_exception import ( SerializableException, SerializableExceptionDict, @@ -352,7 +351,7 @@ def record_cache_hit_type(hit_type: int) -> None: span_tag = "cache_wait" elif hit_type == SIMPLE_READTHROUGH: stats["cache_hit_simple"] = 1 - sentry_sdk.set_attribute("cache_status", span_tag) + set_tag_and_attribute("cache_status", span_tag) cache_partition = _get_cache_partition(reader) metrics.increment( @@ -541,7 +540,7 @@ def _raw_query( # The old `if scope.span:` guard is gone: in stream mode `scope.span` is # always None, which would have silently dropped this attribute. - sentry_sdk.set_attribute("slo_status", request_status.status.value) + set_tag_and_attribute("slo_status", request_status.status.value) stats = update_with_status( status=status or QueryStatus.ERROR, diff --git a/snuba/web/views.py b/snuba/web/views.py index bec4ead8d21..2c78b6c73b2 100644 --- a/snuba/web/views.py +++ b/snuba/web/views.py @@ -249,17 +249,18 @@ def _trace_transaction(dataset_name: str) -> None: # the service span the WSGI integration opened before we got here. Dual-write # tags + attributes while telemetry is mid-transition: tags land on error # events, attributes on streamed spans. + # Normalize a missing Referer header once, so the transaction name and the + # referrer tag/attribute agree instead of the name carrying a literal "None". + referrer = http_request.referrer or "" set_tag_and_attribute("dataset", dataset_name) - set_tag_and_attribute("referrer", http_request.referrer or "") + set_tag_and_attribute("referrer", referrer) # `scope.transaction` is None in stream mode, so we can't read the current # name back off it. Rebuild it from the same source the Flask integration # uses (the endpoint, its default transaction_style) and rename the segment # through the scope, which does work in stream mode. endpoint = http_request.url_rule.endpoint if http_request.url_rule else "unknown" - sentry_sdk.get_current_scope().set_transaction_name( - f"{endpoint}__{dataset_name}__{http_request.referrer}" - ) + sentry_sdk.get_current_scope().set_transaction_name(f"{endpoint}__{dataset_name}__{referrer}") def _set_snql_api_error_tags(body: dict[str, Any], http_referrer: str | None) -> None: From 1c457a9a926c211d81fa0e22bcd5c764455bd77b Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 2 Aug 2026 13:25:46 -0700 Subject: [PATCH 5/6] ref(metrics): Cast tags instead of copying them in SentryMetricsBackend The SDK annotates metrics attributes as an invariant dict while Tags is a Mapping, so the call needed narrowing. _attributes did that with dict(tags), which is a redundant copy: _capture_metric already iterates .items() into a fresh dict and never retains or mutates the caller's mapping. Cast instead. Verified the SDK accepts a non-dict Mapping at runtime and that all four metric types still emit with correct type, unit, and attributes. Narrowing the override signatures instead would violate LSP against MetricsBackend, and callers reach this through the abstract interface, so the conversion has to stay at the call boundary. --- snuba/utils/metrics/backends/sentry.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/snuba/utils/metrics/backends/sentry.py b/snuba/utils/metrics/backends/sentry.py index 3c82cb944bd..1d1e75557e9 100644 --- a/snuba/utils/metrics/backends/sentry.py +++ b/snuba/utils/metrics/backends/sentry.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Any +from typing import Any, cast from sentry_sdk import metrics @@ -9,8 +9,18 @@ def _attributes(tags: Tags | None) -> dict[str, Any] | None: - """The SDK takes an invariant ``dict``; ``Tags`` is a ``Mapping``.""" - return dict(tags) if tags is not None else None + """Pass ``Tags`` to the SDK's ``attributes`` parameter. + + The metrics functions annotate ``attributes`` as an invariant ``dict`` while + ``Tags`` is a ``Mapping``, so mypy rejects the call. The annotation is + narrower than the behaviour: ``_capture_metric`` only iterates ``.items()`` + into a fresh dict, so any ``Mapping`` works and the caller's object is + neither retained nor mutated. + + Cast rather than copy. ``dict(tags)`` would be a second copy of something the + SDK already copies, on a hot path. + """ + return cast("dict[str, Any] | None", tags) class SentryMetricsBackend(MetricsBackend): From c6faaab019dfa36a26758fdeff8bb3aa1d1cb58c Mon Sep 17 00:00:00 2001 From: Pierre Massat Date: Sun, 2 Aug 2026 13:43:19 -0700 Subject: [PATCH 6/6] ref(tracing): Trim verbose comments added for span streaming --- .pre-commit-config.yaml | 1 - snuba/cli/__init__.py | 6 ++-- snuba/cli/consumer.py | 3 +- snuba/clickhouse/connect.py | 21 ++++-------- snuba/environment.py | 11 ++----- snuba/query/allocation_policies/__init__.py | 4 +-- snuba/replacers/projects_query_flags.py | 3 +- snuba/request/validation.py | 1 - snuba/utils/metrics/backends/sentry.py | 15 +++------ snuba/utils/metrics/util.py | 8 ++--- snuba/utils/sentry.py | 10 ++---- snuba/web/db_query.py | 6 ++-- snuba/web/rpc/__init__.py | 6 +--- .../routing_strategies/storage_routing.py | 7 ++-- snuba/web/views.py | 33 ++++--------------- tests/clickhouse/test_connect.py | 1 - 16 files changed, 34 insertions(+), 102 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3c571cc4538..60c66f90d14 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,7 +37,6 @@ repos: name: validate-configs-syntax entry: env SNUBA_SETTINGS=test python3 -m snuba.validate_configs language: python - # sentry_sdk must be >=2.62 for `sentry_sdk.traces` (span streaming API). additional_dependencies: [ 'fastjsonschema', 'pyyaml', 'sentry_sdk>=2.66.1' ] pass_filenames: false files: 'snuba/datasets/configuration/.*' diff --git a/snuba/cli/__init__.py b/snuba/cli/__init__.py index 5e456b0ff56..66097652643 100644 --- a/snuba/cli/__init__.py +++ b/snuba/cli/__init__.py @@ -47,10 +47,8 @@ def get_command(self, ctx: Any, name: str) -> click.Command: # by default. To mimic this behavior we have to do that here # since all of our infrastructure depends on this being the # case - # `parent_span=None` forces a service span (the stream-mode equivalent of - # a transaction). Note there is no stream-mode equivalent of the old - # `sampled=True`, so this now obeys SENTRY_TRACE_SAMPLE_RATE like every - # other span; the `snuba_init_time` metric below is unaffected. + # parent_span=None => service span. No stream-mode sampled=True; obeys + # SENTRY_TRACE_SAMPLE_RATE. snuba_init_time metric below is unaffected. with traces.start_span( name=f"[cli init] {name}", attributes={SENTRY_OP: "snuba_init"}, diff --git a/snuba/cli/consumer.py b/snuba/cli/consumer.py index 6f0ba5cfd92..f961175a07f 100644 --- a/snuba/cli/consumer.py +++ b/snuba/cli/consumer.py @@ -204,8 +204,7 @@ def consumer( logger.info("Consumer Starting") storage_key = StorageKey(storage_name) - # Deliberately a tag, not an attribute: consumers have no active span, and - # scope tags (unlike scope attributes) are what reach error events. + # Tag (not attribute): consumers have no active span; tags reach errors. sentry_sdk.set_tag("storage", storage_name) logger.info("Checking Clickhouse connections...") diff --git a/snuba/clickhouse/connect.py b/snuba/clickhouse/connect.py index 53abfbdd2fa..0b1a959a578 100644 --- a/snuba/clickhouse/connect.py +++ b/snuba/clickhouse/connect.py @@ -35,12 +35,10 @@ def _driver_params(params: Params) -> Sequence[Any] | dict[str, Any] | None: - """Narrow ``Params`` to what clickhouse-connect accepts. + """Narrow ``Params`` to clickhouse-connect's accepted forms. - ``Params`` allows any ``Mapping``, but the driver's signature takes an - invariant ``dict``. Falsy params (including an empty sequence) mean "no - parameters" here, matching the previous ``params if params else None``. - Sequence params are passed through; mappings are copied to a plain ``dict``. + Falsy => None. Mappings are copied to a plain ``dict`` (driver wants an + invariant dict); sequences pass through as positional binds. """ if not params: return None @@ -50,15 +48,11 @@ def _driver_params(params: Params) -> Sequence[Any] | dict[str, Any] | None: def _insert_statement(table: str, column_names: Sequence[str]) -> str: - """Rebuild the INSERT statement clickhouse-connect sends for ``client.insert``. + """Rebuild the INSERT statement clickhouse-connect sends on the wire. - ``client.insert`` takes a row matrix rather than SQL, but it still builds and - sends ``INSERT INTO
() FORMAT Native`` on the wire. Mirror it so - ``db.query.text`` is populated on this path too, matching the native pool, - where ``insert`` routes through ``execute`` with an explicit statement. - - The row data is deliberately excluded: it is unbounded in size and may hold - PII. + ``client.insert`` takes a row matrix, not SQL, but still emits + ``INSERT INTO
() FORMAT Native``. Row data is excluded + (unbounded, may hold PII). """ columns = ", ".join(quote_identifier(name) for name in column_names) return f"INSERT INTO {table} ({columns}) FORMAT Native" @@ -503,7 +497,6 @@ def insert( ), }, ) as span: - # Always set so a missing id is obvious when inspecting the span. span.set_attribute( "query_id", query_id if query_id is not None else "unknown-query-id", diff --git a/snuba/environment.py b/snuba/environment.py index 08bc342bce5..f689c3dcc51 100644 --- a/snuba/environment.py +++ b/snuba/environment.py @@ -181,15 +181,8 @@ def setup_sentry() -> None: release=os.getenv("SNUBA_RELEASE"), traces_sample_rate=settings.SENTRY_TRACE_SAMPLE_RATE, profiles_sample_rate=settings.SNUBA_PROFILES_SAMPLE_RATE, - # Send spans to Sentry in batches as they finish, instead of buffering a - # whole trace in memory and shipping it as a single transaction event. - # This removes the 1000-span-per-transaction cap, keeps memory flat in - # long-running consumers, and preserves the spans that already finished - # when a process is killed mid-trace (e.g. an OOM-kill). - # - # NOTE: this disables the legacy tracing API. `sentry_sdk.start_span()`, - # `start_transaction()`, `update_current_span()` and `scope.span` all - # silently become no-ops, so instrumentation must use `sentry_sdk.traces`. + # Stream spans as they finish. Disables the legacy tracing API + # (start_span/start_transaction/update_current_span/scope.span). trace_lifecycle="stream", ) diff --git a/snuba/query/allocation_policies/__init__.py b/snuba/query/allocation_policies/__init__.py index 0503ea82e66..95cccd8a9c7 100644 --- a/snuba/query/allocation_policies/__init__.py +++ b/snuba/query/allocation_policies/__init__.py @@ -534,9 +534,7 @@ def get_quota_allowance( # make sure we always know which storage key we rejected a query from allowance.explanation["storage_key"] = self._resource_identifier.value for k, v in allowance.to_dict().items(): - # Span attributes only accept scalars; stringify the nested - # values (e.g. `explanation`) rather than letting the SDK - # safe_repr() them into something unreadable. + # Attributes only accept scalars; stringify nested values. span.set_attribute( f"quota_allowance.{k}", v if isinstance(v, (str, int, float, bool)) else json.dumps(v, default=repr), diff --git a/snuba/replacers/projects_query_flags.py b/snuba/replacers/projects_query_flags.py index bea1fadfd58..c8019ffb193 100644 --- a/snuba/replacers/projects_query_flags.py +++ b/snuba/replacers/projects_query_flags.py @@ -145,8 +145,7 @@ def load_from_redis( name="process_redis_results", attributes={SENTRY_OP: "function"} ) as span: flags = cls._process_redis_results(results, len(s_project_ids)) - # Span attributes only accept scalars and homogeneous lists, so - # the sets/datetime here have to be converted explicitly. + # Attributes need scalars/homogeneous lists; convert explicitly. span.set_attribute("projects", sorted(s_project_ids)) span.set_attribute("exclude_groups", sorted(flags.group_ids_to_exclude)) span.set_attribute("len(exclude_groups)", len(flags.group_ids_to_exclude)) diff --git a/snuba/request/validation.py b/snuba/request/validation.py index 6f313dbd4fd..79fb3696db4 100644 --- a/snuba/request/validation.py +++ b/snuba/request/validation.py @@ -237,7 +237,6 @@ def _build_request( ) -> Request: org_ids = get_object_ids_in_query_ast(query, "org_id") if org_ids is not None and len(org_ids) == 1: - # Dual-write tags + attributes while telemetry is mid-transition. set_tag_and_attribute("snuba_org_id", org_ids.pop()) query_project_id = _get_project_id(query) diff --git a/snuba/utils/metrics/backends/sentry.py b/snuba/utils/metrics/backends/sentry.py index 1d1e75557e9..2ae10e46773 100644 --- a/snuba/utils/metrics/backends/sentry.py +++ b/snuba/utils/metrics/backends/sentry.py @@ -9,16 +9,10 @@ def _attributes(tags: Tags | None) -> dict[str, Any] | None: - """Pass ``Tags`` to the SDK's ``attributes`` parameter. + """Cast ``Tags`` for the SDK's invariant ``dict`` annotation. - The metrics functions annotate ``attributes`` as an invariant ``dict`` while - ``Tags`` is a ``Mapping``, so mypy rejects the call. The annotation is - narrower than the behaviour: ``_capture_metric`` only iterates ``.items()`` - into a fresh dict, so any ``Mapping`` works and the caller's object is - neither retained nor mutated. - - Cast rather than copy. ``dict(tags)`` would be a second copy of something the - SDK already copies, on a hot path. + The SDK only iterates ``.items()`` into a fresh dict, so a ``Mapping`` is + fine at runtime and copying here would be pure waste. """ return cast("dict[str, Any] | None", tags) @@ -56,8 +50,7 @@ def timing( tags: Tags | None = None, unit: str | None = None, ) -> None: - # The SDK dropped the dedicated `timing` API; a timing is a distribution - # measured in milliseconds. + # SDK dropped timing; emit as a millisecond distribution. metrics.distribution(name, value, unit or "millisecond", _attributes(tags)) def distribution( diff --git a/snuba/utils/metrics/util.py b/snuba/utils/metrics/util.py index 982f398040e..8697b7c23ea 100644 --- a/snuba/utils/metrics/util.py +++ b/snuba/utils/metrics/util.py @@ -74,7 +74,6 @@ def with_span(op: str = "function") -> Callable[[F], F]: def decorator(func: F) -> F: frame_info = inspect.stack()[1] - # Built once at decoration time rather than on every call. attributes: dict[str, Any] = {SENTRY_OP: op, "filename": frame_info.filename} @wraps(func) @@ -88,12 +87,9 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: def set_current_span_attributes(attributes: Mapping[str, Any]) -> None: - """Set attributes on the currently active span, if there is one. + """Set attributes on the active stream-mode span, if any. - The stream-mode replacement for ``sentry_sdk.update_current_span()``, which - is a no-op once ``trace_lifecycle="stream"`` is enabled. There is no active - span when the surrounding code runs outside a traced request (e.g. on a - consumer path), or when the trace was not sampled. + Replaces ``sentry_sdk.update_current_span()``, a no-op under stream mode. """ span = traces.get_current_span() if span is None: diff --git a/snuba/utils/sentry.py b/snuba/utils/sentry.py index 399aa8a18ff..527c969c982 100644 --- a/snuba/utils/sentry.py +++ b/snuba/utils/sentry.py @@ -4,17 +4,11 @@ import sentry_sdk -# Stream-mode span op lives as an attribute rather than a dedicated field. -# Centralize the key so call sites stay consistent. +# Stream-mode span op is an attribute, not a dedicated field. SENTRY_OP = "sentry.op" def set_tag_and_attribute(key: str, value: Any) -> None: - """Write both a scope tag and a scope attribute. - - During the transition to attribute-based telemetry, tags still land on - error events while attributes land on spans/logs/metrics. Write both so - the value is available on errors and on streamed spans. - """ + """Dual-write a scope tag (errors) and attribute (spans) during the transition.""" sentry_sdk.set_tag(key, value) sentry_sdk.set_attribute(key, value) diff --git a/snuba/web/db_query.py b/snuba/web/db_query.py index 40fc40cd65c..a945f697f8e 100644 --- a/snuba/web/db_query.py +++ b/snuba/web/db_query.py @@ -538,8 +538,7 @@ def _raw_query( elif isinstance(cause, (TimeoutError, ExecutionTimeoutError)): status = QueryStatus.TIMEOUT - # The old `if scope.span:` guard is gone: in stream mode `scope.span` is - # always None, which would have silently dropped this attribute. + # No `if scope.span:` guard: scope.span is always None in stream mode. set_tag_and_attribute("slo_status", request_status.status.value) stats = update_with_status( @@ -876,8 +875,7 @@ def _apply_allocation_policies_quota( allowance = allocation_policy.get_quota_allowance(attribution_info.tenant_ids, query_id) can_run &= allowance.can_run quota_allowances[allocation_policy.class_name()] = allowance - # QuotaAllowance is not a valid attribute value; serialize it rather - # than letting the SDK safe_repr() it. + # QuotaAllowance isn't a valid attribute value; serialize it. span.set_attribute( "quota_allowance", json.dumps( diff --git a/snuba/web/rpc/__init__.py b/snuba/web/rpc/__init__.py index 8849c790695..6d04682bd75 100644 --- a/snuba/web/rpc/__init__.py +++ b/snuba/web/rpc/__init__.py @@ -86,11 +86,8 @@ def _flush_logs() -> None: def _set_rpc_error_tags(in_msg: ProtobufMessage) -> None: - # Dual-write tags + attributes while telemetry is mid-transition: tags land - # on error events, attributes on streamed spans. set_tag_and_attribute("source", "rpc_api") - # Extract and annotate fields from meta if available if hasattr(in_msg, "meta"): meta = in_msg.meta @@ -206,8 +203,7 @@ def _uses_storage_routing(self, in_msg: Tin) -> bool: @final def execute(self, in_msg: Tin) -> Tout: scope = sentry_sdk.get_current_scope() - # In stream mode this renames the service span (the segment) directly, - # which is what the old `span.description = ...` line did separately. + # Renames the service span (segment) directly in stream mode. scope.set_transaction_name(self.config_key()) self.routing_context = RoutingContext( timer=self._timer, diff --git a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py index eaa49b245c8..f6f9daced73 100644 --- a/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py +++ b/snuba/web/rpc/storage_routing/routing_strategies/storage_routing.py @@ -214,9 +214,7 @@ def get_stats_dict( def _construct_hacky_querylog_payload( strategy: BaseRoutingStrategy, routing_decision: RoutingDecision ) -> snuba_queries_v1.Querylog: - # Read the trace id off the propagation context rather than the active span: - # it is populated whether or not a span is active and whether or not the - # trace was sampled, so the querylog always gets a real id. + # Propagation context has a trace id even with no active/sampled span. propagation_context = sentry_sdk.get_current_scope().get_active_propagation_context() trace_id = propagation_context.trace_id if propagation_context is not None else "" assert routing_decision.routing_context is not None @@ -475,8 +473,7 @@ def _get_recommendations_from_allocation_policies( routing_context.tenant_ids, routing_context.query_id, ) - # QuotaAllowance is not a valid attribute value; serialize it - # rather than letting the SDK safe_repr() it. + # QuotaAllowance isn't a valid attribute value; serialize it. span.set_attribute( f"{allocation_policy_name}_quota_allowance", json.dumps(recommendations[allocation_policy_name].to_dict(), default=repr), diff --git a/snuba/web/views.py b/snuba/web/views.py index 2c78b6c73b2..aceb3da41ba 100644 --- a/snuba/web/views.py +++ b/snuba/web/views.py @@ -245,55 +245,36 @@ def parse_request_body(http_request: Request) -> dict[str, Any]: def _trace_transaction(dataset_name: str) -> None: - # Set on the scope so every span in this request inherits them, including - # the service span the WSGI integration opened before we got here. Dual-write - # tags + attributes while telemetry is mid-transition: tags land on error - # events, attributes on streamed spans. - # Normalize a missing Referer header once, so the transaction name and the - # referrer tag/attribute agree instead of the name carrying a literal "None". + # Scope-level so every span in the request inherits them. Normalize missing + # Referer once so the transaction name and referrer tag/attribute agree. referrer = http_request.referrer or "" set_tag_and_attribute("dataset", dataset_name) set_tag_and_attribute("referrer", referrer) - # `scope.transaction` is None in stream mode, so we can't read the current - # name back off it. Rebuild it from the same source the Flask integration - # uses (the endpoint, its default transaction_style) and rename the segment - # through the scope, which does work in stream mode. + # scope.transaction is None in stream mode; rebuild the name from the + # endpoint (Flask's default transaction_style) and rename via the scope. endpoint = http_request.url_rule.endpoint if http_request.url_rule else "unknown" sentry_sdk.get_current_scope().set_transaction_name(f"{endpoint}__{dataset_name}__{referrer}") def _set_snql_api_error_tags(body: dict[str, Any], http_referrer: str | None) -> None: - """Set Sentry scope tags and attributes for SnQL API error tracking. + """Annotate SnQL API errors with source, referrer, and tenant_ids. - Annotates all errors in the SnQL API with: - - source: snql_api - - referrer: from HTTP header or request body - - tenant_ids: as context and individual tags/attributes - - Dual-writes tags and attributes while telemetry is mid-transition: tags - land on error events, attributes on streamed spans. - - This function is wrapped in a try-except to ensure that any failure - in setting tags/attributes does not crash the API request. + Failures here must not crash the request. """ try: set_tag_and_attribute("source", "snql_api") - # Extract and annotate referrer referrer = http_referrer or body.get("tenant_ids", {}).get("referrer", "") set_tag_and_attribute("referrer", referrer) - # Extract and set tenant_ids as context for better error tracking tenant_ids = body.get("tenant_ids", {}) if tenant_ids: sentry_sdk.set_context("tenant_ids", tenant_ids) - # Also set individual tenant_id tags/attributes for easier filtering for key, value in tenant_ids.items(): - if key != "referrer": # Skip referrer as it's already set above + if key != "referrer": set_tag_and_attribute(f"tenant_id.{key}", str(value)) except Exception as e: - # Log the error but don't let it crash the API request logger.warning("Failed to set Sentry tags/attributes for SnQL API", exc_info=e) diff --git a/tests/clickhouse/test_connect.py b/tests/clickhouse/test_connect.py index d5cc14e8f44..c583a15d8df 100644 --- a/tests/clickhouse/test_connect.py +++ b/tests/clickhouse/test_connect.py @@ -133,7 +133,6 @@ def test_insert_multiple_rows_build_a_matrix() -> None: def test_insert_statement_mirrors_what_the_driver_sends() -> None: - # client.insert takes a row matrix, but sends this statement on the wire. assert ( _insert_statement("migrations_local", ["group", "migration_id"]) == "INSERT INTO migrations_local (`group`, `migration_id`) FORMAT Native"