fix: fix exporting grpc metrics#13376
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a CUSTOM_METRIC_PREFIX constant in MetricsImpl and updates the metric filtering logic. Specifically, the built-in OpenTelemetry exporter filter is modified to only allow metrics registered in metricRegistry that do not start with the custom metric prefix. Additionally, CustomAttemptLatency is updated to use the new prefix constant. Feedback on these changes highlights that the new filtering logic will inadvertently filter out gRPC metrics (which start with grpc.) because they are not registered in metricRegistry, and suggests updating the filter to explicitly allow them.
| input -> { | ||
| String name = input.getName(); | ||
| return metricRegistry.getMetric(name) != null | ||
| && !name.startsWith(CUSTOM_METRIC_PREFIX); | ||
| }), |
There was a problem hiding this comment.
The current filter only allows metrics that are registered in metricRegistry. Since gRPC metrics (which typically start with grpc.) are registered directly by the gRPC library and are not present in metricRegistry, they will be filtered out and not exported. To fix this, explicitly allow metrics starting with grpc. in the filter.
| input -> { | |
| String name = input.getName(); | |
| return metricRegistry.getMetric(name) != null | |
| && !name.startsWith(CUSTOM_METRIC_PREFIX); | |
| }), | |
| input -> { | |
| String name = input.getName(); | |
| return (metricRegistry.getMetric(name) != null || name.startsWith("grpc.")) | |
| && !name.startsWith(CUSTOM_METRIC_PREFIX); | |
| }), |
There was a problem hiding this comment.
I don't think this is true. Grpc metrics are registered: https://github.com/googleapis/google-cloud-java/blob/main/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/csm/MetricRegistry.java#L127
No description provided.