From 3ba6262e70bf8650a420c3e9bcc1e9e30a48fffb Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Thu, 7 Nov 2024 17:21:34 -0500 Subject: [PATCH 1/2] fix(profiling): Format all_examples in functions stats response The result of calling `all_examples()` in the stats endpoint needs to be formatted into the expected format for easier reuse on the frontend. --- .../events/builder/profile_functions.py | 19 +++++-- src/sentry/snuba/functions.py | 1 + .../test_organization_events_stats.py | 54 ++++++++++++++++--- 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/sentry/search/events/builder/profile_functions.py b/src/sentry/search/events/builder/profile_functions.py index ea42a0dd7a5d50..6d0db91b80bc98 100644 --- a/src/sentry/search/events/builder/profile_functions.py +++ b/src/sentry/search/events/builder/profile_functions.py @@ -45,8 +45,15 @@ def get_field_type(self: ProfileFunctionsQueryBuilderProtocol, field: str) -> st def process_profiling_function_columns(self, row: SnubaRow): if "all_examples()" in row: + key = "all_examples()" + elif "all_examples" in row: + key = "all_examples" + else: + key = None + + if key is not None: parsed_examples = [] - for example in row["all_examples()"]: + for example in row[key]: profile_id, thread_id, start, end = example # This is shaped like the `ExampleMetaData` in vroom @@ -66,7 +73,7 @@ def process_profiling_function_columns(self, row: SnubaRow): } ) - row["all_examples()"] = parsed_examples + row[key] = parsed_examples class ProfileFunctionsQueryBuilder(ProfileFunctionsQueryBuilderMixin, BaseQueryBuilder): @@ -108,10 +115,12 @@ def time_column(self) -> SelectType: return custom_time_processor(self.interval, Function("toUInt32", [Column("timestamp")])) def process_results(self, results: Any) -> EventsResponse: - processed: EventsResponse = super().process_results(results) - for row in processed["data"]: + # Not sure why exactly but calling `super().process_results(results)` + # on the timeseries data mutates the data in such a way that breaks + # the zerofill later + for row in results["data"]: self.process_profiling_function_columns(row) - return processed + return results class ProfileTopFunctionsTimeseriesQueryBuilder(ProfileFunctionsTimeseriesQueryBuilder): diff --git a/src/sentry/snuba/functions.py b/src/sentry/snuba/functions.py index e1bac0a1394536..3936f137acf911 100644 --- a/src/sentry/snuba/functions.py +++ b/src/sentry/snuba/functions.py @@ -105,6 +105,7 @@ def timeseries_query( ) results = builder.run_query(referrer=referrer, query_source=query_source) results = builder.strip_alias_prefix(results) + results = builder.process_results(results) return SnubaTSResult( { diff --git a/tests/snuba/api/endpoints/test_organization_events_stats.py b/tests/snuba/api/endpoints/test_organization_events_stats.py index 6b0748cd98b789..ad787c2efda0c7 100644 --- a/tests/snuba/api/endpoints/test_organization_events_stats.py +++ b/tests/snuba/api/endpoints/test_organization_events_stats.py @@ -2726,47 +2726,89 @@ def setUp(self): ) def test_functions_dataset_simple(self): - self.store_functions( + transaction_function = self.store_functions( [ { - "self_times_ns": [100 for _ in range(100)], + "self_times_ns": [100_000_000 for _ in range(100)], "package": "foo", "function": "bar", "in_app": True, }, ], project=self.project, - timestamp=self.two_days_ago, + timestamp=self.two_days_ago - timedelta(hours=12), + ) + + continuous_timestamp = self.two_days_ago + timedelta(hours=12) + continuous_function = self.store_functions_chunk( + [ + { + "self_times_ns": [200_000_000 for _ in range(100)], + "package": "bar", + "function": "bar", + "thread_id": "1", + "in_app": True, + }, + ], + project=self.project, + timestamp=continuous_timestamp, ) + y_axes = [ + "cpm()", + "p95(function.duration)", + "all_examples()", + ] + data = { "dataset": "profileFunctions", "start": self.three_days_ago.isoformat(), "end": self.one_day_ago.isoformat(), "interval": "1d", - "yAxis": ["cpm()", "p95(function.duration)"], + "yAxis": y_axes, } response = self.client.get(self.url, data=data, format="json") assert response.status_code == 200, response.content assert sum(row[1][0]["count"] for row in response.data["cpm()"]["data"]) == pytest.approx( - 100 / ((self.one_day_ago - self.three_days_ago).total_seconds() / 60), rel=1e-3 + 200 / ((self.one_day_ago - self.three_days_ago).total_seconds() / 60), rel=1e-3 ) assert any( row[1][0]["count"] > 0 for row in response.data["p95(function.duration)"]["data"] ) - for y_axis in ["cpm()", "p95(function.duration)"]: + examples = [row[1][0]["count"] for row in response.data["all_examples()"]["data"]] + assert examples == [ + [ + { + "profile_id": transaction_function["transaction"]["contexts"]["profile"][ + "profile_id" + ], + }, + ], + [ + { + "profiler_id": continuous_function["profiler_id"], + "thread_id": "1", + "start": continuous_timestamp.timestamp(), + "end": (continuous_timestamp + timedelta(microseconds=200_000)).timestamp(), + }, + ], + ] + + for y_axis in y_axes: assert response.data[y_axis]["meta"]["fields"] == { "time": "date", "cpm": "number", "p95_function_duration": "duration", + "all_examples": "string", } assert response.data[y_axis]["meta"]["units"] == { "time": None, "cpm": None, "p95_function_duration": "nanosecond", + "all_examples": None, } From 205fcd4e3fca89fbcc6130f655ee8dfc8fb081fd Mon Sep 17 00:00:00 2001 From: Tony Xiao Date: Thu, 7 Nov 2024 17:59:31 -0500 Subject: [PATCH 2/2] feat(profiling): Support continuous profiling in function trends In order to support continuous profiling functions, introduce a new list called `examples` that the frontend needs to migrate to that will contain the meta data to link to either a transaction profile or a continuous profile. --- src/sentry/api/bases/organization_events.py | 6 +++--- .../endpoints/organization_profiling_functions.py | 14 +++++++++----- .../test_organization_profiling_functions.py | 2 ++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/sentry/api/bases/organization_events.py b/src/sentry/api/bases/organization_events.py index a461391f22766f..330fe952ec3a61 100644 --- a/src/sentry/api/bases/organization_events.py +++ b/src/sentry/api/bases/organization_events.py @@ -420,15 +420,15 @@ def get_event_stats_data( allow_partial_buckets: bool = False, zerofill_results: bool = True, comparison_delta: timedelta | None = None, - additional_query_column: str | None = None, + additional_query_columns: list[str] | None = None, dataset: Any | None = None, ) -> dict[str, Any]: with handle_query_errors(): with sentry_sdk.start_span(op="discover.endpoint", name="base.stats_query_creation"): _columns = [query_column] # temporary change to make topN query work for multi-axes requests - if additional_query_column is not None: - _columns.append(additional_query_column) + if additional_query_columns is not None: + _columns.extend(additional_query_columns) columns = request.GET.getlist("yAxis", _columns) diff --git a/src/sentry/api/endpoints/organization_profiling_functions.py b/src/sentry/api/endpoints/organization_profiling_functions.py index 60504290081e42..3fd9e68acb0366 100644 --- a/src/sentry/api/endpoints/organization_profiling_functions.py +++ b/src/sentry/api/endpoints/organization_profiling_functions.py @@ -100,7 +100,6 @@ def get(self, request: Request, organization: Organization) -> Response: "package", "function", "count()", - "examples()", ], query=data.get("query"), snuba_params=snuba_params, @@ -135,7 +134,7 @@ def get_event_stats( # It's possible to override the columns via # the `yAxis` qs. So we explicitly ignore the # columns, and hard code in the columns we want. - timeseries_columns=[data["function"], "examples()"], + timeseries_columns=[data["function"], "examples()", "all_examples()"], config=QueryBuilderConfig( skip_tag_resolution=True, ), @@ -196,7 +195,7 @@ def get_trends_data(stats_data) -> list[BreakpointData]: get_event_stats, top_events=FUNCTIONS_PER_QUERY, query_column=data["function"], - additional_query_column="examples()", + additional_query_columns=["examples()", "all_examples()"], snuba_params=snuba_params, query=data.get("query"), ) @@ -244,11 +243,16 @@ def get_stats_data_for_trending_events(results): key = f"{result['project']},{result['transaction']}" formatted_result = { "stats": stats_data[key][data["function"]], - "worst": [ + "worst": [ # deprecated, migrate to `examples` (ts, data[0]["count"][0]) for ts, data in stats_data[key]["examples()"]["data"] if data[0]["count"] # filter out entries without an example ], + "examples": [ + (ts, data[0]["count"][0]) + for ts, data in stats_data[key]["all_examples()"]["data"] + if data[0]["count"] # filter out entries without an example + ], } formatted_result.update( { @@ -269,7 +273,7 @@ def get_stats_data_for_trending_events(results): formatted_result.update( { k: functions[key][k] - for k in ["fingerprint", "package", "function", "count()", "examples()"] + for k in ["fingerprint", "package", "function", "count()"] } ) formatted_results.append(formatted_result) diff --git a/tests/sentry/api/endpoints/test_organization_profiling_functions.py b/tests/sentry/api/endpoints/test_organization_profiling_functions.py index c25d3047ea5632..27dece81abaaf4 100644 --- a/tests/sentry/api/endpoints/test_organization_profiling_functions.py +++ b/tests/sentry/api/endpoints/test_organization_profiling_functions.py @@ -225,6 +225,7 @@ def test_regression(self, mock_detect_breakpoints): assert trend_percentages == [10.0, 5.0] for data in results: assert isinstance(data["worst"], list) + assert isinstance(data["examples"], list) @mock.patch("sentry.api.endpoints.organization_profiling_functions.detect_breakpoints") def test_improvement(self, mock_detect_breakpoints): @@ -310,6 +311,7 @@ def test_improvement(self, mock_detect_breakpoints): assert trend_percentages == [0.1, 0.2] for data in results: assert isinstance(data["worst"], list) + assert isinstance(data["examples"], list) def test_get_rollup_from_range_max_buckets():