Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(metrics): Remodel metric submission flow #3867

Merged
merged 3 commits into from
Aug 1, 2024
Merged

Conversation

Dav1dde
Copy link
Member

@Dav1dde Dav1dde commented Jul 25, 2024

Remove all the load of processing metrics from the project cache and move it to the processor.

We slowly moved more and more logic into the project cache, which over time became quite expensive to execute. Especially for heavily batched metrics. To alleviate this problem, the project cache only enriches messages with project information and dispatches based on this information instead of also dealing with filtering/validating metrics and rate limiting metrics.
Validation and rate limits are now enforced in the Processor.

The old flow for Metrics:

sequenceDiagram
    Endpoint->>Processor: ProcessMetrics
    Processor->>Project Cache: AddMetricBuckets
    activate Project Cache
    Note right of Project Cache: [check_buckets]
    Project Cache->>Aggregator: MergeBuckets
    deactivate Project Cache
    Aggregator->>Project Cache: FlushBuckets
    activate Project Cache
    Note right of Project Cache: [check_buckets]
    Project Cache->>Processor: EncodeMetrics
    deactivate Project Cache
    Processor->>Store: SendRequest
Loading

The new flow:

sequenceDiagram
    participant Endpoint
    participant Processor
    participant Project Cache
    participant Aggregator
    participant Store
    Endpoint->>Project Cache: ProcessMetrics
    Project Cache->>Processor: ProcessProjectMetrics
    activate Processor
    Note right of Processor: [check_buckets]
    Processor->>Aggregator: MergeBuckets
    deactivate Processor
    Aggregator->>Project Cache: FlushBuckets
    Project Cache->>Processor: EncodeMetrics
    activate Processor
    Note right of Processor: [check_buckets]
    Processor->>Store: SendRequest
    deactivate Processor
Loading

Batched metrics just have a slightly adjusted flow, they go from the endpoint to the processor for parsing, then move forward with the same ProcessMetrics flow.

#skip-changelog

@Dav1dde Dav1dde force-pushed the dav1d/flushbuckets-perf branch 4 times, most recently from 5313173 to 8c70b12 Compare July 26, 2024 08:37
@Dav1dde Dav1dde self-assigned this Jul 26, 2024
@Dav1dde Dav1dde marked this pull request as ready for review July 26, 2024 09:05
@Dav1dde Dav1dde requested a review from a team as a code owner July 26, 2024 09:05
Copy link
Member

@iambriccardo iambriccardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good job!

let mut buckets = match data {
MetricData::Parsed(buckets) => buckets,
MetricData::Raw(items) => {
let mut buckets = Vec::new();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we lift this into a function that parses them? Might be more readable.

@Dav1dde Dav1dde merged commit 890e1d1 into master Aug 1, 2024
23 of 24 checks passed
@Dav1dde Dav1dde deleted the dav1d/flushbuckets-perf branch August 1, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants