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.

Sign up for GitHub

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

Jump to bottom

metrics-generator: support per-tenant processor configuration #1434

Merged
merged 8 commits into from
May 18, 2022
Merged

metrics-generator: support per-tenant processor configuration #1434

merged 8 commits into from
May 18, 2022

Conversation

kvrhdn
Copy link
Member

@kvrhdn kvrhdn commented May 18, 2022

What this PR does:
Allow per-tenant configuration of the processor histogram buckets and dimensions.

The following overrides are added:

metrics_generator_processor_service_graphs_histogram_buckets
metrics_generator_processor_service_graphs_dimensions
metrics_generator_processor_span_metrics_histogram_buckets
metrics_generator_processor_span_metrics_dimensions

Which issue(s) this PR fixes:
Related to #1303

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

joe-elliott
joe-elliott reviewed May 18, 2022
View reviewed changes
Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

overall looks good. some thoughts in the comments.

per offline discussion there's one more update to this to prevent multiple series from being registered? after that's pushed i will do a final review.

modules/generator/instance.go Show resolved Hide resolved
modules/generator/instance.go Outdated Show resolved Hide resolved
modules/generator/instance.go Show resolved Hide resolved
@kvrhdn kvrhdn marked this pull request as ready for review May 18, 2022 14:47
@kvrhdn kvrhdn requested a review from joe-elliott May 18, 2022 14:47
joe-elliott
joe-elliott approved these changes May 18, 2022
View reviewed changes
Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

a nit and a question. looks good!

modules/generator/registry/histogram.go Outdated Show resolved Hide resolved
modules/generator/registry/registry.go Outdated Show resolved Hide resolved
knylander-grafana
knylander-grafana approved these changes May 18, 2022
View reviewed changes
Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Minor suggestion to change future tense (will ...) to present tense.

docs/tempo/website/configuration/_index.md Outdated Show resolved Hide resolved
kvrhdn and others added 2 commits May 18, 2022 19:07
@kvrhdn
code review suggestions
f092bcc
@kvrhdn @knylander-grafana
Update docs/tempo/website/configuration/_index.md
f408ebc 
Co-authored-by: Kim Nylander <104772500+knylander-grafana@users.noreply.github.com>
@kvrhdn kvrhdn enabled auto-merge (squash) May 18, 2022 17:08
@kvrhdn kvrhdn merged commit 75249be into grafana:main May 18, 2022
@kvrhdn kvrhdn deleted the kvrhdn/metrics-generator-per-tenant-config branch May 18, 2022 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joe-elliott joe-elliott joe-elliott approved these changes

@knylander-grafana knylander-grafana knylander-grafana approved these changes

@KMiller-Grafana KMiller-Grafana Awaiting requested review from KMiller-Grafana

@annanay25 annanay25 Awaiting requested review from annanay25 annanay25 is a code owner

@mdisibio mdisibio Awaiting requested review from mdisibio mdisibio is a code owner

@dgzlopes dgzlopes Awaiting requested review from dgzlopes

@mapno mapno Awaiting requested review from mapno mapno is a code owner

@zalegrala zalegrala Awaiting requested review from zalegrala zalegrala is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kvrhdn @joe-elliott @knylander-grafana