feat(gengapic): update generator to inject client_span_name for client spans#1728
feat(gengapic): update generator to inject client_span_name for client spans#1728westarle wants to merge 5 commits into
Conversation
westarle
force-pushed
the
feat/generator-t3-name
branch
from
8279b29 to
82dc283
Compare
westarle
force-pushed
the
feat/generator-t3-name
branch
from
82dc283 to
f890060
Compare
|
|
||
| override := g.getServiceNameOverride(serv) | ||
| servName := pbinfo.ReduceServNameWithOverride(serv.GetName(), g.cfg.pkgName, override) | ||
| clientSpanName := fmt.Sprintf("%s.%s.%s", g.cfg.pkgPath, servName, m.GetName()) |
There was a problem hiding this comment.
Do you think we need any guarding logic that ensures all the values are populated? I don't know across all our generated services if there are exceptional cases here that need more scrutiny.
There was a problem hiding this comment.
Great catch! I looked into this, and here is what we have:
-
pkgPathandm.GetName()are guaranteed to be populated by generator-level validation and protobuf specifications. -
servNamecan indeed be empty. If the service name matches the Go package name (e.g., serviceSecretManagerServicein packagesecretmanager),pbinfo.ReduceServNamereturns""so the generator can produce a cleansecretmanager.Clientinstead ofsecretmanager.SecretManagerClient.
If servName is empty, the generated code would have ended up with an invalid double dot in the middle of the span name (e.g., cloud.google.com/go/secretmanager/apiv1..AccessSecretVersion).
To address this, I have refactored this to introduce a new helper inside the pbinfo package. PTAL.
bhshkh
force-pushed
the
feat/generator-t3-name
branch
from
58df77e to
d17dd93
Compare
bhshkh
force-pushed
the
feat/generator-t3-name
branch
from
d17dd93 to
9fdbbad
Compare
Avoids performing expensive runtime string map lookups and sync operations via `gax.IsFeatureEnabled` on the critical hot path of every RPC call. Evaluating these flags once at client initialization and caching them as booleans on the client struct reduces the runtime check to a single, high-performance boolean evaluation. Two separate flags are cached to respect metrics vs. tracing/logging safety: - `telemetryEnabled` (METRICS || TRACING || LOGGING): Guards low-cardinality attributes like `rpc_method` and `client_span_name`. - `tracingOrLoggingEnabled` (TRACING || LOGGING): Guards high-cardinality attributes like `resource_name` to prevent metric cardinality explosion. Also refactored `ReduceServNameTelemetry` to `ReduceServNameWithoutPackage` in `pbinfo` to improve encapsulation, and updated all generator unit tests and goldens.
4 participants
This PR updates the Go GAPIC generator to inject a client_span_name into the telemetry context for every RPC method. This is a prerequisite for implementing T3 spans (Client Request) in gax-go, allowing the runtime to know the logical name of the call (e.g., GoPackage.Struct.Method).
The attribute is injected inside the existing telemetry feature flag guard (gax.IsFeatureEnabled("METRICS") || gax.IsFeatureEnabled("TRACING") || ...), respecting the user opt-in model.