Skip to content

[baseserver] Expose gRPC prometheus metrics #9879

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

Merged
merged 1 commit into from
May 10, 2022

Conversation

easyCZ
Copy link
Member

@easyCZ easyCZ commented May 9, 2022

Description

Currently, prometheus metrics are not part of our default interceptors used across all gRPC servers. This is because every server handles is slightly differently. As part of ongoing WIP migration of existing gRPC servers onto base-server, it's necessary to be able to expose gRPC metrics with configurable prometheus registry.

This PR ensures that with, or without, a custom prometheus.Registry, gRPC metrics are exposed as long as the server serves gRPC traffic.

Related Issue(s)

Fixes #

How to test

Unit tests

or

  1. Start public-api-server on this branch with cd components/public-api-server && go run main.go
  2. Hit the server with a CLI cd dev/gpctl && go run main.go api workspaces get --id 123 --token foo --address localhost:9501 --insecure - the token here doesn't matter, we just need to hit it even with bad auth
  3. Grab the server exposed metrics with curl localhost:9500/metrics and observe at least the following metrics: "grpc_server_handled_total", "grpc_server_handling_seconds", "grpc_server_started_total"

Release Notes

NONE

Documentation

NONE

@easyCZ easyCZ requested a review from a team May 9, 2022 21:08
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label May 9, 2022
@easyCZ easyCZ force-pushed the mp/baseserver-grpc-metrics branch from d5d2ecb to 6178b2d Compare May 9, 2022 21:13
@easyCZ easyCZ force-pushed the mp/baseserver-grpc-metrics branch from 6178b2d to fc98703 Compare May 10, 2022 08:12
@laushinka laushinka self-assigned this May 10, 2022
@@ -77,3 +82,43 @@ func TestServer_ServesPprof(t *testing.T) {
require.NoError(t, err)
require.Equalf(t, http.StatusOK, resp.StatusCode, "must serve pprof on %s", pprof.Path)
}

func TestServer_Metrics_gRPC(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this test and the comments 🔥

@laushinka
Copy link
Contributor

laushinka commented May 10, 2022

Nice! Works as advertised 💪🏼

LGTM

@roboquat roboquat merged commit 262cba9 into main May 10, 2022
@roboquat roboquat deleted the mp/baseserver-grpc-metrics branch May 10, 2022 10:18
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/M team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants