refactor(otel): unify compute driver tracing - #2995
Conversation
|
🌿 Preview your docs: https://nvidia-preview-pr-2995.docs.buildwithfern.com/openshell |
afd2908 to
f4d7e74
Compare
elezar
left a comment
elezar
left a comment
There was a problem hiding this comment.
Request changes before approval:
- [P2] Standalone Docker tracing remains on its legacy trace layer (
driver.*,rpc.system, andrpc.grpc.status_code), while the in-process path is migrated. Please bring the standalone path to the same contract; this file is outside the PR diff, so I could not anchor this finding. - [P2] The inline findings cover missing cancellation outcomes for in-process Kubernetes and Podman watch spans, plus duplicate re-export mapping tests.
The operation-mapper simplification is non-blocking.
f4d7e74 to
6d88a7c
Compare
krishicks
changed the title
|
@elezar I think I got all the feedback, plus did some further refactoring. |
elezar
left a comment
elezar
left a comment
There was a problem hiding this comment.
The refactor resolves the earlier shared-tracing concerns, but the emitted gRPC semantic-convention attributes need adjustment.
rpc.method is a stable gRPC semantic-convention attribute whose value must be the fully qualified logical method name (for example, openshell.compute.v1.ComputeDriver/CreateSandbox). This changes it to the short method name, while adding rpc.service, which is not part of the current stable gRPC semantic conventions. That leaves traces with a non-standard field and makes rpc.method ambiguous for standard telemetry queries and dashboards.
Please keep the fully qualified value in rpc.method (and otel.name) and remove the emitted rpc.service attribute. The descriptor can still store service and method separately, concatenating them at span creation to avoid repeating the service prefix in every RPC constant. OpenTelemetry gRPC semantic conventions
Centralize compute-driver RPC descriptors, stream instrumentation, provider routing, and standalone installation in openshell-otel. Use typed RPC constants so gateway and in-process driver paths cannot panic on unknown operation strings or repeat runtime method parsing. Emit semantic-convention rpc.service and rpc.method attributes, preserve trace context and resource identity across deployment modes, and route both RPC boundary and backend crate spans to each selected driver provider. Leave consumer-dropped watch spans unset while recording observed terminal status, and avoid reboxing untraced external-driver streams. Derive each driver tracing identity from Cargo package and crate metadata and attach its descriptor to the compute-driver registration, keeping provider selection and target routing tied to the registered implementation. Share tracing setup and round-trip test support across Docker, Podman, Kubernetes, and VM, and update the gateway tracing documentation. Signed-off-by: Kris Hicks <khicks@nvidia.com>
6d88a7c to
b8fdc6b
Compare
|
Updated rpc.method to use the fully qualified protobuf operation name across client and server spans, removed the obsolete rpc.service attribute, and aligned the tests and documentation with the current OpenTelemetry RPC semantic conventions. |
2 participants
Summary
This changes compute-driver tracing to use fully qualified protobuf operation names and stable semantic-convention gRPC status attributes across the gateway and drivers. It also keeps WatchSandboxes client and server spans open for the stream lifetime, distinguishes backend implementation spans, and updates the related tests and documentation.
Before:
After:
The above example is for docker, but all drivers were updated.
Related Issue
Changes
Testing
mise run pre-commitpassesChecklist