From 1d3be546400a0ae8463ec41843863a5eac374451 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 16 Jan 2025 15:39:30 -0600 Subject: [PATCH 01/14] Rewrite tags queries --- src/sentry/search/events/builder/base.py | 87 ++++++++++++++++--- src/sentry/search/snuba/executors.py | 39 +++++++++ .../test_organization_group_index.py | 16 ++++ 3 files changed, 130 insertions(+), 12 deletions(-) diff --git a/src/sentry/search/events/builder/base.py b/src/sentry/search/events/builder/base.py index 1ee01eb56605d5..10a4a6861e0b34 100644 --- a/src/sentry/search/events/builder/base.py +++ b/src/sentry/search/events/builder/base.py @@ -1504,21 +1504,84 @@ def _get_entity_name(self) -> str: def get_snql_query(self) -> Request: self.validate_having_clause() + def check_tags(expression: Column | Function) -> bool: + if isinstance(expression, Column): + if ( + expression.entity.name == "events" + and expression.name.startswith("tags[") + and expression.name.endswith("]") + ): + return True + elif isinstance(expression, Function): + for parameter in expression.parameters: + if isinstance(parameter, (Column, Function)): + return check_tags(parameter) + return False + + def check_tags_in_condition(condition: WhereType): + if isinstance(condition, Condition): + return check_tags(condition.lhs) + elif isinstance(condition, BooleanCondition): + return any(check_tags_in_condition(cond) for cond in condition.conditions) + else: + return False + + def replace_tags(expression: Column | Function) -> bool: + if isinstance(expression, Column): + if expression.name.startswith("tags[") and expression.name.endswith("]"): + return Column( + name=f"features[{expression.name[5:-1]}]", + entity=expression.entity, + ) + elif isinstance(expression, Function): + for parameter in expression.parameters: + if isinstance(parameter, (Column, Function)): + return replace_tags(parameter) + return expression + + def replace_tags_in_condition(condition: WhereType): + if isinstance(condition, Condition): + return replace_tags(condition.lhs) + elif isinstance(condition, BooleanCondition): + return any(replace_tags_in_condition(cond) for cond in condition.conditions) + else: + return False + + new_where = [ + ( + Or( + conditions=[ + condition, + Condition( + lhs=replace_tags_in_condition(condition.lhs), + op=condition.op, + rhs=condition.rhs, + ), + ] + ) + if check_tags_in_condition(condition.lhs) + else condition + ) + for condition in self.where + ] + + query = Query( + match=Entity(self._get_entity_name(), sample=self.sample_rate), + select=self.columns, + array_join=self.array_join, + where=new_where, + having=self.having, + groupby=self.groupby, + orderby=self.orderby, + limit=self.limit, + offset=self.offset, + limitby=self.limitby, + ) + return Request( dataset=self.dataset.value, app_id="default", - query=Query( - match=Entity(self._get_entity_name(), sample=self.sample_rate), - select=self.columns, - array_join=self.array_join, - where=self.where, - having=self.having, - groupby=self.groupby, - orderby=self.orderby, - limit=self.limit, - offset=self.offset, - limitby=self.limitby, - ), + query=query, flags=Flags(turbo=self.turbo), tenant_ids=self.tenant_ids, ) diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index f05b1c81a709d9..b0a11672fa92e7 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -32,6 +32,7 @@ OrderBy, Request, ) +from snuba_sdk.conditions import Or from snuba_sdk.expressions import Expression from snuba_sdk.query import Query from snuba_sdk.relationships import Relationship @@ -1810,6 +1811,44 @@ def query( ), ) if condition is not None: + + def recursive_check(condition: Column | Function) -> bool: + if isinstance(condition, Column): + if ( + condition.entity.name == "events" + and condition.name.startswith("tags[") + and condition.name.endswith("]") + ): + return True + elif isinstance(condition, Function): + for parameter in condition.parameters: + if isinstance(parameter, (Column, Function)): + return recursive_check(parameter) + return False + + def recursive_replace(condition: Column | Function) -> bool: + if isinstance(condition, Column): + if condition.name.startswith("tags[") and condition.name.endswith( + "]" + ): + return Column( + name=f"features[{condition.name[5:-1]}]", + entity=condition.entity, + ) + elif isinstance(condition, Function): + for parameter in condition.parameters: + if isinstance(parameter, (Column, Function)): + return recursive_replace(parameter) + return condition + + if recursive_check(condition.lhs): + feature_condition = Condition( + lhs=recursive_replace(condition.lhs), + op=condition.op, + rhs=condition.rhs, + ) + condition = Or(conditions=[condition, feature_condition]) + where_conditions.append(condition) # handle types based on issue.type and issue.category diff --git a/tests/sentry/issues/endpoints/test_organization_group_index.py b/tests/sentry/issues/endpoints/test_organization_group_index.py index 37bca50a66b1ca..2ccdc8b518a9c7 100644 --- a/tests/sentry/issues/endpoints/test_organization_group_index.py +++ b/tests/sentry/issues/endpoints/test_organization_group_index.py @@ -4032,6 +4032,22 @@ def test_feedback_category_filter_no_snuba_search(self, _: MagicMock) -> None: def test_feedback_category_filter_use_snuba_search(self, _: MagicMock) -> None: self.run_feedback_category_filter_test(True) + def test_flags_and_tags_query(self, _: MagicMock) -> None: + self.login_as(self.user) + project = self.project + self.store_event( + data={ + "timestamp": before_now(seconds=1).isoformat(), + "contexts": {"flags": {"values": [{"flag": "abc", "result": True}]}}, + }, + project_id=project.id, + ) + response = self.get_success_response(query="abc:true") + assert len(json.loads(response.content)) == 1 + + response = self.get_success_response(query="abc:false") + assert len(json.loads(response.content)) == 0 + class GroupUpdateTest(APITestCase, SnubaTestCase): endpoint = "sentry-api-0-organization-group-index" From f3536ae3395821b27bccd5276f11d2bda33b58d3 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 16 Jan 2025 15:42:55 -0600 Subject: [PATCH 02/14] Add comments --- src/sentry/search/events/builder/base.py | 1 + src/sentry/search/snuba/executors.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sentry/search/events/builder/base.py b/src/sentry/search/events/builder/base.py index 10a4a6861e0b34..31910b6560072d 100644 --- a/src/sentry/search/events/builder/base.py +++ b/src/sentry/search/events/builder/base.py @@ -1547,6 +1547,7 @@ def replace_tags_in_condition(condition: WhereType): else: return False + # This should help us find the recommended event? new_where = [ ( Or( diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index b0a11672fa92e7..c67f29dda9974b 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -1811,7 +1811,8 @@ def query( ), ) if condition is not None: - + # This should update the search box so we can fetch the correct + # issues. def recursive_check(condition: Column | Function) -> bool: if isinstance(condition, Column): if ( From 8a3b02c01ece0ce1136a1e5cd89d0ef9ded51a0e Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 17 Jan 2025 08:56:12 -0600 Subject: [PATCH 03/14] Add coverage --- .../api/endpoints/test_organization_events.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/snuba/api/endpoints/test_organization_events.py b/tests/snuba/api/endpoints/test_organization_events.py index 45d22e0f572570..a58f2ee72acc13 100644 --- a/tests/snuba/api/endpoints/test_organization_events.py +++ b/tests/snuba/api/endpoints/test_organization_events.py @@ -6783,3 +6783,23 @@ def test_count_scores(self): "count_scores(measurements.score.lcp)": 1, "count_scores(measurements.score.total)": 3, } + + def test_query_tags_and_flags(self): + self.store_event( + data={ + "event_id": "a" * 32, + "environment": "staging", + "timestamp": self.ten_mins_ago_iso, + "contexts": {"flags": {"values": [{"flag": "abc", "result": "123"}]}}, + }, + project_id=self.project.id, + ) + + query = { + "field": ["event_id"], + "query": "abc:123", + "dataset": "errors", + } + response = self.do_request(query) + assert response.status_code == 200, response.content + assert False, response.content From baa9f33ba07d6e140b88b8a56b17758b25cb63ca Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 17 Jan 2025 12:09:09 -0600 Subject: [PATCH 04/14] Remove organization_events code --- src/sentry/search/events/builder/base.py | 88 +++---------------- .../api/endpoints/test_organization_events.py | 20 ----- 2 files changed, 12 insertions(+), 96 deletions(-) diff --git a/src/sentry/search/events/builder/base.py b/src/sentry/search/events/builder/base.py index 31910b6560072d..1ee01eb56605d5 100644 --- a/src/sentry/search/events/builder/base.py +++ b/src/sentry/search/events/builder/base.py @@ -1504,85 +1504,21 @@ def _get_entity_name(self) -> str: def get_snql_query(self) -> Request: self.validate_having_clause() - def check_tags(expression: Column | Function) -> bool: - if isinstance(expression, Column): - if ( - expression.entity.name == "events" - and expression.name.startswith("tags[") - and expression.name.endswith("]") - ): - return True - elif isinstance(expression, Function): - for parameter in expression.parameters: - if isinstance(parameter, (Column, Function)): - return check_tags(parameter) - return False - - def check_tags_in_condition(condition: WhereType): - if isinstance(condition, Condition): - return check_tags(condition.lhs) - elif isinstance(condition, BooleanCondition): - return any(check_tags_in_condition(cond) for cond in condition.conditions) - else: - return False - - def replace_tags(expression: Column | Function) -> bool: - if isinstance(expression, Column): - if expression.name.startswith("tags[") and expression.name.endswith("]"): - return Column( - name=f"features[{expression.name[5:-1]}]", - entity=expression.entity, - ) - elif isinstance(expression, Function): - for parameter in expression.parameters: - if isinstance(parameter, (Column, Function)): - return replace_tags(parameter) - return expression - - def replace_tags_in_condition(condition: WhereType): - if isinstance(condition, Condition): - return replace_tags(condition.lhs) - elif isinstance(condition, BooleanCondition): - return any(replace_tags_in_condition(cond) for cond in condition.conditions) - else: - return False - - # This should help us find the recommended event? - new_where = [ - ( - Or( - conditions=[ - condition, - Condition( - lhs=replace_tags_in_condition(condition.lhs), - op=condition.op, - rhs=condition.rhs, - ), - ] - ) - if check_tags_in_condition(condition.lhs) - else condition - ) - for condition in self.where - ] - - query = Query( - match=Entity(self._get_entity_name(), sample=self.sample_rate), - select=self.columns, - array_join=self.array_join, - where=new_where, - having=self.having, - groupby=self.groupby, - orderby=self.orderby, - limit=self.limit, - offset=self.offset, - limitby=self.limitby, - ) - return Request( dataset=self.dataset.value, app_id="default", - query=query, + query=Query( + match=Entity(self._get_entity_name(), sample=self.sample_rate), + select=self.columns, + array_join=self.array_join, + where=self.where, + having=self.having, + groupby=self.groupby, + orderby=self.orderby, + limit=self.limit, + offset=self.offset, + limitby=self.limitby, + ), flags=Flags(turbo=self.turbo), tenant_ids=self.tenant_ids, ) diff --git a/tests/snuba/api/endpoints/test_organization_events.py b/tests/snuba/api/endpoints/test_organization_events.py index a58f2ee72acc13..45d22e0f572570 100644 --- a/tests/snuba/api/endpoints/test_organization_events.py +++ b/tests/snuba/api/endpoints/test_organization_events.py @@ -6783,23 +6783,3 @@ def test_count_scores(self): "count_scores(measurements.score.lcp)": 1, "count_scores(measurements.score.total)": 3, } - - def test_query_tags_and_flags(self): - self.store_event( - data={ - "event_id": "a" * 32, - "environment": "staging", - "timestamp": self.ten_mins_ago_iso, - "contexts": {"flags": {"values": [{"flag": "abc", "result": "123"}]}}, - }, - project_id=self.project.id, - ) - - query = { - "field": ["event_id"], - "query": "abc:123", - "dataset": "errors", - } - response = self.do_request(query) - assert response.status_code == 200, response.content - assert False, response.content From 8890661a5318bfaf8f1e31f2a1705d5749c733aa Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 17 Jan 2025 12:41:13 -0600 Subject: [PATCH 05/14] Clean up --- src/sentry/search/snuba/executors.py | 66 ++++++++++++++-------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index c67f29dda9974b..0a858b06d9b15a 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -1811,40 +1811,9 @@ def query( ), ) if condition is not None: - # This should update the search box so we can fetch the correct - # issues. - def recursive_check(condition: Column | Function) -> bool: - if isinstance(condition, Column): - if ( - condition.entity.name == "events" - and condition.name.startswith("tags[") - and condition.name.endswith("]") - ): - return True - elif isinstance(condition, Function): - for parameter in condition.parameters: - if isinstance(parameter, (Column, Function)): - return recursive_check(parameter) - return False - - def recursive_replace(condition: Column | Function) -> bool: - if isinstance(condition, Column): - if condition.name.startswith("tags[") and condition.name.endswith( - "]" - ): - return Column( - name=f"features[{condition.name[5:-1]}]", - entity=condition.entity, - ) - elif isinstance(condition, Function): - for parameter in condition.parameters: - if isinstance(parameter, (Column, Function)): - return recursive_replace(parameter) - return condition - - if recursive_check(condition.lhs): + if recursive_tag_check(condition.lhs): feature_condition = Condition( - lhs=recursive_replace(condition.lhs), + lhs=recursive_tag_replace(condition.lhs), op=condition.op, rhs=condition.rhs, ) @@ -1980,3 +1949,34 @@ def recursive_replace(condition: Column | Function) -> bool: paginator_results.results = [groups[k] for k in paginator_results.results if k in groups] # TODO: Add types to paginators and remove this return cast(CursorResult[Group], paginator_results) + + +# This should update the search box so we can fetch the correct +# issues. +def recursive_tag_check(condition: Column | Function) -> bool: + if isinstance(condition, Column): + if ( + condition.entity.name == "events" + and condition.name.startswith("tags[") + and condition.name.endswith("]") + ): + return True + elif isinstance(condition, Function): + for parameter in condition.parameters: + if isinstance(parameter, (Column, Function)): + return recursive_tag_check(parameter) + return False + + +def recursive_tag_replace(condition: Column | Function) -> bool: + if isinstance(condition, Column): + if condition.name.startswith("tags[") and condition.name.endswith("]"): + return Column( + name=f"features[{condition.name[5:-1]}]", + entity=condition.entity, + ) + elif isinstance(condition, Function): + for parameter in condition.parameters: + if isinstance(parameter, (Column, Function)): + return recursive_tag_replace(parameter) + return condition From cdaa79c4b8bee4a27952e5dac22c243c10529bad Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 22 Jan 2025 08:28:04 -0600 Subject: [PATCH 06/14] Rename to flags --- src/sentry/search/snuba/executors.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index 0a858b06d9b15a..1a307f16fbf204 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -1972,7 +1972,7 @@ def recursive_tag_replace(condition: Column | Function) -> bool: if isinstance(condition, Column): if condition.name.startswith("tags[") and condition.name.endswith("]"): return Column( - name=f"features[{condition.name[5:-1]}]", + name=f"flags[{condition.name[5:-1]}]", entity=condition.entity, ) elif isinstance(condition, Function): From ebcc1f3abbcb71336c1db004a60eb96d85273e94 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 22 Jan 2025 10:11:50 -0600 Subject: [PATCH 07/14] Add flags autocomplete blueprint --- src/sentry/flags/docs/api.md | 100 +++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/src/sentry/flags/docs/api.md b/src/sentry/flags/docs/api.md index e16a5731ff1b23..d166b1ce3b9991 100644 --- a/src/sentry/flags/docs/api.md +++ b/src/sentry/flags/docs/api.md @@ -205,3 +205,103 @@ Any request content-type is acceptable (JSON, XML, binary-formats) so long as th - Request - Response 201 + +## Flag Keys [/organizations//flags/keys/] + +- Parameters + - end (optional, string) - ISO 8601 format. Required if `start` is set. + - project (number) - The project to search. + - sort (string) - A field to sort by. Optionally prepended with a hyphen to indicate descending order. + - start (optional, string) - ISO 8601 format (`YYYY-MM-DDTHH:mm:ss.sssZ`) + - statsPeriod (optional, string) - A positive integer suffixed with a unit type. + - useCache (number) - Boolean number which determines if we should use the cache or not. 0 for false; 1 for true. + - cursor (optional, string) + - per_page (optional, number) + Default: 10 + - offset (optional, number) + Default: 0 + +**Attributes** + +| Column | Type | Description | +| ----------- | ------ | ----------- | +| key | string | | +| name | string | | +| totalValues | number | | + +### Browse Flag Keys [GET] + +Retrieve a collection of flag keys. + +- Response 200 + + ```json + [ + { + "key": "sdk_name", + "name": "Sdk Name", + "totalValues": 2444 + } + ] + ``` + +## Flag Key [/organizations//flags/keys//] + +### Fetch Flag Key [GET] + +Fetch a flag key. + +- Response 200 + + ```json + { + "key": "sdk_name", + "name": "Sdk Name", + "totalValues": 2444 + } + ``` + +## Flag Values [/organizations//flags/keys//values/] + +- Parameters + - end (optional, string) - ISO 8601 format. Required if `start` is set. + - project (number) - The project to search. + - sort (string) - A field to sort by. Optionally prepended with a hyphen to indicate descending order. + - start (optional, string) - ISO 8601 format (`YYYY-MM-DDTHH:mm:ss.sssZ`) + - statsPeriod (optional, string) - A positive integer suffixed with a unit type. + - useCache (number) - Boolean number which determines if we should use the cache or not. 0 for false; 1 for true. + - cursor (optional, string) + - per_page (optional, number) + Default: 10 + - offset (optional, number) + Default: 0 + +**Attributes** + +| Column | Type | Description | +| --------- | ------ | ----------- | +| key | string | | +| name | string | | +| value | string | | +| count | number | | +| lastSeen | string | | +| firstSeen | string | | + +### Browse Flag Values [GET] + +Retrieve a collection of flag values. + +- Response 200 + + ```json + [ + { + "key": "isCustomerDomain", + "name": "yes", + "value": "yes", + "count": 15525, + "lastSeen": "2025-01-22T15:59:13Z", + "firstSeen": "2025-01-21T15:59:02Z" + } + ] + ``` From ab5dd40114468899cf946b73729735c4903d26e1 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Wed, 22 Jan 2025 12:18:54 -0600 Subject: [PATCH 08/14] Add flag backend to tagstore --- .../api/endpoints/project_tagkey_values.py | 10 +- src/sentry/api/endpoints/project_tags.py | 12 ++- src/sentry/conf/server.py | 4 + src/sentry/flags/docs/api.md | 100 ------------------ src/sentry/tagstore/__init__.py | 6 ++ src/sentry/tagstore/snuba/__init__.py | 2 +- src/sentry/tagstore/snuba/backend.py | 69 +++++++++--- .../endpoints/test_project_tagkey_values.py | 36 +++++++ .../snuba/api/endpoints/test_project_tags.py | 41 +++++++ 9 files changed, 160 insertions(+), 120 deletions(-) diff --git a/src/sentry/api/endpoints/project_tagkey_values.py b/src/sentry/api/endpoints/project_tagkey_values.py index 83bed72538978c..7d6218a5831ab8 100644 --- a/src/sentry/api/endpoints/project_tagkey_values.py +++ b/src/sentry/api/endpoints/project_tagkey_values.py @@ -42,8 +42,14 @@ def get(self, request: Request, project, key) -> Response: # if the environment doesn't exist then the tag can't possibly exist raise ResourceDoesNotExist + # Flags also autocomplete. We can switch the dataset we target. + if request.GET.get("useFlagsBackend") == "1": + backend = tagstore.flag_backend + else: + backend = tagstore.backend + try: - tagkey = tagstore.backend.get_tag_key( + tagkey = backend.get_tag_key( project.id, environment_id, lookup_key, @@ -54,7 +60,7 @@ def get(self, request: Request, project, key) -> Response: start, end = get_date_range_from_params(request.GET) - paginator = tagstore.backend.get_tag_value_paginator( + paginator = backend.get_tag_value_paginator( project.id, environment_id, tagkey.key, diff --git a/src/sentry/api/endpoints/project_tags.py b/src/sentry/api/endpoints/project_tags.py index 3389c62074051d..691e3fe68f2642 100644 --- a/src/sentry/api/endpoints/project_tags.py +++ b/src/sentry/api/endpoints/project_tags.py @@ -27,8 +27,16 @@ def get(self, request: Request, project) -> Response: if request.GET.get("onlySamplingTags") == "1": kwargs["denylist"] = DS_DENYLIST + # Flags also autocomplete. We switch our backend and also enforce the + # events dataset. Flags are limited to errors. + use_flag_backend = request.GET.get("useFlagsBackend") == "1" + if use_flag_backend: + backend = tagstore.flag_backend + else: + backend = tagstore.backend + tag_keys = sorted( - tagstore.backend.get_tag_keys( + backend.get_tag_keys( project.id, environment_id, tenant_ids={"organization_id": project.organization_id}, @@ -48,7 +56,7 @@ def get(self, request: Request, project) -> Response: "key": tagstore.backend.get_standardized_key(tag_key.key), "name": tagstore.backend.get_tag_key_label(tag_key.key), "uniqueValues": tag_key.values_seen, - "canDelete": tag_key.key not in PROTECTED_TAG_KEYS, + "canDelete": tag_key.key not in PROTECTED_TAG_KEYS and not use_flag_backend, } ) diff --git a/src/sentry/conf/server.py b/src/sentry/conf/server.py index 5afdf18efe14ce..e0b651003f903a 100644 --- a/src/sentry/conf/server.py +++ b/src/sentry/conf/server.py @@ -1764,6 +1764,10 @@ def custom_parameter_sort(parameter: dict) -> tuple[str, int]: SENTRY_TAGSTORE = os.environ.get("SENTRY_TAGSTORE", "sentry.tagstore.snuba.SnubaTagStorage") SENTRY_TAGSTORE_OPTIONS: dict[str, Any] = {} +# Flag storage backend +SENTRY_FLAGSTORE = os.environ.get("SENTRY_FLAGSTORE", "sentry.tagstore.snuba.SnubaFlagStorage") +SENTRY_FLAGSTORE_OPTIONS: dict[str, Any] = {} + # Search backend SENTRY_SEARCH = os.environ.get( "SENTRY_SEARCH", "sentry.search.snuba.EventsDatasetSnubaSearchBackend" diff --git a/src/sentry/flags/docs/api.md b/src/sentry/flags/docs/api.md index d166b1ce3b9991..e16a5731ff1b23 100644 --- a/src/sentry/flags/docs/api.md +++ b/src/sentry/flags/docs/api.md @@ -205,103 +205,3 @@ Any request content-type is acceptable (JSON, XML, binary-formats) so long as th - Request - Response 201 - -## Flag Keys [/organizations//flags/keys/] - -- Parameters - - end (optional, string) - ISO 8601 format. Required if `start` is set. - - project (number) - The project to search. - - sort (string) - A field to sort by. Optionally prepended with a hyphen to indicate descending order. - - start (optional, string) - ISO 8601 format (`YYYY-MM-DDTHH:mm:ss.sssZ`) - - statsPeriod (optional, string) - A positive integer suffixed with a unit type. - - useCache (number) - Boolean number which determines if we should use the cache or not. 0 for false; 1 for true. - - cursor (optional, string) - - per_page (optional, number) - Default: 10 - - offset (optional, number) - Default: 0 - -**Attributes** - -| Column | Type | Description | -| ----------- | ------ | ----------- | -| key | string | | -| name | string | | -| totalValues | number | | - -### Browse Flag Keys [GET] - -Retrieve a collection of flag keys. - -- Response 200 - - ```json - [ - { - "key": "sdk_name", - "name": "Sdk Name", - "totalValues": 2444 - } - ] - ``` - -## Flag Key [/organizations//flags/keys//] - -### Fetch Flag Key [GET] - -Fetch a flag key. - -- Response 200 - - ```json - { - "key": "sdk_name", - "name": "Sdk Name", - "totalValues": 2444 - } - ``` - -## Flag Values [/organizations//flags/keys//values/] - -- Parameters - - end (optional, string) - ISO 8601 format. Required if `start` is set. - - project (number) - The project to search. - - sort (string) - A field to sort by. Optionally prepended with a hyphen to indicate descending order. - - start (optional, string) - ISO 8601 format (`YYYY-MM-DDTHH:mm:ss.sssZ`) - - statsPeriod (optional, string) - A positive integer suffixed with a unit type. - - useCache (number) - Boolean number which determines if we should use the cache or not. 0 for false; 1 for true. - - cursor (optional, string) - - per_page (optional, number) - Default: 10 - - offset (optional, number) - Default: 0 - -**Attributes** - -| Column | Type | Description | -| --------- | ------ | ----------- | -| key | string | | -| name | string | | -| value | string | | -| count | number | | -| lastSeen | string | | -| firstSeen | string | | - -### Browse Flag Values [GET] - -Retrieve a collection of flag values. - -- Response 200 - - ```json - [ - { - "key": "isCustomerDomain", - "name": "yes", - "value": "yes", - "count": 15525, - "lastSeen": "2025-01-22T15:59:13Z", - "firstSeen": "2025-01-21T15:59:02Z" - } - ] - ``` diff --git a/src/sentry/tagstore/__init__.py b/src/sentry/tagstore/__init__.py index c202b6c6a3e21a..241ade3a11560b 100644 --- a/src/sentry/tagstore/__init__.py +++ b/src/sentry/tagstore/__init__.py @@ -7,3 +7,9 @@ backend = LazyServiceWrapper(TagStorage, settings.SENTRY_TAGSTORE, settings.SENTRY_TAGSTORE_OPTIONS) backend.expose(locals()) + +# Searches the "flags" columns instead of "tags". +flag_backend = LazyServiceWrapper( + TagStorage, settings.SENTRY_FLAGSTORE, settings.SENTRY_FLAGSTORE_OPTIONS +) +flag_backend.expose(locals()) diff --git a/src/sentry/tagstore/snuba/__init__.py b/src/sentry/tagstore/snuba/__init__.py index 8e5b8504df9dd4..375b9ceeae1e6e 100644 --- a/src/sentry/tagstore/snuba/__init__.py +++ b/src/sentry/tagstore/snuba/__init__.py @@ -1 +1 @@ -from .backend import SnubaTagStorage # NOQA +from .backend import SnubaFlagStorage, SnubaTagStorage # NOQA diff --git a/src/sentry/tagstore/snuba/backend.py b/src/sentry/tagstore/snuba/backend.py index 2b4df05b768462..37c00409ed6787 100644 --- a/src/sentry/tagstore/snuba/backend.py +++ b/src/sentry/tagstore/snuba/backend.py @@ -116,7 +116,7 @@ class _OptimizeKwargs(TypedDict, total=False): sample: int -class SnubaTagStorage(TagStorage): +class _SnubaTagStorage(TagStorage): def __get_tag_key_and_top_values( self, project_id, @@ -128,7 +128,7 @@ def __get_tag_key_and_top_values( tenant_ids=None, **kwargs, ): - tag = f"tags[{key}]" + tag = self.format_string.format(key) filters = {"project_id": get_project_list(project_id)} if environment_id: filters["environment"] = [environment_id] @@ -260,10 +260,10 @@ def __get_tag_keys_for_projects( dataset, filters = self.apply_group_filters(group, filters) if keys is not None: - filters["tags_key"] = sorted(keys) + filters[self.key_column] = sorted(keys) if include_values_seen: - aggregations.append(["uniq", "tags_value", "values_seen"]) + aggregations.append(["uniq", self.value_column, "values_seen"]) should_cache = use_cache and group is None result = None @@ -303,7 +303,7 @@ def __get_tag_keys_for_projects( dataset=dataset, start=start, end=end, - groupby=["tags_key"], + groupby=[self.key_column], conditions=[], filter_keys=filters, aggregations=aggregations, @@ -480,7 +480,9 @@ def __get_group_list_tag_value( filters = {"project_id": project_ids, "group_id": group_id_list} if environment_ids: filters["environment"] = environment_ids - conditions = (extra_conditions if extra_conditions else []) + [[f"tags[{key}]", "=", value]] + conditions = (extra_conditions if extra_conditions else []) + [ + [self.format_string.format(key), "=", value] + ] aggregations = (extra_aggregations if extra_aggregations else []) + [ ["count()", "", "times_seen"], ["min", SEEN_COLUMN, "first_seen"], @@ -537,7 +539,7 @@ def get_generic_group_list_tag_value( Condition(Column("group_id"), Op.IN, group_id_list), Condition(Column("timestamp"), Op.LT, end), Condition(Column("timestamp"), Op.GTE, start), - Condition(Column(f"tags[{key}]"), Op.EQ, value), + Condition(Column(self.format_string.format(key)), Op.EQ, value), ] if translated_params.get("environment"): Condition(Column("environment"), Op.IN, translated_params["environment"]), @@ -582,7 +584,7 @@ def apply_group_filters(self, group: Group | None, filters): return dataset, filters def get_group_tag_value_count(self, group, environment_id, key, tenant_ids=None): - tag = f"tags[{key}]" + tag = self.format_string.format(key) filters = {"project_id": get_project_list(group.project_id)} if environment_id: filters["environment"] = [environment_id] @@ -633,7 +635,7 @@ def get_group_tag_keys_and_top_values( if environment_ids: filters["environment"] = environment_ids if keys is not None: - filters["tags_key"] = keys + filters[self.key_column] = keys dataset, filters = self.apply_group_filters(group, filters) aggregations = kwargs.get("aggregations", []) aggregations += [ @@ -646,12 +648,12 @@ def get_group_tag_keys_and_top_values( dataset=dataset, start=kwargs.get("start"), end=kwargs.get("end"), - groupby=["tags_key", "tags_value"], + groupby=[self.key_column, self.value_column], conditions=conditions, filter_keys=filters, aggregations=aggregations, orderby="-count", - limitby=[value_limit, "tags_key"], + limitby=[value_limit, self.key_column], referrer="tagstore._get_tag_keys_and_top_values", tenant_ids=tenant_ids, ) @@ -1047,6 +1049,9 @@ def _get_tag_values_for_releases_across_all_datasets(self, projects, environment [(i, TagValue(RELEASE_ALIAS, v, None, None, None)) for i, v in enumerate(versions)] ) + def get_snuba_column_name(self, key: str, dataset: Dataset): + return snuba.get_snuba_column_name(key, dataset=dataset) + def get_tag_value_paginator_for_projects( self, projects, @@ -1077,7 +1082,7 @@ def get_tag_value_paginator_for_projects( if include_replays: dataset = Dataset.Replays - snuba_key = snuba.get_snuba_column_name(key, dataset=dataset) + snuba_key = self.get_snuba_column_name(key, dataset=dataset) # We cannot search the values of these columns like we do other columns because they are # a different type, and as such, LIKE and != do not work on them. Furthermore, because the @@ -1192,7 +1197,7 @@ def get_tag_value_paginator_for_projects( snuba_name = FIELD_ALIASES[USER_DISPLAY_ALIAS].get_field() snuba.resolve_complex_column(snuba_name, resolver, []) elif snuba_name in BLACKLISTED_COLUMNS: - snuba_name = f"tags[{key}]" + snuba_name = self.format_string.format(key) if query: query = query.replace("\\", "\\\\") @@ -1299,7 +1304,7 @@ def get_group_tag_value_iter( ) -> list[GroupTagValue]: filters = { "project_id": get_project_list(group.project_id), - "tags_key": [key], + self.key_column: [key], } dataset, filters = self.apply_group_filters(group, filters) @@ -1307,7 +1312,7 @@ def get_group_tag_value_iter( filters["environment"] = environment_ids results = snuba.query( dataset=dataset, - groupby=["tags_value"], + groupby=[self.value_column], filter_keys=filters, conditions=[], aggregations=[ @@ -1358,3 +1363,37 @@ def get_group_tag_value_paginator( [(int(getattr(gtv, score_field).timestamp() * 1000), gtv) for gtv in group_tag_values], reverse=desc, ) + + +class SnubaTagStorage(_SnubaTagStorage): + key_column = "tags_key" + value_column = "tags_value" + format_string = "tags[{}]" + + +# Quick and dirty overload to support flag aggregations. This probably deserves +# a better refactor for now we're just raising within the functions we don't want +# to support. This sort of refactor couples flags behavior to a lot of tags +# specific behavior. The interfaces are compatible and flags are basically tags +# just with a different column to live on. + + +class SnubaFlagStorage(_SnubaTagStorage): + key_column = "flags_key" + value_column = "flags_value" + format_string = "flags[{}]" + + def get_release_tags(self, *args, **kwargs): + raise NotImplementedError + + def __get_groups_user_counts(self, *args, **kwargs): + raise NotImplementedError + + def get_groups_user_counts(self, *args, **kwargs): + raise NotImplementedError + + def get_generic_groups_user_counts(self, *args, **kwargs): + raise NotImplementedError + + def get_snuba_column_name(self, key: str, dataset: Dataset): + return f"flags[{key}]" diff --git a/tests/sentry/api/endpoints/test_project_tagkey_values.py b/tests/sentry/api/endpoints/test_project_tagkey_values.py index 72d881b48ca7d8..7881b1ca0f0a0f 100644 --- a/tests/sentry/api/endpoints/test_project_tagkey_values.py +++ b/tests/sentry/api/endpoints/test_project_tagkey_values.py @@ -137,3 +137,39 @@ def test_start_end_query(self): assert response.status_code == 200 assert len(response.data) == 1 assert response.data[0]["value"] == "bar" + + def test_simple_flags(self): + project = self.create_project() + self.store_event( + data={ + "contexts": {"flags": {"values": [{"flag": "abc", "result": True}]}}, + "timestamp": before_now(seconds=1).isoformat(), + }, + project_id=project.id, + ) + self.store_event( + data={ + "contexts": {"flags": {"values": [{"flag": "abc", "result": False}]}}, + "timestamp": before_now(seconds=1).isoformat(), + }, + project_id=project.id, + ) + + self.login_as(user=self.user) + + url = reverse( + "sentry-api-0-project-tagkey-values", + kwargs={ + "organization_id_or_slug": project.organization.slug, + "project_id_or_slug": project.slug, + "key": "abc", + }, + ) + + response = self.client.get(url + "?useFlagsBackend=1") + assert response.status_code == 200 + assert len(response.data) == 2 + + results = sorted(response.data, key=lambda i: i["value"]) + assert results[0]["value"] == "false" + assert results[1]["value"] == "true" diff --git a/tests/snuba/api/endpoints/test_project_tags.py b/tests/snuba/api/endpoints/test_project_tags.py index 589c103522e167..87ddff9c57645b 100644 --- a/tests/snuba/api/endpoints/test_project_tags.py +++ b/tests/snuba/api/endpoints/test_project_tags.py @@ -72,3 +72,44 @@ def test_simple_remove_tags_in_denylist_remove_all_tags(self): data = {v["key"]: v for v in response.data} assert len(data) == 0 assert data == {} + + def test_simple_flags(self): + self.store_event( + data={ + "contexts": { + "flags": { + "values": [ + {"flag": "abc", "result": True}, + {"flag": "def", "result": False}, + ] + } + }, + "timestamp": before_now(minutes=1).isoformat(), + }, + project_id=self.project.id, + ) + self.store_event( + data={ + "contexts": { + "flags": { + "values": [ + {"flag": "abc", "result": False}, + ] + } + }, + "timestamp": before_now(minutes=1).isoformat(), + }, + project_id=self.project.id, + ) + + response = self.get_success_response( + self.project.organization.slug, self.project.slug, useFlagsBackend="1" + ) + + data = {v["key"]: v for v in response.data} + assert len(data) == 2 + + assert data["def"]["canDelete"] is False + assert data["def"]["uniqueValues"] == 1 + assert data["abc"]["canDelete"] is False + assert data["abc"]["uniqueValues"] == 2 From 90130d602ee0238ebc9110c1d5772b750a628610 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Thu, 23 Jan 2025 09:57:41 -0600 Subject: [PATCH 09/14] Gate search behind a feature flag --- src/sentry/api/endpoints/project_tagkey_values.py | 10 +++++++--- src/sentry/api/endpoints/project_tags.py | 11 +++++++---- src/sentry/features/temporary.py | 8 ++++++++ .../api/endpoints/test_project_tagkey_values.py | 15 ++++++++------- tests/snuba/api/endpoints/test_project_tags.py | 7 ++++--- 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/sentry/api/endpoints/project_tagkey_values.py b/src/sentry/api/endpoints/project_tagkey_values.py index 7d6218a5831ab8..8e444e8725f83f 100644 --- a/src/sentry/api/endpoints/project_tagkey_values.py +++ b/src/sentry/api/endpoints/project_tagkey_values.py @@ -1,7 +1,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import tagstore +from sentry import features, tagstore from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import EnvironmentMixin, region_silo_endpoint @@ -42,8 +42,12 @@ def get(self, request: Request, project, key) -> Response: # if the environment doesn't exist then the tag can't possibly exist raise ResourceDoesNotExist - # Flags also autocomplete. We can switch the dataset we target. - if request.GET.get("useFlagsBackend") == "1": + # Flags are stored on the same table as tags but on a different column. Ideally both + # could be queried in a single request. But at present we're not sure if we want to + # treat tags and flags as the same or different and in which context. + if request.GET.get("useFlagsBackend") == "1" and features.has( + "organizations:feature-flag-search", project.organization, actor=request.user + ): backend = tagstore.flag_backend else: backend = tagstore.backend diff --git a/src/sentry/api/endpoints/project_tags.py b/src/sentry/api/endpoints/project_tags.py index 691e3fe68f2642..44e06645c8fe97 100644 --- a/src/sentry/api/endpoints/project_tags.py +++ b/src/sentry/api/endpoints/project_tags.py @@ -1,7 +1,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import tagstore +from sentry import features, tagstore from sentry.api.api_owners import ApiOwner from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import EnvironmentMixin, region_silo_endpoint @@ -27,9 +27,12 @@ def get(self, request: Request, project) -> Response: if request.GET.get("onlySamplingTags") == "1": kwargs["denylist"] = DS_DENYLIST - # Flags also autocomplete. We switch our backend and also enforce the - # events dataset. Flags are limited to errors. - use_flag_backend = request.GET.get("useFlagsBackend") == "1" + # Flags are stored on the same table as tags but on a different column. Ideally both + # could be queried in a single request. But at present we're not sure if we want to + # treat tags and flags as the same or different and in which context. + use_flag_backend = request.GET.get("useFlagsBackend") == "1" and features.has( + "organizations:feature-flag-search", project.organization, actor=request.user + ) if use_flag_backend: backend = tagstore.flag_backend else: diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index 56df8b2656d475..d6a9c85f0faeba 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -625,3 +625,11 @@ def register_temporary_features(manager: FeatureManager): FeatureHandlerStrategy.FLAGPOLE, api_expose=True, ) + + # Feature Flags Features. + manager.add( + "organizations:feature-flag-search", + OrganizationFeature, + FeatureHandlerStrategy.FLAGPOLE, + api_expose=True, + ) diff --git a/tests/sentry/api/endpoints/test_project_tagkey_values.py b/tests/sentry/api/endpoints/test_project_tagkey_values.py index 7881b1ca0f0a0f..c82f775dc615ed 100644 --- a/tests/sentry/api/endpoints/test_project_tagkey_values.py +++ b/tests/sentry/api/endpoints/test_project_tagkey_values.py @@ -166,10 +166,11 @@ def test_simple_flags(self): }, ) - response = self.client.get(url + "?useFlagsBackend=1") - assert response.status_code == 200 - assert len(response.data) == 2 - - results = sorted(response.data, key=lambda i: i["value"]) - assert results[0]["value"] == "false" - assert results[1]["value"] == "true" + with self.feature({"organizations:feature-flag-search": True}): + response = self.client.get(url + "?useFlagsBackend=1") + assert response.status_code == 200 + assert len(response.data) == 2 + + results = sorted(response.data, key=lambda i: i["value"]) + assert results[0]["value"] == "false" + assert results[1]["value"] == "true" diff --git a/tests/snuba/api/endpoints/test_project_tags.py b/tests/snuba/api/endpoints/test_project_tags.py index 87ddff9c57645b..0a566a77fa8595 100644 --- a/tests/snuba/api/endpoints/test_project_tags.py +++ b/tests/snuba/api/endpoints/test_project_tags.py @@ -102,9 +102,10 @@ def test_simple_flags(self): project_id=self.project.id, ) - response = self.get_success_response( - self.project.organization.slug, self.project.slug, useFlagsBackend="1" - ) + with self.feature({"organizations:feature-flag-search": True}): + response = self.get_success_response( + self.project.organization.slug, self.project.slug, useFlagsBackend="1" + ) data = {v["key"]: v for v in response.data} assert len(data) == 2 From 93266c4fe8497dae3712bcc0e53a546524aa3639 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 24 Jan 2025 09:00:46 -0600 Subject: [PATCH 10/14] Rename feature flag --- src/sentry/api/endpoints/project_tagkey_values.py | 2 +- src/sentry/api/endpoints/project_tags.py | 2 +- src/sentry/features/temporary.py | 2 +- tests/sentry/api/endpoints/test_project_tagkey_values.py | 2 +- tests/snuba/api/endpoints/test_project_tags.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sentry/api/endpoints/project_tagkey_values.py b/src/sentry/api/endpoints/project_tagkey_values.py index 8e444e8725f83f..a04775d5a91f4e 100644 --- a/src/sentry/api/endpoints/project_tagkey_values.py +++ b/src/sentry/api/endpoints/project_tagkey_values.py @@ -46,7 +46,7 @@ def get(self, request: Request, project, key) -> Response: # could be queried in a single request. But at present we're not sure if we want to # treat tags and flags as the same or different and in which context. if request.GET.get("useFlagsBackend") == "1" and features.has( - "organizations:feature-flag-search", project.organization, actor=request.user + "organizations:feature-flag-autocomplete", project.organization, actor=request.user ): backend = tagstore.flag_backend else: diff --git a/src/sentry/api/endpoints/project_tags.py b/src/sentry/api/endpoints/project_tags.py index 44e06645c8fe97..d02fd87ab891d4 100644 --- a/src/sentry/api/endpoints/project_tags.py +++ b/src/sentry/api/endpoints/project_tags.py @@ -31,7 +31,7 @@ def get(self, request: Request, project) -> Response: # could be queried in a single request. But at present we're not sure if we want to # treat tags and flags as the same or different and in which context. use_flag_backend = request.GET.get("useFlagsBackend") == "1" and features.has( - "organizations:feature-flag-search", project.organization, actor=request.user + "organizations:feature-flag-autocomplete", project.organization, actor=request.user ) if use_flag_backend: backend = tagstore.flag_backend diff --git a/src/sentry/features/temporary.py b/src/sentry/features/temporary.py index d6a9c85f0faeba..757ed4a2cf2ef7 100644 --- a/src/sentry/features/temporary.py +++ b/src/sentry/features/temporary.py @@ -628,7 +628,7 @@ def register_temporary_features(manager: FeatureManager): # Feature Flags Features. manager.add( - "organizations:feature-flag-search", + "organizations:feature-flag-autocomplete", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True, diff --git a/tests/sentry/api/endpoints/test_project_tagkey_values.py b/tests/sentry/api/endpoints/test_project_tagkey_values.py index c82f775dc615ed..f528188cc585da 100644 --- a/tests/sentry/api/endpoints/test_project_tagkey_values.py +++ b/tests/sentry/api/endpoints/test_project_tagkey_values.py @@ -166,7 +166,7 @@ def test_simple_flags(self): }, ) - with self.feature({"organizations:feature-flag-search": True}): + with self.feature({"organizations:feature-flag-autocomplete": True}): response = self.client.get(url + "?useFlagsBackend=1") assert response.status_code == 200 assert len(response.data) == 2 diff --git a/tests/snuba/api/endpoints/test_project_tags.py b/tests/snuba/api/endpoints/test_project_tags.py index 0a566a77fa8595..5f73b727a087bf 100644 --- a/tests/snuba/api/endpoints/test_project_tags.py +++ b/tests/snuba/api/endpoints/test_project_tags.py @@ -102,7 +102,7 @@ def test_simple_flags(self): project_id=self.project.id, ) - with self.feature({"organizations:feature-flag-search": True}): + with self.feature({"organizations:feature-flag-autocomplete": True}): response = self.get_success_response( self.project.organization.slug, self.project.slug, useFlagsBackend="1" ) From a6959fd40947a9ebd1b5217d531673025b518681 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 24 Jan 2025 10:12:43 -0600 Subject: [PATCH 11/14] Gate tag rewrite behind feature flag --- src/sentry/search/snuba/executors.py | 4 +++- .../endpoints/test_organization_group_index.py | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index 1a307f16fbf204..37821f071d06fe 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -1811,7 +1811,9 @@ def query( ), ) if condition is not None: - if recursive_tag_check(condition.lhs): + if features.has( + "organizations:feature-flag-autocomplete", organization + ) and recursive_tag_check(condition.lhs): feature_condition = Condition( lhs=recursive_tag_replace(condition.lhs), op=condition.op, diff --git a/tests/sentry/issues/endpoints/test_organization_group_index.py b/tests/sentry/issues/endpoints/test_organization_group_index.py index 2ccdc8b518a9c7..ac732dc3969dd8 100644 --- a/tests/sentry/issues/endpoints/test_organization_group_index.py +++ b/tests/sentry/issues/endpoints/test_organization_group_index.py @@ -4042,11 +4042,18 @@ def test_flags_and_tags_query(self, _: MagicMock) -> None: }, project_id=project.id, ) - response = self.get_success_response(query="abc:true") - assert len(json.loads(response.content)) == 1 - response = self.get_success_response(query="abc:false") - assert len(json.loads(response.content)) == 0 + with self.feature({"organizations:feature-flag-autocomplete": True}): + response = self.get_success_response(query="abc:true") + assert len(json.loads(response.content)) == 1 + + with self.feature({"organizations:feature-flag-autocomplete": False}): + response = self.get_success_response(query="abc:true") + assert len(json.loads(response.content)) == 0 + + with self.feature({"organizations:feature-flag-autocomplete": True}): + response = self.get_success_response(query="abc:false") + assert len(json.loads(response.content)) == 0 class GroupUpdateTest(APITestCase, SnubaTestCase): From 1cf4ec3e0cbeb722dcb74a603a672b28b9c7b78c Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 24 Jan 2025 14:32:28 -0600 Subject: [PATCH 12/14] Renames --- src/sentry/search/snuba/executors.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sentry/search/snuba/executors.py b/src/sentry/search/snuba/executors.py index 37821f071d06fe..2e79f41ee5fb36 100644 --- a/src/sentry/search/snuba/executors.py +++ b/src/sentry/search/snuba/executors.py @@ -1813,9 +1813,9 @@ def query( if condition is not None: if features.has( "organizations:feature-flag-autocomplete", organization - ) and recursive_tag_check(condition.lhs): + ) and has_tags_filter(condition.lhs): feature_condition = Condition( - lhs=recursive_tag_replace(condition.lhs), + lhs=substitute_tags_filter(condition.lhs), op=condition.op, rhs=condition.rhs, ) @@ -1955,7 +1955,7 @@ def query( # This should update the search box so we can fetch the correct # issues. -def recursive_tag_check(condition: Column | Function) -> bool: +def has_tags_filter(condition: Column | Function) -> bool: if isinstance(condition, Column): if ( condition.entity.name == "events" @@ -1966,11 +1966,11 @@ def recursive_tag_check(condition: Column | Function) -> bool: elif isinstance(condition, Function): for parameter in condition.parameters: if isinstance(parameter, (Column, Function)): - return recursive_tag_check(parameter) + return has_tags_filter(parameter) return False -def recursive_tag_replace(condition: Column | Function) -> bool: +def substitute_tags_filter(condition: Column | Function) -> bool: if isinstance(condition, Column): if condition.name.startswith("tags[") and condition.name.endswith("]"): return Column( @@ -1980,5 +1980,5 @@ def recursive_tag_replace(condition: Column | Function) -> bool: elif isinstance(condition, Function): for parameter in condition.parameters: if isinstance(parameter, (Column, Function)): - return recursive_tag_replace(parameter) + return substitute_tags_filter(parameter) return condition From d6998ba61702ea16179de3c606c59c0a31b7caa1 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 24 Jan 2025 14:45:52 -0600 Subject: [PATCH 13/14] Add assert for counts --- tests/sentry/api/endpoints/test_project_tagkey_values.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/sentry/api/endpoints/test_project_tagkey_values.py b/tests/sentry/api/endpoints/test_project_tagkey_values.py index f528188cc585da..d5781b52b4a918 100644 --- a/tests/sentry/api/endpoints/test_project_tagkey_values.py +++ b/tests/sentry/api/endpoints/test_project_tagkey_values.py @@ -174,3 +174,5 @@ def test_simple_flags(self): results = sorted(response.data, key=lambda i: i["value"]) assert results[0]["value"] == "false" assert results[1]["value"] == "true" + assert results[0]["count"] == 1 + assert results[1]["count"] == 1 From 68a83a7b541a941ae1837a068e57fb1ad7f39481 Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 24 Jan 2025 15:23:49 -0600 Subject: [PATCH 14/14] Update organization endpoints --- .../endpoints/organization_tagkey_values.py | 14 ++++- src/sentry/api/endpoints/organization_tags.py | 13 ++++- .../test_organization_tagkey_values.py | 31 +++++++++++ .../api/endpoints/test_organization_tags.py | 55 +++++++++++++++++++ 4 files changed, 110 insertions(+), 3 deletions(-) diff --git a/src/sentry/api/endpoints/organization_tagkey_values.py b/src/sentry/api/endpoints/organization_tagkey_values.py index 94b4916764a991..c4f270c5a85cbd 100644 --- a/src/sentry/api/endpoints/organization_tagkey_values.py +++ b/src/sentry/api/endpoints/organization_tagkey_values.py @@ -3,7 +3,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from sentry import tagstore +from sentry import features, tagstore from sentry.api.api_publish_status import ApiPublishStatus from sentry.api.base import region_silo_endpoint from sentry.api.bases import NoProjects, OrganizationEventsEndpointBase @@ -54,7 +54,17 @@ def get(self, request: Request, organization, key) -> Response: paginator: SequencePaginator[TagValue] = SequencePaginator([]) else: with handle_query_errors(): - paginator = tagstore.backend.get_tag_value_paginator_for_projects( + # Flags are stored on the same table as tags but on a different column. Ideally + # both could be queried in a single request. But at present we're not sure if we + # want to treat tags and flags as the same or different and in which context. + if request.GET.get("useFlagsBackend") == "1" and features.has( + "organizations:feature-flag-autocomplete", organization, actor=request.user + ): + backend = tagstore.flag_backend + else: + backend = tagstore.backend + + paginator = backend.get_tag_value_paginator_for_projects( snuba_params.project_ids, snuba_params.environment_ids, key, diff --git a/src/sentry/api/endpoints/organization_tags.py b/src/sentry/api/endpoints/organization_tags.py index ccaeeeaa10d658..95c8fd5d7ffbf4 100644 --- a/src/sentry/api/endpoints/organization_tags.py +++ b/src/sentry/api/endpoints/organization_tags.py @@ -53,7 +53,18 @@ def get(self, request: Request, organization) -> Response: ), ) - results = tagstore.backend.get_tag_keys_for_projects( + # Flags are stored on the same table as tags but on a different column. Ideally + # both could be queried in a single request. But at present we're not sure if we + # want to treat tags and flags as the same or different and in which context. + use_flag_backend = request.GET.get("useFlagsBackend") == "1" and features.has( + "organizations:feature-flag-autocomplete", organization, actor=request.user + ) + if use_flag_backend: + backend = tagstore.flag_backend + else: + backend = tagstore.backend + + results = backend.get_tag_keys_for_projects( filter_params["project_id"], filter_params.get("environment"), start, diff --git a/tests/snuba/api/endpoints/test_organization_tagkey_values.py b/tests/snuba/api/endpoints/test_organization_tagkey_values.py index d1573fc11c8ac0..1ea68cd2f41d98 100644 --- a/tests/snuba/api/endpoints/test_organization_tagkey_values.py +++ b/tests/snuba/api/endpoints/test_organization_tagkey_values.py @@ -459,6 +459,37 @@ def test_release_filter_for_all_releases_with_env_and_project_filters(self): expected=[("aaa@1.0", None), ("aba@1.0", None)], ) + def test_simple_flags(self): + self.store_event( + data={ + "contexts": {"flags": {"values": [{"flag": "abc", "result": True}]}}, + "timestamp": before_now(seconds=1).isoformat(), + }, + project_id=self.project.id, + ) + self.store_event( + data={ + "contexts": {"flags": {"values": [{"flag": "abc", "result": False}]}}, + "timestamp": before_now(seconds=1).isoformat(), + }, + project_id=self.project.id, + ) + + with self.feature({"organizations:feature-flag-autocomplete": True}): + url = reverse( + "sentry-api-0-organization-tagkey-values", + kwargs={"organization_id_or_slug": self.org.slug, "key": "abc"}, + ) + response = self.client.get(url + "?useFlagsBackend=1") + assert response.status_code == 200 + assert len(response.data) == 2 + + results = sorted(response.data, key=lambda i: i["value"]) + assert results[0]["value"] == "false" + assert results[1]["value"] == "true" + assert results[0]["count"] == 1 + assert results[1]["count"] == 1 + class TransactionTagKeyValues(OrganizationTagKeyTestCase): def setUp(self): diff --git a/tests/snuba/api/endpoints/test_organization_tags.py b/tests/snuba/api/endpoints/test_organization_tags.py index 3c66e2568253cc..b70ca9e42adf57 100644 --- a/tests/snuba/api/endpoints/test_organization_tags.py +++ b/tests/snuba/api/endpoints/test_organization_tags.py @@ -60,6 +60,61 @@ def test_simple(self): {"name": "Some Tag", "key": "some_tag", "totalValues": 1}, ] + def test_simple_flags(self): + user = self.create_user() + org = self.create_organization() + team = self.create_team(organization=org) + self.create_member(organization=org, user=user, teams=[team]) + + self.login_as(user=user) + + project = self.create_project(organization=org, teams=[team]) + self.store_event( + data={ + "contexts": { + "flags": { + "values": [ + {"flag": "abc", "result": True}, + {"flag": "def", "result": False}, + ] + } + }, + "timestamp": before_now(minutes=1).isoformat(), + }, + project_id=project.id, + ) + self.store_event( + data={ + "contexts": { + "flags": { + "values": [ + {"flag": "abc", "result": False}, + ] + } + }, + "timestamp": before_now(minutes=1).isoformat(), + }, + project_id=project.id, + ) + + url = reverse( + "sentry-api-0-organization-tags", kwargs={"organization_id_or_slug": org.slug} + ) + + with self.feature({"organizations:feature-flag-autocomplete": True}): + response = self.client.get( + url, + {"statsPeriod": "14d", "useFlagsBackend": "1", "dataset": "events"}, + format="json", + ) + assert response.status_code == 200, response.content + data = response.data + data.sort(key=lambda val: val["totalValues"], reverse=True) + assert data == [ + {"key": "abc", "name": "Abc", "totalValues": 2}, + {"key": "def", "name": "Def", "totalValues": 1}, + ] + def test_dataset_events(self): user = self.create_user() org = self.create_organization()