Skip to content

blockbuilder: fix fetch metric types#6578

Merged
mdisibio merged 2 commits intografana:mainfrom
WinterCabbage:fix/blockbuilder-fetch-metric-type
Mar 19, 2026
Merged

blockbuilder: fix fetch metric types#6578
mdisibio merged 2 commits intografana:mainfrom
WinterCabbage:fix/blockbuilder-fetch-metric-type

Conversation

@WinterCabbage
Copy link
Copy Markdown
Contributor

@WinterCabbage WinterCabbage commented Feb 27, 2026 •
edited
Loading

What this PR does:

  • Follow-up to the Incorrect metric types for the deduplicated spans in compactor #6558 note: “Probably good to scan all other metrics and make sure we’re using the right types.”
  • Changes block-builder fetch metrics from gauges to counters while keeping the existing metric names:
    • tempo_block_builder_fetch_bytes_total
    • tempo_block_builder_fetch_records_total
  • Why: these metrics are monotonic totals, so counter is the correct Prometheus semantic for rate()/increase().
  • Updates metric definitions in modules/blockbuilder/blockbuilder.go.
  • Adds unit coverage in modules/blockbuilder/blockbuilder_test.go to verify fetch metric increments.
  • Updates affected tests and assertions.
  • Documents the change in CHANGELOG.md.

Which issue(s) this PR fixes:
Fixes #6558

Checklist

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

@mdisibio
Copy link
Copy Markdown
Contributor

mdisibio commented Mar 4, 2026

Hi, thanks for catching this. I'm good with fixing the metric type, but do we need to rename them? In our minds fetch already implies from the queue.

@WinterCabbage
Copy link
Copy Markdown
Contributor Author

WinterCabbage commented Mar 4, 2026

Hi, thanks for catching this. I'm good with fixing the metric type, but do we need to rename them? In our minds fetch already implies from the queue.

Hi, the original name is indeed better. I mainly changed it to follow the suggestion in issue #6558. Let me know how you'd like to proceed: revert to the original name if we're okay with the type change, or go with any other name you suggest.

@mdisibio
Copy link
Copy Markdown
Contributor

mdisibio commented Mar 11, 2026

@WinterCabbage Discussed with @zhxiaogg and we want to keep the original metric name. Just changing the type should be fine.

@WinterCabbage WinterCabbage force-pushed the fix/blockbuilder-fetch-metric-type branch from 580b49b to 6d3a2ba Compare March 12, 2026 11:42
@mdisibio
Copy link
Copy Markdown
Contributor

mdisibio commented Mar 16, 2026

@WinterCabbage Hi the PR lgtm after the last changes. Can you please resolve the merge conflict and update the PR description, and then we can merge.

@WinterCabbage
blockbuilder: fix fetch metric types
35e26d5 
Follow-up to grafana#6558 metric type audit.

  - switch block-builder fetch metrics from gauge to counter
  - rename metrics to tempo_block_builder_queue_fetch_bytes_total and
  tempo_block_builder_queue_fetch_records_total
  - update blockbuilder unit/integration tests
  - update mixin dashboard sources and compiled JSON
  - document breaking metric rename in upgrade notes and changelog

  Refs grafana#6558.
@WinterCabbage WinterCabbage force-pushed the fix/blockbuilder-fetch-metric-type branch from 6d3a2ba to eeaa6f4 Compare March 17, 2026 00:29
@WinterCabbage
Copy link
Copy Markdown
Contributor Author

WinterCabbage commented Mar 17, 2026

@WinterCabbage Hi the PR lgtm after the last changes. Can you please resolve the merge conflict and update the PR description, and then we can merge.

Hi thanks for the review. I have rebased the commit and updated the PR description.

mdisibio
mdisibio reviewed Mar 17, 2026
View reviewed changes
Comment thread CHANGELOG.md Outdated
* [BUGFIX] fix: skip per-label limiter and sanitizer for target_info and host_info metrics in metrics-generator [#6660](https://github.com/grafana/tempo/pull/6660) (@electron0zero)
* [BUGFIX] fix(traceql): err on division by zero [#6580](https://github.com/grafana/tempo/pull/6580) (@Proximyst)
* [BUGFIX] Return 400 instead of 500 when query_range or query_instant requests have unparseable start/end parameters [#6694](https://github.com/grafana/tempo/pull/6694) (@ruslan-mikhailov)
* [CHANGE] jsonnet: Add emptyDir data volume to block-builder StatefulSet [#6648](https://github.com/grafana/tempo/pull/6648) (@mapno)
Copy link
Copy Markdown
Contributor

@mdisibio mdisibio Mar 17, 2026

Choose a reason for hiding this comment

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

Suggested change
* [CHANGE] jsonnet: Add emptyDir data volume to block-builder StatefulSet [#6648](https://github.com/grafana/tempo/pull/6648) (@mapno)

This looks like a spurious copy from above, let's remove.

Copy link
Copy Markdown
Contributor Author

@WinterCabbage WinterCabbage Mar 18, 2026

Choose a reason for hiding this comment

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

Good catch, sorry about that. It was accidentally copied in during the rebase. I'll remove it now.

@WinterCabbage
blockbuilder: keep fetch metric names and fix type only
e0bfc74 
Also removed an accidental duplicate changelog entry introduced earlier in this branch.
@WinterCabbage WinterCabbage force-pushed the fix/blockbuilder-fetch-metric-type branch from eeaa6f4 to e0bfc74 Compare March 18, 2026 02:27
@mdisibio mdisibio merged commit 533c17c into grafana:main Mar 19, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdisibio mdisibio mdisibio approved these changes

@knylander-grafana knylander-grafana Awaiting requested review from knylander-grafana

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

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

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

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

@ie-pham ie-pham Awaiting requested review from ie-pham ie-pham is a code owner

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

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

@carles-grafana carles-grafana Awaiting requested review from carles-grafana carles-grafana is a code owner

@ruslan-mikhailov ruslan-mikhailov Awaiting requested review from ruslan-mikhailov ruslan-mikhailov is a code owner

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

@oleg-kozlyuk-grafana oleg-kozlyuk-grafana Awaiting requested review from oleg-kozlyuk-grafana oleg-kozlyuk-grafana 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.

Incorrect metric types for the deduplicated spans in compactor

2 participants

@WinterCabbage @mdisibio