diff --git a/pyproject.toml b/pyproject.toml index a1a5c35681..741322794a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ dependencies = [ "pyyaml>=6.0", "redis>=4.5.4", "sentry-arroyo>=2.39.2", - "sentry-conventions>=0.6.0", + "sentry-conventions>=0.8.0", "sentry-kafka-schemas>=2.1.24", "sentry-protos>=0.8.14", "sentry-redis-tools>=0.5.1", diff --git a/snuba/protos/common.py b/snuba/protos/common.py index 9dac037ba2..3ae0b3e0fb 100644 --- a/snuba/protos/common.py +++ b/snuba/protos/common.py @@ -62,19 +62,40 @@ class MalformedAttributeException(Exception): } -def _build_deprecated_attributes() -> dict[str, set[str]]: - current_to_deprecated: dict[str, set[str]] = defaultdict(set) +def _resolve_canonical(name: str) -> str: + visited: set[str] = set() + current = name + while current in ATTRIBUTE_METADATA: + meta = ATTRIBUTE_METADATA[current] + if not meta.deprecation or not meta.deprecation.replacement: + return current + if current in visited: + return current + visited.add(current) + current = meta.deprecation.replacement + return current + + +def _build_deprecated_attributes() -> dict[str, list[str]]: + groups: dict[str, set[str]] = defaultdict(set) for name, metadata in ATTRIBUTE_METADATA.items(): if metadata.deprecation and metadata.deprecation.replacement: - replacement = metadata.deprecation.replacement - deprecated = {name} - if metadata.aliases: - deprecated.update(metadata.aliases) - current_to_deprecated[replacement].update(deprecated) - return current_to_deprecated - - -ATTRIBUTES_TO_COALESCE: dict[str, set[str]] = _build_deprecated_attributes() + canonical = _resolve_canonical(name) + groups[canonical].add(name) + + result: dict[str, list[str]] = {} + for canonical, deprecated_names in groups.items(): + full_group = {canonical} | deprecated_names + for member in full_group: + others = full_group - {member} + if member == canonical: + result[member] = sorted(others) + else: + result[member] = [canonical] + sorted(others - {canonical}) + return result + + +ATTRIBUTES_TO_COALESCE: dict[str, list[str]] = _build_deprecated_attributes() def _build_label_mapping_key(attribute_key: AttributeKey) -> str: diff --git a/tests/protos/test_protos_common.py b/tests/protos/test_protos_common.py index fd5ab69300..965d4edcc7 100644 --- a/tests/protos/test_protos_common.py +++ b/tests/protos/test_protos_common.py @@ -1,14 +1,16 @@ import pytest +from sentry_conventions.attributes import ATTRIBUTE_METADATA from sentry_protos.snuba.v1.trace_item_attribute_pb2 import AttributeKey from snuba.protos.common import ( ATTRIBUTES_TO_COALESCE, MalformedAttributeException, + _resolve_canonical, attribute_key_to_expression, ) from snuba.query.dsl import Functions as f from snuba.query.dsl import arrayElement, column, literal -from snuba.query.expressions import SubscriptableReference +from snuba.query.expressions import FunctionCall, SubscriptableReference class TestAttributeKeyToExpression: @@ -96,6 +98,52 @@ def test_coalesce(self) -> None: alias=f"{new_attribute}_TYPE_STRING", ) + def test_coalesce_queried_attribute_is_first(self) -> None: + for name in ATTRIBUTES_TO_COALESCE: + result = attribute_key_to_expression( + AttributeKey(type=AttributeKey.TYPE_STRING, name=name), + ) + assert isinstance(result, FunctionCall) + assert result.function_name == "coalesce" + first_param = result.parameters[0] + assert isinstance(first_param, SubscriptableReference) + assert first_param.key.value == name, ( + f"Expected {name} as first coalesce argument, got {first_param.key.value}" + ) + remaining: list[str] = [ + str(p.key.value) + for p in result.parameters[1:] + if isinstance(p, SubscriptableReference) + ] + meta = ATTRIBUTE_METADATA.get(name) + is_deprecated = ( + meta is not None + and meta.deprecation is not None + and meta.deprecation.replacement is not None + ) + if is_deprecated: + canonical = _resolve_canonical(name) + assert remaining[0] == canonical, ( + f"Expected canonical {canonical} as second coalesce argument for deprecated {name}, got {remaining[0]}" + ) + assert remaining[1:] == sorted(remaining[1:]), ( + f"Coalesce arguments after canonical for {name} are not sorted: {remaining[1:]}" + ) + else: + assert remaining == sorted(remaining), ( + f"Coalesce arguments after {name} are not sorted: {remaining}" + ) + + def test_coalesce_bidirectional(self) -> None: + for name, others in ATTRIBUTES_TO_COALESCE.items(): + for other in others: + assert other in ATTRIBUTES_TO_COALESCE, ( + f"{other} (in group with {name}) is not a key in ATTRIBUTES_TO_COALESCE" + ) + assert name in ATTRIBUTES_TO_COALESCE[other], ( + f"{name} not in ATTRIBUTES_TO_COALESCE[{other}]" + ) + def test_coalesce_map_does_not_include_none_key(self) -> None: assert None not in ATTRIBUTES_TO_COALESCE diff --git a/uv.lock b/uv.lock index bbef8991c2..d88cc21129 100644 --- a/uv.lock +++ b/uv.lock @@ -906,10 +906,10 @@ wheels = [ [[package]] name = "sentry-conventions" -version = "0.6.0" +version = "0.8.0" source = { registry = "https://pypi.devinfra.sentry.io/simple" } wheels = [ - { url = "https://pypi.devinfra.sentry.io/wheels/sentry_conventions-0.6.0-py3-none-any.whl", hash = "sha256:672604d689a5e1e7833fe3ef9508550fd826e06c57ba0aab95086a973abf9bba" }, + { url = "https://pypi.devinfra.sentry.io/wheels/sentry_conventions-0.8.0-py3-none-any.whl", hash = "sha256:a4c788d78f8ab09068d25fe1fc0345fa874a7e733016571368938698ae2168c9" }, ] [[package]] @@ -1147,7 +1147,7 @@ requires-dist = [ { name = "redis", specifier = ">=4.5.4" }, { name = "rust-snuba", editable = "rust_snuba" }, { name = "sentry-arroyo", specifier = ">=2.39.2" }, - { name = "sentry-conventions", specifier = ">=0.6.0" }, + { name = "sentry-conventions", specifier = ">=0.8.0" }, { name = "sentry-kafka-schemas", specifier = ">=2.1.24" }, { name = "sentry-protos", specifier = ">=0.8.14" }, { name = "sentry-redis-tools", specifier = ">=0.5.1" },