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

Add grpc server and client metrics to Teleport #11534

Merged
merged 9 commits into from
Apr 4, 2022

Conversation

rcanderson23
Copy link
Contributor

@rcanderson23 rcanderson23 commented Mar 29, 2022

This is part of https://github.com/gravitational/cloud/issues/1468

This add grpc client metrics to the Proxy service and server metrics to the Auth service via a grpc interceptor. By default these metrics are lazy loaded and latency is disabled by default. The latency metrics are turned off by default as they are high cardinality. Users can enable them enabling the metrics_service using the following stanza:

metrics_service:
  enabled: yes
  listen_addr: 0.0.0.0:9090
  optional_metrics:
    grpc_server_latency: true
    grpc_client_latency: true

I went back and forth on having these in their respective service stanzas, it might be more clear if they configured in their respective service and will change if feedback agrees.

Documentation is missing from this PR which I will add once I have feedback on rest to avoid having to make the changes twice.

Mock up dashboard with these can be seen here (still WIP)

@github-actions github-actions bot requested review from jakule and rosstimothy March 29, 2022 14:25
@rcanderson23 rcanderson23 added observability Used for metrics and insight into Teleport. cloud Cloud labels Mar 31, 2022
Copy link
Contributor

@rosstimothy rosstimothy left a comment

Choose a reason for hiding this comment

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

LGTM. Could you add the new Metrics config items to TestConfigReading before merging?

@@ -63,6 +65,8 @@ type TLSServerConfig struct {
AcceptedUsage []string
// ID is an optional debugging ID
ID string
// Metrics are optional TLSServer metrics
Metrics *Metrics
Copy link
Contributor

Choose a reason for hiding this comment

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

If this wasn't a pointer you could rely on default values to do the right thing at the moment since Metrics only contains one boolean. If Metrics was to be extended in the future then relying on default values wouldn't work so maybe it's fine to leave as it is.

@rcanderson23 rcanderson23 enabled auto-merge (squash) April 4, 2022 16:21
@rcanderson23 rcanderson23 merged commit 1b758ce into master Apr 4, 2022
@rcanderson23 rcanderson23 deleted the carson/grpc-metrics branch April 4, 2022 16:55
rcanderson23 added a commit that referenced this pull request Apr 6, 2022
Adds grpc metrics on the auth and and proxy service with the option to enable grpc latency via the metrics service.
rcanderson23 added a commit that referenced this pull request Apr 7, 2022
…#11776)

* Adds grpc metrics on the auth and and proxy service with the option to enable grpc latency via the metrics service.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cloud Cloud observability Used for metrics and insight into Teleport.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants