feat: Provide label value by http header

[jkroepke]

Fixes #117

Closes #64

[kfox1111]

Can we get this merged? Would be very useful.

[jkroepke]

@squat could you take a look here?

[ThisIsQasim]

Tagging all owners since there have been related PRs (#54 #64 #117 ) with the same feature pending for quite a while.

@brancz @krasi-georgiev @metalmatze @paulfantom @pgier @s-urbaniak @simonpasquier @squat @lilic

[jkroepke]

@fpetkovski maybe?

[Atomique]

Would also be interested into this. Also into a new release because the actual 0.5.0 doesnt support the -values parameter (only the version in main does this).

[simonpasquier]

I'd expect that users pass the name of the header from the command-line (e.g. --header). And one of --query-param and --header needs to be not empty.

For backward-compatibility and principle of least surprise, the default value of --header should be "" IMHO. Thus when you upgrade to the latest version, it doesn't become automatically possible to pass the label value via HTTP header (existing setups might not filter HTTP headers so end-users could bypass prom-label-proxy's enforcement).

[simonpasquier]

If we go the route of adding a --query-param flag (which is fine by me because more explicit), I think that we should deprecate the --label flag.

[jkroepke]

How we can proceed here?

[simonpasquier]

@jkroepke sorry for the lag! I have some comments on the CLI help. I've submitted jkroepke#1 to your fork with some refactoring suggestions :)

[jkroepke]

Hey @simonpasquier, I saw you suggestions, but I recommend to create an additional PR which superseeds this PR or modify to existing one. "Allow edits by maintainers" is enabled. if you have maintainer permissions on https://github.com/prometheus-community/prom-label-proxy, feel free to commit here. I wont do any refactorings, since I don't have the full golang knowledge.

[simonpasquier]

@jkroepke can you reopen my PR on your fork and merge it then? I could have pushed my changes on top of your branch but I didn't want to do it without you being aware :)

[jkroepke]

Hey @simonpasquier please take a look that the merge conflict now which appears now after merging the PR

[simonpasquier]

@jkroepke sure, let me resolve this!

[simonpasquier]

@fpetkovski I'd like your input on this PR. In particular there's a slight difference compared to what you implemented in #116. With this change, users have to select the method to retrieve the label value from incoming requests:

• from the HTTP form/query parameters (-query-param or -label)
• from the HTTP headers (-header-name)
• statically defined (-label-value)

It isn't possible to pass -label-value and -label at the same time hence when a static label value is used, the proxy will not remove anything from the HTTP parameters. Did you have a concrete use case for cleaning up the HTTP parameters when using the static configuration?

[simonpasquier]

It isn't possible to pass -label-value and -label at the same time hence when a static label value is used, the proxy will not remove anything from the HTTP parameters. Did you have a concrete use case for cleaning up the HTTP parameters when using the static configuration?

After discussing offline with @fpetkovski the statement above isn't correct: in the current version, the proxy will return an HTTP error code when configured with -label-value and the incoming HTTP request has a label query parameter. With this PR, the proxy configured with -label-value won't remove any HTTP query/form parameter but it sounds ok as it will be ignored anyway.

[simonpasquier]

thanks a lot @jkroepke for your work and patience!