-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
feat(generic-metrics): Add gauges to docker compose, re-try #3177
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #3177 +/- ##
=======================================
Coverage 99.01% 99.01%
=======================================
Files 3 3
Lines 203 203
=======================================
Hits 201 201
Misses 2 2 ☔ View full report in Codecov by Sentry. |
docker-compose.yml
Outdated
@@ -293,6 +293,9 @@ services: | |||
snuba-generic-metrics-counters-consumer: | |||
<<: *snuba_defaults | |||
command: rust-consumer --storage generic_metrics_counters_raw --consumer-group snuba-gen-metrics-counters-consumers --auto-offset-reset=latest --max-batch-time-ms 750 --no-strict-offset-reset | |||
snuba-generic-metrics-gauges-consumer: | |||
<<: *snuba_defaults | |||
command: consumer --storage generic_metrics_gauges_raw --consumer-group snuba-gen-metrics-gauges-consumers --auto-offset-reset=latest --max-batch-time-ms 750 --no-strict-offset-reset |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that you don't want to use rust-consumer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, let me fix that.
Wouldn't this also require us to flip the feature flags for custom metrics? Otherwise, what is this consumer used for? |
Could you point me to the feature flags you're referring to? But yes, you are correct that today the gauges consumer is only used by the custom metrics functionality. I reopened this PR as I heard there was interest/questions around using this. |
I think it's fine since AFAIK Metrics isn't GA yet. We can add the flags in a separate PR. |
@aldy505 I'd like to avoid adding bloat to the docker compose file if it pertains to a feature that is not yet used.
@ayirr7 Is this the case here? |
Ping @ayirr7. We are 5 more days until the next release. |
Is there any news on when this might be merged? I understand this has now missed the window for .7 |
I believe this should be the case here. The consumer will ingest any relevant data. Once any feature(s) are enabled, they can be used to query the data. I'm not sure what the timelines are for these features being available/being used. I have asked in relevant spots but have not heard back. cc @hubertdeng123 , should I go ahead with merging this? |
Thanks Riya! @ayirr7 |
Adding this missing consumer, this is a repeat of #2757