fix metrics type metadata when some quantiles are cleaned due to summary window by jiekun · Pull Request #121 · VictoriaMetrics/metrics

jiekun · 2026-03-30T10:20:22Z

For summary data, it contains *Summary and multiple *quantileValue. When printing, they'll be sorted by their metricType:

*Summary has unsupported.
*quantileValue has summary.

So *quantileValue are always printed first.

However, the *quantileValue could be removed if stale for 5 minutes, in this case, the metricType of *Summary will be printed (as unsupported), while the expectation would be summary.

This pull request specific the metric type for both to summary and sort them by isAux.

Summary by cubic

Fixes TYPE metadata for summary metrics when quantiles expire. Always emits TYPE summary and sorts quantiles before _sum/_count using an auxiliary flag, not metric-type sorting.

Bug Fixes
- Set Summary.metricType() to return "summary" so metadata stays correct even if only _sum/_count remain.
- Use isAux to sort quantiles before _sum/_count within the same family; remove metric-type-based sorting.
- Add a unit test for a short-window SummaryExt to verify TYPE summary after quantiles expire; add an isAux comment to clarify the sorting behavior.

^{Written for commit ef2c19a. Summary will update on new commits.}

codecov · 2026-03-30T10:22:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.75%. Comparing base (368479b) to head (ef2c19a).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #121      +/-   ##
==========================================
+ Coverage   88.34%   89.75%   +1.40%     
==========================================
  Files          12       12              
  Lines        1399     1572     +173     
==========================================
+ Hits         1236     1411     +175     
- Misses        112      115       +3     
+ Partials       51       46       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

cubic-dev-ai

No issues found across 4 files

Copilot

Pull request overview

Fixes incorrect # TYPE ... unsupported metadata for summary metric families when quantile samples are no longer emitted (e.g., after the summary window rotates), ensuring the family always exposes summary while preserving Prometheus-like ordering (quantiles before _sum/_count).

Changes:

Set Summary.metricType() to return summary (matching quantileValue) so metadata remains correct even when quantiles aren’t printed.
Replace metricType-based ordering with a summaryMetric/sortPriority() mechanism to keep quantiles ordered before the summary’s _sum/_count.
Extend ExampleExposeMetadata to cover the “quantiles expire” scenario and assert that TYPE remains summary.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
summary.go	Unifies summary metric type and adds summary-specific sorting helper.
set.go	Updates stable sorting to handle summary vs quantiles without relying on metricType differences.
metrics.go	Introduces `summaryMetric` interface for summary-specific ordering.
set_example_test.go	Adds example coverage for summary metadata correctness after quantiles stop emitting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hagen1778

Thanks

hagen1778 · 2026-04-02T08:21:23Z

 }

