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

nri-prom histogram improvement: add _count counter by default #1890

Open
jackwilsdon opened this issue Jul 8, 2024 · 2 comments
Open

nri-prom histogram improvement: add _count counter by default #1890

jackwilsdon opened this issue Jul 8, 2024 · 2 comments
Labels
bug Categorizes issue or PR as related to a bug.

Comments

@jackwilsdon
Copy link

Description

The _count counter is not included in data pushed to New Relic for Prometheus histograms.

Expected Behavior

The _count counter should be included.

Steps to Reproduce

Scrape a Prometheus histogram with the infrastructure agent and nri-prometheus. Note that the _count counter is not exported to New Relic.

Additional context

The summary type already has support for _count:

metricName = metric.Name + "_count"
countMetric, ok := p.calculate.get(metricName, metric.Attributes, value.SampleCount, metric.Time())
if ok {
result = append(result, countMetric)
} else {
telemetryLogger.WithField("name", metricName).WithField("metric-type", metric.Type).Debug(noCalculationMadeErrMsg)
}

But it looks like the histogram type doesn't:

metricName := metric.Name + "_sum"
sumCount, ok := ph.calculate.get(metricName, metric.Attributes, float64(*value.SampleCount), metric.Time())
if ok {
result = append(result, telemetry.Summary{
Name: metricName,
Attributes: metric.Attributes,
Count: 1,
Sum: sumCount.Value,
Min: math.NaN(),
Max: math.NaN(),
Timestamp: metric.Time(),
Interval: sumCount.Interval,
})
} else {
telemetryLogger.WithField("name", metricName).WithField("metric-type", metric.Type).Debug(noCalculationMadeErrMsg)
}
metricName = metric.Name + "_bucket"
for _, b := range value.Buckets {
bucketAttrs := metric.CopyAttrs()
bucketAttrs["le"] = fmt.Sprintf("%g", *b.UpperBound)
bucketCount, ok := ph.calculate.get(
metricName,
bucketAttrs,
float64(*b.CumulativeCount),
metric.Time(),
)
if ok {
result = append(result, bucketCount)
} else {
telemetryLogger.WithField("name", metricName).WithField("metric-type", metric.Type).Debug(noCalculationMadeErrMsg)
}
}

For Maintainers Only or Hero Triaging this bug

Suggested Priority (P1,P2,P3,P4,P5):
Suggested T-Shirt size (S, M, L, XL, Unknown):

@jackwilsdon jackwilsdon added the bug Categorizes issue or PR as related to a bug. label Jul 8, 2024
@workato-integration
Copy link

@workato-integration workato-integration bot changed the title _count is missing from Prometheus histograms nri-prom historgram improvement: add _count counter by default Sep 25, 2024
@workato-integration workato-integration bot changed the title nri-prom historgram improvement: add _count counter by default nri-prom histogram improvement: add _count counter by default Sep 25, 2024
@josemore
Copy link
Contributor

this has been added to backlog as low-priority, see #1889 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

2 participants