traefik icon indicating copy to clipboard operation
traefik copied to clipboard

Extend `headerLabels` Support to All Prometheus Metrics

Open leonlyu1996 opened this issue 1 year ago • 6 comments

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

leonlyu1996 avatar Jun 05 '24 07:06 leonlyu1996

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!

leonlyu1996 avatar Nov 28 '24 09:11 leonlyu1996

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.

mmatur avatar Nov 28 '24 10:11 mmatur

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?

mgerasimchuk avatar Nov 28 '24 10:11 mgerasimchuk

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 avatar Nov 28 '24 14:11 mmatur

@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

mgerasimchuk avatar Nov 28 '24 17:11 mgerasimchuk

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.

leonlyu1996 avatar Nov 29 '24 02:11 leonlyu1996

Hey @leonlyu1996,

Have you been able to dig on the solution proposed by @mmatur?

nmengin avatar Mar 03 '25 13:03 nmengin

Hi @leonlyu1996 @mgerasimchuk,

We are closing this pull request due to lack of activity. Feel free to reopen it at any time.

rtribotte avatar Jun 04 '25 09:06 rtribotte

Hi @rtribotte,

Thank you for informing, and sorry that I didn't find a time to continue @leonlyu1996's work as I intended to

mgerasimchuk avatar Jun 04 '25 09:06 mgerasimchuk

Hi @mgerasimchuk, no worries! Don't hesitate to supersede this PR if you find the time for it at some point!

rtribotte avatar Jun 04 '25 15:06 rtribotte