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

CFP: Expose URL Path in HTTP metrics #26521

Open
zhilingc opened this issue Jun 28, 2023 · 10 comments
Open

CFP: Expose URL Path in HTTP metrics #26521

zhilingc opened this issue Jun 28, 2023 · 10 comments
Labels
kind/feature This introduces new functionality. sig/hubble Impacts hubble server or relay

Comments

@zhilingc
Copy link

Cilium Feature Proposal

Thanks for taking time to make a feature request for Cilium! If you have usage questions, please try the slack channel and see the FAQ first.

Is your feature request related to a problem?

Hubble L7 metrics currently does not export the http URL path, even though its available in Hubble flows; Not having access to this information is a huge bummer and a large part of why we want L7 visibility in the first place.

The only way to access this information outside of hubble currently is to export the file using --hubble-export-file-path and then parse that information, which isn't ideal...

Describe the feature you'd like

Expose the URL path as labels for the hubble http metrics http_requests_total, http_responses_total and http_request_duration_seconds.

(Optional) Describe your proposed solution

I'm happy to give this a shot if acceptable :)

@zhilingc zhilingc added the kind/feature This introduces new functionality. label Jun 28, 2023
@ldelossa ldelossa added the sig/hubble Impacts hubble server or relay label Jun 28, 2023
@Joffref
Copy link
Contributor

Joffref commented Jul 4, 2023

Take my contribution with caution, I'm new to Cilium

As far as I understand it L7 observability with Hubble monitor each HTTP request sent to your services, meaning that cardinality of such a thing might be infinite.

I'm letting you check out this comment: prometheus/client_golang#491 (comment)

@rolinh
Copy link
Member

rolinh commented Jul 6, 2023

Thanks for the CFP @zhilingc. @Joffref is absolutely right: the cardinality of the metrics could literally explose which means we unfortunately cannot expose URL path in HTTP metrics.

@rolinh rolinh closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 2023
@zhilingc
Copy link
Author

zhilingc commented Jul 6, 2023

@rolinh @Joffref That's regrettable, since it nerfs the out of the box metrics significantly for a large swath of applications, but i see where the decision is coming from. Thanks for the replies :)

@dbazhal
Copy link

dbazhal commented Oct 1, 2023

This feature is available in istio service mesh, it uses same envoy as cilium does, and it does not contain such metrics out of the box too. So one nice solution would be to let user decide turning these dimensions on.
Could we /reopen?

@Joffref
Copy link
Contributor

Joffref commented Oct 3, 2023

I understand such metric, in case of a low cardinality in url path, but I'm not sure about implementing this directly inside cilium. Is there any workaround we can think about to enable this dimension without modifying the code base ?

@michi-covalent
Copy link
Contributor

ok reopen and have some more discussion 🚀

@chancez
Copy link
Contributor

chancez commented Feb 22, 2024

I think if we can consider supporting this, but it would need to support a few things to keep cardinality reasonable:

  • Needs to be opt in.
  • The URLs should be "allow listed" with a regex to match, and then remove any parameters from the URL and replace them with placeholders.

Example:
URLs might be:

/user/chance/details
/user/michi/details

And the config/regex would need to support transforming the URL into /user/USERNAME/details within the metrics labels.

Copy link

This issue has been automatically marked as stale because it has not
had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 23, 2024
Copy link

github-actions bot commented May 7, 2024

This issue has not seen any activity since it was marked stale.
Closing.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 7, 2024
@dbazhal
Copy link

dbazhal commented May 7, 2024

/reopen

@xmulligan xmulligan reopened this May 7, 2024
@github-actions github-actions bot removed the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature This introduces new functionality. sig/hubble Impacts hubble server or relay
Projects
None yet
Development

No branches or pull requests

8 participants