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

frontend: lint Prometheus metrics #1076

Merged
merged 1 commit into from
Jan 20, 2025
Merged

frontend: lint Prometheus metrics #1076

merged 1 commit into from
Jan 20, 2025

Conversation

simonpasquier
Copy link
Collaborator

What this PR does

This change renames the frontend_count metric to frontend_requests_total to comply with the Prometheus guidelines.

It also bounds the cardinality of the route label by using the matched pattern instead of the actual path.

Jira:
Link to demo recording:

Before this PR

# TYPE frontend_count counter
frontend_count{api_version="",code="200",route="/healthz",state="Unknown",verb="GET"} 2
frontend_count{api_version="",code="404",route="/notfound",state="Unknown",verb="GET"} 1
frontend_count{api_version="",code="404",route="/notfound2",state="Unknown",verb="GET"} 1
frontend_count{api_version="",code="404",route="/notfound3",state="Unknown",verb="GET"} 1
frontend_count{api_version="2.0",code="201",route="/subscriptions/1d3378d3-5a3f-4712-85a1-2485495dfc4b",state="Registered",verb="PUT"} 1
frontend_count{api_version="2024-06-10-preview",code="200",route="/subscriptions/1d3378d3-5a3f-4712-85a1-2485495dfc4b/resourcegroups/simon-net-rg/providers/microsoft.redhatopenshift/hcpopenshiftclusters",state="Registered",verb="GET"} 2
frontend_count{api_version="2024-06-10-preview",code="404",route="/subscriptions/1d3378d3-5a3f-4712-85a1-2485495dfc4b/resourcegroups/simon-net-rg/providers/microsoft.redhatopenshift/hcpopenshiftclusters/simon",state="Registered",verb="GET"} 2
# HELP frontend_duration 
# TYPE frontend_duration gauge
frontend_duration{api_version="",code="200",route="/healthz",verb="GET"} 0
frontend_duration{api_version="",code="404",route="/notfound",verb="GET"} 0
frontend_duration{api_version="",code="404",route="/notfound2",verb="GET"} 0
frontend_duration{api_version="",code="404",route="/notfound3",verb="GET"} 0
frontend_duration{api_version="2.0",code="201",route="/subscriptions/1d3378d3-5a3f-4712-85a1-2485495dfc4b",verb="PUT"} 1
frontend_duration{api_version="2024-06-10-preview",code="200",route="/subscriptions/1d3378d3-5a3f-4712-85a1-2485495dfc4b/resourcegroups/simon-net-rg/providers/microsoft.redhatopenshift/hcpopenshiftclusters",verb="GET"} 431
frontend_duration{api_version="2024-06-10-preview",code="404",route="/subscriptions/1d3378d3-5a3f-4712-85a1-2485495dfc4b/resourcegroups/simon-net-rg/providers/microsoft.redhatopenshift/hcpopenshiftclusters/simon",verb="GET"} 0
# HELP frontend_health 
# TYPE frontend_health gauge
frontend_health{endpoint="/healthz"} 1

After this PR (note how all the 404 paths are folded into the / route):

# HELP frontend_duration 
# TYPE frontend_duration gauge
frontend_duration{api_version="",code="200",route="/healthz",verb="GET"} 5.8412e-05
frontend_duration{api_version="",code="404",route="/",verb="GET"} 2.7196e-05
frontend_duration{api_version="2.0",code="201",route="/subscriptions/{subscriptionid}",verb="PUT"} 0.000267956
frontend_duration{api_version="2024-06-10-preview",code="200",route="/subscriptions/{subscriptionid}/resourcegroups/{resourcegroupname}/providers/microsoft.redhatopenshift/hcpopenshiftclusters",verb="GET"} 0.829475008
frontend_duration{api_version="2024-06-10-preview",code="400",route="/subscriptions/{subscriptionid}/resourcegroups/{resourcegroupname}/providers/microsoft.redhatopenshift/hcpopenshiftclusters/{resourcename}",verb="GET"} 0.000129962
frontend_duration{api_version="2024-06-10-preview",code="404",route="/subscriptions/{subscriptionid}/resourcegroups/{resourcegroupname}/providers/microsoft.redhatopenshift/hcpopenshiftclusters/{resourcename}",verb="GET"} 0.000103378
# HELP frontend_health 
# TYPE frontend_health gauge
frontend_health{endpoint="/healthz"} 1
# HELP frontend_requests_total 
# TYPE frontend_requests_total counter
frontend_requests_total{api_version="",code="200",route="/healthz",state="Unknown",verb="GET"} 1
frontend_requests_total{api_version="",code="404",route="/",state="Unknown",verb="GET"} 1
frontend_requests_total{api_version="2.0",code="201",route="/subscriptions/{subscriptionid}",state="Registered",verb="PUT"} 1
frontend_requests_total{api_version="2024-06-10-preview",code="200",route="/subscriptions/{subscriptionid}/resourcegroups/{resourcegroupname}/providers/microsoft.redhatopenshift/hcpopenshiftclusters",state="Registered",verb="GET"} 1
frontend_requests_total{api_version="2024-06-10-preview",code="400",route="/subscriptions/{subscriptionid}/resourcegroups/{resourcegroupname}/providers/microsoft.redhatopenshift/hcpopenshiftclusters/{resourcename}",state="Unknown",verb="GET"} 1
frontend_requests_total{api_version="2024-06-10-preview",code="404",route="/subscriptions/{subscriptionid}/resourcegroups/{resourcegroupname}/providers/microsoft.redhatopenshift/hcpopenshiftclusters/{resourcename}",state="Registered",verb="GET"} 2
...

Special notes for your reviewer

Copy link

Please rebase pull request.

@simonpasquier
Copy link
Collaborator Author

@SudoBrendan @mbarnes could you please review?

Copy link

Please rebase pull request.

@mbarnes
Copy link
Collaborator

mbarnes commented Jan 17, 2025

Sorry for the delay on this, I haven't been watching incoming PRs closely.
This is on my radar now. If you can resolve the current conflicts I'll give it a review ASAP.

This change renames the `frontend_count` metric to
`frontend_requests_total` to comply with the Prometheus guidelines.

It also bounds the cardinality of the `route` label by using the
matched pattern instead of the actual path.

Signed-off-by: Simon Pasquier <[email protected]>
Copy link
Collaborator

@mbarnes mbarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fantastic use of http.Request.Pattern, which I didn't even know existed! Changes look great, thank you.

@mbarnes mbarnes merged commit 9fd787e into main Jan 20, 2025
20 checks passed
@mbarnes mbarnes deleted the lint-frontend-metrics branch January 20, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants