PROM-39: Provide PromQL info annotations when rate()/increase() over series without __type__="counter" label
#16632
Conversation
__type__="counter" label__type__="counter" label
There was a problem hiding this comment.
Couple of questions:
-
Should we add the warning in the case there's no type label? Currently that's what's happening. I'm worried that might be noisy. In a perfect world, all metrics will have a type label. But if only a subset of the traffic has the type label, these warnings could just get in the way for the non-type-label traffic. Or if they've recently started using type and unit metadata labels, their old series will be missing these labels and queries over longer periods of time could return warnings.
-
Is it worth splitting out the query type checks to a separate flag? I could imagine in a migration scenario, people first start manually adding type and unit metadata labels to a subset of of the metrics rather than enabling type and unit metadata labels to be added automatically upon ingestion, as the latter would affect all their metrics. So they might only want the query-time checks first. This relates to the first question - in a scenario where there's a phased migration it's likely a portion of the metrics will be missing the type label.
Yeah, I was not really sure what to do in the event of the absence of those labels. I feel we should warn in the case of absence. It's not the situation right now, but the proposal aims to add those labels when ingesting metrics from all possible ways metrics could be ingested:
So, by the time the feature flag is ready to be promoted to stable, a metric cannot be ingested without those labels. I agree that while development is ongoing, the experience won't be perfect, but maybe that's an okay trade-off (that we don't need to care about this in the future)?
I'd like to avoid another feature flag if possible. A PromQL warning is just a highlight in the UI; query results are still returned, and everything works as always. The more feature flags we introduce, the more work we'll have in the future to migrate folks who haven't enabled them before and it creates more cognitive load for users trying to understand which toggles to switch to have what they want 😬 |
…thout counter label Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
a306b1b to
56e09ac
Compare
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
56e09ac to
1ef361b
Compare
| !strings.HasSuffix(metricName, "_sum") || | ||
| !strings.HasSuffix(metricName, "_count") || | ||
| !strings.HasSuffix(metricName, "_bucket") { | ||
| // Fallback to name suffix checking |
There was a problem hiding this comment.
This comment should be above the check?
Part of #16610
This PR implements PromQL warning when performing rate() and increase() over series without the correct
__type__label.