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

Allow to disable collection of general metrics (of the whole Sidekiq setup) #20

Merged

Conversation

mrexox
Copy link
Contributor

@mrexox mrexox commented May 7, 2021

If you run Kubernetes and have several pods running sidekiq, these general metrics can be overkill. So it is better to choose one pod that will publish these metrics instead of all pods publishing the same values.

To make it simple, I offer the solution with ENV variable checking. But I am open to discussion about how to make it more accurate.

UPD: Also added the setting to force collecting metrics even if an application is not a Sidekiq runner.

@mrexox mrexox force-pushed the feature/add-env-var-to-control-general-metrics branch 2 times, most recently from a2eb2c7 to 3cba342 Compare May 7, 2021 13:11
Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>
@mrexox mrexox force-pushed the feature/add-env-var-to-control-general-metrics branch from 3cba342 to 25c1ce0 Compare May 7, 2021 13:11
@Envek Envek changed the title Add YABEDA_SIDEKIQ_GENERAL_METRICS_DISABLE to control general metrics Allow to disable collection of general metrics (of the whole Sidekiq setup) May 9, 2021
@Envek
Copy link
Member

Envek commented May 9, 2021

Hey! Thanks for the pull request! I like the idea as it can be really useful on large Sidekiq installation.

I just pushed few changes: using anyway_config for configuration (it allows to use not only environment variables, but many other sources) and renaming of setting (however, I'm still not sure about correct naming).

Let's discuss!

@mrexox
Copy link
Contributor Author

mrexox commented May 11, 2021

I like the name general or common, but general sounds more like typical. We mean here the metrics that are not specific to any sidekiq runner. What about aggregate or overall name for these metrics?

mrexox and others added 3 commits May 11, 2021 16:05
Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>
Signed-off-by: Valentin Kiselev <mrexox@evilmartians.com>
@Envek
Copy link
Member

Envek commented May 11, 2021

After further thinking I thought that it is enough to use single setting as force switch for general/global/aggregate metrics (it is by default on in Sidekiq processes and off in non-Sidekiq, thanks for the idea!).

@Envek Envek merged commit fdb1d34 into yabeda-rb:master May 12, 2021
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