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

Init histogram counters with 0 #4140

Merged
merged 4 commits into from
Oct 3, 2024

Conversation

mapno
Copy link
Member

@mapno mapno commented Sep 28, 2024

What this PR does:

Pushes a 0 to classic histogram's counter when the series is new to allow Prometheus to start from a non-null value.

Which issue(s) this PR fixes:
Fixes #

Checklist

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

// If we are about to call Append for the first time on a series,
// we need to first insert a 0 value to allow Prometheus to start from a non-null value.
if s.isNew() {
lb.Set(labels.MetricName, h.nameCount)
Copy link
Contributor

Choose a reason for hiding this comment

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

Question, maybe it makes sense to create a new ìnit` method in the interface:

type metric interface {
	name() string
        initMetrics(appender storage.Appender,...)
	collectMetrics(appender storage.Appender, timeMs int64, externalLabels map[string]string) (activeSeries int, err error)
	removeStaleSeries(staleTimeMs int64)
}

since I see we do the same logic in the counter

Copy link
Member Author

Choose a reason for hiding this comment

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

This logic is actually of the series, not the metric. It needs passing a couple of values, the function would actually encapsulate very little. I don't think it's worth the hassle.

func (co *counterSeries) init(appender storage.Appender, lbls labels.Labels, ts int64) {
	_, _ = appender.Append(0, lbls, ts, 0)
	co.registerSeenSeries()
}

@mapno mapno force-pushed the histogram-counter-intial-zero branch from 7f5d504 to 1b672e0 Compare October 1, 2024 09:15
@mapno mapno merged commit 4a2b78f into grafana:main Oct 3, 2024
16 checks passed
@mapno mapno deleted the histogram-counter-intial-zero branch October 3, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants