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

fix(metrics): Fetch project states during aggregator shutdown [INGEST-784 INGEST-997] #1205

Merged
merged 5 commits into from
Mar 7, 2022

Conversation

jjbayer
Copy link
Member

@jjbayer jjbayer commented Mar 7, 2022

The metrics aggregator currently force flushes all metrics during graceful shutdown, but only for projects which have up-to-date project configs (metrics for stale projects are dropped). This PR makes the aggregator attempt to flush everything multiple times, such that the project cache has time to update the project state.

@jjbayer jjbayer changed the title fix(metrics): Fetch project states during aggregator shutdown fix(metrics): Fetch project states during aggregator shutdown [INGEST-997] Mar 7, 2022
@jjbayer jjbayer changed the title fix(metrics): Fetch project states during aggregator shutdown [INGEST-997] fix(metrics): Fetch project states during aggregator shutdown [INGEST-784 INGEST-997] Mar 7, 2022
@jjbayer jjbayer marked this pull request as ready for review March 7, 2022 11:41
@jjbayer jjbayer requested a review from a team March 7, 2022 11:41
Copy link
Member

@untitaker untitaker left a comment

Choose a reason for hiding this comment

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

does this obsolete #1203 ?

relay_statsd::metric!(gauge(MetricGauges::Buckets) = self.buckets.len() as u64);

let mut buckets = HashMap::<ProjectKey, Vec<Bucket>>::new();

let force = matches!(&self.state, AggregatorState::ShuttingDown);

relay_statsd::metric!(timer(MetricTimers::BucketsScanDuration), {
let bucket_interval = self.config.bucket_interval;
self.buckets.retain(|key, entry| {
Copy link
Member Author

Choose a reason for hiding this comment

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

@jan-auer I did not refactor this into a force and non-force branch after all. Since we need to iterate through self.buckets in order to construct the return value, I did not see much benefit.

@jjbayer
Copy link
Member Author

jjbayer commented Mar 7, 2022

does this obsolete #1203 ?

Yes.

CHANGELOG.md Outdated Show resolved Hide resolved
More descriptive changelog.

Co-authored-by: Jan Michael Auer <mail@jauer.org>
@jjbayer jjbayer merged commit ee2ea71 into master Mar 7, 2022
@jjbayer jjbayer deleted the feat/shutdown-fetch-project-states branch March 7, 2022 13:17
jan-auer added a commit that referenced this pull request Mar 9, 2022
* master:
  ref(make): Simplify M1 exports in Makefile (#1206)
  fix(metrics): Wait for project states during aggregator shutdown (#1205)
  fix(test): Find librdkafka on Apple M1 (#1204)
  build: Bump sentry-relay in dev dependencies to 0.8.9 (#1202)
  ref(metrics): Tag backdated bucket creations in statsd (#1200)
  feat(metrics): Extract user satisfaction as tag (#1197)
  fix(statsd): Add new metric_type tag to existing metrics (#1199)
  fix: Apply clippy 1.59 suggestions (#1198)
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.

3 participants