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

feat(generic-metrics): Add gauges to docker compose file #2757

Closed
wants to merge 1 commit into from

Conversation

ayirr7
Copy link
Member

@ayirr7 ayirr7 commented Feb 1, 2024

This was missing by default, adding this consumer to the docker-compose file as well

@@ -283,6 +287,9 @@ services:
snuba-generic-metrics-counters-consumer:
<<: *snuba_defaults
command: 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:
Copy link
Member

Choose a reason for hiding this comment

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

is this a new consumer that was missed? 😅

Copy link
Collaborator

@aldy505 aldy505 left a comment

Choose a reason for hiding this comment

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

I know that this is missing and as far as I know, this is only used by the DDM backend? If that's the case, can we hold this until DDM is at least in beta or GA?

Adding a new container to the compose file is... an undesirable thing to do for self-hosted Sentry.

Copy link
Member

@hubertdeng123 hubertdeng123 left a comment

Choose a reason for hiding this comment

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

If this is only used for DDM, I agree with @aldy505 that we should hold off on this for now

@ale-cota
Copy link

ale-cota commented Feb 5, 2024

Since a lot of people are bound to run into this issue while trying to run metrics on self-hosted, would it be possible/acceptable to add it as a comment at least? So as to inform users that if they want to use DDM, they need to add the additional consumer to their docker-compose file.

@aldy505
Copy link
Collaborator

aldy505 commented Feb 5, 2024

Since a lot of people are bound to run into this issue while trying to run metrics on self-hosted, would it be possible/acceptable to add it as a comment at least? So as to inform users that if they want to use DDM, they need to add the additional consumer to their docker-compose file.

@ale-cota The self-hosted community never do that. For new features we always put an issue and point people to run/enable some things on their config in order to enable a new feature. For this specific issue, it's here: #2698

For other features, see #2646, #1993 (comment), and.. I forgot where is the issue for Performance Queries

@chadwhitacre
Copy link
Member

to inform users that if they want to use DDM, they need to add the additional consumer to their docker-compose file

I like using a GitHub issue for this. Let's start formalizing that as our process. I've added a Type: Pre-release Feature label to this repo and applied it to #2698. I think it'd be great to have a paragraph on our self-hosted docs about enabling pre-release features and links out to the issue label listing for current pre-release features. We have some reference under feature flags but nothing specific under self-hosted afaict.

@Ale Can you help us share #2698 with customers who are interested trying out Metrics in self-hosted?

@ayirr7 I'm gonna close this PR. 🙏

@chadwhitacre chadwhitacre deleted the add-gauges-docker-compose branch February 5, 2024 14:58
@github-actions github-actions bot locked and limited conversation to collaborators Feb 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants