From 010a447f16a667d6f7c9dd2cb68d4f74faafdf20 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 29 Sep 2026 16:40:09 +0200 Subject: [PATCH 1/2] feat(inbound-filters): Compare release versions in custom filter conditions A release condition value that starts with >, >=, <, <= or = now compiles to Relay's semver rule condition instead of a glob, so a filter can drop data from a version range such as >=1.2.0 without listing each version as a pattern. The API rejects a comparison whose release carries no version, as Relay would never match it. Co-Authored-By: Claude Fable 5.1 --- .../project_custom_inbound_filters.py | 30 ++++++- src/sentry/ingest/inbound_filters.py | 87 +++++++++++++++++-- src/sentry/relay/types/rule_condition.py | 15 ++++ .../test_project_custom_inbound_filters.py | 36 ++++++++ tests/sentry/ingest/test_inbound_filters.py | 66 +++++++++++--- 5 files changed, 212 insertions(+), 22 deletions(-) diff --git a/src/sentry/api/endpoints/project_custom_inbound_filters.py b/src/sentry/api/endpoints/project_custom_inbound_filters.py index c496d1f2cb25..d21b2c02424a 100644 --- a/src/sentry/api/endpoints/project_custom_inbound_filters.py +++ b/src/sentry/api/endpoints/project_custom_inbound_filters.py @@ -25,7 +25,11 @@ ValidationErrorResponse, as_validation_errors, ) -from sentry.ingest.inbound_filters import get_supported_condition_types +from sentry.ingest.inbound_filters import ( + get_supported_condition_types, + is_release_version, + parse_release_comparison, +) from sentry.models.custominboundfilter import ( ConditionType, CustomInboundFilter, @@ -115,19 +119,37 @@ class CustomInboundFilterConditionSerializer(serializers.Serializer[CustomInboun allow_empty=False, help_text=( "Glob patterns the field is matched against. The condition matches when any " - "pattern matches, so multiple values act as OR." + "pattern matches, so multiple values act as OR. A `release` value that starts " + "with `>`, `>=`, `<`, `<=` or `=` compares versions instead, e.g. `>=1.2.0` " + "or ` CustomInboundFilterCondition: - # Relay drops an entry it cannot parse as an address or range, so a typo would - # silently disable part of the filter. + # Relay never matches an entry it cannot parse, so a typo would silently + # disable part of the filter. if attrs["type"] == ConditionType.IP_ADDRESS: invalid = [value for value in attrs["value"] if not _is_ip_address_or_range(value)] if invalid: raise serializers.ValidationError( {"value": f"{', '.join(invalid)} is not an IP address or CIDR range."} ) + if attrs["type"] == ConditionType.RELEASE: + invalid = [ + value + for value in attrs["value"] + if (comparison := parse_release_comparison(value)) + and not is_release_version(comparison.release) + ] + if invalid: + raise serializers.ValidationError( + { + "value": ( + f"{', '.join(invalid)} does not compare against a version " + "such as 1.2.0 or myapp@1.2.0." + ) + } + ) return attrs diff --git a/src/sentry/ingest/inbound_filters.py b/src/sentry/ingest/inbound_filters.py index 19d6d40133fc..d0b37daa67c6 100644 --- a/src/sentry/ingest/inbound_filters.py +++ b/src/sentry/ingest/inbound_filters.py @@ -1,10 +1,14 @@ +import re from collections.abc import Callable, Mapping, Sequence from dataclasses import dataclass -from typing import Any, cast +from typing import Any, Literal, cast +import orjson from django.conf import settings from rest_framework import serializers +from sentry_relay.processing import parse_release +from sentry.constants import SEMVER_FAKE_PACKAGE from sentry.models.custominboundfilter import ( ConditionType, CustomInboundFilter, @@ -562,7 +566,7 @@ def _custom_error_type_condition(values: list[str]) -> RuleCondition: ) -# Builds the Relay condition that matches one filter condition's glob values. +# Builds the Relay condition that matches one filter condition's values. _ConditionMatcher = Callable[[list[str]], RuleCondition] # The matcher for each condition type a data type supports. @@ -586,6 +590,77 @@ def match(values: list[str]) -> RuleCondition: _client_ip_matcher = _cidr_matcher("envelope.client_ip") +SemverComparator = Literal["eq", "gt", "gte", "lt", "lte"] + +_RELEASE_COMPARATORS: Mapping[str, SemverComparator] = { + ">=": "gte", + "<=": "lte", + ">": "gt", + "<": "lt", + "=": "eq", +} + +_RELEASE_COMPARISON_RE = re.compile(r"(>=|<=|>|<|=)\s*(.+)") + + +@dataclass(frozen=True) +class ReleaseComparison: + comparator: SemverComparator + # The release to compare against, such as `1.2.0` or `myapp@1.2.0`. + release: str + + +def parse_release_comparison(value: str) -> ReleaseComparison | None: + """ + Reads a release condition value written as a version comparison, such as + `>=1.2.0` or ` bool: + """ + Whether Relay reads a version out of the release, so that it can compare it. + A release such as `1.2`, `1.2.3.4-rc.1` or `myapp@1.2.0` has one. A commit hash + or a glob pattern has none. + """ + if "@" not in release: + release = f"{SEMVER_FAKE_PACKAGE}@{release}" + return parse_release(release, json_loads=orjson.loads).get("version_parsed") is not None + + +def _release_matcher(name: str) -> _ConditionMatcher: + # Glob values share one condition; each version comparison is a condition of + # its own, as Relay takes one comparator and release per `semver` condition. + def match(values: list[str]) -> RuleCondition: + globs: list[str] = [] + conditions: list[RuleCondition] = [] + for value in values: + comparison = parse_release_comparison(value) + if comparison is None: + globs.append(value) + else: + conditions.append( + { + "op": "semver", + "name": name, + "comparator": comparison.comparator, + "value": comparison.release, + } + ) + if globs: + conditions.insert(0, {"op": "glob", "name": name, "value": globs}) + if len(conditions) == 1: + return conditions[0] + return {"op": "or", "inner": conditions} + + return match + + _CONDITION_MATCHERS: Mapping[ ConditionType, _ConditionMatcher | Mapping[DataType, _ConditionMatcher], @@ -603,10 +678,10 @@ def match(values: list[str]) -> RuleCondition: DataType.METRIC: _field_matcher("trace_metric.name"), }, ConditionType.RELEASE: { - DataType.ERROR: _field_matcher("event.release"), - DataType.LOG: _field_matcher("log.attributes.sentry.release.value"), - DataType.METRIC: _field_matcher("trace_metric.attributes.sentry.release.value"), - DataType.SPAN: _field_matcher("span.attributes.sentry.release.value"), + DataType.ERROR: _release_matcher("event.release"), + DataType.LOG: _release_matcher("log.attributes.sentry.release.value"), + DataType.METRIC: _release_matcher("trace_metric.attributes.sentry.release.value"), + DataType.SPAN: _release_matcher("span.attributes.sentry.release.value"), }, ConditionType.IP_ADDRESS: _client_ip_matcher, } diff --git a/src/sentry/relay/types/rule_condition.py b/src/sentry/relay/types/rule_condition.py index 89c3564586bf..08ec521dd659 100644 --- a/src/sentry/relay/types/rule_condition.py +++ b/src/sentry/relay/types/rule_condition.py @@ -84,6 +84,20 @@ class CidrCondition(TypedDict): value: list[str] +class SemverCondition(TypedDict): + """Release version comparison condition + + Compares the version of the release in a field against `value`, a release such + as `1.2.0` or `myapp@1.2.0`. A value with a package only matches releases of that + package. A field or value without a version, such as a commit hash, never matches. + """ + + op: Literal["semver"] + name: str + comparator: Literal["eq", "gt", "gte", "lt", "lte"] + value: str + + class IterableCondition(TypedDict): """Condition for iterating over a list and applying a nested condition""" @@ -117,4 +131,5 @@ class NotCondition(TypedDict): LtCondition, GlobCondition, CidrCondition, + SemverCondition, ] diff --git a/tests/sentry/api/endpoints/test_project_custom_inbound_filters.py b/tests/sentry/api/endpoints/test_project_custom_inbound_filters.py index cba05956b823..03c218cfadc2 100644 --- a/tests/sentry/api/endpoints/test_project_custom_inbound_filters.py +++ b/tests/sentry/api/endpoints/test_project_custom_inbound_filters.py @@ -305,6 +305,42 @@ def test_rejects_ip_address_that_does_not_parse(self) -> None: "10.0.0.*, nope is not an IP address or CIDR range." ) + def test_post_release_version_comparison(self) -> None: + """A release value that starts with a comparator is stored as typed.""" + conditions = [{"type": "release", "value": [">=1.2.0", " None: + with self.feature(self.features): + response = self.get_error_response( + self.organization.slug, + self.project.slug, + method="post", + name="Typo", + dataType="error", + conditions=[ + {"type": "release", "value": [">=1.2.0", ">2*", "2*, None: """The catch-all filters whichever data types the organization ingests.""" with self.feature(self.features): diff --git a/tests/sentry/ingest/test_inbound_filters.py b/tests/sentry/ingest/test_inbound_filters.py index c35f313c357f..f351de944d47 100644 --- a/tests/sentry/ingest/test_inbound_filters.py +++ b/tests/sentry/ingest/test_inbound_filters.py @@ -246,19 +246,29 @@ def error_type_rule_condition(values: list[str]) -> dict: } +RELEASE_FIELDS = [ + "event.release", + "log.attributes.sentry.release.value", + "trace_metric.attributes.sentry.release.value", + "span.attributes.sentry.release.value", +] + + def release_rule_condition(values: list[str]) -> dict: """The catch-all shape: the release field of every data type, combined with OR.""" + return { + "op": "or", + "inner": [{"op": "glob", "name": name, "value": values} for name in RELEASE_FIELDS], + } + + +def release_version_rule_condition(comparator: str, release: str) -> dict: + """The catch-all shape of a version comparison.""" return { "op": "or", "inner": [ - {"op": "glob", "name": "event.release", "value": values}, - {"op": "glob", "name": "log.attributes.sentry.release.value", "value": values}, - { - "op": "glob", - "name": "trace_metric.attributes.sentry.release.value", - "value": values, - }, - {"op": "glob", "name": "span.attributes.sentry.release.value", "value": values}, + {"op": "semver", "name": name, "comparator": comparator, "value": release} + for name in RELEASE_FIELDS ], } @@ -383,18 +393,50 @@ def release_rule_condition(values: list[str]) -> dict: pytest.param( "all", [ - {"type": "release", "value": [">2*"]}, - {"type": "release", "value": ["<4*"]}, + {"type": "release", "value": [">=2"]}, + {"type": "release", "value": ["<4"]}, ], { "op": "and", "inner": [ - release_rule_condition([">2*"]), - release_rule_condition(["<4*"]), + release_version_rule_condition("gte", "2"), + release_version_rule_condition("lt", "4"), ], }, id="catch_all_release_range", ), + pytest.param( + "error", + [{"type": "release", "value": [">1.2.0", ">= 1.3", "=3.0", "2.5.*"]}], + { + "op": "or", + "inner": [ + {"op": "glob", "name": "event.release", "value": ["1.*", "2.5.*"]}, + {"op": "semver", "name": "event.release", "comparator": "gte", "value": "3.0"}, + ], + }, + id="release_globs_and_version_comparison_mixed", + ), pytest.param( "error", [{"type": "ip_address", "value": ["10.0.0.0/8", "203.0.113.7"]}], From a25d41f3b3af36741ed921c936e6665b3e3bb013 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Tue, 29 Sep 2026 16:46:28 +0200 Subject: [PATCH 2/2] feat(inbound-filters): Compare a plain release version as equal A release value without a comparator that carries a version, such as 1.2.0 or myapp@1.2.0, now compares as equal to that version instead of matching the text. So 1.2 matches 1.2.0 of every package, and a build code does not get in the way. A value without a version, such as a commit hash or a glob, still matches the text. Co-Authored-By: Claude Fable 5.1 --- .../project_custom_inbound_filters.py | 7 ++-- src/sentry/ingest/inbound_filters.py | 18 ++++++--- tests/sentry/ingest/test_inbound_filters.py | 39 ++++++++++++++++--- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/sentry/api/endpoints/project_custom_inbound_filters.py b/src/sentry/api/endpoints/project_custom_inbound_filters.py index d21b2c02424a..e80316fb71b5 100644 --- a/src/sentry/api/endpoints/project_custom_inbound_filters.py +++ b/src/sentry/api/endpoints/project_custom_inbound_filters.py @@ -119,9 +119,10 @@ class CustomInboundFilterConditionSerializer(serializers.Serializer[CustomInboun allow_empty=False, help_text=( "Glob patterns the field is matched against. The condition matches when any " - "pattern matches, so multiple values act as OR. A `release` value that starts " - "with `>`, `>=`, `<`, `<=` or `=` compares versions instead, e.g. `>=1.2.0` " - "or ``, `>=`, `<`, " + "`<=` or `=`, compares versions instead of matching the text. `ip_address` " + "values are addresses or CIDR ranges." ), ) diff --git a/src/sentry/ingest/inbound_filters.py b/src/sentry/ingest/inbound_filters.py index d0b37daa67c6..8806a60c2198 100644 --- a/src/sentry/ingest/inbound_filters.py +++ b/src/sentry/ingest/inbound_filters.py @@ -612,14 +612,20 @@ class ReleaseComparison: def parse_release_comparison(value: str) -> ReleaseComparison | None: """ - Reads a release condition value written as a version comparison, such as - `>=1.2.0` or `=1.2.0` or ` bool: diff --git a/tests/sentry/ingest/test_inbound_filters.py b/tests/sentry/ingest/test_inbound_filters.py index f351de944d47..4b1d7a14c8d5 100644 --- a/tests/sentry/ingest/test_inbound_filters.py +++ b/tests/sentry/ingest/test_inbound_filters.py @@ -332,7 +332,7 @@ def release_version_rule_condition(comparator: str, release: str) -> dict: "log", [ {"type": "log_message", "value": ["*DEBUG*"]}, - {"type": "release", "value": ["1.2.3"]}, + {"type": "release", "value": ["1.2.*"]}, ], { "op": "and", @@ -341,7 +341,7 @@ def release_version_rule_condition(comparator: str, release: str) -> dict: { "op": "glob", "name": "log.attributes.sentry.release.value", - "value": ["1.2.3"], + "value": ["1.2.*"], }, ], }, @@ -351,7 +351,7 @@ def release_version_rule_condition(comparator: str, release: str) -> dict: "metric", [ {"type": "metric_name", "value": ["checkout.*"]}, - {"type": "release", "value": ["1.2.3"]}, + {"type": "release", "value": ["1.2.*"]}, ], { "op": "and", @@ -360,7 +360,7 @@ def release_version_rule_condition(comparator: str, release: str) -> dict: { "op": "glob", "name": "trace_metric.attributes.sentry.release.value", - "value": ["1.2.3"], + "value": ["1.2.*"], }, ], }, @@ -374,10 +374,37 @@ def release_version_rule_condition(comparator: str, release: str) -> dict: ), pytest.param( "span", - [{"type": "release", "value": ["1.2.3"]}], - {"op": "glob", "name": "span.attributes.sentry.release.value", "value": ["1.2.3"]}, + [{"type": "release", "value": ["1.2.*"]}], + {"op": "glob", "name": "span.attributes.sentry.release.value", "value": ["1.2.*"]}, id="release_on_spans", ), + pytest.param( + "span", + [{"type": "release", "value": ["1.2.3", "myapp@2.0", "a4b7e0f9c2d1"]}], + { + "op": "or", + "inner": [ + { + "op": "glob", + "name": "span.attributes.sentry.release.value", + "value": ["a4b7e0f9c2d1"], + }, + { + "op": "semver", + "name": "span.attributes.sentry.release.value", + "comparator": "eq", + "value": "1.2.3", + }, + { + "op": "semver", + "name": "span.attributes.sentry.release.value", + "comparator": "eq", + "value": "myapp@2.0", + }, + ], + }, + id="plain_version_compares_as_equal", + ), pytest.param( "error", [{"type": "release", "value": ["1.*"]}],