+// sortableMetric is a special metric that may require sorting (backed summary and quantileValue).
+type sortableMetric interface {


Thanks for the quick fix. But I think it kinda gets overcomplicated.
I think we did not make the best decision by relying on logic based on unsupported type here https://github.com/VictoriaMetrics/metrics/pull/105/changes#diff-0543e8750caa826e80d331a4c6c9234e227ebaaacd232db41e060b0b9380a7eeR129. It is brittle and not reliable. And the linked issue to this PR only demonstrates it.

I suggest we try to make it simpler.
The {quantile=<>} metrics in this library are marked as auxilary (see isAux attr). These are only metrics that are marked like this, it seems. We need to guarantee the following sorting logic:

Sort by metric family, so all my_metric_whatever metrics go together

Sort by metric name, so all my_summary{quantile="0.5"} and my_summary{quantile="0.75"} go one after another, respecting quantile="" sorting

Special sorting for summaries that consists of auxilary quantile metrics and main _sum and _count metrics.

p1 and p2 are easy to achieve. And the struggle was only with p3, so we added sorting by type relying on sorting order between summary and unsupported, which I don't like.

Let's try to solve p3 by simply saying that auxilary metrics go first:

lessFunc := func(i, j int) bool { a, b := s.a[i], s.a[j] fName1, fName2 := getMetricFamily(a.name), getMetricFamily(b.name) if fName1 != fName2 { return fName1 < fName2 } if a.isAux != b.isAux { return a.isAux } return a.name < b.name }

That should work for all described cases. @jiekun could you please check if my suggestion is viable?
If yes, we can get rid of all the rest complicated sorting logic.

Thank you. I had trouble understanding isAux by the git history, which was the motivation to use a seperate attribute for sorting, and it's clear now. Let me see if I can wrote a better one with this.

I preserved the unit test, and it's all green for the suggested sorting logic. Thank you.

…les are expired

hagen1778 · 2026-04-02T08:51:41Z

-		if mType1 != mType2 {
-			return mType1 < mType2
+		// if both has same family, check the auxiliary first.
+		// only summary quantile(s) is registered as auxiliary, and quantile(s) in summary should go first.


It would be nice to add comment to the isAux field in metric struct to note where and why it is used. So in future we can quickly figure out what it is for.

hagen1778 · 2026-04-02T08:55:07Z

Once merged, this fix should be delivered to vm, vl and vt projects. It also should be backported to vm lts branches.

f41gh7

LGTM

hagen1778 · 2026-04-03T07:06:49Z

Thanks!

hagen1778 · 2026-04-03T07:24:34Z

This change was included into https://github.com/VictoriaMetrics/metrics/releases/tag/v1.43.1
@jiekun pls take care of making corresponding update PRs to depending repos.

…ummary (#10745) Fix `unsupported` metric type display in exposed metric metadata for summaries and quantiles by bumping `metrics` SDK version. This `unsupported` type exists when a summary is not updated within a certain time window. See VictoriaMetrics/metrics#120 and pull request VictoriaMetrics/metrics#121 for details. Signed-off-by: Zhu Jiekun <jiekun@victoriametrics.com> Signed-off-by: Max Kotliar <kotlyar.maksim@gmail.com> Co-authored-by: Max Kotliar <mkotlyar@victoriametrics.com>

…ary window (VictoriaMetrics#121) Summary metric type, when exposed in Prometheus format, consists of quantiles, _sum and _count metrics. The order of these metrics should be respected as listed above. Before, this order was implemented by comparing `metricType`, which returned `summary` for quantiles and `unsupported` for _sum and _count. The sorting logic was based on the fact that quantiles are always present and they will be always printed first, so their `metricType` will be printed first too. This logic is complicated and unreliable, as shown in VictoriaMetrics#120. This commit removes logic based on `metricType`. The `{quantile=<>}` metrics are currently marked as auxilary (see `isAux` attr). These are only metrics that are marked like this. We need to guarantee the following sorting logic: 1. Sort by metric family, so all my_metric_whatever metrics go together 2. Sort by metric name, so all my_summary{quantile="0.5"} and my_summary{quantile="0.75"} go one after another, respecting quantile="" sorting 3. Special sorting for summaries that consists of auxilary quantile metrics and main _sum and _count metrics. The new sorting logic now relies on `isAux` field for printing quantiles before _sum and _count metrics for summaries.

makasim · 2026-04-17T17:19:21Z

The fix is available in VictoriaMetrics since v1.140.0, v1.136.4, and v1.122.19.

jiekun requested review from Copilot, hagen1778 and makasim March 30, 2026 10:20

Copilot started reviewing on behalf of jiekun March 30, 2026 10:22 View session

cubic-dev-ai Bot reviewed Mar 30, 2026

View reviewed changes

Copilot AI reviewed Mar 30, 2026

View reviewed changes

Comment thread set_example_test.go Outdated

Comment thread set_example_test.go Outdated

Comment thread summary.go Outdated

Comment thread summary.go Outdated

Comment thread summary.go Outdated

Comment thread metrics.go Outdated

jiekun requested a review from Copilot March 30, 2026 11:22

Copilot started reviewing on behalf of jiekun March 30, 2026 11:24 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

Comment thread set.go Outdated

hagen1778 reviewed Apr 2, 2026

View reviewed changes

feature: fix metrics sorting and type when some of the summary quanti…

1d02475

…les are expired

jiekun force-pushed the fix/metrics-metadata-after-cleanup branch from 6d9232e to 1d02475 Compare April 2, 2026 08:47

jiekun requested a review from hagen1778 April 2, 2026 08:49

hagen1778 approved these changes Apr 2, 2026

View reviewed changes

doc: add extra comment for isAux field

a8ace47

f41gh7 approved these changes Apr 2, 2026

View reviewed changes

f41gh7 reviewed Apr 2, 2026

View reviewed changes

Comment thread set_example_test.go Outdated

test: move unit test to test file instead of output example

ef2c19a

hagen1778 merged commit 5dbd3cd into master Apr 3, 2026
30 checks passed

hagen1778 deleted the fix/metrics-metadata-after-cleanup branch April 3, 2026 07:16

BrewTestBot mentioned this pull request Apr 13, 2026

victoriametrics 1.140.0 Homebrew/homebrew-core#277326

Merged

Conversation

jiekun commented Mar 30, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

codecov Bot commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

hagen1778 left a comment

Choose a reason for hiding this comment

Uh oh!

hagen1778 Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiekun Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiekun Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

hagen1778 Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hagen1778 commented Apr 2, 2026

Uh oh!

f41gh7 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hagen1778 commented Apr 3, 2026

Uh oh!

Uh oh!

hagen1778 commented Apr 3, 2026

Uh oh!

makasim commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jiekun commented Mar 30, 2026 •

edited by cubic-dev-ai Bot

Loading

codecov Bot commented Mar 30, 2026 •

edited

Loading

hagen1778 Apr 2, 2026 •

edited

Loading

jiekun Apr 2, 2026 •

edited

Loading