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

Optionally disable m3msg consumer metric scope #3802

Merged
merged 3 commits into from
Oct 1, 2021

Conversation

ryanhall07
Copy link
Collaborator

For large m3msg deployments the number of consumers can add a lot of
cardinality to the metrics. Now users can disable the "consumer" label
via the config option.

What this PR does / why we need it:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:


Does this PR require updating code package or user-facing documentation?:


For large m3msg deployments the number of consumers can add a lot of
cardinality to the metrics. Now users can disable the "consumer" label
via the config option.
Copy link
Collaborator

@robskillington robskillington left a comment

Choose a reason for hiding this comment

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

LGTM

@ryanhall07 ryanhall07 enabled auto-merge (squash) October 1, 2021 19:55
@@ -107,6 +107,9 @@ type WriterConfiguration struct {
// IgnoreCutoffCutover allows producing writes ignoring cutoff/cutover timestamp.
// Must be in sync with AggregatorConfiguration.WritesIgnoreCutoffCutover.
IgnoreCutoffCutover bool `yaml:"ignoreCutoffCutover"`
// WithoutConsumerScope drops the consumer tag from the metrics. For large m3msg deployments the consumer tag can
// add a lot of cardinality to the metrics.
WithoutConsumerScope bool `ymal:"withoutConsumerScope"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo: ymal

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lol how did the linter not catch this (at least locally)

@ryanhall07 ryanhall07 merged commit ddeddc4 into master Oct 1, 2021
@ryanhall07 ryanhall07 deleted the rhall-disable-consumer-label branch October 1, 2021 20:19
@codecov
Copy link

codecov bot commented Oct 1, 2021

Codecov Report

Merging #3802 (0fa08d8) into master (0225ecb) will decrease coverage by 0.2%.
The diff coverage is 88.8%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3802     +/-   ##
========================================
- Coverage    57.0%   56.8%   -0.3%     
========================================
  Files         552     552             
  Lines       63363   63077    -286     
========================================
- Hits        36180   35880    -300     
- Misses      23983   23998     +15     
+ Partials     3200    3199      -1     
Flag Coverage Δ
aggregator 63.4% <ø> (+<0.1%) ⬆️
cluster ∅ <ø> (∅)
collector 58.4% <ø> (ø)
dbnode 60.4% <ø> (-0.1%) ⬇️
m3em 46.4% <ø> (ø)
metrics 19.7% <ø> (ø)
msg 74.4% <88.8%> (-2.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0225ecb...0fa08d8. Read the comment docs.

Antanukas pushed a commit that referenced this pull request Oct 5, 2021
For large m3msg deployments the number of consumers can add a lot of
cardinality to the metrics. Now users can disable the "consumer" label
via the config option.
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