OpenTelemetryMetricsModule : Ensure method name is recorded instead of "other" in OpenTelemetry metrics. #12325
Conversation
…r" in OpenTelemetry metrics.
| @@ -466,6 +466,7 @@ private static final class ServerTracer extends ServerStreamTracer { | |||
| this.fullMethodName = fullMethodName; | |||
| this.streamPlugins = checkNotNull(streamPlugins, "streamPlugins"); | |||
| this.stopwatch = module.stopwatchSupplier.get().start(); | |||
| isGeneratedMethod = fullMethodName != null; | |||
There was a problem hiding this comment.
This overrides early the reasoning specified in serverCallStarted but only till serverCallStarted is called. There will be higher cardinality metrics only in the small number of cases where the stream is closed before the server call is started.
@ejona86
There was a problem hiding this comment.
An attacker has control of whether the stream is closed before the server call is started.
I'm not sure there's anything to do to fix that issue, except maybe by making some processing occur in a certain order in the case of cancellation.
There was a problem hiding this comment.
This != null check is also garbage. It is guaranteed to be non-null (see CallAttemptsTracerFactory's constructor), so this is the same as isGeneratedMethod = true
Fixes: #12117
current change sets isGenerated = true whenever fullMethodName != null in ServerTracer. This ensures OpenTelemetry metrics don’t fall back to "other" in early-cancel cases.