feat: eap support formulas in timeseries endpoint #6854

kylemumma · 2025-02-04T22:46:27Z

this PR implements support for formulas in the timeseries endpoint. it closes this ticket https://github.com/getsentry/eap-planning/issues/27

major changes:

auto-convert TimeSeriesRequest.aggregations to TimeSeriesRequest.expressions
implement support for formula

tests:

I have a test for a non-extrapolated formula as well as an extrapolated one

design decisions

reliability doesnt work with formulas
formulas dont work w uptime checks or logs

codecov · 2025-02-05T00:00:26Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
2806	1	2805	11

View the top 1 failed test(s) by shortest run time

tests.web.rpc.v1.test_endpoint_time_series.test_endpoint_time_series.TestTimeSeriesApi::test_formula

Stack Traces | 0.982s run time

Traceback (most recent call last):
  File ".../v1/test_endpoint_time_series/test_endpoint_time_series.py", line 1015, in test_formula
    assert sorted(response.result_timeseries, key=lambda x: x.label) == [
AssertionError: assert [label: "sum + avg"\nbuckets {\n  seconds: 1738828800\n}\nbuckets {\n  seconds: 1738829100\n}\nbuckets {\n  seconds: 1738829400\n}\nbuckets {\n  seconds: 1738829700\n}\nbuckets {\n  seconds: 1738830000\n}\nbuckets {\n  seconds: 1738830300\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\n] == [label: "sum + avg"\nbuckets {\n  seconds: 1738828800\n}\nbuckets {\n  seconds: 1738829100\n}\nbuckets {\n  seconds: 1738829400\n}\nbuckets {\n  seconds: 1738829700\n}\nbuckets {\n  seconds: 1738830000\n}\nbuckets {\n  seconds: 1738830300\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\n]
  At index 0 diff: label: "sum + avg"\nbuckets {\n  seconds: 1738828800\n}\nbuckets {\n  seconds: 1738829100\n}\nbuckets {\n  seconds: 1738829400\n}\nbuckets {\n  seconds: 1738829700\n}\nbuckets {\n  seconds: 1738830000\n}\nbuckets {\n  seconds: 1738830300\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\ndata_points {\n}\n != label: "sum + avg"\nbuckets {\n  seconds: 1738828800\n}\nbuckets {\n  seconds: 1738829100\n}\nbuckets {\n  seconds: 1738829400\n}\nbuckets {\n  seconds: 1738829700\n}\nbuckets {\n  seconds: 1738830000\n}\nbuckets {\n  seconds: 1738830300\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\ndata_points {\n  data: 301\n  data_present: true\n  sample_count: 1\n}\n
  Full diff:
    [
     label: "sum + avg"
    buckets {
      seconds: 1738828800
    }
    buckets {
      seconds: 1738829100
    }
    buckets {
      seconds: 1738829400
    }
    buckets {
      seconds: 1738829700
    }
    buckets {
      seconds: 1738830000
    }
    buckets {
      seconds: 1738830300
    }
    data_points {
  -   data: 301
  -   data_present: true
  -   sample_count: 1
    }
    data_points {
  -   data: 301
  -   data_present: true
  -   sample_count: 1
    }
    data_points {
  -   data: 301
  -   data_present: true
  -   sample_count: 1
    }
    data_points {
  -   data: 301
  -   data_present: true
  -   sample_count: 1
    }
    data_points {
  -   data: 301
  -   data_present: true
  -   sample_count: 1
    }
    data_points {
  -   data: 301
  -   data_present: true
  -   sample_count: 1
    }
    ,
    ]

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

kylemumma · 2025-02-07T22:16:57Z

snuba/web/rpc/v1/endpoint_time_series.py

@@ -107,5 +121,6 @@ def _execute(self, in_msg: TimeSeriesRequest) -> TimeSeriesResponse:
            raise BadSnubaRPCRequestException(
                "This endpoint requires meta.trace_item_type to be set (are you requesting spans? logs?)"
            )
+        in_msg = _convert_aggregations_to_expressions(in_msg)


in_msg.aggregations was deprecated to be replaced with in_msg.expressions
https://github.com/getsentry/sentry-protos/pull/105/files#diff-82c0471037e5c909123fcef5d5283670bf14ee7af2fe80b3fee909143e46f4adR26

this change makes it so any time a user passes aggregations it get converted to expressions before hitting the resolver

kylemumma · 2025-02-07T22:19:32Z

snuba/web/rpc/v1/resolvers/R_eap_spans/resolver_time_series.py

-        schema=get_entity(EntityKey("eap_spans")).get_data_model(),
-        sample=None,
-    )
+def _get_reliability_context_columns(


existing logic extracted into a new function

kylemumma · 2025-02-07T22:20:41Z

snuba/web/rpc/v1/resolvers/R_uptime_checks/resolver_time_series.py

all changes to this resolver is just to support the transition from TimeSeriesRequest.aggregations to TimeSeriesRequest.expressions

kylemumma · 2025-02-08T00:15:26Z

snuba/web/rpc/v1/resolvers/R_eap_spans/resolver_time_series.py

@@ -154,7 +167,7 @@ def _convert_result_timeseries(
                extrapolation_context = ExtrapolationContext.from_row(
                    timeseries.label, row_data
                )
-                if extrapolation_context.is_data_present:
+                if row_data.get(timeseries.label, None) is not None:


@davidtsuk this line may interest you

why? What's the relevant or interesting change here?

We used to rely on logic inside ExtrapolationContext to determine if we leave something out of the results.

We now are just relying on whether the data itself has the value None.

This change can happen now that davids pr to support null values properly is merged

… aggregates field to expression field

…of formula count, look for bug in item table

volokluev · 2025-02-10T22:44:19Z

snuba/web/rpc/v1/resolvers/R_eap_spans/resolver_time_series.py

+def _get_reliability_context_columns(
+    expressions: Iterable[ProtoExpression],
+) -> list[SelectedExpression]:
+    # this reliability logic ignores formulas, meaning formulas may not properly support reliability


is this a final state of things or is this something that needs to be done later?

The final state until a product team complains about it. I should probably let them know about it.

kylemumma force-pushed the krm/formulasts branch from 95b2935 to 11f6569 Compare February 4, 2025 23:52

kylemumma force-pushed the krm/formulasts branch from a5490fb to 0d6eff8 Compare February 7, 2025 20:24

kylemumma changed the title ~~Krm/formulasts~~ feat: eap support formulas in timeseries endpoint Feb 7, 2025

kylemumma commented Feb 7, 2025

View reviewed changes

kylemumma marked this pull request as ready for review February 8, 2025 00:14

kylemumma requested review from a team as code owners February 8, 2025 00:14

kylemumma commented Feb 8, 2025

View reviewed changes

kylemumma added 10 commits February 10, 2025 11:16

init, need to add a test of a formula and a test of a transition from…

a074b05

… aggregates field to expression field

fix expression import

bfc5412

deprecate aggregations field in timeseries

7c8d1ea

deprecate aggregation field in timeseries

742f294

inital formula support using the count hack, todo: expected behavior …

1122349

…of formula count, look for bug in item table

remove count column for formulas

a6b319d

fix uptime check resolver, idk why it changed like that before

416060f

fix mypy

8a31070

count column ignored, formula working

2925d33

add a test w extrapolation

b45d1f4

kylemumma force-pushed the krm/formulasts branch from 87f630b to b45d1f4 Compare February 10, 2025 19:16

volokluev reviewed Feb 10, 2025

View reviewed changes

volokluev approved these changes Feb 11, 2025

View reviewed changes

kylemumma merged commit 6d8de6d into master Feb 11, 2025
32 checks passed

kylemumma deleted the krm/formulasts branch February 11, 2025 18:01

DominikB2014 mentioned this pull request Mar 14, 2025

feat(eap-spans): Support formulas with time series endpoint getsentry/sentry#87031

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: eap support formulas in timeseries endpoint #6854

feat: eap support formulas in timeseries endpoint #6854

kylemumma commented Feb 4, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

kylemumma Feb 7, 2025 •

edited

Loading

kylemumma Feb 7, 2025

kylemumma Feb 7, 2025

kylemumma Feb 8, 2025

volokluev Feb 10, 2025

kylemumma Feb 11, 2025

volokluev Feb 10, 2025

kylemumma Feb 10, 2025

feat: eap support formulas in timeseries endpoint #6854

feat: eap support formulas in timeseries endpoint #6854

Conversation

kylemumma commented Feb 4, 2025 • edited Loading

codecov bot commented Feb 5, 2025 • edited Loading

❌ 1 Tests Failed:

kylemumma Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

kylemumma Feb 7, 2025

Choose a reason for hiding this comment

kylemumma Feb 7, 2025

Choose a reason for hiding this comment

kylemumma Feb 8, 2025

Choose a reason for hiding this comment

volokluev Feb 10, 2025

Choose a reason for hiding this comment

kylemumma Feb 11, 2025

Choose a reason for hiding this comment

volokluev Feb 10, 2025

Choose a reason for hiding this comment

kylemumma Feb 10, 2025

Choose a reason for hiding this comment

kylemumma commented Feb 4, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

kylemumma Feb 7, 2025 •

edited

Loading