Extend `headerLabels` Support to All Prometheus Metrics
What does this PR do?
Extend headerLabels support to all Prometheus metrics.
Motivation
Fixes https://github.com/traefik/traefik/issues/10774
More
- [x] Added/updated tests
- [x] Added/updated documentation
Additional Notes
Hi @mmatur,
It's been 5 months since this PR was submitted, but there hasn't been any progress. Since others are also facing the same issue, I believe this feature is necessary. Please let me know if there's anything I need to do to help move this forward.
Thanks!
Hi @leonlyu1996 ,
Thank you for following up on this PR and for your interest in this feature. I apologize for the delayed response.
After careful review, we've determined that this change would be a breaking change to Traefik's behavior. As such, we cannot include it in a minor version release, as it would violate semantic versioning principles and potentially disrupt existing implementations.
However, if you have ideas for implementing this feature in a non-breaking way, we would be happy to review and consider including it in a minor version release.
This feature, as currently proposed, will need to be considered for our next major version release to ensure we maintain backwards compatibility for our users.
Thank you for your understanding and patience.
Hi @mmatur,
Could you point out what particular will be broken by these changes?
*cos as I can see, these changes only add additional labels without any deletions
... I believe this feature is necessary ...
@leonlyu1996 it's definitely and extremely useful feature (thank you for your attention and contribution)
Maybe It will help to avoid the major version changes if these changes will focus only around Prometheus ? cos in the documentation, the headerLabels available only for the Prometheus, @mmatur @leonlyu1996 what do you think guys?
Hi,
I completely understand your point about only adding labels without removing anything. However, the breaking change here isn't about removing functionality, but rather about the significant impact on cardinality that this change introduces.
Currently, headerLabels are only applied to specific metrics, which keeps cardinality under control. The proposed change would allow headerLabels on all metrics, which could lead to an explosion in cardinality - potentially creating thousands of new time series depending on the header values. This dramatic increase in cardinality could cause performance issues and resource consumption problems for existing users who have their monitoring systems sized for the current behavior.
While I absolutely agree this feature would be valuable (and thank you again for the contribution), we need to be very careful about changes that could impact the resource usage patterns that our users have come to expect and have built their systems around.
If we can find a way to implement this that carefully controls cardinality (perhaps through explicit opt-in per metric or other controls), we could consider it for a minor version. Would you be interested in exploring that approach?
@mmatur, thank you so much for the detailed description of the situation; I now understand.
You are right - if some users already use headerLabels for the counter metrics, they might face an issue of Prometheus time series increasing around headerLabelsCount * bucketCount. We must also remember that every histogram metric has additional *_duration_seconds_count and *_duration_seconds_sum series. This is definitely a concern.
On a positive note, as you mentioned, the opt-in per metric (disabled by default) could be a viable solution. I would be glad to try to help with that.
@leonlyu1996 , please let me know if you need any assistance or if you don't have time to continue supporting your initiative. If that's the case, I will try to continue working on your existing solution
Hi @mgerasimchuk,
I'm glad that we can continue advancing this feature. I don't have time to continue working on this at the moment, please feel free to proceed with the work. Thank you very much.
Hey @leonlyu1996,
Have you been able to dig on the solution proposed by @mmatur?
Hi @leonlyu1996 @mgerasimchuk,
We are closing this pull request due to lack of activity. Feel free to reopen it at any time.
Hi @rtribotte,
Thank you for informing, and sorry that I didn't find a time to continue @leonlyu1996's work as I intended to
Hi @mgerasimchuk, no worries! Don't hesitate to supersede this PR if you find the time for it at some point!