feat(metric-outcomes): Add initial topic and schema #231

Dav1dde · 2024-03-08T08:50:03Z

Preliminary schema for the outcomes-metrics topic, work described and tracked in getsentry/relay#3147.

The cardinality description is vague on purpose, custom metrics will most likely have cardinality tracked per hour, but this is subject to change and may be different for other metric types (or even completely missing).

Dav1dde · 2024-03-08T08:50:28Z

topics/outcomes-metrics.yaml

@@ -0,0 +1,16 @@
+topic: outcomes-metrics
+pipeline: outcomes


Is this correct?

I think this is used primarily to identify a pipeline logically. @lynnagara and @untitaker can provide more context.
But it should be correct as we call outcomes everything that populates the outcomes and billing data,

I assume the metrics outcomes eventually makes it's way to the shared outcomes table in clickhouse with all the other outcomes? So this pipeline is probably fine

It will be parallel to the current outcomes with their own schema and from the looks of it in a separate clickhouse cluster.

If everything is fully separate, I think you can change this to outcomes-metrics or something like that

iker-barriocanal

LGTM, but I don't have context on how these schemas are used so you may want to get another approval.

jjbayer · 2024-03-08T11:09:43Z

schemas/outcomes-metrics.v1.schema.json

+        },
+        "outcome": {
+          "description": "Outcome ID, metric outcomes share the same numeric outcome ID with regular outcomes.",
+          "$ref": "#/definitions/U64"


I wonder if we should limit this to existing outcome IDs, or deliberately leave it open. I tend towards leaving it open.

I decided to follow the outcomes schema and leave it open.

schemas/outcomes-metrics.v1.schema.json

fpacifici · 2024-03-08T17:41:54Z

topics/outcomes-metrics.yaml

@@ -0,0 +1,16 @@
+topic: outcomes-metrics
+pipeline: outcomes


I think this is used primarily to identify a pipeline logically. @lynnagara and @untitaker can provide more context.
But it should be correct as we call outcomes everything that populates the outcomes and billing data,

fpacifici · 2024-03-08T17:42:46Z

topics/outcomes-metrics.yaml

+  producers:
+    - getsentry/relay
+  consumers:
+    - getsentry/snuba
+    - getsentry/super-big-consumers


Is this pipeline meant to bypass the current metrics outcomes pipeline that takes data in after the indexer ? If yes can we get rid of the old one

I am not familiar with any (other) metrics outcomes pipeline, so I assume the answer is yes.

This is the billing metrics consumer https://github.com/getsentry/sentry/blob/0730d277c3ad16c4c8aaebd7b2edd51f08819dea/src/sentry/ingest/billing_metrics_consumer.py#L41

I cannot find the design docs anymore.

fpacifici · 2024-03-08T17:45:40Z

schemas/outcomes-metrics.v1.schema.json

+        "cardinality": {
+          "description": "Maximum observed cardinality of the metric.",
+          "$ref": "#/definitions/U64"
+        }


I'd assume for "max cardinality" we mean the number of distinct combinations of tags keys:values observed in the bucket ? OR over a period of time ?
This would be useful information in the schema for who decides to consume from this topic

It is over time, current plan for custom metrics is over the span of an hour (because that's what people are interested and and what Relay will be enforcing).

I'll try to improve the description, but I was a bit vague on purpose because the time frame may change and may be different per usecase.

I'll try to improve the description, but I was a bit vague on purpose because the time frame may change and may be different per usecase.

I think this may highlight a concern. If the time frame changes, that likely concerns both the producer and the consumers which can easily end up in incidents where the billing code miscounts metrics for assuming a wrong time frame. max_cardinality over a day vs over a minute is likely going to mean different things for the billing code.

I would suggest an alternative approach to make this more robust: add a time_frame field in the message that indicates what the time frame is.
This puts the responsibility to manage the time frame on the consumer in an explicit way and it allows you to change the time frame in a safe and backwards compatible way as the consumer immediately knows how to interpret the message

Introduced cardinality_window which is required when a cardinality is emitted.

fpacifici · 2024-03-08T17:46:59Z

schemas/outcomes-metrics.v1.schema.json

+        "quantity": {
+          "description": "Amount of metric buckets accepted by Relay (volume).",
+          "$ref": "#/definitions/U64"
+        },


Same as below. What's the time range we are talking about here? Is it fixed or is it dependent on relay bucketing time ?

This is independent of time, this is the amount of statsd elements Relay receives and parses out of envelopes without any aggregation.

lynnagara · 2024-03-08T18:00:12Z

schemas/outcomes-metrics.v1.schema.json

@@ -0,0 +1,50 @@
+{


To what extent is the outcomes-metrics schema actually different from outcomes or is it mostly the same?

In Snuba, all of outcomes, outcomes-billing and loadbalancer-outcomes all share the exact same consumer code, and we just re-use the same schema in those scenarios. In the past we generally focused on having schemas reflect the minimum set of requirements for a consumer to work correctly. If the consumer is the same one, it might be simpler to use the existing outcomes schema for the new topic rather than maintain a separate one. On the other hand if the consumer code is different, I think a different schema here makes sense.

The biggest differences to the existing outcomes are:

Metric outcomes are tracked by MRI (consumer is expected to parse type and namespace from the MRI and materialize it in the database)

Metric outcomes have quantity and cardinality tracked, where latter is aggregated max by hour instead of sum

We considered trying to make this work with the current outcomes but came to the conclusion that the metric outcomes are too different and we'd rather have a separate concept just for metrics.

lynnagara

This seems good to me assuming that there will be a fully separate snuba consumer (and possibly clickhouse schema) and it's not shared with existing outcomes. If this ends up not being the case, I'd suggest later merging this together with outcomes so there would be one less schema to maintain.

Dav1dde · 2024-03-20T13:53:31Z

We've decided to use generic metrics for now, I'll be cleaning up the Kafka topic.

feat(metric-outcomes): Add initial topic and schema

7d49d5f

Dav1dde requested review from a team as code owners March 8, 2024 08:50

Dav1dde commented Mar 8, 2024

View reviewed changes

Dav1dde self-assigned this Mar 8, 2024

style(lint): Auto commit lint changes

66f8280

Dav1dde requested a review from a team March 8, 2024 08:50

Dav1dde mentioned this pull request Mar 8, 2024

[EPIC] Metric Stats getsentry/relay#3147

Closed

add missing code owners

3940fa8

iker-barriocanal approved these changes Mar 8, 2024

View reviewed changes

Dav1dde added 2 commits March 8, 2024 10:27

fix title

62a8094

type object

4f09363

jjbayer approved these changes Mar 8, 2024

View reviewed changes

use u64 type for quantity and cardinality

6e753a2

fpacifici reviewed Mar 8, 2024

View reviewed changes

lynnagara reviewed Mar 8, 2024

View reviewed changes

lynnagara approved these changes Mar 8, 2024

View reviewed changes

add cardinality_window

ca37824

Dav1dde closed this Mar 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(metric-outcomes): Add initial topic and schema #231

feat(metric-outcomes): Add initial topic and schema #231

Dav1dde commented Mar 8, 2024 •

edited

Loading

Dav1dde Mar 8, 2024

fpacifici Mar 8, 2024

lynnagara Mar 8, 2024

Dav1dde Mar 8, 2024

lynnagara Mar 8, 2024

iker-barriocanal left a comment

jjbayer Mar 8, 2024

Dav1dde Mar 8, 2024

fpacifici Mar 8, 2024

fpacifici Mar 8, 2024

Dav1dde Mar 8, 2024

fpacifici Mar 8, 2024

fpacifici Mar 8, 2024

Dav1dde Mar 8, 2024

fpacifici Mar 8, 2024

Dav1dde Mar 11, 2024

fpacifici Mar 8, 2024

Dav1dde Mar 8, 2024

lynnagara Mar 8, 2024

Dav1dde Mar 8, 2024

lynnagara left a comment •

edited

Loading

Dav1dde commented Mar 20, 2024

feat(metric-outcomes): Add initial topic and schema #231

feat(metric-outcomes): Add initial topic and schema #231

Conversation

Dav1dde commented Mar 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iker-barriocanal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lynnagara left a comment • edited Loading

Choose a reason for hiding this comment

Dav1dde commented Mar 20, 2024

Dav1dde commented Mar 8, 2024 •

edited

Loading

lynnagara left a comment •

edited

Loading