getsentry / relay

Sentry event forwarding and ingestion service.
https://docs.sentry.io/product/relay/
Other
319 stars 92 forks source link

Update (Billing) Consumers to emit accepted outcomes for indexed transactions. #3696

Open Dav1dde opened 3 months ago

Dav1dde commented 3 months ago

After #3662

This will make our counts overall more consistent.

Todo: investigate the same for Spans

jjbayer commented 3 months ago

These changes should be deployed together, but even if they are, there might be some wrong reporting because consumers do not restart at exactly the same time. I.e. maybe this new behavior should be controlled by a global option.

jjbayer commented 3 months ago

Let's discuss priority again in Monday's meeting. As we saw for profiles, after metrics extraction the "total" data category (e.g. Transaction) is currently double-represented after metrics extraction, so it is possible to report multiple outcomes for the same payload if it gets dropped after metrics extraction:

flowchart LR
    o -->|payload| metrics_extraction
    metrics_extraction -->|payload| more_processing
    more_processing -->|outcome: dropped| outcomes 
    metrics_extraction -->|metrics| metrics_consumer
    metrics_consumer -->|outcome: accepted| outcomes

We need to either implement this ticket to give the payload full ownership over the data category, or introduce a if metrics_extracted condition to the index_category() function to make sure the payload stops representing the "total" category after metrics extraction.

jjbayer commented 3 months ago

See also https://github.com/getsentry/relay/pull/3767.

jjbayer commented 2 months ago

Decision: implement ownership in consumer as the description states. Requires tagging of the usage metric with a sampled tag.

Idea: Pass the sampled state into metrics extraction by having a composite impl Getter that combines the event with the dynamic sampling decision.