diff --git a/Cargo.lock b/Cargo.lock index 72f615e934..6389ebee6a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4178,6 +4178,7 @@ dependencies = [ name = "openshell-otel" version = "0.0.0" dependencies = [ + "futures", "http 1.4.0", "opentelemetry", "opentelemetry-otlp", @@ -4195,10 +4196,13 @@ dependencies = [ name = "openshell-otel-test-support" version = "0.0.0" dependencies = [ + "openshell-otel", "opentelemetry-proto", "tokio", "tokio-stream", "tonic", + "tracing", + "tracing-subscriber", ] [[package]] diff --git a/architecture/gateway.md b/architecture/gateway.md index f86411511b..c523e08045 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -756,9 +756,22 @@ identity and configured compute driver. The gateway forwards OTLP configuration, its configured gateway name, and W3C trace context to managed external drivers. Built-in drivers use dedicated -in-process providers that preserve the same RPC trace boundary. Each driver +in-process providers that preserve the same RPC trace boundary. A shared +compute-driver tracing descriptor derives provider layers and standalone +installation from each driver's service identity, and routes both the shared +RPC boundary target and that driver's crate target prefix. A shared RPC tracer +owns unary and streaming boundary outcomes. Each driver exports to the configured collector under its own service name and carries the -gateway name as a resource attribute. +gateway name and configured compute driver as resource attributes. +Compute-driver client and server spans share the fully qualified protobuf +operation name, such as `openshell.compute.v1.ComputeDriver/CreateSandbox`, +in both the span name and `rpc.method`; the current RPC semantic conventions +integrate the service into that fully qualified method and do not emit +`rpc.service`. `service.name` and span kind distinguish the two sides. +Backend-prefixed spans describe implementation work beneath that boundary. +Streaming `WatchSandboxes` spans remain open for the stream lifetime on both +sides. A terminal stream status records its outcome; +consumer teardown without a terminal status leaves the span status unset. Two invariants shape the failure behavior. Telemetry is diagnostic, so no OTLP failure stops the gateway from serving: a malformed endpoint is logged at diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index d599cac169..f19560ef97 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -57,12 +57,10 @@ use openshell_core::proto_struct::{ use openshell_core::{Error, Result as CoreResult}; use opentelemetry::trace::TraceContextExt as _; use std::collections::{HashMap, HashSet}; -use std::future::Future; use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use std::path::{Path, PathBuf}; use std::pin::Pin; use std::sync::Arc; -use std::task::{Context, Poll}; use std::time::Duration; use tokio::sync::{Mutex, broadcast, mpsc}; use tokio::task::JoinHandle; @@ -413,55 +411,15 @@ fn default_true() -> bool { type WatchStream = Pin> + Send + 'static>>; -struct TracedWatchStream { - inner: WatchStream, - span: tracing::Span, - finished: bool, -} - -impl Stream for TracedWatchStream { - type Item = Result; - - fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - let span = self.span.clone(); - let _entered = span.enter(); - let result = self.inner.as_mut().poll_next(cx); - if !self.finished { - match &result { - Poll::Ready(Some(Err(status))) => { - openshell_otel::mark_error(&self.span); - self.span - .record("rpc.grpc.status_code", status.code() as i32); - self.finished = true; - } - Poll::Ready(None) => { - self.span - .record("rpc.grpc.status_code", tonic::Code::Ok as i32); - self.finished = true; - } - Poll::Pending | Poll::Ready(Some(Ok(_))) => {} - } - } - result - } -} - -impl Drop for TracedWatchStream { - fn drop(&mut self) { - if !self.finished { - openshell_otel::mark_error(&self.span); - self.span - .record("rpc.grpc.status_code", tonic::Code::Cancelled as i32); - } - } -} +#[cfg(test)] +type TracedWatchStream = openshell_otel::TracedGrpcStream; /// Compute-driver service wrapper that preserves the standalone RPC trace /// boundary while Docker runs in the gateway process. #[derive(Clone)] pub struct ComputeDriverService { driver: DockerComputeDriver, - trace_in_process_rpc: bool, + rpc_tracer: openshell_otel::InProcessRpcTracer, } impl ComputeDriverService { @@ -469,7 +427,7 @@ impl ComputeDriverService { pub fn new(driver: DockerComputeDriver) -> Self { Self { driver, - trace_in_process_rpc: false, + rpc_tracer: openshell_otel::InProcessRpcTracer::disabled(), } } @@ -477,53 +435,9 @@ impl ComputeDriverService { pub fn new_in_process(driver: DockerComputeDriver) -> Self { Self { driver, - trace_in_process_rpc: true, + rpc_tracer: openshell_otel::InProcessRpcTracer::enabled(), } } - - fn in_process_rpc_span( - &self, - operation: &'static str, - method: &'static str, - ) -> Option { - self.trace_in_process_rpc.then(|| { - tracing::info_span!( - target: "openshell_driver_docker::otel_tracing", - "driver_rpc", - otel.name = operation, - otel.kind = "server", - otel.status_code = tracing::field::Empty, - rpc.system = "grpc", - rpc.service = "openshell.compute.v1.ComputeDriver", - rpc.method = method, - rpc.grpc.status_code = tracing::field::Empty, - ) - }) - } - - async fn trace_rpc( - &self, - operation: &'static str, - method: &'static str, - future: impl Future>, - ) -> Result { - use tracing::Instrument as _; - - let Some(span) = self.in_process_rpc_span(operation, method) else { - return future.await; - }; - let result = future.instrument(span.clone()).await; - match &result { - Ok(_) => { - span.record("rpc.grpc.status_code", tonic::Code::Ok as i32); - } - Err(status) => { - openshell_otel::mark_error(&span); - span.record("rpc.grpc.status_code", status.code() as i32); - } - } - result - } } /// Return the first responsive local Docker API socket. @@ -971,16 +885,6 @@ impl DockerComputeDriver { } } - #[tracing::instrument( - name = "docker.provision_sandbox", - skip(self, sandbox), - fields( - otel.name = "docker.provision_sandbox", - otel.status_code = tracing::field::Empty, - sandbox.id = %sandbox.id, - sandbox.name = %sandbox.name, - ) - )] async fn provision_sandbox_inner( &self, sandbox: &DriverSandbox, @@ -1769,6 +1673,9 @@ impl DockerComputeDriver { } } +// Standalone and in-process servers both use this wrapper. Delegating to the +// driver's canonical tonic implementation keeps request validation and Docker +// operation spans identical across both deployment modes. #[tonic::async_trait] impl ComputeDriver for ComputeDriverService { type WatchSandboxesStream = WatchStream; @@ -1778,169 +1685,159 @@ impl ComputeDriver for ComputeDriverService { request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.authenticate_sandbox", - "authenticate_sandbox", - ComputeDriver::authenticate_sandbox(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::AUTHENTICATE_SANDBOX, + ComputeDriver::authenticate_sandbox(&self.driver, request), + ) + .await } async fn get_capabilities( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.get_capabilities", - "get_capabilities", - ComputeDriver::get_capabilities(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::GET_CAPABILITIES, + ComputeDriver::get_capabilities(&self.driver, request), + ) + .await } async fn get_gateway_listener_requirements( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - ComputeDriver::get_gateway_listener_requirements(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::GET_GATEWAY_LISTENER_REQUIREMENTS, + ComputeDriver::get_gateway_listener_requirements(&self.driver, request), + ) + .await } async fn validate_sandbox_create( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.validate_sandbox_create", - "validate_sandbox_create", - ComputeDriver::validate_sandbox_create(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::VALIDATE_SANDBOX_CREATE, + ComputeDriver::validate_sandbox_create(&self.driver, request), + ) + .await } async fn get_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.get_sandbox", - "get_sandbox", - ComputeDriver::get_sandbox(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::GET_SANDBOX, + ComputeDriver::get_sandbox(&self.driver, request), + ) + .await } async fn list_sandboxes( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.list_sandboxes", - "list_sandboxes", - ComputeDriver::list_sandboxes(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::LIST_SANDBOXES, + ComputeDriver::list_sandboxes(&self.driver, request), + ) + .await } async fn create_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.create_sandbox", - "create_sandbox", - ComputeDriver::create_sandbox(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::CREATE_SANDBOX, + ComputeDriver::create_sandbox(&self.driver, request), + ) + .await } async fn stop_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.stop_sandbox", - "stop_sandbox", - ComputeDriver::stop_sandbox(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::STOP_SANDBOX, + ComputeDriver::stop_sandbox(&self.driver, request), + ) + .await } async fn start_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.start_sandbox", - "start_sandbox", - ComputeDriver::start_sandbox(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::START_SANDBOX, + ComputeDriver::start_sandbox(&self.driver, request), + ) + .await } async fn delete_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.delete_sandbox", - "delete_sandbox", - ComputeDriver::delete_sandbox(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::DELETE_SANDBOX, + ComputeDriver::delete_sandbox(&self.driver, request), + ) + .await } async fn watch_sandboxes( &self, request: Request, ) -> Result, Status> { - use tracing::Instrument as _; - - let create_stream = ComputeDriver::watch_sandboxes(&self.driver, request); - let Some(span) = self.in_process_rpc_span("driver.watch_sandboxes", "watch_sandboxes") - else { - return create_stream.await; + let create_stream = async { + ComputeDriver::watch_sandboxes(&self.driver, request) + .await + .map(Response::into_inner) }; - match create_stream.instrument(span.clone()).await { - Ok(response) => Ok(Response::new(Box::pin(TracedWatchStream { - inner: response.into_inner(), - span, - finished: false, - }))), - Err(status) => { - openshell_otel::mark_error(&span); - span.record("rpc.grpc.status_code", status.code() as i32); - Err(status) - } - } + self.rpc_tracer + .trace_stream(openshell_otel::rpc::WATCH_SANDBOXES, create_stream) + .await + .map(Response::new) } async fn ensure_workspace( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.ensure_workspace", - "ensure_workspace", - ComputeDriver::ensure_workspace(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::ENSURE_WORKSPACE, + ComputeDriver::ensure_workspace(&self.driver, request), + ) + .await } async fn delete_workspace( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.delete_workspace", - "delete_workspace", - ComputeDriver::delete_workspace(&self.driver, request), - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::DELETE_WORKSPACE, + ComputeDriver::delete_workspace(&self.driver, request), + ) + .await } } diff --git a/crates/openshell-driver-docker/src/main.rs b/crates/openshell-driver-docker/src/main.rs index 11a2762849..a5ca29887a 100644 --- a/crates/openshell-driver-docker/src/main.rs +++ b/crates/openshell-driver-docker/src/main.rs @@ -8,11 +8,8 @@ use clap::Parser; use miette::{IntoDiagnostic, Result}; use openshell_core::VERSION; use openshell_core::proto::compute::v1::compute_driver_server::ComputeDriverServer; -use openshell_driver_docker::otel_tracing::compute_driver_rpc_layer; use openshell_driver_docker::{ComputeDriverService, DockerComputeConfig, DockerComputeDriver}; use tracing::info; -use tracing_subscriber::EnvFilter; -use tracing_subscriber::prelude::*; #[derive(Debug, Parser)] #[command(name = "openshell-driver-docker", version = VERSION)] @@ -46,24 +43,15 @@ struct Args { #[tokio::main] async fn main() -> Result<()> { let args = Args::parse(); - let (tracer_provider, setup_error) = openshell_driver_docker::otel_tracing::provider_for( - args.otlp_endpoint.as_deref(), - args.gateway_name.as_deref(), + let _tracing = openshell_otel::install_driver_tracing( + openshell_driver_docker::otel_tracing::TRACING, + openshell_otel::DriverTracingConfig { + endpoint: args.otlp_endpoint.as_deref(), + gateway_name: args.gateway_name.as_deref(), + service_version: VERSION, + log_level: &args.log_level, + }, ); - tracing_subscriber::registry() - .with(EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(&args.log_level))) - .with(tracing_subscriber::fmt::layer()) - .with( - tracer_provider - .as_ref() - .map(openshell_driver_docker::otel_tracing::layer), - ) - .init(); - if let Some(error) = setup_error { - tracing::error!(%error, "OTLP exporting could not be started"); - } else if let Some(endpoint) = &args.otlp_endpoint { - info!(endpoint, "OTLP exporting enabled"); - } let config_source = std::fs::read_to_string(&args.config).into_diagnostic()?; let docker_config: DockerComputeConfig = toml::from_str(&config_source).into_diagnostic()?; @@ -76,21 +64,15 @@ async fn main() -> Result<()> { let _cleanup = openshell_core::external_driver_socket::SocketCleanup::new(args.bind_socket.clone()); info!(socket = %args.bind_socket.display(), "Starting Docker compute driver"); - let result = tonic::transport::Server::builder() - .layer(compute_driver_rpc_layer()) + tonic::transport::Server::builder() + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(ComputeDriverServer::new(ComputeDriverService::new(driver))) .serve_with_incoming_shutdown( openshell_core::external_driver_socket::SameUidUnixIncoming::new(listener), shutdown_signal(), ) .await - .into_diagnostic(); - if let Some(provider) = &tracer_provider - && let Err(error) = provider.shutdown() - { - tracing::warn!(%error, "OTLP tracer provider shutdown failed"); - } - result + .into_diagnostic() } async fn shutdown_signal() { diff --git a/crates/openshell-driver-docker/src/otel_tracing.rs b/crates/openshell-driver-docker/src/otel_tracing.rs index 7623279c16..03090c2eda 100644 --- a/crates/openshell-driver-docker/src/otel_tracing.rs +++ b/crates/openshell-driver-docker/src/otel_tracing.rs @@ -1,201 +1,19 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! OpenTelemetry trace exporting for the Docker compute driver. +//! OpenTelemetry tracing identity for the Docker compute driver. -use http::Request; -use openshell_otel::{ - HeaderMapExtractor, OtlpTraceConfig, RecordGrpcFailure, RecordGrpcStatus, SdkTracerProvider, - ServiceName, SetupError, -}; -use opentelemetry::propagation::TextMapPropagator as _; -use opentelemetry::trace::TraceContextExt as _; -use opentelemetry_sdk::propagation::TraceContextPropagator; -use tower_http::trace::{GrpcMakeClassifier, MakeSpan, TraceLayer}; -use tracing::{Span, Subscriber}; -use tracing_opentelemetry::OpenTelemetrySpanExt as _; -use tracing_subscriber::registry::LookupSpan; - -const SERVICE_NAME: &str = "openshell-driver-docker"; -const INSTRUMENTATION_SCOPE: &str = "openshell-driver-docker"; -const COMPUTE_DRIVER_SERVICE: &str = "openshell.compute.v1.ComputeDriver"; -pub const IN_PROCESS_TARGET_PREFIX: &str = "openshell_driver_docker"; - -/// Trace inbound standalone compute-driver RPCs and continue gateway context. -pub fn compute_driver_rpc_layer() -> TraceLayer< - GrpcMakeClassifier, - ComputeDriverRpcSpan, - (), - RecordGrpcStatus, - (), - RecordGrpcStatus, - RecordGrpcFailure, -> { - TraceLayer::new_for_grpc() - .make_span_with(ComputeDriverRpcSpan) - .on_request(()) - .on_response(RecordGrpcStatus) - .on_body_chunk(()) - .on_eos(RecordGrpcStatus) - .on_failure(RecordGrpcFailure) -} - -#[derive(Debug, Clone, Copy)] -pub struct ComputeDriverRpcSpan; - -impl MakeSpan for ComputeDriverRpcSpan { - fn make_span(&mut self, request: &Request) -> Span { - let (operation, method) = compute_driver_rpc_operation(request.uri().path()); - let span = tracing::info_span!( - "driver_rpc", - otel.name = operation, - otel.kind = "server", - otel.status_code = tracing::field::Empty, - rpc.system = "grpc", - rpc.service = COMPUTE_DRIVER_SERVICE, - rpc.method = method, - rpc.grpc.status_code = tracing::field::Empty, - ); - let parent = TraceContextPropagator::new().extract_with_context( - &opentelemetry::Context::new(), - &HeaderMapExtractor::new(request.headers()), - ); - if parent.span().span_context().is_valid() { - let _ = span.set_parent(parent); - } - span - } -} - -pub(crate) fn compute_driver_rpc_operation(path: &str) -> (&'static str, &'static str) { - match path.rsplit('/').next() { - Some("GetCapabilities") => ("driver.get_capabilities", "get_capabilities"), - Some("GetGatewayListenerRequirements") => ( - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - ), - Some("ValidateSandboxCreate") => { - ("driver.validate_sandbox_create", "validate_sandbox_create") - } - Some("CreateSandbox") => ("driver.create_sandbox", "create_sandbox"), - Some("GetSandbox") => ("driver.get_sandbox", "get_sandbox"), - Some("ListSandboxes") => ("driver.list_sandboxes", "list_sandboxes"), - Some("StopSandbox") => ("driver.stop_sandbox", "stop_sandbox"), - Some("StartSandbox") => ("driver.start_sandbox", "start_sandbox"), - Some("DeleteSandbox") => ("driver.delete_sandbox", "delete_sandbox"), - Some("WatchSandboxes") => ("driver.watch_sandboxes", "watch_sandboxes"), - Some("EnsureWorkspace") => ("driver.ensure_workspace", "ensure_workspace"), - Some("DeleteWorkspace") => ("driver.delete_workspace", "delete_workspace"), - _ => ("driver.unknown", "unknown"), - } -} - -/// Build a tracer provider for the configured OTLP/gRPC endpoint and gateway. -#[must_use] -pub fn provider_for( - endpoint: Option<&str>, - gateway_name: Option<&str>, -) -> (Option, Option) { - openshell_otel::provider_for(endpoint.map(|endpoint| { - OtlpTraceConfig { - endpoint, - service_name: ServiceName::Fixed(SERVICE_NAME), - service_version: Some(openshell_core::VERSION), - resource_attributes: gateway_name - .map(str::trim) - .filter(|name| !name.is_empty()) - .map(|name| { - vec![opentelemetry::KeyValue::new( - "openshell.gateway.name", - name.to_string(), - )] - }) - .unwrap_or_default(), - } - })) -} - -pub fn layer(provider: &SdkTracerProvider) -> openshell_otel::OtlpLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - openshell_otel::layer(provider, INSTRUMENTATION_SCOPE) -} - -pub fn in_process_layer(provider: &SdkTracerProvider) -> openshell_otel::TargetOtlpLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - openshell_otel::layer_for_target_prefix( - provider, - INSTRUMENTATION_SCOPE, - IN_PROCESS_TARGET_PREFIX, - ) -} - -#[cfg(test)] -pub(crate) async fn test_lock() -> tokio::sync::MutexGuard<'static, ()> { - static LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); - static INITIALIZED: std::sync::LazyLock<()> = std::sync::LazyLock::new(|| { - tracing::subscriber::set_global_default(tracing_subscriber::registry()) - .expect("test tracing subscriber installs once"); - }); - - let guard = LOCK.lock().await; - std::sync::LazyLock::force(&INITIALIZED); - guard -} +pub const TRACING: openshell_otel::ComputeDriverTracing = openshell_otel::compute_driver_tracing!(); #[cfg(test)] mod tests { - use openshell_otel_test_support::OtlpTestServer; - use tracing_subscriber::layer::SubscriberExt as _; - - #[test] - fn compute_driver_rpc_names_are_explicitly_mapped_and_schema_bounded() { - assert_eq!( - super::compute_driver_rpc_operation( - "/openshell.compute.v1.ComputeDriver/CreateSandbox" - ), - ("driver.create_sandbox", "create_sandbox") - ); - assert_eq!( - super::compute_driver_rpc_operation("/openshell.compute.v1.ComputeDriver/FutureMethod"), - ("driver.unknown", "unknown") - ); - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn tracing_docker_driver_spans_reach_otlp_collector_with_resource_identity() { - let _tracing_lock = super::test_lock().await; - let collector = OtlpTestServer::start().await; - - let (provider, error) = super::provider_for(Some(collector.endpoint()), Some("docker-dev")); - assert!(error.is_none()); - let provider = provider.expect("provider"); - let subscriber = tracing_subscriber::registry().with(super::layer(&provider)); - tracing::subscriber::with_default(subscriber, || { - let span = tracing::info_span!("docker.schedule_sandbox", sandbox.id = "sb-otlp"); - drop(span.enter()); - drop(span); - }); - provider.force_flush().unwrap(); - collector.wait_for_export().await; - provider.shutdown().unwrap(); - let received = collector.shutdown().await; - - assert!( - received - .spans - .iter() - .any(|span| span.name == "docker.schedule_sandbox") - ); - assert_eq!(received.gateway_names, ["docker-dev"]); - assert!( - received - .service_names - .iter() - .any(|name| name == "openshell-driver-docker") - ); + async fn driver_spans_reach_otlp_collector_with_resource_identity() { + openshell_otel_test_support::assert_compute_driver_tracing( + super::TRACING, + openshell_core::VERSION, + "docker.schedule_sandbox", + ) + .await; } } diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 07fb2b0ffb..71855ed4b5 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -168,13 +168,125 @@ fn test_driver_with_config(config: DockerDriverRuntimeConfig) -> DockerComputeDr } } +type TestDriverClient = + openshell_core::proto::compute::v1::compute_driver_client::ComputeDriverClient< + tonic::transport::Channel, + >; + +fn request_with_traceparent(message: T) -> Request { + let mut request = Request::new(message); + request.metadata_mut().insert( + "traceparent", + "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01" + .parse() + .unwrap(), + ); + request +} + +async fn standalone_traced_client() -> ( + TestDriverClient, + tokio::sync::oneshot::Sender<()>, + JoinHandle>, +) { + use openshell_core::proto::compute::v1::compute_driver_server::ComputeDriverServer; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let (shutdown, shutdown_rx) = tokio::sync::oneshot::channel(); + let service = ComputeDriverService::new(test_driver_with_config(runtime_config())); + let server = tokio::spawn(async move { + tonic::transport::Server::builder() + .layer(openshell_otel::compute_driver_rpc_layer()) + .add_service(ComputeDriverServer::new(service)) + .serve_with_incoming_shutdown( + tokio_stream::wrappers::TcpListenerStream::new(listener), + async { + let _ = shutdown_rx.await; + }, + ) + .await + }); + let client = TestDriverClient::connect(format!("http://{address}")) + .await + .unwrap(); + (client, shutdown, server) +} + +#[tokio::test] +async fn tracing_standalone_rpc_layer_propagates_context_and_records_errors() { + use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; + use tracing_subscriber::layer::SubscriberExt as _; + + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; + let exporter = InMemorySpanExporterBuilder::new().build(); + let provider = SdkTracerProvider::builder() + .with_simple_exporter(exporter.clone()) + .build(); + let dispatch = tracing::Dispatch::new( + tracing_subscriber::registry().with(otel_tracing::TRACING.layer(&provider)), + ); + let _dispatch = tracing::dispatcher::set_default(&dispatch); + let (mut client, shutdown, server) = standalone_traced_client().await; + + client + .get_capabilities(request_with_traceparent(GetCapabilitiesRequest {})) + .await + .expect("capabilities should succeed"); + client + .validate_sandbox_create(request_with_traceparent(ValidateSandboxCreateRequest { + sandbox: None, + })) + .await + .expect_err("missing sandbox should fail"); + drop(client); + shutdown.send(()).unwrap(); + tokio::time::timeout(Duration::from_secs(5), server) + .await + .expect("standalone test server should stop") + .expect("standalone test server should not panic") + .expect("standalone test server should stop cleanly"); + provider.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + let capabilities = spans + .iter() + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") + .expect("capabilities RPC span"); + assert_eq!( + capabilities.span_context.trace_id().to_string(), + "4bf92f3577b34da6a3ce929d0e0e4736" + ); + assert_eq!(capabilities.parent_span_id.to_string(), "00f067aa0ba902b7"); + assert!(capabilities.attributes.iter().any(|attribute| { + attribute.key.as_str() == "rpc.method" + && attribute.value.to_string() == "openshell.compute.v1.ComputeDriver/GetCapabilities" + })); + assert!( + capabilities + .attributes + .iter() + .all(|attribute| attribute.key.as_str() != "rpc.service"), + "the current RPC semantic conventions integrate the service into rpc.method" + ); + let failed = spans + .iter() + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate") + .expect("failed RPC span"); + assert!(matches!( + failed.status, + opentelemetry::trace::Status::Error { .. } + )); + provider.shutdown().unwrap(); +} + #[tokio::test] async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; use tracing::{Instrument as _, instrument::WithSubscriber as _}; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let gateway_exporter = InMemorySpanExporterBuilder::new().build(); let gateway_provider = SdkTracerProvider::builder() .with_simple_exporter(gateway_exporter.clone()) @@ -184,19 +296,19 @@ async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { .with_simple_exporter(driver_exporter.clone()) .build(); let subscriber = tracing_subscriber::registry() - .with(openshell_otel::layer_excluding_target_prefix( + .with(openshell_otel::layer_excluding_target_prefixes( &gateway_provider, "gateway-test", - Some(otel_tracing::IN_PROCESS_TARGET_PREFIX), + otel_tracing::TRACING.in_process_targets(), )) - .with(otel_tracing::in_process_layer(&driver_provider)); + .with(otel_tracing::TRACING.in_process_layer(&driver_provider)); let service = ComputeDriverService::new_in_process(test_driver_with_config(runtime_config())); async { let gateway_span = tracing::info_span!( target: "openshell_server::compute", "driver", - otel.name = "driver.get_capabilities", + otel.name = "openshell.compute.v1.ComputeDriver/GetCapabilities", otel.kind = "client" ); ComputeDriver::get_capabilities(&service, Request::new(GetCapabilitiesRequest {})) @@ -209,6 +321,12 @@ async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { ); drop(unrelated.enter()); drop(unrelated); + let selected_backend = tracing::info_span!( + target: "openshell_driver_docker::compute", + "docker.operation" + ); + drop(selected_backend.enter()); + drop(selected_backend); Ok::<_, Status>(()) } .with_subscriber(subscriber) @@ -218,7 +336,7 @@ async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { let gateway_span = tracing::info_span!( target: "openshell_server::compute", "driver", - otel.name = "driver.validate_sandbox_create", + otel.name = "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate", otel.kind = "client" ); ComputeDriver::validate_sandbox_create( @@ -230,12 +348,12 @@ async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { } .with_subscriber( tracing_subscriber::registry() - .with(openshell_otel::layer_excluding_target_prefix( + .with(openshell_otel::layer_excluding_target_prefixes( &gateway_provider, "gateway-test", - Some(otel_tracing::IN_PROCESS_TARGET_PREFIX), + otel_tracing::TRACING.in_process_targets(), )) - .with(otel_tracing::in_process_layer(&driver_provider)), + .with(otel_tracing::TRACING.in_process_layer(&driver_provider)), ) .await .expect_err("missing sandbox should fail"); @@ -246,11 +364,11 @@ async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { let driver_spans = driver_exporter.get_finished_spans().unwrap(); let client = gateway_spans .iter() - .find(|span| span.name == "driver.get_capabilities") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .unwrap(); let server = driver_spans .iter() - .find(|span| span.name == "driver.get_capabilities") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .expect("in-process server span"); assert_eq!( server.span_context.trace_id(), @@ -259,8 +377,18 @@ async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { assert_eq!(server.parent_span_id, client.span_context.span_id()); assert_eq!(server.span_kind, opentelemetry::trace::SpanKind::Server); assert!(server.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Ok as i32).to_string() + attribute.key.as_str() == "rpc.method" + && attribute.value.to_string() == "openshell.compute.v1.ComputeDriver/GetCapabilities" + })); + assert!( + server + .attributes + .iter() + .all(|attribute| attribute.key.as_str() != "rpc.service"), + "the current RPC semantic conventions integrate the service into rpc.method" + ); + assert!(server.attributes.iter().any(|attribute| { + attribute.key.as_str() == "rpc.response.status_code" && attribute.value.to_string() == "OK" })); assert!( gateway_spans @@ -274,17 +402,29 @@ async fn tracing_in_process_service_preserves_the_driver_rpc_server_boundary() { .all(|span| span.name != "kubernetes.operation"), "the Docker provider must not claim unrelated driver spans" ); + assert!( + gateway_spans + .iter() + .all(|span| span.name != "docker.operation"), + "the gateway provider must not claim the selected driver's backend spans" + ); + assert!( + driver_spans + .iter() + .any(|span| span.name == "docker.operation"), + "the Docker provider must export backend spans from the selected driver" + ); let failed = driver_spans .iter() - .find(|span| span.name == "driver.validate_sandbox_create") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate") .expect("failed in-process server span"); assert!(matches!( failed.status, opentelemetry::trace::Status::Error { .. } )); assert!(failed.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::InvalidArgument as i32).to_string() + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "INVALID_ARGUMENT" })); gateway_provider.shutdown().unwrap(); driver_provider.shutdown().unwrap(); @@ -296,12 +436,12 @@ async fn tracing_lifecycle_rpc_failures_export_docker_operation_spans() { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(otel_tracing::layer(&provider)); + let subscriber = tracing_subscriber::registry().with(otel_tracing::TRACING.layer(&provider)); let driver = test_driver_with_config(runtime_config()); async { @@ -350,12 +490,12 @@ async fn tracing_direct_start_exports_a_docker_start_span() { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(otel_tracing::layer(&provider)); + let subscriber = tracing_subscriber::registry().with(otel_tracing::TRACING.layer(&provider)); let driver = test_driver_with_config(runtime_config()); DockerComputeDriver::start_sandbox(&driver, "", "") @@ -379,30 +519,37 @@ async fn tracing_direct_start_exports_a_docker_start_span() { #[tokio::test] async fn tracing_image_preparation_failure_exports_nested_failed_spans() { use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; - use tracing::instrument::WithSubscriber as _; + use tracing::{Instrument as _, instrument::WithSubscriber as _}; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(otel_tracing::layer(&provider)); + let subscriber = tracing_subscriber::registry().with(otel_tracing::TRACING.layer(&provider)); let mut config = runtime_config(); config.image_pull_policy = "unsupported".to_string(); let driver = test_driver_with_config(config); - driver - .provision_sandbox_inner(&test_sandbox()) - .with_subscriber(subscriber) - .await - .expect_err("unsupported image pull policy should fail provisioning"); + async { + driver + .provision_sandbox_inner(&test_sandbox()) + .instrument(tracing::info_span!( + "docker.provision", + otel.status_code = tracing::field::Empty + )) + .await + } + .with_subscriber(subscriber) + .await + .expect_err("unsupported image pull policy should fail provisioning"); provider.force_flush().unwrap(); let spans = exporter.get_finished_spans().unwrap(); let provision = spans .iter() - .find(|span| span.name == "docker.provision_sandbox") + .find(|span| span.name == "docker.provision") .expect("provisioning span should be exported"); assert!(matches!( provision.status, @@ -430,12 +577,12 @@ async fn background_provisioning_does_not_extend_the_scheduling_span_lifetime() use tracing_opentelemetry::OpenTelemetrySpanExt as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(otel_tracing::layer(&provider)); + let subscriber = tracing_subscriber::registry().with(otel_tracing::TRACING.layer(&provider)); let dispatch = tracing::Dispatch::new(subscriber); let _dispatch = tracing::dispatcher::set_default(&dispatch); @@ -472,30 +619,27 @@ async fn tracing_in_process_stream_span_lives_until_stream_failure() { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(otel_tracing::in_process_layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(otel_tracing::TRACING.in_process_layer(&provider)); async { let span = tracing::info_span!( - target: "openshell_driver_docker::otel_tracing", + target: otel_tracing::TRACING.in_process_target(), "driver_rpc", - otel.name = "driver.watch_sandboxes", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); let inner: WatchStream = Box::pin(futures::stream::iter([Err(Status::internal( "watch failed", ))])); - let mut stream = TracedWatchStream { - inner, - span, - finished: false, - }; + let mut stream = TracedWatchStream::new(inner, span); provider.force_flush().unwrap(); assert!( @@ -516,7 +660,7 @@ async fn tracing_in_process_stream_span_lives_until_stream_failure() { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "driver.watch_sandboxes") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") .expect("watch server span should be exported when the stream ends"); assert!(matches!( span.status, @@ -531,28 +675,25 @@ async fn tracing_in_process_stream_records_ok_when_stream_completes() { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(otel_tracing::in_process_layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(otel_tracing::TRACING.in_process_layer(&provider)); async { let span = tracing::info_span!( - target: "openshell_driver_docker::otel_tracing", + target: otel_tracing::TRACING.in_process_target(), "driver_rpc", - otel.name = "driver.watch_sandboxes", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); let inner: WatchStream = Box::pin(futures::stream::empty()); - let mut stream = TracedWatchStream { - inner, - span, - finished: false, - }; + let mut stream = TracedWatchStream::new(inner, span); assert!(stream.next().await.is_none()); drop(stream); @@ -564,43 +705,39 @@ async fn tracing_in_process_stream_records_ok_when_stream_completes() { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "driver.watch_sandboxes") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") .expect("watch server span should be exported when the stream completes"); assert!(span.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Ok as i32).to_string() + attribute.key.as_str() == "rpc.response.status_code" && attribute.value.to_string() == "OK" })); provider.shutdown().unwrap(); } #[tokio::test] -async fn tracing_in_process_stream_records_cancelled_when_dropped() { +async fn tracing_in_process_stream_leaves_status_unset_when_dropped() { use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(otel_tracing::in_process_layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(otel_tracing::TRACING.in_process_layer(&provider)); async { let span = tracing::info_span!( - target: "openshell_driver_docker::otel_tracing", + target: otel_tracing::TRACING.in_process_target(), "driver_rpc", - otel.name = "driver.watch_sandboxes", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); let inner: WatchStream = Box::pin(futures::stream::pending()); - let stream = TracedWatchStream { - inner, - span, - finished: false, - }; + let stream = TracedWatchStream::new(inner, span); drop(stream); } @@ -611,16 +748,14 @@ async fn tracing_in_process_stream_records_cancelled_when_dropped() { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "driver.watch_sandboxes") - .expect("watch server span should be exported when the stream is cancelled"); - assert!(matches!( - span.status, - opentelemetry::trace::Status::Error { .. } - )); - assert!(span.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Cancelled as i32).to_string() - })); + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") + .expect("watch server span should be exported when the stream is dropped"); + assert!(matches!(span.status, opentelemetry::trace::Status::Unset)); + assert!( + span.attributes + .iter() + .all(|attribute| attribute.key.as_str() != "rpc.response.status_code") + ); provider.shutdown().unwrap(); } diff --git a/crates/openshell-driver-kubernetes/src/driver.rs b/crates/openshell-driver-kubernetes/src/driver.rs index afe3f579e0..bb1b75e8a9 100644 --- a/crates/openshell-driver-kubernetes/src/driver.rs +++ b/crates/openshell-driver-kubernetes/src/driver.rs @@ -1428,10 +1428,10 @@ impl KubernetesComputeDriver { #[allow(clippy::similar_names)] #[tracing::instrument( - name = "kubernetes.create_sandbox", + name = "kubernetes.provision", skip(self, sandbox), fields( - otel.name = "kubernetes.create_sandbox", + otel.name = "kubernetes.provision", otel.status_code = tracing::field::Empty, sandbox.id = %sandbox.id, sandbox.name = %sandbox.name, @@ -4945,12 +4945,13 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)); let driver = KubernetesComputeDriver::new_for_test(KubernetesComputeConfig::default()); driver @@ -4963,7 +4964,7 @@ mod tests { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "kubernetes.create_sandbox") + .find(|span| span.name == "kubernetes.provision") .expect("create operation span"); assert!(matches!( span.status, @@ -4977,15 +4978,16 @@ mod tests { use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter) .build(); - let subscriber = tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)); let annotations = tracing::subscriber::with_default(subscriber, || { - let span = tracing::info_span!("kubernetes.create_sandbox"); + let span = tracing::info_span!("kubernetes.provision"); let _entered = span.enter(); let mut annotations = BTreeMap::new(); add_trace_context_annotation(&mut annotations); diff --git a/crates/openshell-driver-kubernetes/src/grpc.rs b/crates/openshell-driver-kubernetes/src/grpc.rs index 027c350a26..095752d842 100644 --- a/crates/openshell-driver-kubernetes/src/grpc.rs +++ b/crates/openshell-driver-kubernetes/src/grpc.rs @@ -15,11 +15,8 @@ use openshell_core::proto::compute::v1::{ ValidateSandboxCreateResponse, WatchSandboxesEvent, WatchSandboxesRequest, compute_driver_server::ComputeDriver, }; -use std::future::Future; use std::pin::Pin; -use std::task::{Context, Poll}; use tonic::{Request, Response, Status}; -use tracing::Instrument as _; use crate::KubernetesComputeDriver; use crate::WorkspaceMode; @@ -27,38 +24,13 @@ use crate::WorkspaceMode; type ComputeDriverWatchStream = Pin> + Send + 'static>>; -struct TracedWatchStream { - inner: ComputeDriverWatchStream, - span: tracing::Span, -} - -impl Stream for TracedWatchStream { - type Item = Result; - - fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - let span = self.span.clone(); - let _entered = span.enter(); - let result = self.inner.as_mut().poll_next(cx); - match &result { - Poll::Ready(Some(Err(status))) => { - openshell_otel::mark_error(&self.span); - self.span - .record("rpc.grpc.status_code", status.code() as i32); - } - Poll::Ready(None) => { - self.span - .record("rpc.grpc.status_code", tonic::Code::Ok as i32); - } - Poll::Pending | Poll::Ready(Some(Ok(_))) => {} - } - result - } -} +#[cfg(test)] +type TracedWatchStream = openshell_otel::TracedGrpcStream; #[derive(Debug, Clone)] pub struct ComputeDriverService { driver: KubernetesComputeDriver, - trace_in_process_rpc: bool, + rpc_tracer: openshell_otel::InProcessRpcTracer, } impl ComputeDriverService { @@ -66,7 +38,7 @@ impl ComputeDriverService { pub fn new(driver: KubernetesComputeDriver) -> Self { Self { driver, - trace_in_process_rpc: false, + rpc_tracer: openshell_otel::InProcessRpcTracer::disabled(), } } @@ -74,50 +46,8 @@ impl ComputeDriverService { pub fn new_in_process(driver: KubernetesComputeDriver) -> Self { Self { driver, - trace_in_process_rpc: true, - } - } - - fn in_process_rpc_span( - &self, - operation: &'static str, - method: &'static str, - ) -> Option { - self.trace_in_process_rpc.then(|| { - tracing::info_span!( - target: "openshell_driver_kubernetes::otel_tracing", - "driver_rpc", - otel.name = operation, - otel.kind = "server", - otel.status_code = tracing::field::Empty, - rpc.system = "grpc", - rpc.service = "openshell.compute.v1.ComputeDriver", - rpc.method = method, - rpc.grpc.status_code = tracing::field::Empty, - ) - }) - } - - async fn trace_rpc( - &self, - operation: &'static str, - method: &'static str, - future: impl Future>, - ) -> Result { - let Some(span) = self.in_process_rpc_span(operation, method) else { - return future.await; - }; - let result = future.instrument(span.clone()).await; - match &result { - Ok(_) => { - span.record("rpc.grpc.status_code", tonic::Code::Ok as i32); - } - Err(status) => { - openshell_otel::mark_error(&span); - span.record("rpc.grpc.status_code", status.code() as i32); - } + rpc_tracer: openshell_otel::InProcessRpcTracer::enabled(), } - result } } @@ -127,170 +57,182 @@ impl ComputeDriver for ComputeDriverService { &self, request: Request, ) -> Result, Status> { - let credential = request.into_inner().credential; - if credential.is_empty() { - return Err(Status::invalid_argument("credential is required")); - } - let sandbox_id = self.driver.authenticate_sandbox(&credential).await?; - Ok(Response::new(AuthenticateSandboxResponse { sandbox_id })) + self.rpc_tracer + .trace(openshell_otel::rpc::AUTHENTICATE_SANDBOX, async { + let credential = request.into_inner().credential; + if credential.is_empty() { + return Err(Status::invalid_argument("credential is required")); + } + let sandbox_id = self.driver.authenticate_sandbox(&credential).await?; + Ok(Response::new(AuthenticateSandboxResponse { sandbox_id })) + }) + .await } async fn get_capabilities( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc("driver.get_capabilities", "get_capabilities", async { - self.driver - .capabilities() - .map(Response::new) - .map_err(Status::internal) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::GET_CAPABILITIES, async { + self.driver + .capabilities() + .map(Response::new) + .map_err(Status::internal) + }) + .await } async fn get_gateway_listener_requirements( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - async { - Ok(Response::new(GetGatewayListenerRequirementsResponse { - requirements: Vec::new(), - })) - }, - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::GET_GATEWAY_LISTENER_REQUIREMENTS, + async { + Ok(Response::new(GetGatewayListenerRequirementsResponse { + requirements: Vec::new(), + })) + }, + ) + .await } async fn validate_sandbox_create( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.validate_sandbox_create", - "validate_sandbox_create", - async { + self.rpc_tracer + .trace(openshell_otel::rpc::VALIDATE_SANDBOX_CREATE, async { let sandbox = request .into_inner() .sandbox .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; self.driver.validate_sandbox_create(&sandbox).await?; Ok(Response::new(ValidateSandboxCreateResponse {})) - }, - ) - .await + }) + .await } async fn get_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.get_sandbox", "get_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - let sandbox = self - .driver - .get_sandbox(&request.sandbox_id) - .await - .map_err(Status::internal)? - .ok_or_else(|| Status::not_found("sandbox not found"))?; - Ok(Response::new(GetSandboxResponse { - sandbox: Some(sandbox), - })) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::GET_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + let sandbox = self + .driver + .get_sandbox(&request.sandbox_id) + .await + .map_err(Status::internal)? + .ok_or_else(|| Status::not_found("sandbox not found"))?; + Ok(Response::new(GetSandboxResponse { + sandbox: Some(sandbox), + })) + }) + .await } async fn list_sandboxes( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc("driver.list_sandboxes", "list_sandboxes", async { - let sandboxes = self - .driver - .list_sandboxes() - .await - .map_err(Status::internal)?; - Ok(Response::new(ListSandboxesResponse { sandboxes })) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::LIST_SANDBOXES, async { + let sandboxes = self + .driver + .list_sandboxes() + .await + .map_err(Status::internal)?; + Ok(Response::new(ListSandboxesResponse { sandboxes })) + }) + .await } async fn create_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.create_sandbox", "create_sandbox", async { - let sandbox = request - .into_inner() - .sandbox - .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; - self.driver - .create_sandbox(&sandbox) - .await - .map_err(|e| Status::from(openshell_core::ComputeDriverError::from(e)))?; - Ok(Response::new(CreateSandboxResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::CREATE_SANDBOX, async { + let sandbox = request + .into_inner() + .sandbox + .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; + self.driver + .create_sandbox(&sandbox) + .await + .map_err(|e| Status::from(openshell_core::ComputeDriverError::from(e)))?; + Ok(Response::new(CreateSandboxResponse {})) + }) + .await } async fn stop_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.stop_sandbox", "stop_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - self.driver - .stop_sandbox(&request.sandbox_id) - .await - .map_err(|error| Status::from(openshell_core::ComputeDriverError::from(error)))?; - Ok(Response::new(StopSandboxResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::STOP_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + self.driver + .stop_sandbox(&request.sandbox_id) + .await + .map_err(|error| { + Status::from(openshell_core::ComputeDriverError::from(error)) + })?; + Ok(Response::new(StopSandboxResponse {})) + }) + .await } async fn start_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.start_sandbox", "start_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - self.driver - .start_sandbox(&request.sandbox_id) - .await - .map_err(|error| Status::from(openshell_core::ComputeDriverError::from(error)))?; - Ok(Response::new(StartSandboxResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::START_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + self.driver + .start_sandbox(&request.sandbox_id) + .await + .map_err(|error| { + Status::from(openshell_core::ComputeDriverError::from(error)) + })?; + Ok(Response::new(StartSandboxResponse {})) + }) + .await } async fn delete_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.delete_sandbox", "delete_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - let deleted = self - .driver - .delete_sandbox(&request.sandbox_id) - .await - .map_err(Status::internal)?; - Ok(Response::new(DeleteSandboxResponse { deleted })) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::DELETE_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + let deleted = self + .driver + .delete_sandbox(&request.sandbox_id) + .await + .map_err(Status::internal)?; + Ok(Response::new(DeleteSandboxResponse { deleted })) + }) + .await } type WatchSandboxesStream = ComputeDriverWatchStream; @@ -308,81 +250,74 @@ impl ComputeDriver for ComputeDriverService { let stream = stream.map(|item| item.map_err(|err| Status::internal(err.to_string()))); Ok::(Box::pin(stream)) }; - let Some(span) = self.in_process_rpc_span("driver.watch_sandboxes", "watch_sandboxes") - else { - return create_stream.await.map(Response::new); - }; - match create_stream.instrument(span.clone()).await { - Ok(stream) => Ok(Response::new(Box::pin(TracedWatchStream { - inner: stream, - span, - }))), - Err(status) => { - openshell_otel::mark_error(&span); - span.record("rpc.grpc.status_code", status.code() as i32); - Err(status) - } - } + self.rpc_tracer + .trace_stream(openshell_otel::rpc::WATCH_SANDBOXES, create_stream) + .await + .map(Response::new) } async fn ensure_workspace( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.ensure_workspace", "ensure_workspace", async { - let workspace = request.into_inner().workspace; - if workspace.is_empty() { - return Err(Status::invalid_argument("workspace is required")); - } - self.driver - .validate_workspace_namespace(&workspace) - .map_err(|error| Status::from(openshell_core::ComputeDriverError::from(error)))?; - match self.driver.workspace_mode() { - WorkspaceMode::Managed => { - self.driver - .ensure_namespace(&workspace) - .await - .map_err(|e| Status::internal(e.to_string()))?; + self.rpc_tracer + .trace(openshell_otel::rpc::ENSURE_WORKSPACE, async { + let workspace = request.into_inner().workspace; + if workspace.is_empty() { + return Err(Status::invalid_argument("workspace is required")); } - WorkspaceMode::Operator => { - if let Some(allowlist) = self.driver.operator_allowlist() - && !allowlist.contains(&workspace) - { - return Err(Status::permission_denied(format!( - "workspace '{workspace}' is not in the operator namespace allowlist" - ))); + self.driver + .validate_workspace_namespace(&workspace) + .map_err(|error| { + Status::from(openshell_core::ComputeDriverError::from(error)) + })?; + match self.driver.workspace_mode() { + WorkspaceMode::Managed => { + self.driver + .ensure_namespace(&workspace) + .await + .map_err(|e| Status::internal(e.to_string()))?; } + WorkspaceMode::Operator => { + if let Some(allowlist) = self.driver.operator_allowlist() + && !allowlist.contains(&workspace) + { + return Err(Status::permission_denied(format!( + "workspace '{workspace}' is not in the operator namespace allowlist" + ))); + } + } + WorkspaceMode::Shared => {} } - WorkspaceMode::Shared => {} - } - Ok(Response::new(EnsureWorkspaceResponse {})) - }) - .await + Ok(Response::new(EnsureWorkspaceResponse {})) + }) + .await } async fn delete_workspace( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.delete_workspace", "delete_workspace", async { - let workspace = request.into_inner().workspace; - if workspace.is_empty() { - return Err(Status::invalid_argument("workspace is required")); - } - if workspace_delete_requires_namespace_access(self.driver.workspace_mode()) { - self.driver - .validate_workspace_namespace(&workspace) - .map_err(|error| { - Status::from(openshell_core::ComputeDriverError::from(error)) - })?; - self.driver - .delete_namespace(&workspace) - .await - .map_err(|e| Status::internal(e.to_string()))?; - } - Ok(Response::new(DeleteWorkspaceResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::DELETE_WORKSPACE, async { + let workspace = request.into_inner().workspace; + if workspace.is_empty() { + return Err(Status::invalid_argument("workspace is required")); + } + if workspace_delete_requires_namespace_access(self.driver.workspace_mode()) { + self.driver + .validate_workspace_namespace(&workspace) + .map_err(|error| { + Status::from(openshell_core::ComputeDriverError::from(error)) + })?; + self.driver + .delete_namespace(&workspace) + .await + .map_err(|e| Status::internal(e.to_string()))?; + } + Ok(Response::new(DeleteWorkspaceResponse {})) + }) + .await } } @@ -402,10 +337,10 @@ mod tests { use super::*; use crate::KubernetesComputeConfig; use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; - use tracing::instrument::WithSubscriber as _; + use tracing::{Instrument as _, instrument::WithSubscriber as _}; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let gateway_exporter = InMemorySpanExporterBuilder::new().build(); let gateway_provider = SdkTracerProvider::builder() .with_simple_exporter(gateway_exporter.clone()) @@ -415,12 +350,12 @@ mod tests { .with_simple_exporter(driver_exporter.clone()) .build(); let subscriber = tracing_subscriber::registry() - .with(openshell_otel::layer_excluding_target_prefix( + .with(openshell_otel::layer_excluding_target_prefixes( &gateway_provider, "gateway-test", - Some(crate::otel_tracing::IN_PROCESS_TARGET_PREFIX), + crate::otel_tracing::TRACING.in_process_targets(), )) - .with(crate::otel_tracing::in_process_layer(&driver_provider)); + .with(crate::otel_tracing::TRACING.in_process_layer(&driver_provider)); let service = ComputeDriverService::new_in_process(KubernetesComputeDriver::new_for_test( KubernetesComputeConfig::default(), )); @@ -429,7 +364,7 @@ mod tests { let gateway_span = tracing::info_span!( target: "openshell_server::compute", "driver", - otel.name = "driver.get_capabilities", + otel.name = "openshell.compute.v1.ComputeDriver/GetCapabilities", otel.kind = "client" ); ComputeDriver::get_capabilities(&service, Request::new(GetCapabilitiesRequest {})) @@ -452,11 +387,11 @@ mod tests { let driver_spans = driver_exporter.get_finished_spans().unwrap(); let client = gateway_spans .iter() - .find(|span| span.name == "driver.get_capabilities") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .expect("gateway client span"); let server = driver_spans .iter() - .find(|span| span.name == "driver.get_capabilities") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .expect("in-process server span"); assert_eq!( server.span_context.trace_id(), @@ -465,12 +400,24 @@ mod tests { assert_eq!(server.parent_span_id, client.span_context.span_id()); assert_eq!(server.span_kind, opentelemetry::trace::SpanKind::Server); assert!(server.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Ok as i32).to_string() + attribute.key.as_str() == "rpc.method" + && attribute.value.to_string() + == "openshell.compute.v1.ComputeDriver/GetCapabilities" + })); + assert!( + server + .attributes + .iter() + .all(|attribute| attribute.key.as_str() != "rpc.service"), + "the current RPC semantic conventions integrate the service into rpc.method" + ); + assert!(server.attributes.iter().any(|attribute| { + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "OK" })); let failed = driver_spans .iter() - .find(|span| span.name == "driver.validate_sandbox_create") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate") .expect("failed in-process server span"); assert!(matches!( failed.status, @@ -487,27 +434,27 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = - tracing_subscriber::registry().with(crate::otel_tracing::in_process_layer(&provider)); + let subscriber = tracing_subscriber::registry() + .with(crate::otel_tracing::TRACING.in_process_layer(&provider)); async { let span = tracing::info_span!( - target: "openshell_driver_kubernetes::otel_tracing", + target: crate::otel_tracing::TRACING.in_process_target(), "driver_rpc", - otel.name = "driver.watch_sandboxes", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); let inner: ComputeDriverWatchStream = Box::pin(futures::stream::iter([Err( Status::internal("watch failed"), )])); - let mut stream = TracedWatchStream { inner, span }; + let mut stream = TracedWatchStream::new(inner, span); provider.force_flush().unwrap(); assert!( @@ -528,7 +475,7 @@ mod tests { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "driver.watch_sandboxes") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") .expect("watch server span should be exported when the stream ends"); assert!(matches!( span.status, @@ -544,25 +491,25 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = - tracing_subscriber::registry().with(crate::otel_tracing::in_process_layer(&provider)); + let subscriber = tracing_subscriber::registry() + .with(crate::otel_tracing::TRACING.in_process_layer(&provider)); async { let span = tracing::info_span!( - target: "openshell_driver_kubernetes::otel_tracing", + target: crate::otel_tracing::TRACING.in_process_target(), "driver_rpc", - otel.name = "driver.watch_sandboxes", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); let inner: ComputeDriverWatchStream = Box::pin(futures::stream::empty()); - let mut stream = TracedWatchStream { inner, span }; + let mut stream = TracedWatchStream::new(inner, span); assert!(stream.next().await.is_none()); drop(stream); @@ -574,15 +521,63 @@ mod tests { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "driver.watch_sandboxes") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") .expect("watch server span should be exported when the stream completes"); assert!(span.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Ok as i32).to_string() + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "OK" })); provider.shutdown().unwrap(); } + #[tokio::test] + async fn tracing_in_process_stream_leaves_status_unset_when_dropped() { + use super::*; + use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; + use tracing::instrument::WithSubscriber as _; + use tracing_subscriber::layer::SubscriberExt as _; + + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; + let exporter = InMemorySpanExporterBuilder::new().build(); + let provider = SdkTracerProvider::builder() + .with_simple_exporter(exporter.clone()) + .build(); + let subscriber = tracing_subscriber::registry() + .with(crate::otel_tracing::TRACING.in_process_layer(&provider)); + + async { + let span = tracing::info_span!( + target: crate::otel_tracing::TRACING.in_process_target(), + "driver_rpc", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", + otel.kind = "server", + otel.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, + ); + let inner: ComputeDriverWatchStream = Box::pin(futures::stream::pending()); + let stream = TracedWatchStream::new(inner, span); + + drop(stream); + } + .with_subscriber(subscriber) + .await; + provider.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + let span = spans + .iter() + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") + .expect("watch server span should be exported when the stream is dropped"); + assert!(matches!(span.status, opentelemetry::trace::Status::Unset)); + assert!( + span.attributes + .iter() + .all(|attribute| attribute.key.as_str() != "rpc.response.status_code") + ); + provider.shutdown().unwrap(); + } + #[test] fn precondition_driver_errors_map_to_failed_precondition_status() { let status: Status = ComputeDriverError::from(KubernetesDriverError::Precondition( diff --git a/crates/openshell-driver-kubernetes/src/main.rs b/crates/openshell-driver-kubernetes/src/main.rs index 7690ccee85..9b11f5da2a 100644 --- a/crates/openshell-driver-kubernetes/src/main.rs +++ b/crates/openshell-driver-kubernetes/src/main.rs @@ -7,12 +7,9 @@ use std::collections::BTreeMap; use std::net::SocketAddr; use std::path::PathBuf; use tracing::info; -use tracing_subscriber::EnvFilter; -use tracing_subscriber::prelude::*; use openshell_core::VERSION; use openshell_core::proto::compute::v1::compute_driver_server::ComputeDriverServer; -use openshell_driver_kubernetes::otel_tracing::compute_driver_rpc_layer; use openshell_driver_kubernetes::{ AppArmorProfile, ComputeDriverService, DEFAULT_GATEWAY_ID, DEFAULT_PROXY_UID, DEFAULT_SANDBOX_SERVICE_ACCOUNT_NAME, KubernetesComputeConfig, KubernetesComputeDriver, @@ -218,24 +215,15 @@ async fn shutdown_signal() { #[tokio::main] async fn main() -> Result<()> { let args = Args::parse(); - let (tracer_provider, setup_error) = openshell_driver_kubernetes::otel_tracing::provider_for( - args.otlp_endpoint.as_deref(), - args.gateway_name.as_deref(), + let _tracing = openshell_otel::install_driver_tracing( + openshell_driver_kubernetes::otel_tracing::TRACING, + openshell_otel::DriverTracingConfig { + endpoint: args.otlp_endpoint.as_deref(), + gateway_name: args.gateway_name.as_deref(), + service_version: VERSION, + log_level: &args.log_level, + }, ); - tracing_subscriber::registry() - .with(EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(&args.log_level))) - .with(tracing_subscriber::fmt::layer()) - .with( - tracer_provider - .as_ref() - .map(openshell_driver_kubernetes::otel_tracing::layer), - ) - .init(); - if let Some(error) = setup_error { - tracing::error!(%error, "OTLP exporting could not be started"); - } else if let Some(endpoint) = &args.otlp_endpoint { - info!(endpoint, "OTLP exporting enabled"); - } let managed_ssh_gateway_pod_selector = args .managed_ssh_gateway_pod_selector @@ -317,14 +305,14 @@ async fn main() -> Result<()> { shutdown_signal().await; let _ = shutdown_tx.send(true); }; - let result = if let Some(socket_path) = args.bind_socket { + if let Some(socket_path) = args.bind_socket { let listener = openshell_core::external_driver_socket::bind_private(&socket_path) .map_err(|err| miette::miette!("{err}"))?; let _cleanup = openshell_core::external_driver_socket::SocketCleanup::new(socket_path.clone()); info!(socket = %socket_path.display(), "Starting Kubernetes compute driver"); tonic::transport::Server::builder() - .layer(compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(service) .serve_with_incoming_shutdown( openshell_core::external_driver_socket::SameUidUnixIncoming::new(listener), @@ -335,18 +323,12 @@ async fn main() -> Result<()> { } else { info!(address = %args.bind_address, "Starting Kubernetes compute driver"); tonic::transport::Server::builder() - .layer(compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(service) .serve_with_shutdown(args.bind_address, shutdown) .await .into_diagnostic() - }; - if let Some(provider) = &tracer_provider - && let Err(error) = provider.shutdown() - { - tracing::warn!(%error, "OTLP tracer provider shutdown failed"); } - result } #[cfg(test)] diff --git a/crates/openshell-driver-kubernetes/src/otel_tracing.rs b/crates/openshell-driver-kubernetes/src/otel_tracing.rs index 02501651f8..37e69c65bd 100644 --- a/crates/openshell-driver-kubernetes/src/otel_tracing.rs +++ b/crates/openshell-driver-kubernetes/src/otel_tracing.rs @@ -1,125 +1,19 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! OpenTelemetry trace exporting for the Kubernetes compute driver. +//! OpenTelemetry tracing identity for the Kubernetes compute driver. -use openshell_otel::{OtlpTraceConfig, SdkTracerProvider, ServiceName, SetupError}; -pub use openshell_otel::{compute_driver_rpc_layer, compute_driver_rpc_operation}; -use tracing::Subscriber; -use tracing_subscriber::registry::LookupSpan; - -const SERVICE_NAME: &str = "openshell-driver-kubernetes"; -const INSTRUMENTATION_SCOPE: &str = "openshell-driver-kubernetes"; -pub const IN_PROCESS_TARGET_PREFIX: &str = "openshell_driver_kubernetes"; - -/// Build a tracer provider for the configured OTLP/gRPC endpoint and gateway. -#[must_use] -pub fn provider_for( - endpoint: Option<&str>, - gateway_name: Option<&str>, -) -> (Option, Option) { - openshell_otel::provider_for(endpoint.map(|endpoint| { - OtlpTraceConfig { - endpoint, - service_name: ServiceName::Fixed(SERVICE_NAME), - service_version: Some(openshell_core::VERSION), - resource_attributes: gateway_name - .map(str::trim) - .filter(|name| !name.is_empty()) - .map(|name| { - vec![opentelemetry::KeyValue::new( - "openshell.gateway.name", - name.to_string(), - )] - }) - .unwrap_or_default(), - } - })) -} - -pub fn layer(provider: &SdkTracerProvider) -> openshell_otel::OtlpLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - openshell_otel::layer(provider, INSTRUMENTATION_SCOPE) -} - -pub fn in_process_layer(provider: &SdkTracerProvider) -> openshell_otel::TargetOtlpLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - openshell_otel::layer_for_target_prefix( - provider, - INSTRUMENTATION_SCOPE, - IN_PROCESS_TARGET_PREFIX, - ) -} - -#[cfg(test)] -pub(crate) async fn test_lock() -> tokio::sync::MutexGuard<'static, ()> { - static LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); - static INITIALIZED: std::sync::LazyLock<()> = std::sync::LazyLock::new(|| { - tracing::subscriber::set_global_default(tracing_subscriber::registry()) - .expect("test tracing subscriber installs once"); - }); - - let guard = LOCK.lock().await; - std::sync::LazyLock::force(&INITIALIZED); - guard -} +pub const TRACING: openshell_otel::ComputeDriverTracing = openshell_otel::compute_driver_tracing!(); #[cfg(test)] mod tests { - use openshell_otel_test_support::OtlpTestServer; - use tracing_subscriber::layer::SubscriberExt as _; - - #[test] - fn compute_driver_rpc_names_are_explicitly_mapped_and_schema_bounded() { - assert_eq!( - super::compute_driver_rpc_operation( - "/openshell.compute.v1.ComputeDriver/GetCapabilities" - ), - ("driver.get_capabilities", "get_capabilities") - ); - assert_eq!( - super::compute_driver_rpc_operation( - "/openshell.compute.v1.ComputeDriver/AttackerControlled12345" - ), - ("driver.unknown", "unknown") - ); - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn tracing_kubernetes_driver_spans_reach_otlp_collector_with_resource_identity() { - let _tracing_lock = super::test_lock().await; - let collector = OtlpTestServer::start().await; - let (provider, error) = - super::provider_for(Some(collector.endpoint()), Some("kubernetes-dev")); - assert!(error.is_none()); - let provider = provider.expect("provider"); - let subscriber = tracing_subscriber::registry().with(super::layer(&provider)); - tracing::subscriber::with_default(subscriber, || { - let span = tracing::info_span!("kubernetes.create_sandbox", sandbox.id = "sb-otlp"); - drop(span.enter()); - drop(span); - }); - provider.force_flush().unwrap(); - collector.wait_for_export().await; - provider.shutdown().unwrap(); - let received = collector.shutdown().await; - - assert!( - received - .spans - .iter() - .any(|span| span.name == "kubernetes.create_sandbox") - ); - assert_eq!(received.gateway_names, ["kubernetes-dev"]); - assert!( - received - .service_names - .iter() - .any(|name| name == super::SERVICE_NAME) - ); + async fn driver_spans_reach_otlp_collector_with_resource_identity() { + openshell_otel_test_support::assert_compute_driver_tracing( + super::TRACING, + openshell_core::VERSION, + "kubernetes.provision", + ) + .await; } } diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index aa41017663..16c5780780 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -728,10 +728,10 @@ impl PodmanComputeDriver { /// Create a sandbox container. #[tracing::instrument( - name = "podman.create_sandbox", + name = "podman.provision", skip(self, sandbox), fields( - otel.name = "podman.create_sandbox", + otel.name = "podman.provision", otel.status_code = tracing::field::Empty, sandbox.id = %sandbox.id, sandbox.name = %sandbox.name, @@ -1808,7 +1808,7 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let (socket_path, _requests, handle) = spawn_podman_stub( "trace-stop", vec![ @@ -1824,7 +1824,8 @@ mod tests { let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)); test_driver(socket_path.clone()) .stop_sandbox("sandbox-1") @@ -1857,7 +1858,7 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let (socket_path, _requests, handle) = spawn_podman_stub( "trace-create", vec![ @@ -1876,7 +1877,8 @@ mod tests { let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)); test_driver(socket_path.clone()) .create_sandbox(&plain_sandbox("sandbox-trace", "demo")) @@ -1889,7 +1891,7 @@ mod tests { let spans = exporter.get_finished_spans().unwrap(); let create = spans .iter() - .find(|span| span.name == "podman.create_sandbox") + .find(|span| span.name == "podman.provision") .expect("create operation should be exported"); for name in [ "podman.prepare_images", @@ -1917,7 +1919,7 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let (socket_path, _requests, handle) = spawn_podman_stub( "trace-image-failure", vec![ @@ -1929,7 +1931,8 @@ mod tests { let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)); test_driver(socket_path.clone()) .create_sandbox(&plain_sandbox("sandbox-trace", "demo")) @@ -1958,12 +1961,13 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)); let (start_socket, _requests, start_handle) = spawn_podman_stub( "trace-start", @@ -1990,7 +1994,8 @@ mod tests { StubResponse::new(StatusCode::NO_CONTENT, ""), ], ); - let subscriber = tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)); + let subscriber = + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)); test_driver(delete_socket.clone()) .delete_sandbox("sandbox-1") .with_subscriber(subscriber) diff --git a/crates/openshell-driver-podman/src/grpc.rs b/crates/openshell-driver-podman/src/grpc.rs index 222c617c2f..fedeea3068 100644 --- a/crates/openshell-driver-podman/src/grpc.rs +++ b/crates/openshell-driver-podman/src/grpc.rs @@ -14,55 +14,21 @@ use openshell_core::proto::compute::v1::{ ValidateSandboxCreateRequest, ValidateSandboxCreateResponse, WatchSandboxesEvent, WatchSandboxesRequest, compute_driver_server::ComputeDriver, }; -use std::future::Future; use std::pin::Pin; -use std::task::{Context, Poll}; use tonic::{Request, Response, Status}; -use tracing::Instrument as _; use crate::PodmanComputeDriver; type ComputeDriverWatchStream = Pin> + Send + 'static>>; -struct TracedWatchStream { - inner: ComputeDriverWatchStream, - span: tracing::Span, -} - -impl TracedWatchStream { - fn new(inner: ComputeDriverWatchStream, span: tracing::Span) -> Self { - Self { inner, span } - } -} - -impl Stream for TracedWatchStream { - type Item = Result; - - fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - let span = self.span.clone(); - let _entered = span.enter(); - let result = self.inner.as_mut().poll_next(cx); - match &result { - Poll::Ready(Some(Err(status))) => { - openshell_otel::mark_error(&self.span); - self.span - .record("rpc.grpc.status_code", status.code() as i32); - } - Poll::Ready(None) => { - self.span - .record("rpc.grpc.status_code", tonic::Code::Ok as i32); - } - Poll::Pending | Poll::Ready(Some(Ok(_))) => {} - } - result - } -} +#[cfg(test)] +type TracedWatchStream = openshell_otel::TracedGrpcStream; #[derive(Debug, Clone)] pub struct ComputeDriverService { driver: PodmanComputeDriver, - trace_in_process_rpc: bool, + rpc_tracer: openshell_otel::InProcessRpcTracer, } impl ComputeDriverService { @@ -70,7 +36,7 @@ impl ComputeDriverService { pub fn new(driver: PodmanComputeDriver) -> Self { Self { driver, - trace_in_process_rpc: false, + rpc_tracer: openshell_otel::InProcessRpcTracer::disabled(), } } @@ -78,51 +44,9 @@ impl ComputeDriverService { pub fn new_in_process(driver: PodmanComputeDriver) -> Self { Self { driver, - trace_in_process_rpc: true, + rpc_tracer: openshell_otel::InProcessRpcTracer::enabled(), } } - - fn in_process_rpc_span( - &self, - operation: &'static str, - method: &'static str, - ) -> Option { - self.trace_in_process_rpc.then(|| { - tracing::info_span!( - target: "openshell_driver_podman::otel_tracing", - "driver_rpc", - otel.name = operation, - otel.kind = "server", - otel.status_code = tracing::field::Empty, - rpc.system = "grpc", - rpc.service = "openshell.compute.v1.ComputeDriver", - rpc.method = method, - rpc.grpc.status_code = tracing::field::Empty, - ) - }) - } - - async fn trace_rpc( - &self, - operation: &'static str, - method: &'static str, - future: impl Future>, - ) -> Result { - let Some(span) = self.in_process_rpc_span(operation, method) else { - return future.await; - }; - let result = future.instrument(span.clone()).await; - match &result { - Ok(_) => { - span.record("rpc.grpc.status_code", tonic::Code::Ok as i32); - } - Err(status) => { - openshell_otel::mark_error(&span); - span.record("rpc.grpc.status_code", status.code() as i32); - } - } - result - } } #[tonic::async_trait] @@ -132,51 +56,54 @@ impl ComputeDriver for ComputeDriverService { _request: Request, ) -> Result, Status> { - Err(Status::unimplemented( - "podman does not authenticate sandbox credentials", - )) + self.rpc_tracer + .trace(openshell_otel::rpc::AUTHENTICATE_SANDBOX, async { + Err(Status::unimplemented( + "podman does not authenticate sandbox credentials", + )) + }) + .await } async fn get_capabilities( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc("driver.get_capabilities", "get_capabilities", async { - self.driver - .capabilities() - .map(Response::new) - .map_err(Status::from) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::GET_CAPABILITIES, async { + self.driver + .capabilities() + .map(Response::new) + .map_err(Status::from) + }) + .await } async fn get_gateway_listener_requirements( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - async { - Ok(Response::new(GetGatewayListenerRequirementsResponse { - requirements: self - .driver - .gateway_listener_requirements() - .map_err(Status::from)?, - })) - }, - ) - .await + self.rpc_tracer + .trace( + openshell_otel::rpc::GET_GATEWAY_LISTENER_REQUIREMENTS, + async { + Ok(Response::new(GetGatewayListenerRequirementsResponse { + requirements: self + .driver + .gateway_listener_requirements() + .map_err(Status::from)?, + })) + }, + ) + .await } async fn validate_sandbox_create( &self, request: Request, ) -> Result, Status> { - self.trace_rpc( - "driver.validate_sandbox_create", - "validate_sandbox_create", - async { + self.rpc_tracer + .trace(openshell_otel::rpc::VALIDATE_SANDBOX_CREATE, async { let sandbox = request .into_inner() .sandbox @@ -186,115 +113,120 @@ impl ComputeDriver for ComputeDriverService { .await .map_err(Status::from)?; Ok(Response::new(ValidateSandboxCreateResponse {})) - }, - ) - .await + }) + .await } async fn get_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.get_sandbox", "get_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - let sandbox = self - .driver - .get_sandbox(&request.sandbox_id) - .await - .map_err(Status::from)? - .ok_or_else(|| Status::not_found("sandbox not found"))?; - Ok(Response::new(GetSandboxResponse { - sandbox: Some(sandbox), - })) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::GET_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + let sandbox = self + .driver + .get_sandbox(&request.sandbox_id) + .await + .map_err(Status::from)? + .ok_or_else(|| Status::not_found("sandbox not found"))?; + Ok(Response::new(GetSandboxResponse { + sandbox: Some(sandbox), + })) + }) + .await } async fn list_sandboxes( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc("driver.list_sandboxes", "list_sandboxes", async { - let sandboxes = self.driver.list_sandboxes().await.map_err(Status::from)?; - Ok(Response::new(ListSandboxesResponse { sandboxes })) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::LIST_SANDBOXES, async { + let sandboxes = self.driver.list_sandboxes().await.map_err(Status::from)?; + Ok(Response::new(ListSandboxesResponse { sandboxes })) + }) + .await } async fn create_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.create_sandbox", "create_sandbox", async { - let sandbox = request - .into_inner() - .sandbox - .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; - self.driver - .create_sandbox(&sandbox) - .await - .map_err(Status::from)?; - Ok(Response::new(CreateSandboxResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::CREATE_SANDBOX, async { + let sandbox = request + .into_inner() + .sandbox + .ok_or_else(|| Status::invalid_argument("sandbox is required"))?; + self.driver + .create_sandbox(&sandbox) + .await + .map_err(Status::from)?; + Ok(Response::new(CreateSandboxResponse {})) + }) + .await } async fn stop_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.stop_sandbox", "stop_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - self.driver - .stop_sandbox(&request.sandbox_id) - .await - .map_err(Status::from)?; - Ok(Response::new(StopSandboxResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::STOP_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + self.driver + .stop_sandbox(&request.sandbox_id) + .await + .map_err(Status::from)?; + Ok(Response::new(StopSandboxResponse {})) + }) + .await } async fn start_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.start_sandbox", "start_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - self.driver - .start_sandbox(&request.sandbox_id) - .await - .map_err(Status::from)?; - Ok(Response::new(StartSandboxResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::START_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + self.driver + .start_sandbox(&request.sandbox_id) + .await + .map_err(Status::from)?; + Ok(Response::new(StartSandboxResponse {})) + }) + .await } async fn delete_sandbox( &self, request: Request, ) -> Result, Status> { - self.trace_rpc("driver.delete_sandbox", "delete_sandbox", async { - let request = request.into_inner(); - if request.sandbox_id.is_empty() { - return Err(Status::invalid_argument("sandbox_id is required")); - } - let deleted = self - .driver - .delete_sandbox(&request.sandbox_id) - .await - .map_err(Status::from)?; - Ok(Response::new(DeleteSandboxResponse { deleted })) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::DELETE_SANDBOX, async { + let request = request.into_inner(); + if request.sandbox_id.is_empty() { + return Err(Status::invalid_argument("sandbox_id is required")); + } + let deleted = self + .driver + .delete_sandbox(&request.sandbox_id) + .await + .map_err(Status::from)?; + Ok(Response::new(DeleteSandboxResponse { deleted })) + }) + .await } type WatchSandboxesStream = ComputeDriverWatchStream; @@ -308,40 +240,32 @@ impl ComputeDriver for ComputeDriverService { let stream = stream.map(|item| item.map_err(|err| Status::internal(err.to_string()))); Ok::(Box::pin(stream)) }; - let Some(span) = self.in_process_rpc_span("driver.watch_sandboxes", "watch_sandboxes") - else { - return create_stream.await.map(Response::new); - }; - match create_stream.instrument(span.clone()).await { - Ok(stream) => Ok(Response::new(Box::pin(TracedWatchStream::new( - stream, span, - )))), - Err(status) => { - openshell_otel::mark_error(&span); - span.record("rpc.grpc.status_code", status.code() as i32); - Err(status) - } - } + self.rpc_tracer + .trace_stream(openshell_otel::rpc::WATCH_SANDBOXES, create_stream) + .await + .map(Response::new) } async fn ensure_workspace( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc("driver.ensure_workspace", "ensure_workspace", async { - Ok(Response::new(EnsureWorkspaceResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::ENSURE_WORKSPACE, async { + Ok(Response::new(EnsureWorkspaceResponse {})) + }) + .await } async fn delete_workspace( &self, _request: Request, ) -> Result, Status> { - self.trace_rpc("driver.delete_workspace", "delete_workspace", async { - Ok(Response::new(DeleteWorkspaceResponse {})) - }) - .await + self.rpc_tracer + .trace(openshell_otel::rpc::DELETE_WORKSPACE, async { + Ok(Response::new(DeleteWorkspaceResponse {})) + }) + .await } } @@ -386,7 +310,7 @@ mod tests { )); let server = tokio::spawn(async move { tonic::transport::Server::builder() - .layer(crate::otel_tracing::compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(ComputeDriverServer::new(service)) .serve_with_incoming_shutdown( tokio_stream::wrappers::TcpListenerStream::new(listener), @@ -429,7 +353,7 @@ mod tests { use tracing::{Instrument as _, instrument::WithSubscriber as _}; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let gateway_exporter = InMemorySpanExporterBuilder::new().build(); let gateway_provider = SdkTracerProvider::builder() .with_simple_exporter(gateway_exporter.clone()) @@ -439,18 +363,18 @@ mod tests { .with_simple_exporter(driver_exporter.clone()) .build(); let subscriber = tracing_subscriber::registry() - .with(openshell_otel::layer_excluding_target_prefix( + .with(openshell_otel::layer_excluding_target_prefixes( &gateway_provider, "gateway-test", - Some(crate::otel_tracing::IN_PROCESS_TARGET_PREFIX), + crate::otel_tracing::TRACING.in_process_targets(), )) - .with(crate::otel_tracing::in_process_layer(&driver_provider)); + .with(crate::otel_tracing::TRACING.in_process_layer(&driver_provider)); let service = ComputeDriverService::new_in_process(PodmanComputeDriver::for_tests( PodmanComputeConfig::default(), )); async { - let gateway_span = tracing::info_span!(target: "openshell_server::compute", "driver", otel.name = "driver.get_capabilities", otel.kind = "client"); + let gateway_span = tracing::info_span!(target: "openshell_server::compute", "driver", otel.name = "openshell.compute.v1.ComputeDriver/GetCapabilities", otel.kind = "client"); ComputeDriver::get_capabilities(&service, Request::new(GetCapabilitiesRequest {})) .instrument(gateway_span) .await @@ -465,11 +389,11 @@ mod tests { let driver_spans = driver_exporter.get_finished_spans().unwrap(); let client = gateway_spans .iter() - .find(|span| span.name == "driver.get_capabilities") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .unwrap(); let server = driver_spans .iter() - .find(|span| span.name == "driver.get_capabilities") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .expect("in-process server span"); assert_eq!( server.span_context.trace_id(), @@ -478,8 +402,20 @@ mod tests { assert_eq!(server.parent_span_id, client.span_context.span_id()); assert_eq!(server.span_kind, opentelemetry::trace::SpanKind::Server); assert!(server.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Ok as i32).to_string() + attribute.key.as_str() == "rpc.method" + && attribute.value.to_string() + == "openshell.compute.v1.ComputeDriver/GetCapabilities" + })); + assert!( + server + .attributes + .iter() + .all(|attribute| attribute.key.as_str() != "rpc.service"), + "the current RPC semantic conventions integrate the service into rpc.method" + ); + assert!(server.attributes.iter().any(|attribute| { + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "OK" })); gateway_provider.shutdown().unwrap(); driver_provider.shutdown().unwrap(); @@ -490,13 +426,13 @@ mod tests { use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); let dispatch = tracing::Dispatch::new( - tracing_subscriber::registry().with(crate::otel_tracing::layer(&provider)), + tracing_subscriber::registry().with(crate::otel_tracing::TRACING.layer(&provider)), ); let _dispatch = tracing::dispatcher::set_default(&dispatch); let (mut client, shutdown, server) = standalone_traced_client().await; @@ -523,7 +459,7 @@ mod tests { let spans = exporter.get_finished_spans().unwrap(); let capabilities = spans .iter() - .filter(|span| span.name == "driver.get_capabilities") + .filter(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .collect::>(); assert_eq!(capabilities.len(), 1, "exactly one RPC span is expected"); assert_eq!( @@ -539,21 +475,21 @@ mod tests { opentelemetry::trace::SpanKind::Server ); assert!(capabilities[0].attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Ok as i32).to_string() + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "OK" })); let failed = spans .iter() - .find(|span| span.name == "driver.validate_sandbox_create") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate") .expect("failed RPC span should be exported"); assert!(matches!( failed.status, opentelemetry::trace::Status::Error { .. } )); assert!(failed.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::InvalidArgument as i32).to_string() + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "INVALID_ARGUMENT" })); provider.shutdown().unwrap(); } @@ -564,22 +500,22 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = - tracing_subscriber::registry().with(crate::otel_tracing::in_process_layer(&provider)); + let subscriber = tracing_subscriber::registry() + .with(crate::otel_tracing::TRACING.in_process_layer(&provider)); async { let span = tracing::info_span!( - target: "openshell_driver_podman::otel_tracing", + target: crate::otel_tracing::TRACING.in_process_target(), "driver_rpc", - otel.name = "driver.watch_sandboxes", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); let inner: ComputeDriverWatchStream = Box::pin(futures::stream::iter([Err( Status::internal("watch failed"), @@ -605,7 +541,7 @@ mod tests { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "driver.watch_sandboxes") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") .expect("watch server span should be exported when the stream ends"); assert!(matches!( span.status, @@ -620,22 +556,22 @@ mod tests { use tracing::instrument::WithSubscriber as _; use tracing_subscriber::layer::SubscriberExt as _; - let _tracing_lock = crate::otel_tracing::test_lock().await; + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; let exporter = InMemorySpanExporterBuilder::new().build(); let provider = SdkTracerProvider::builder() .with_simple_exporter(exporter.clone()) .build(); - let subscriber = - tracing_subscriber::registry().with(crate::otel_tracing::in_process_layer(&provider)); + let subscriber = tracing_subscriber::registry() + .with(crate::otel_tracing::TRACING.in_process_layer(&provider)); async { let span = tracing::info_span!( - target: "openshell_driver_podman::otel_tracing", + target: crate::otel_tracing::TRACING.in_process_target(), "driver_rpc", - otel.name = "driver.watch_sandboxes", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); let inner: ComputeDriverWatchStream = Box::pin(futures::stream::empty()); let mut stream = TracedWatchStream::new(inner, span); @@ -650,15 +586,62 @@ mod tests { let spans = exporter.get_finished_spans().unwrap(); let span = spans .iter() - .find(|span| span.name == "driver.watch_sandboxes") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") .expect("watch server span should be exported when the stream completes"); assert!(span.attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" - && attribute.value.to_string() == (tonic::Code::Ok as i32).to_string() + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "OK" })); provider.shutdown().unwrap(); } + #[tokio::test] + async fn in_process_stream_leaves_status_unset_when_dropped() { + use opentelemetry_sdk::trace::{InMemorySpanExporterBuilder, SdkTracerProvider}; + use tracing::instrument::WithSubscriber as _; + use tracing_subscriber::layer::SubscriberExt as _; + + let _tracing_lock = openshell_otel_test_support::tracing_test_lock().await; + let exporter = InMemorySpanExporterBuilder::new().build(); + let provider = SdkTracerProvider::builder() + .with_simple_exporter(exporter.clone()) + .build(); + let subscriber = tracing_subscriber::registry() + .with(crate::otel_tracing::TRACING.in_process_layer(&provider)); + + async { + let span = tracing::info_span!( + target: crate::otel_tracing::TRACING.in_process_target(), + "driver_rpc", + otel.name = "openshell.compute.v1.ComputeDriver/WatchSandboxes", + otel.kind = "server", + otel.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, + ); + let inner: ComputeDriverWatchStream = Box::pin(futures::stream::pending()); + let stream = TracedWatchStream::new(inner, span); + + drop(stream); + } + .with_subscriber(subscriber) + .await; + provider.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + let span = spans + .iter() + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") + .expect("watch server span should be exported when the stream is dropped"); + assert!(matches!(span.status, opentelemetry::trace::Status::Unset)); + assert!( + span.attributes + .iter() + .all(|attribute| attribute.key.as_str() != "rpc.response.status_code") + ); + provider.shutdown().unwrap(); + } + fn test_service(socket_path: PathBuf) -> ComputeDriverService { let config = PodmanComputeConfig { socket_path: Some(socket_path), diff --git a/crates/openshell-driver-podman/src/main.rs b/crates/openshell-driver-podman/src/main.rs index a0fa85d018..4c42ba9699 100644 --- a/crates/openshell-driver-podman/src/main.rs +++ b/crates/openshell-driver-podman/src/main.rs @@ -7,8 +7,6 @@ use std::future::Future; use std::net::SocketAddr; use std::path::PathBuf; use tracing::info; -use tracing_subscriber::EnvFilter; -use tracing_subscriber::prelude::*; use openshell_core::VERSION; use openshell_core::proto::compute::v1::compute_driver_server::ComputeDriverServer; @@ -16,7 +14,6 @@ use openshell_driver_podman::config::{ DEFAULT_NETWORK_NAME, DEFAULT_PODMAN_STOP_TIMEOUT_SECS, DEFAULT_SANDBOX_PIDS_LIMIT, ImagePullPolicy, }; -use openshell_driver_podman::otel_tracing::compute_driver_rpc_layer; use openshell_driver_podman::{ComputeDriverService, PodmanComputeConfig, PodmanComputeDriver}; #[derive(Parser)] @@ -177,24 +174,15 @@ struct Args { #[tokio::main] async fn main() -> Result<()> { let args = Args::parse(); - let (tracer_provider, setup_error) = openshell_driver_podman::otel_tracing::provider_for( - args.otlp_endpoint.as_deref(), - args.gateway_name.as_deref(), + let _tracing = openshell_otel::install_driver_tracing( + openshell_driver_podman::otel_tracing::TRACING, + openshell_otel::DriverTracingConfig { + endpoint: args.otlp_endpoint.as_deref(), + gateway_name: args.gateway_name.as_deref(), + service_version: VERSION, + log_level: &args.log_level, + }, ); - tracing_subscriber::registry() - .with(EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(&args.log_level))) - .with(tracing_subscriber::fmt::layer()) - .with( - tracer_provider - .as_ref() - .map(openshell_driver_podman::otel_tracing::layer), - ) - .init(); - if let Some(error) = setup_error { - tracing::error!(%error, "OTLP exporting could not be started"); - } else if let Some(endpoint) = &args.otlp_endpoint { - info!(endpoint, "OTLP exporting enabled"); - } let driver = PodmanComputeDriver::new(PodmanComputeConfig { socket_path: args.podman_socket, @@ -231,14 +219,14 @@ async fn main() -> Result<()> { .into_diagnostic()?; let service = ComputeDriverServer::new(ComputeDriverService::new(driver)); - let result = if let Some(socket_path) = args.bind_socket { + if let Some(socket_path) = args.bind_socket { let listener = openshell_core::external_driver_socket::bind_private(&socket_path) .map_err(|err| miette::miette!("{err}"))?; let _cleanup = openshell_core::external_driver_socket::SocketCleanup::new(socket_path.clone()); info!(socket = %socket_path.display(), "Starting Podman compute driver"); tonic::transport::Server::builder() - .layer(compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(service) .serve_with_incoming_shutdown( openshell_core::external_driver_socket::SameUidUnixIncoming::new(listener), @@ -249,18 +237,12 @@ async fn main() -> Result<()> { } else { info!(address = %args.bind_address, "Starting Podman compute driver"); tonic::transport::Server::builder() - .layer(compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(service) .serve_with_shutdown(args.bind_address, shutdown_signal()) .await .into_diagnostic() - }; - if let Some(provider) = &tracer_provider - && let Err(error) = provider.shutdown() - { - tracing::warn!(%error, "OTLP tracer provider shutdown failed"); } - result } async fn select_shutdown_signal( diff --git a/crates/openshell-driver-podman/src/otel_tracing.rs b/crates/openshell-driver-podman/src/otel_tracing.rs index 646472c8ed..8d3314f821 100644 --- a/crates/openshell-driver-podman/src/otel_tracing.rs +++ b/crates/openshell-driver-podman/src/otel_tracing.rs @@ -1,165 +1,19 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! OpenTelemetry trace exporting for the Podman compute driver. +//! OpenTelemetry tracing identity for the Podman compute driver. -use openshell_otel::{OtlpTraceConfig, SdkTracerProvider, ServiceName, SetupError}; -pub use openshell_otel::{compute_driver_rpc_layer, compute_driver_rpc_operation}; -use tracing::Subscriber; -use tracing_subscriber::registry::LookupSpan; - -const SERVICE_NAME: &str = "openshell-driver-podman"; -const INSTRUMENTATION_SCOPE: &str = "openshell-driver-podman"; -pub const IN_PROCESS_TARGET_PREFIX: &str = "openshell_driver_podman"; - -/// Build a tracer provider for the configured OTLP/gRPC endpoint and gateway. -#[must_use] -pub fn provider_for( - endpoint: Option<&str>, - gateway_name: Option<&str>, -) -> (Option, Option) { - openshell_otel::provider_for(endpoint.map(|endpoint| { - OtlpTraceConfig { - endpoint, - service_name: ServiceName::Fixed(SERVICE_NAME), - service_version: Some(openshell_core::VERSION), - resource_attributes: gateway_name - .map(str::trim) - .filter(|name| !name.is_empty()) - .map(|name| { - vec![opentelemetry::KeyValue::new( - "openshell.gateway.name", - name.to_string(), - )] - }) - .unwrap_or_default(), - } - })) -} - -pub fn layer(provider: &SdkTracerProvider) -> openshell_otel::OtlpLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - openshell_otel::layer(provider, INSTRUMENTATION_SCOPE) -} - -pub fn in_process_layer(provider: &SdkTracerProvider) -> openshell_otel::TargetOtlpLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - openshell_otel::layer_for_target_prefix( - provider, - INSTRUMENTATION_SCOPE, - IN_PROCESS_TARGET_PREFIX, - ) -} - -#[cfg(test)] -pub(crate) async fn test_lock() -> tokio::sync::MutexGuard<'static, ()> { - static LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); - static INITIALIZED: std::sync::LazyLock<()> = std::sync::LazyLock::new(|| { - tracing::subscriber::set_global_default(tracing_subscriber::registry()) - .expect("test tracing subscriber installs once"); - }); - - let guard = LOCK.lock().await; - std::sync::LazyLock::force(&INITIALIZED); - guard -} +pub const TRACING: openshell_otel::ComputeDriverTracing = openshell_otel::compute_driver_tracing!(); #[cfg(test)] mod tests { - use openshell_otel_test_support::OtlpTestServer; - use tracing_subscriber::layer::SubscriberExt as _; - - #[test] - fn compute_driver_rpc_names_are_explicitly_mapped_and_schema_bounded() { - for (rpc, operation, method) in [ - ( - "GetCapabilities", - "driver.get_capabilities", - "get_capabilities", - ), - ( - "GetGatewayListenerRequirements", - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - ), - ( - "ValidateSandboxCreate", - "driver.validate_sandbox_create", - "validate_sandbox_create", - ), - ("CreateSandbox", "driver.create_sandbox", "create_sandbox"), - ("GetSandbox", "driver.get_sandbox", "get_sandbox"), - ("ListSandboxes", "driver.list_sandboxes", "list_sandboxes"), - ("StopSandbox", "driver.stop_sandbox", "stop_sandbox"), - ("StartSandbox", "driver.start_sandbox", "start_sandbox"), - ("DeleteSandbox", "driver.delete_sandbox", "delete_sandbox"), - ( - "WatchSandboxes", - "driver.watch_sandboxes", - "watch_sandboxes", - ), - ( - "EnsureWorkspace", - "driver.ensure_workspace", - "ensure_workspace", - ), - ( - "DeleteWorkspace", - "driver.delete_workspace", - "delete_workspace", - ), - ] { - assert_eq!( - super::compute_driver_rpc_operation(&format!( - "/openshell.compute.v1.ComputeDriver/{rpc}" - )), - (operation, method), - ); - } - assert_eq!( - super::compute_driver_rpc_operation( - "/openshell.compute.v1.ComputeDriver/AttackerControlled12345" - ), - ("driver.unknown", "unknown"), - ); - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn podman_driver_spans_reach_otlp_collector_with_resource_identity() { - let _tracing_lock = super::test_lock().await; - let collector = OtlpTestServer::start().await; - - let (provider, error) = - super::provider_for(Some(collector.endpoint()), Some("production-us-west")); - assert!(error.is_none()); - let provider = provider.expect("provider"); - let subscriber = tracing_subscriber::registry().with(super::layer(&provider)); - tracing::subscriber::with_default(subscriber, || { - let span = tracing::info_span!("podman.create", sandbox.id = "sb-otlp"); - drop(span.enter()); - drop(span); - }); - provider.force_flush().unwrap(); - collector.wait_for_export().await; - provider.shutdown().unwrap(); - let received = collector.shutdown().await; - - assert!( - received - .spans - .iter() - .any(|span| span.name == "podman.create") - ); - assert_eq!(received.gateway_names, ["production-us-west"]); - assert!( - received - .service_names - .iter() - .any(|name| name == "openshell-driver-podman") - ); + async fn driver_spans_reach_otlp_collector_with_resource_identity() { + openshell_otel_test_support::assert_compute_driver_tracing( + super::TRACING, + openshell_core::VERSION, + "podman.create", + ) + .await; } } diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 46133f38fe..af55998a2a 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -1347,10 +1347,10 @@ impl VmDriver { } #[tracing::instrument( - name = "vm.delete", + name = "vm.teardown", skip(self), fields( - otel.name = "vm.delete", + otel.name = "vm.teardown", otel.status_code = tracing::field::Empty, sandbox.id = %sandbox_id, sandbox.name = %sandbox_name, @@ -6301,7 +6301,7 @@ mod tests { let (shutdown, shutdown_rx) = tokio::sync::oneshot::channel(); let server = tokio::spawn(async move { tonic::transport::Server::builder() - .layer(crate::otel_tracing::compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(ComputeDriverServer::new(driver)) .serve_with_incoming_shutdown( tokio_stream::wrappers::TcpListenerStream::new(listener), @@ -6338,7 +6338,7 @@ mod tests { let spans = traced.exporter.get_finished_spans().unwrap(); let rpc_spans = spans .iter() - .filter(|span| span.name == "driver.get_capabilities") + .filter(|span| span.name == "openshell.compute.v1.ComputeDriver/GetCapabilities") .collect::>(); assert_eq!( rpc_spans.len(), @@ -6419,14 +6419,14 @@ mod tests { let spans = traced.exporter.get_finished_spans().unwrap(); let expected = [ - "driver.get_capabilities", - "driver.validate_sandbox_create", - "driver.create_sandbox", - "driver.get_sandbox", - "driver.list_sandboxes", - "driver.stop_sandbox", - "driver.delete_sandbox", - "driver.watch_sandboxes", + "openshell.compute.v1.ComputeDriver/GetCapabilities", + "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate", + "openshell.compute.v1.ComputeDriver/CreateSandbox", + "openshell.compute.v1.ComputeDriver/GetSandbox", + "openshell.compute.v1.ComputeDriver/ListSandboxes", + "openshell.compute.v1.ComputeDriver/StopSandbox", + "openshell.compute.v1.ComputeDriver/DeleteSandbox", + "openshell.compute.v1.ComputeDriver/WatchSandboxes", ]; for name in expected { let span = spans @@ -6437,10 +6437,10 @@ mod tests { assert_has_parent(span); } for name in [ - "driver.validate_sandbox_create", - "driver.create_sandbox", - "driver.get_sandbox", - "driver.stop_sandbox", + "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate", + "openshell.compute.v1.ComputeDriver/CreateSandbox", + "openshell.compute.v1.ComputeDriver/GetSandbox", + "openshell.compute.v1.ComputeDriver/StopSandbox", ] { let span = spans.iter().find(|span| span.name == name).unwrap(); assert!( @@ -6451,12 +6451,12 @@ mod tests { } let delete_rpc = spans .iter() - .find(|span| span.name == "driver.delete_sandbox") + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/DeleteSandbox") .expect("delete RPC span"); let cleanup = spans .iter() .find(|span| { - span.name == "vm.delete" + span.name == "vm.teardown" && span.span_context.trace_id() == delete_rpc.span_context.trace_id() }) .expect("delete cleanup span"); @@ -6625,7 +6625,7 @@ mod tests { async fn background_provisioning_does_not_extend_the_rpc_span_lifetime() { let traced = TestTracing::new(); let _dispatch = tracing::dispatcher::set_default(&traced.dispatch); - let rpc = tracing::info_span!("driver.create_sandbox"); + let rpc = tracing::info_span!("openshell.compute.v1.ComputeDriver/CreateSandbox"); let entered = rpc.enter(); let provisioning = provisioning_span(&rpc.context(), "sb-lifetime", "invalid image reference"); @@ -6638,7 +6638,7 @@ mod tests { .get_finished_spans() .unwrap() .iter() - .any(|span| span.name == "driver.create_sandbox"), + .any(|span| span.name == "openshell.compute.v1.ComputeDriver/CreateSandbox"), "the RPC span should finish while background provisioning is still active" ); drop(provisioning); @@ -6769,7 +6769,7 @@ mod tests { let spans = traced.exporter.get_finished_spans().unwrap(); let deletion = spans .iter() - .find(|span| span.name == "vm.delete") + .find(|span| span.name == "vm.teardown") .expect("delete span"); assert!( matches!(deletion.status, opentelemetry::trace::Status::Error { .. }), diff --git a/crates/openshell-driver-vm/src/main.rs b/crates/openshell-driver-vm/src/main.rs index 2546cb2606..b8788e4fc9 100644 --- a/crates/openshell-driver-vm/src/main.rs +++ b/crates/openshell-driver-vm/src/main.rs @@ -6,7 +6,6 @@ use futures::Stream; use miette::{IntoDiagnostic, Result}; use openshell_core::VERSION; use openshell_core::proto::compute::v1::compute_driver_server::ComputeDriverServer; -use openshell_driver_vm::otel_tracing::compute_driver_rpc_layer; #[cfg(target_os = "macos")] use openshell_driver_vm::{VM_RUNTIME_DIR_ENV, configured_runtime_dir}; use openshell_driver_vm::{VmBackend, VmDriver, VmDriverConfig, VmLaunchConfig, procguard, run_vm}; @@ -18,8 +17,6 @@ use std::pin::Pin; use std::task::{Context, Poll}; use tokio::net::{UnixListener, UnixStream}; use tracing::info; -use tracing_subscriber::EnvFilter; -use tracing_subscriber::prelude::*; #[derive(Parser, Debug)] #[command(name = "openshell-driver-vm")] @@ -213,24 +210,15 @@ async fn main() -> Result<()> { return Ok(()); } - let (tracer_provider, setup_error) = openshell_driver_vm::otel_tracing::provider_for( - args.otlp_endpoint.as_deref(), - args.gateway_name.as_deref(), + let _tracing = openshell_otel::install_driver_tracing( + openshell_driver_vm::otel_tracing::TRACING, + openshell_otel::DriverTracingConfig { + endpoint: args.otlp_endpoint.as_deref(), + gateway_name: args.gateway_name.as_deref(), + service_version: VERSION, + log_level: &args.log_level, + }, ); - tracing_subscriber::registry() - .with(EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(&args.log_level))) - .with(tracing_subscriber::fmt::layer()) - .with( - tracer_provider - .as_ref() - .map(openshell_driver_vm::otel_tracing::layer), - ) - .init(); - if let Some(error) = setup_error { - tracing::error!(%error, "OTLP exporting could not be started"); - } else if let Some(endpoint) = &args.otlp_endpoint { - info!(endpoint, "OTLP exporting enabled"); - } let listen_mode = compute_driver_listen_mode(&args).map_err(|err| miette::miette!("{err}"))?; @@ -277,7 +265,7 @@ async fn main() -> Result<()> { .await .map_err(|err| miette::miette!("{err}"))?; - let result = match listen_mode { + match listen_mode { ComputeDriverListenMode::Unix { socket_path, expected_peer_pid, @@ -288,7 +276,7 @@ async fn main() -> Result<()> { let listener = UnixListener::bind(&socket_path).into_diagnostic()?; restrict_socket_permissions(&socket_path).map_err(|err| miette::miette!("{err}"))?; let result = tonic::transport::Server::builder() - .layer(compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(ComputeDriverServer::new(driver)) .serve_with_incoming_shutdown( AuthenticatedUnixIncoming::new(listener, expected_peer_pid), @@ -302,19 +290,13 @@ async fn main() -> Result<()> { ComputeDriverListenMode::Tcp(bind_address) => { info!(address = %bind_address, "Starting unauthenticated dev vm compute driver"); tonic::transport::Server::builder() - .layer(compute_driver_rpc_layer()) + .layer(openshell_otel::compute_driver_rpc_layer()) .add_service(ComputeDriverServer::new(driver)) .serve_with_shutdown(bind_address, shutdown_signal()) .await .into_diagnostic() } - }; - if let Some(provider) = &tracer_provider - && let Err(error) = provider.shutdown() - { - tracing::warn!(%error, "OTLP tracer provider shutdown failed"); } - result } async fn shutdown_signal() { diff --git a/crates/openshell-driver-vm/src/otel_tracing.rs b/crates/openshell-driver-vm/src/otel_tracing.rs index 47e2dd5f73..a931ff502e 100644 --- a/crates/openshell-driver-vm/src/otel_tracing.rs +++ b/crates/openshell-driver-vm/src/otel_tracing.rs @@ -1,133 +1,19 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! OpenTelemetry trace exporting. +//! OpenTelemetry tracing identity for the VM compute driver. -use openshell_otel::{OtlpTraceConfig, SdkTracerProvider, ServiceName, SetupError}; -pub use openshell_otel::{compute_driver_rpc_layer, compute_driver_rpc_operation}; -use tracing::Subscriber; -use tracing_subscriber::registry::LookupSpan; - -const SERVICE_NAME: &str = "openshell-driver-vm"; -const INSTRUMENTATION_SCOPE: &str = "openshell-driver-vm"; - -/// Build a tracer provider for the configured OTLP/gRPC endpoint and gateway. -#[must_use] -pub fn provider_for( - endpoint: Option<&str>, - gateway_name: Option<&str>, -) -> (Option, Option) { - openshell_otel::provider_for(endpoint.map(|endpoint| { - OtlpTraceConfig { - endpoint, - service_name: ServiceName::Fixed(SERVICE_NAME), - service_version: Some(openshell_core::VERSION), - resource_attributes: gateway_name - .map(str::trim) - .filter(|name| !name.is_empty()) - .map(|name| { - vec![opentelemetry::KeyValue::new( - "openshell.gateway.name", - name.to_string(), - )] - }) - .unwrap_or_default(), - } - })) -} - -/// Build the tracing layer that exports VM-driver spans. -pub fn layer(provider: &SdkTracerProvider) -> openshell_otel::OtlpLayer -where - S: Subscriber + for<'span> LookupSpan<'span>, -{ - openshell_otel::layer(provider, INSTRUMENTATION_SCOPE) -} +pub const TRACING: openshell_otel::ComputeDriverTracing = openshell_otel::compute_driver_tracing!(); #[cfg(test)] mod tests { - use openshell_otel_test_support::OtlpTestServer; - use tracing_subscriber::layer::SubscriberExt as _; - - #[test] - fn compute_driver_rpc_names_are_explicitly_mapped_and_schema_bounded() { - for (rpc, operation, method) in [ - ( - "GetCapabilities", - "driver.get_capabilities", - "get_capabilities", - ), - ( - "GetGatewayListenerRequirements", - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - ), - ( - "ValidateSandboxCreate", - "driver.validate_sandbox_create", - "validate_sandbox_create", - ), - ("CreateSandbox", "driver.create_sandbox", "create_sandbox"), - ("GetSandbox", "driver.get_sandbox", "get_sandbox"), - ("ListSandboxes", "driver.list_sandboxes", "list_sandboxes"), - ("StopSandbox", "driver.stop_sandbox", "stop_sandbox"), - ("StartSandbox", "driver.start_sandbox", "start_sandbox"), - ("DeleteSandbox", "driver.delete_sandbox", "delete_sandbox"), - ( - "WatchSandboxes", - "driver.watch_sandboxes", - "watch_sandboxes", - ), - ] { - assert_eq!( - super::compute_driver_rpc_operation(&format!( - "/openshell.compute.v1.ComputeDriver/{rpc}" - )), - (operation, method), - "{rpc} must keep an explicit low-cardinality span identity" - ); - } - assert_eq!( - super::compute_driver_rpc_operation( - "/openshell.compute.v1.ComputeDriver/AttackerControlled12345" - ), - ("driver.unknown", "unknown"), - "paths absent from the protobuf schema must not create span names" - ); - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn vm_driver_spans_reach_otlp_collector_with_resource_identity() { - let collector = OtlpTestServer::start().await; - - let (provider, error) = - super::provider_for(Some(collector.endpoint()), Some("production-us-west")); - assert!(error.is_none(), "valid OTLP endpoint should configure"); - let provider = provider.expect("provider"); - let subscriber = tracing_subscriber::registry().with(super::layer(&provider)); - tracing::subscriber::with_default(subscriber, || { - let span = tracing::info_span!("vm.provision", sandbox.id = "sb-otlp"); - drop(span.enter()); - drop(span); - }); - provider.force_flush().unwrap(); - collector.wait_for_export().await; - provider.shutdown().unwrap(); - let received = collector.shutdown().await; - - received - .spans - .iter() - .find(|span| span.name == "vm.provision") - .expect("VM span should reach collector"); - assert!( - received - .service_names - .iter() - .any(|name| name == "openshell-driver-vm"), - "VM spans should use a distinct service name, got {:?}", - received.service_names - ); - assert_eq!(received.gateway_names, ["production-us-west"]); + async fn driver_spans_reach_otlp_collector_with_resource_identity() { + openshell_otel_test_support::assert_compute_driver_tracing( + super::TRACING, + openshell_core::VERSION, + "vm.provision", + ) + .await; } } diff --git a/crates/openshell-gateway/src/lib.rs b/crates/openshell-gateway/src/lib.rs index c5a7ebe0e7..54ae5e5de3 100644 --- a/crates/openshell-gateway/src/lib.rs +++ b/crates/openshell-gateway/src/lib.rs @@ -116,7 +116,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) { registration .with_telemetry_category(TelemetryComputeDriver::anonymous_category("kubernetes")) .without_mtls_user_auth() - .with_tracing_setup(kubernetes_tracing_setup) + .with_in_process_tracing(openshell_driver_kubernetes::otel_tracing::TRACING) .with_inherited_config_keys(&[ "namespace", "default_image", @@ -138,7 +138,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) { registration .with_telemetry_category(TelemetryComputeDriver::anonymous_category("podman")) .with_local_singleplayer() - .with_tracing_setup(podman_tracing_setup) + .with_in_process_tracing(openshell_driver_podman::otel_tracing::TRACING) .with_inherited_config_keys(&[ "default_image", "supervisor_image", @@ -158,7 +158,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) { registration .with_telemetry_category(TelemetryComputeDriver::anonymous_category("docker")) .with_local_singleplayer() - .with_tracing_setup(docker_tracing_setup) + .with_in_process_tracing(openshell_driver_docker::otel_tracing::TRACING) .with_inherited_config_keys(&[ "sandbox_namespace", "default_image", @@ -187,84 +187,6 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) { } } -#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))] -fn kubernetes_tracing_setup( - otlp_endpoint: Option<&str>, - gateway_name: Option<&str>, -) -> openshell_server::ComputeDriverTracingSetup { - let (provider, error) = - openshell_driver_kubernetes::otel_tracing::provider_for(otlp_endpoint, gateway_name); - let layer = provider.as_ref().map(|provider| { - let layer: openshell_server::ComputeDriverTracingLayer = Box::new( - openshell_driver_kubernetes::otel_tracing::in_process_layer(provider), - ); - layer - }); - let shutdown = provider.map(|provider| { - let shutdown: openshell_server::ComputeDriverTracingShutdown = - Box::new(move || provider.shutdown().map_err(|error| error.to_string())); - shutdown - }); - openshell_server::ComputeDriverTracingSetup::new( - layer, - shutdown, - error.map(|error| error.to_string()), - Some(openshell_driver_kubernetes::otel_tracing::IN_PROCESS_TARGET_PREFIX), - ) -} - -#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))] -fn podman_tracing_setup( - otlp_endpoint: Option<&str>, - gateway_name: Option<&str>, -) -> openshell_server::ComputeDriverTracingSetup { - let (provider, error) = - openshell_driver_podman::otel_tracing::provider_for(otlp_endpoint, gateway_name); - let layer = provider.as_ref().map(|provider| { - let layer: openshell_server::ComputeDriverTracingLayer = Box::new( - openshell_driver_podman::otel_tracing::in_process_layer(provider), - ); - layer - }); - let shutdown = provider.map(|provider| { - let shutdown: openshell_server::ComputeDriverTracingShutdown = - Box::new(move || provider.shutdown().map_err(|error| error.to_string())); - shutdown - }); - openshell_server::ComputeDriverTracingSetup::new( - layer, - shutdown, - error.map(|error| error.to_string()), - Some(openshell_driver_podman::otel_tracing::IN_PROCESS_TARGET_PREFIX), - ) -} - -#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))] -fn docker_tracing_setup( - otlp_endpoint: Option<&str>, - gateway_name: Option<&str>, -) -> openshell_server::ComputeDriverTracingSetup { - let (provider, error) = - openshell_driver_docker::otel_tracing::provider_for(otlp_endpoint, gateway_name); - let layer = provider.as_ref().map(|provider| { - let layer: openshell_server::ComputeDriverTracingLayer = Box::new( - openshell_driver_docker::otel_tracing::in_process_layer(provider), - ); - layer - }); - let shutdown = provider.map(|provider| { - let shutdown: openshell_server::ComputeDriverTracingShutdown = - Box::new(move || provider.shutdown().map_err(|error| error.to_string())); - shutdown - }); - openshell_server::ComputeDriverTracingSetup::new( - layer, - shutdown, - error.map(|error| error.to_string()), - Some(openshell_driver_docker::otel_tracing::IN_PROCESS_TARGET_PREFIX), - ) -} - #[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))] #[derive(Clone, Copy)] struct KubernetesFactory; diff --git a/crates/openshell-otel-test-support/Cargo.toml b/crates/openshell-otel-test-support/Cargo.toml index dac6ecbe7b..48be77e1ce 100644 --- a/crates/openshell-otel-test-support/Cargo.toml +++ b/crates/openshell-otel-test-support/Cargo.toml @@ -11,10 +11,13 @@ license.workspace = true repository.workspace = true [dependencies] +openshell-otel = { path = "../openshell-otel" } opentelemetry-proto = { version = "0.32", default-features = false, features = ["gen-tonic", "trace"] } tokio = { workspace = true } tokio-stream = { workspace = true } tonic = { workspace = true } +tracing = { workspace = true } +tracing-subscriber = { workspace = true } [lints] workspace = true diff --git a/crates/openshell-otel-test-support/src/lib.rs b/crates/openshell-otel-test-support/src/lib.rs index 4edbe8b7cc..5d6db481a6 100644 --- a/crates/openshell-otel-test-support/src/lib.rs +++ b/crates/openshell-otel-test-support/src/lib.rs @@ -12,11 +12,65 @@ use opentelemetry_proto::tonic::collector::trace::v1::{ }; use opentelemetry_proto::tonic::trace::v1::Span; +/// Serialize tracing tests and keep their callsites enabled process-wide. +pub async fn tracing_test_lock() -> tokio::sync::MutexGuard<'static, ()> { + static LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + static INITIALIZED: std::sync::LazyLock<()> = std::sync::LazyLock::new(|| { + tracing::subscriber::set_global_default(tracing_subscriber::registry()) + .expect("test tracing subscriber installs once"); + }); + + let guard = LOCK.lock().await; + std::sync::LazyLock::force(&INITIALIZED); + guard +} + +/// Verify one compute-driver descriptor exports spans and shared resources. +pub async fn assert_compute_driver_tracing( + descriptor: openshell_otel::ComputeDriverTracing, + service_version: &'static str, + span_name: &'static str, +) { + use tracing_subscriber::layer::SubscriberExt as _; + + let _tracing_lock = tracing_test_lock().await; + let collector = OtlpTestServer::start().await; + let (provider, error) = descriptor.provider_for( + Some(collector.endpoint()), + service_version, + Some("test-gateway"), + Some(descriptor.compute_driver()), + ); + assert!(error.is_none(), "valid OTLP endpoint should configure"); + let provider = provider.expect("provider"); + let subscriber = tracing_subscriber::registry().with(descriptor.layer(&provider)); + tracing::subscriber::with_default(subscriber, || { + let span = tracing::info_span!("driver.test", otel.name = span_name); + drop(span.enter()); + drop(span); + }); + provider.force_flush().unwrap(); + collector.wait_for_export().await; + provider.shutdown().unwrap(); + let received = collector.shutdown().await; + + assert!(received.spans.iter().any(|span| span.name == span_name)); + assert_eq!(received.gateway_names, ["test-gateway"]); + assert_eq!(received.compute_drivers, [descriptor.compute_driver()]); + assert!( + received + .service_names + .iter() + .any(|name| name == descriptor.service_name()) + ); +} + #[derive(Clone, Debug, Default)] pub struct ReceivedTraces { pub spans: Vec, pub service_names: Vec, pub gateway_names: Vec, + pub compute_drivers: Vec, } #[derive(Clone)] @@ -51,6 +105,9 @@ impl TraceService for Collector { match attribute.key.as_str() { "service.name" => received.service_names.push(value), "openshell.gateway.name" => received.gateway_names.push(value), + "openshell.gateway.compute_driver" => { + received.compute_drivers.push(value); + } _ => {} } } diff --git a/crates/openshell-otel/Cargo.toml b/crates/openshell-otel/Cargo.toml index 8155989824..e4b5de770d 100644 --- a/crates/openshell-otel/Cargo.toml +++ b/crates/openshell-otel/Cargo.toml @@ -11,6 +11,7 @@ license.workspace = true repository.workspace = true [dependencies] +futures = { workspace = true } http = { workspace = true } opentelemetry = { workspace = true } opentelemetry_sdk = { workspace = true } diff --git a/crates/openshell-otel/src/driver.rs b/crates/openshell-otel/src/driver.rs new file mode 100644 index 0000000000..c28f5df55d --- /dev/null +++ b/crates/openshell-otel/src/driver.rs @@ -0,0 +1,258 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Shared compute-driver tracing setup and in-process RPC instrumentation. + +use std::future::Future; +use std::pin::Pin; + +use futures::Stream; +use tracing::Instrument as _; +use tracing_subscriber::EnvFilter; +use tracing_subscriber::prelude::*; +use tracing_subscriber::registry::LookupSpan; + +use crate::{OtlpTraceConfig, SdkTracerProvider, ServiceName, SetupError}; + +/// Target used by every in-process compute-driver RPC boundary. +pub const IN_PROCESS_COMPUTE_DRIVER_TARGET: &str = "openshell_otel::in_process_compute_driver"; + +/// Define the calling driver crate's [`ComputeDriverTracing`] identity from +/// its Cargo package and crate names. +#[macro_export] +macro_rules! compute_driver_tracing { + () => { + $crate::ComputeDriverTracing::new(env!("CARGO_PKG_NAME"), env!("CARGO_CRATE_NAME")) + }; +} + +/// Tracing identity and layer factory for one compute-driver service. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ComputeDriverTracing { + service_name: &'static str, + crate_target_prefix: &'static str, +} + +impl ComputeDriverTracing { + /// `crate_target_prefix` must be the driver's Rust crate name; it routes + /// that crate's spans to the driver's provider. Prefer + /// [`compute_driver_tracing!`](crate::compute_driver_tracing). + #[must_use] + pub const fn new(service_name: &'static str, crate_target_prefix: &'static str) -> Self { + Self { + service_name, + crate_target_prefix, + } + } + + #[must_use] + pub const fn service_name(self) -> &'static str { + self.service_name + } + + /// Canonical compute-driver name derived from `service.name`. + #[must_use] + pub fn compute_driver(self) -> &'static str { + self.service_name + .strip_prefix("openshell-driver-") + .unwrap_or(self.service_name) + } + + #[must_use] + pub const fn in_process_target(self) -> &'static str { + IN_PROCESS_COMPUTE_DRIVER_TARGET + } + + /// Targets routed to this driver's provider when it runs in-process. + #[must_use] + pub const fn in_process_targets(self) -> [&'static str; 2] { + [self.in_process_target(), self.crate_target_prefix] + } + + #[must_use] + pub fn provider_for( + self, + endpoint: Option<&str>, + service_version: &'static str, + gateway_name: Option<&str>, + compute_driver: Option<&str>, + ) -> (Option, Option) { + crate::provider_for(endpoint.map(|endpoint| OtlpTraceConfig { + endpoint, + service_name: ServiceName::Fixed(self.service_name), + service_version: Some(service_version), + resource_attributes: crate::gateway_resource_attributes(gateway_name, compute_driver), + })) + } + + pub fn layer(self, provider: &SdkTracerProvider) -> crate::OtlpLayer + where + S: tracing::Subscriber + for<'span> LookupSpan<'span>, + { + crate::layer(provider, self.service_name) + } + + pub fn in_process_layer(self, provider: &SdkTracerProvider) -> crate::TargetOtlpLayer + where + S: tracing::Subscriber + for<'span> LookupSpan<'span>, + { + crate::layer_for_target_prefixes(provider, self.service_name, self.in_process_targets()) + } +} + +/// Optional in-process server boundary tracing for a compute-driver service. +#[derive(Debug, Clone, Copy)] +pub struct InProcessRpcTracer { + enabled: bool, +} + +impl InProcessRpcTracer { + #[must_use] + pub const fn disabled() -> Self { + Self { enabled: false } + } + + #[must_use] + pub const fn enabled() -> Self { + Self { enabled: true } + } + + fn span(self, rpc: crate::ComputeDriverRpc) -> Option { + self.enabled.then(|| { + tracing::info_span!( + target: IN_PROCESS_COMPUTE_DRIVER_TARGET, + "driver_rpc", + otel.name = rpc.operation, + otel.kind = "server", + otel.status_code = tracing::field::Empty, + rpc.system.name = "grpc", + rpc.method = rpc.operation, + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, + ) + }) + } + + pub async fn trace( + self, + rpc: crate::ComputeDriverRpc, + future: impl Future>, + ) -> Result { + let Some(span) = self.span(rpc) else { + return future.await; + }; + let result = future.instrument(span.clone()).await; + record_result(&span, &result); + result + } + + pub async fn trace_stream( + self, + rpc: crate::ComputeDriverRpc, + future: impl Future, tonic::Status>>, + ) -> Result, tonic::Status> + where + T: 'static, + { + let Some(span) = self.span(rpc) else { + return future.await; + }; + match future.instrument(span.clone()).await { + Ok(stream) => Ok(Box::pin(crate::TracedGrpcStream::new(stream, span))), + Err(status) => { + crate::record_grpc_status(&span, status.code()); + Err(status) + } + } + } +} + +pub type BoxGrpcStream = Pin> + Send + 'static>>; + +fn record_result(span: &tracing::Span, result: &Result) { + crate::record_grpc_status( + span, + result + .as_ref() + .map_or_else(tonic::Status::code, |_| tonic::Code::Ok), + ); +} + +/// Owns and shuts down a standalone compute driver's tracer provider. +pub struct DriverTracingHandle { + provider: Option, +} + +/// Named inputs for standalone compute-driver tracing installation. +#[derive(Debug, Clone, Copy)] +pub struct DriverTracingConfig<'a> { + pub endpoint: Option<&'a str>, + pub gateway_name: Option<&'a str>, + pub service_version: &'static str, + pub log_level: &'a str, +} + +impl Drop for DriverTracingHandle { + fn drop(&mut self) { + if let Some(provider) = &self.provider + && let Err(error) = provider.shutdown() + { + tracing::warn!(%error, "OTLP tracer provider shutdown failed"); + } + } +} + +/// Install standalone compute-driver logging and OTLP tracing. +#[must_use] +pub fn install_driver_tracing( + descriptor: ComputeDriverTracing, + config: DriverTracingConfig<'_>, +) -> DriverTracingHandle { + let (provider, setup_error) = descriptor.provider_for( + config.endpoint, + config.service_version, + config.gateway_name, + Some(descriptor.compute_driver()), + ); + tracing_subscriber::registry() + .with( + EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new(config.log_level)), + ) + .with(tracing_subscriber::fmt::layer()) + .with(provider.as_ref().map(|provider| descriptor.layer(provider))) + .init(); + + if let Some(error) = setup_error { + tracing::error!(%error, "OTLP exporting could not be started"); + } else if let Some(endpoint) = config.endpoint { + tracing::info!(endpoint, "OTLP exporting enabled"); + } + + DriverTracingHandle { provider } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn tracing_macro_derives_identity_from_the_calling_crate() { + const TRACING: ComputeDriverTracing = crate::compute_driver_tracing!(); + + assert_eq!(TRACING.service_name(), "openshell-otel"); + assert_eq!(TRACING.in_process_targets()[1], "openshell_otel"); + } + + #[tokio::test] + async fn disabled_stream_tracing_returns_the_original_box() { + let stream: BoxGrpcStream<()> = Box::pin(futures::stream::empty()); + let original = std::ptr::from_ref(stream.as_ref().get_ref()); + + let returned = InProcessRpcTracer::disabled() + .trace_stream(crate::rpc::WATCH_SANDBOXES, async move { Ok(stream) }) + .await + .unwrap(); + + assert!(std::ptr::eq(original, returned.as_ref().get_ref())); + } +} diff --git a/crates/openshell-otel/src/grpc.rs b/crates/openshell-otel/src/grpc.rs index 3c38ddd74a..65d498eccb 100644 --- a/crates/openshell-otel/src/grpc.rs +++ b/crates/openshell-otel/src/grpc.rs @@ -3,6 +3,7 @@ //! Shared gRPC tracing adapters. +use futures::Stream; use http::Request; use opentelemetry::propagation::TextMapPropagator as _; use opentelemetry::trace::TraceContextExt as _; @@ -12,7 +13,132 @@ use tower_http::trace::{GrpcMakeClassifier, MakeSpan, OnEos, OnFailure, OnRespon use tracing::Span; use tracing_opentelemetry::OpenTelemetrySpanExt as _; -const COMPUTE_DRIVER_SERVICE: &str = "openshell.compute.v1.ComputeDriver"; +/// Keeps a gRPC server span alive for a response stream and records its outcome. +pub struct TracedGrpcStream { + inner: S, + span: Span, + finished: bool, +} + +impl TracedGrpcStream { + #[must_use] + pub fn new(inner: S, span: Span) -> Self { + Self { + inner, + span, + finished: false, + } + } +} + +impl Stream for TracedGrpcStream +where + S: Stream> + Unpin, +{ + type Item = Result; + + fn poll_next( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + let this = self.get_mut(); + let span = this.span.clone(); + let _entered = span.enter(); + let result = std::pin::Pin::new(&mut this.inner).poll_next(cx); + if !this.finished { + match &result { + std::task::Poll::Ready(Some(Err(status))) => { + record_grpc_status(&this.span, status.code()); + this.finished = true; + } + std::task::Poll::Ready(None) => { + record_grpc_status(&this.span, tonic::Code::Ok); + this.finished = true; + } + std::task::Poll::Pending | std::task::Poll::Ready(Some(Ok(_))) => {} + } + } + result + } +} + +pub const COMPUTE_DRIVER_RPC_SERVICE: &str = "openshell.compute.v1.ComputeDriver"; + +/// Low-cardinality semantic-convention identity for a compute-driver RPC. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ComputeDriverRpc { + pub service: &'static str, + pub method: &'static str, + pub operation: &'static str, +} + +impl ComputeDriverRpc { + const fn new(method: &'static str, operation: &'static str) -> Self { + Self { + service: COMPUTE_DRIVER_RPC_SERVICE, + method, + operation, + } + } +} + +/// Typed identities for every RPC in the generated compute-driver service. +pub mod rpc { + use super::ComputeDriverRpc; + + pub const AUTHENTICATE_SANDBOX: ComputeDriverRpc = ComputeDriverRpc::new( + "AuthenticateSandbox", + "openshell.compute.v1.ComputeDriver/AuthenticateSandbox", + ); + pub const GET_CAPABILITIES: ComputeDriverRpc = ComputeDriverRpc::new( + "GetCapabilities", + "openshell.compute.v1.ComputeDriver/GetCapabilities", + ); + pub const GET_GATEWAY_LISTENER_REQUIREMENTS: ComputeDriverRpc = ComputeDriverRpc::new( + "GetGatewayListenerRequirements", + "openshell.compute.v1.ComputeDriver/GetGatewayListenerRequirements", + ); + pub const VALIDATE_SANDBOX_CREATE: ComputeDriverRpc = ComputeDriverRpc::new( + "ValidateSandboxCreate", + "openshell.compute.v1.ComputeDriver/ValidateSandboxCreate", + ); + pub const CREATE_SANDBOX: ComputeDriverRpc = ComputeDriverRpc::new( + "CreateSandbox", + "openshell.compute.v1.ComputeDriver/CreateSandbox", + ); + pub const GET_SANDBOX: ComputeDriverRpc = ComputeDriverRpc::new( + "GetSandbox", + "openshell.compute.v1.ComputeDriver/GetSandbox", + ); + pub const LIST_SANDBOXES: ComputeDriverRpc = ComputeDriverRpc::new( + "ListSandboxes", + "openshell.compute.v1.ComputeDriver/ListSandboxes", + ); + pub const STOP_SANDBOX: ComputeDriverRpc = ComputeDriverRpc::new( + "StopSandbox", + "openshell.compute.v1.ComputeDriver/StopSandbox", + ); + pub const START_SANDBOX: ComputeDriverRpc = ComputeDriverRpc::new( + "StartSandbox", + "openshell.compute.v1.ComputeDriver/StartSandbox", + ); + pub const DELETE_SANDBOX: ComputeDriverRpc = ComputeDriverRpc::new( + "DeleteSandbox", + "openshell.compute.v1.ComputeDriver/DeleteSandbox", + ); + pub const WATCH_SANDBOXES: ComputeDriverRpc = ComputeDriverRpc::new( + "WatchSandboxes", + "openshell.compute.v1.ComputeDriver/WatchSandboxes", + ); + pub const ENSURE_WORKSPACE: ComputeDriverRpc = ComputeDriverRpc::new( + "EnsureWorkspace", + "openshell.compute.v1.ComputeDriver/EnsureWorkspace", + ); + pub const DELETE_WORKSPACE: ComputeDriverRpc = ComputeDriverRpc::new( + "DeleteWorkspace", + "openshell.compute.v1.ComputeDriver/DeleteWorkspace", + ); +} /// Trace every inbound compute-driver RPC at the tonic service boundary. pub fn compute_driver_rpc_layer() -> TraceLayer< @@ -39,16 +165,16 @@ pub struct ComputeDriverRpcSpan; impl MakeSpan for ComputeDriverRpcSpan { fn make_span(&mut self, request: &Request) -> Span { - let (operation, method) = compute_driver_rpc_operation(request.uri().path()); + let rpc = compute_driver_rpc_operation(request.uri().path()); let span = tracing::info_span!( "driver_rpc", - otel.name = operation, + otel.name = rpc.map_or("grpc", |rpc| rpc.operation), otel.kind = "server", otel.status_code = tracing::field::Empty, - rpc.system = "grpc", - rpc.service = COMPUTE_DRIVER_SERVICE, - rpc.method = method, - rpc.grpc.status_code = tracing::field::Empty, + rpc.system.name = "grpc", + rpc.method = rpc.map_or("_OTHER", |rpc| rpc.operation), + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, ); let parent = TraceContextPropagator::new().extract_with_context( &opentelemetry::Context::new(), @@ -62,26 +188,62 @@ impl MakeSpan for ComputeDriverRpcSpan { } /// Maps the generated compute-driver RPC schema to low-cardinality span names. -pub fn compute_driver_rpc_operation(path: &str) -> (&'static str, &'static str) { +pub fn compute_driver_rpc_operation(path: &str) -> Option { match path.rsplit('/').next() { - Some("GetCapabilities") => ("driver.get_capabilities", "get_capabilities"), - Some("GetGatewayListenerRequirements") => ( - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - ), - Some("ValidateSandboxCreate") => { - ("driver.validate_sandbox_create", "validate_sandbox_create") - } - Some("CreateSandbox") => ("driver.create_sandbox", "create_sandbox"), - Some("GetSandbox") => ("driver.get_sandbox", "get_sandbox"), - Some("ListSandboxes") => ("driver.list_sandboxes", "list_sandboxes"), - Some("StopSandbox") => ("driver.stop_sandbox", "stop_sandbox"), - Some("StartSandbox") => ("driver.start_sandbox", "start_sandbox"), - Some("DeleteSandbox") => ("driver.delete_sandbox", "delete_sandbox"), - Some("WatchSandboxes") => ("driver.watch_sandboxes", "watch_sandboxes"), - Some("EnsureWorkspace") => ("driver.ensure_workspace", "ensure_workspace"), - Some("DeleteWorkspace") => ("driver.delete_workspace", "delete_workspace"), - _ => ("driver.unknown", "unknown"), + Some("AuthenticateSandbox") => Some(rpc::AUTHENTICATE_SANDBOX), + Some("GetCapabilities") => Some(rpc::GET_CAPABILITIES), + Some("GetGatewayListenerRequirements") => Some(rpc::GET_GATEWAY_LISTENER_REQUIREMENTS), + Some("ValidateSandboxCreate") => Some(rpc::VALIDATE_SANDBOX_CREATE), + Some("CreateSandbox") => Some(rpc::CREATE_SANDBOX), + Some("GetSandbox") => Some(rpc::GET_SANDBOX), + Some("ListSandboxes") => Some(rpc::LIST_SANDBOXES), + Some("StopSandbox") => Some(rpc::STOP_SANDBOX), + Some("StartSandbox") => Some(rpc::START_SANDBOX), + Some("DeleteSandbox") => Some(rpc::DELETE_SANDBOX), + Some("WatchSandboxes") => Some(rpc::WATCH_SANDBOXES), + Some("EnsureWorkspace") => Some(rpc::ENSURE_WORKSPACE), + Some("DeleteWorkspace") => Some(rpc::DELETE_WORKSPACE), + _ => None, + } +} + +/// Return the stable OpenTelemetry spelling for a gRPC response status. +#[must_use] +pub const fn grpc_status_code_name(code: tonic::Code) -> &'static str { + match code { + tonic::Code::Ok => "OK", + tonic::Code::Cancelled => "CANCELLED", + tonic::Code::Unknown => "UNKNOWN", + tonic::Code::InvalidArgument => "INVALID_ARGUMENT", + tonic::Code::DeadlineExceeded => "DEADLINE_EXCEEDED", + tonic::Code::NotFound => "NOT_FOUND", + tonic::Code::AlreadyExists => "ALREADY_EXISTS", + tonic::Code::PermissionDenied => "PERMISSION_DENIED", + tonic::Code::ResourceExhausted => "RESOURCE_EXHAUSTED", + tonic::Code::FailedPrecondition => "FAILED_PRECONDITION", + tonic::Code::Aborted => "ABORTED", + tonic::Code::OutOfRange => "OUT_OF_RANGE", + tonic::Code::Unimplemented => "UNIMPLEMENTED", + tonic::Code::Internal => "INTERNAL", + tonic::Code::Unavailable => "UNAVAILABLE", + tonic::Code::DataLoss => "DATA_LOSS", + tonic::Code::Unauthenticated => "UNAUTHENTICATED", + } +} + +/// Records a complete gRPC outcome on `span`. +/// +/// Non-OK codes also set `otel.status_code` to `ERROR` and record the code as +/// `error.type`. OK codes leave the OpenTelemetry span status unset. +/// +/// The span must declare `otel.status_code`, `rpc.response.status_code`, and +/// `error.type` when it is created because `tracing` ignores undeclared fields. +pub fn record_grpc_status(span: &Span, code: tonic::Code) { + let code_name = grpc_status_code_name(code); + span.record("rpc.response.status_code", code_name); + if code != tonic::Code::Ok { + crate::mark_error(span); + span.record("error.type", code_name); } } @@ -96,9 +258,11 @@ impl OnFailure for RecordGrpcFailure { _latency: std::time::Duration, span: &Span, ) { - crate::mark_error(span); if let GrpcFailureClass::Code(code) = failure { - span.record("rpc.grpc.status_code", code.get()); + let code = tonic::Code::from_i32(code.get()); + record_grpc_status(span, code); + } else { + crate::mark_error(span); } } } @@ -116,10 +280,8 @@ impl RecordGrpcStatus { else { return; }; - if code != tonic::Code::Ok as i32 { - crate::mark_error(span); - } - span.record("rpc.grpc.status_code", code); + let code = tonic::Code::from_i32(code); + record_grpc_status(span, code); } } @@ -150,57 +312,77 @@ mod tests { #[test] fn compute_driver_rpc_names_are_explicitly_mapped_and_schema_bounded() { - for (rpc, operation, method) in [ - ( - "GetCapabilities", - "driver.get_capabilities", - "get_capabilities", - ), - ( - "GetGatewayListenerRequirements", - "driver.get_gateway_listener_requirements", - "get_gateway_listener_requirements", - ), - ( - "ValidateSandboxCreate", - "driver.validate_sandbox_create", - "validate_sandbox_create", - ), - ("CreateSandbox", "driver.create_sandbox", "create_sandbox"), - ("GetSandbox", "driver.get_sandbox", "get_sandbox"), - ("ListSandboxes", "driver.list_sandboxes", "list_sandboxes"), - ("StopSandbox", "driver.stop_sandbox", "stop_sandbox"), - ("StartSandbox", "driver.start_sandbox", "start_sandbox"), - ("DeleteSandbox", "driver.delete_sandbox", "delete_sandbox"), - ( - "WatchSandboxes", - "driver.watch_sandboxes", - "watch_sandboxes", - ), - ( - "EnsureWorkspace", - "driver.ensure_workspace", - "ensure_workspace", - ), - ( - "DeleteWorkspace", - "driver.delete_workspace", - "delete_workspace", - ), + for rpc in [ + rpc::AUTHENTICATE_SANDBOX, + rpc::GET_CAPABILITIES, + rpc::GET_GATEWAY_LISTENER_REQUIREMENTS, + rpc::VALIDATE_SANDBOX_CREATE, + rpc::CREATE_SANDBOX, + rpc::GET_SANDBOX, + rpc::LIST_SANDBOXES, + rpc::STOP_SANDBOX, + rpc::START_SANDBOX, + rpc::DELETE_SANDBOX, + rpc::WATCH_SANDBOXES, + rpc::ENSURE_WORKSPACE, + rpc::DELETE_WORKSPACE, ] { + assert_eq!(rpc.operation, format!("{}/{}", rpc.service, rpc.method)); assert_eq!( - compute_driver_rpc_operation(&format!("/openshell.compute.v1.ComputeDriver/{rpc}")), - (operation, method) + compute_driver_rpc_operation(&format!("/{}/{}", rpc.service, rpc.method)), + Some(rpc) ); } assert_eq!( compute_driver_rpc_operation( "/openshell.compute.v1.ComputeDriver/AttackerControlled12345" ), - ("driver.unknown", "unknown") + None ); } + #[test] + fn traced_stream_records_an_observed_cancelled_status() { + let _tracing_lock = crate::test_lock(); + let exporter = InMemorySpanExporterBuilder::new().build(); + let provider = SdkTracerProvider::builder() + .with_simple_exporter(exporter.clone()) + .build(); + let subscriber = tracing_subscriber::registry().with(crate::layer(&provider, "test")); + + tracing::subscriber::with_default(subscriber, || { + let span = tracing::info_span!( + "watch", + otel.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, + ); + let inner = + futures::stream::iter([Err::<(), _>(tonic::Status::cancelled("peer cancelled"))]); + let mut stream = TracedGrpcStream::new(inner, span); + let mut cx = std::task::Context::from_waker(futures::task::noop_waker_ref()); + let result = std::pin::Pin::new(&mut stream).poll_next(&mut cx); + assert!(matches!( + result, + std::task::Poll::Ready(Some(Err(status))) + if status.code() == tonic::Code::Cancelled + )); + }); + provider.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + assert_eq!(spans.len(), 1); + assert!(matches!( + spans[0].status, + opentelemetry::trace::Status::Error { .. } + )); + assert!(spans[0].attributes.iter().any(|attribute| { + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "CANCELLED" + })); + provider.shutdown().unwrap(); + } + #[test] fn grpc_status_records_and_marks_non_ok_trailer_status() { let _tracing_lock = crate::test_lock(); @@ -216,7 +398,8 @@ mod tests { let span = tracing::info_span!( "rpc", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, ); RecordGrpcStatus.on_eos(Some(&trailers), std::time::Duration::ZERO, &span); }); @@ -229,8 +412,51 @@ mod tests { opentelemetry::trace::Status::Error { .. } )); assert!(spans[0].attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" && attribute.value.to_string() == "13" + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "INTERNAL" + })); + assert!(spans[0].attributes.iter().any(|attribute| { + attribute.key.as_str() == "error.type" && attribute.value.to_string() == "INTERNAL" + })); + provider.shutdown().unwrap(); + } + + #[test] + fn grpc_status_records_ok_without_marking_an_error() { + let _tracing_lock = crate::test_lock(); + let exporter = InMemorySpanExporterBuilder::new().build(); + let provider = SdkTracerProvider::builder() + .with_simple_exporter(exporter.clone()) + .build(); + let subscriber = tracing_subscriber::registry().with(crate::layer(&provider, "test")); + + tracing::subscriber::with_default(subscriber, || { + let span = tracing::info_span!( + "rpc", + otel.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, + ); + record_grpc_status(&span, tonic::Code::Ok); + }); + provider.force_flush().unwrap(); + + let spans = exporter.get_finished_spans().unwrap(); + assert_eq!(spans.len(), 1); + assert!(matches!( + spans[0].status, + opentelemetry::trace::Status::Unset + )); + assert!(spans[0].attributes.iter().any(|attribute| { + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "OK" })); + assert!( + spans[0] + .attributes + .iter() + .all(|attribute| attribute.key.as_str() != "error.type") + ); provider.shutdown().unwrap(); } @@ -251,7 +477,7 @@ mod tests { let span = tracing::info_span!( "rpc", otel.status_code = tracing::field::Empty, - rpc.grpc.status_code = tracing::field::Empty, + rpc.response.status_code = tracing::field::Empty, ); RecordGrpcStatus.on_response(&response, std::time::Duration::ZERO, &span); RecordGrpcStatus.on_eos(None, std::time::Duration::ZERO, &span); @@ -265,7 +491,8 @@ mod tests { opentelemetry::trace::Status::Error { .. } )); assert!(spans[0].attributes.iter().any(|attribute| { - attribute.key.as_str() == "rpc.grpc.status_code" && attribute.value.to_string() == "13" + attribute.key.as_str() == "rpc.response.status_code" + && attribute.value.to_string() == "INTERNAL" })); provider.shutdown().unwrap(); } diff --git a/crates/openshell-otel/src/lib.rs b/crates/openshell-otel/src/lib.rs index 30a9d29730..7a9162ab92 100644 --- a/crates/openshell-otel/src/lib.rs +++ b/crates/openshell-otel/src/lib.rs @@ -3,12 +3,19 @@ //! Shared OpenTelemetry trace export support for `OpenShell` services. +mod driver; mod grpc; mod propagation; +pub use driver::{ + BoxGrpcStream, ComputeDriverTracing, DriverTracingConfig, DriverTracingHandle, + IN_PROCESS_COMPUTE_DRIVER_TARGET, InProcessRpcTracer, install_driver_tracing, +}; + pub use grpc::{ - ComputeDriverRpcSpan, RecordGrpcFailure, RecordGrpcStatus, compute_driver_rpc_layer, - compute_driver_rpc_operation, + COMPUTE_DRIVER_RPC_SERVICE, ComputeDriverRpc, ComputeDriverRpcSpan, RecordGrpcFailure, + RecordGrpcStatus, TracedGrpcStream, compute_driver_rpc_layer, compute_driver_rpc_operation, + grpc_status_code_name, record_grpc_status, rpc, }; pub use propagation::{ HeaderMapExtractor, MetadataMapInjector, TraceContextInterceptor, current_trace_context_carrier, @@ -28,6 +35,27 @@ use tracing_subscriber::registry::LookupSpan; const SDK_UNKNOWN_SERVICE_PREFIX: &str = "unknown_service"; +/// Build the resource attributes shared by gateway and compute-driver providers. +pub fn gateway_resource_attributes( + gateway_name: Option<&str>, + compute_driver: Option<&str>, +) -> Vec { + let mut attributes = Vec::new(); + if let Some(name) = gateway_name.map(str::trim).filter(|name| !name.is_empty()) { + attributes.push(KeyValue::new("openshell.gateway.name", name.to_string())); + } + if let Some(driver) = compute_driver + .map(str::trim) + .filter(|driver| !driver.is_empty()) + { + attributes.push(KeyValue::new( + "openshell.gateway.compute_driver", + driver.to_string(), + )); + } + attributes +} + /// Mark `span` as failed. /// /// The field must be declared on the span at creation because `tracing` drops @@ -202,17 +230,18 @@ pub type OtlpLayer = tracing_subscriber::filter::Filtered< pub type TargetOtlpLayer = tracing_subscriber::filter::Filtered, TargetPrefixFilter, S>; -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] pub struct TargetPrefixFilter { - prefix: Option<&'static str>, + prefixes: Vec<&'static str>, include: bool, } impl Filter for TargetPrefixFilter { fn enabled(&self, metadata: &tracing::Metadata<'_>, _ctx: &Context<'_, S>) -> bool { let matches_prefix = self - .prefix - .is_some_and(|prefix| metadata.target().starts_with(prefix)); + .prefixes + .iter() + .any(|prefix| metadata.target().starts_with(prefix)); metadata.is_span() && !metadata.target().starts_with("opentelemetry") && (matches_prefix == self.include) @@ -237,13 +266,25 @@ pub fn layer_for_target_prefix( instrumentation_scope: &'static str, target_prefix: &'static str, ) -> TargetOtlpLayer +where + S: Subscriber + for<'span> LookupSpan<'span>, +{ + layer_for_target_prefixes(provider, instrumentation_scope, [target_prefix]) +} + +/// Build a tracing layer that exports only spans from `target_prefixes`. +pub fn layer_for_target_prefixes( + provider: &SdkTracerProvider, + instrumentation_scope: &'static str, + target_prefixes: impl IntoIterator, +) -> TargetOtlpLayer where S: Subscriber + for<'span> LookupSpan<'span>, { tracing_opentelemetry::layer() .with_tracer(provider.tracer(instrumentation_scope)) .with_filter(TargetPrefixFilter { - prefix: Some(target_prefix), + prefixes: target_prefixes.into_iter().collect(), include: true, }) } @@ -256,13 +297,27 @@ pub fn layer_excluding_target_prefix( instrumentation_scope: &'static str, target_prefix: Option<&'static str>, ) -> TargetOtlpLayer +where + S: Subscriber + for<'span> LookupSpan<'span>, +{ + layer_excluding_target_prefixes(provider, instrumentation_scope, target_prefix) +} + +/// Build a tracing layer that excludes spans from `target_prefixes`. +/// +/// An empty iterator excludes no application spans. +pub fn layer_excluding_target_prefixes( + provider: &SdkTracerProvider, + instrumentation_scope: &'static str, + target_prefixes: impl IntoIterator, +) -> TargetOtlpLayer where S: Subscriber + for<'span> LookupSpan<'span>, { tracing_opentelemetry::layer() .with_tracer(provider.tracer(instrumentation_scope)) .with_filter(TargetPrefixFilter { - prefix: target_prefix, + prefixes: target_prefixes.into_iter().collect(), include: false, }) } diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index f18dbe07aa..33008774b9 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -513,11 +513,9 @@ async fn run_from_args( Some(prepared.config.name.as_str()), Some(prepared.compute_driver.name()), ); - let compute_driver_tracing = compute_drivers.tracing_setup( + let compute_driver_tracing = compute_drivers.in_process_tracing( &prepared.compute_driver, &prepared.config.compute_driver_endpoints, - otlp_config.map(|config| config.endpoint.as_str()), - Some(prepared.config.name.as_str()), ); let (tracing_handle, setup_error) = crate::tracing_setup::install( EnvFilter::try_from_default_env() diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 6aa1f70a0c..86ac0b5e88 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -75,7 +75,9 @@ mod traced_driver { use tonic::Status; use tracing::Instrument as _; - use super::SharedComputeDriver; + use super::{DriverWatchStream, SharedComputeDriver}; + + type TracedWatchStream = openshell_otel::TracedGrpcStream; #[derive(Clone)] pub(super) struct TracedDriver { @@ -88,45 +90,83 @@ mod traced_driver { Self { inner, name } } - /// Run one call across the driver boundary inside its span. - /// - /// Takes a closure rather than a future so the call cannot be built - /// without going through here. - pub(super) async fn call( + fn span( &self, - operation: &'static str, + rpc: openshell_otel::ComputeDriverRpc, sandbox_id: Option<&str>, - call: impl FnOnce(SharedComputeDriver) -> Fut, - ) -> Result - where - Fut: Future>, - { + ) -> tracing::Span { let span = tracing::info_span!( "driver", - otel.name = operation, + otel.name = rpc.operation, otel.kind = "client", otel.status_code = tracing::field::Empty, driver.name = %self.name, sandbox.id = tracing::field::Empty, - grpc.code = tracing::field::Empty, + rpc.system.name = "grpc", + rpc.method = rpc.operation, + rpc.response.status_code = tracing::field::Empty, + error.type = tracing::field::Empty, ); if let Some(sandbox_id) = sandbox_id { span.record("sandbox.id", sandbox_id); } + span + } + + /// Run one call across the driver boundary inside its span. + /// + /// Takes a closure rather than a future so the call cannot be built + /// without going through here. + pub(super) async fn call( + &self, + rpc: openshell_otel::ComputeDriverRpc, + sandbox_id: Option<&str>, + call: impl FnOnce(SharedComputeDriver) -> Fut, + ) -> Result + where + Fut: Future>, + { + let span = self.span(rpc, sandbox_id); let future = call(self.inner.clone()); async { let result = future.await; - if let Err(status) = &result { - let current = tracing::Span::current(); - crate::otel_tracing::mark_error(¤t); - current.record("grpc.code", status.code() as i32); + let current = tracing::Span::current(); + match &result { + Ok(_) => { + openshell_otel::record_grpc_status(¤t, tonic::Code::Ok); + } + Err(status) => { + openshell_otel::record_grpc_status(¤t, status.code()); + } } result } .instrument(span) .await } + + /// Open a driver watch while keeping the client span alive with the stream. + pub(super) async fn watch(&self) -> Result, Status> { + let span = self.span(openshell_otel::rpc::WATCH_SANDBOXES, None); + let result = self + .inner + .clone() + .watch_sandboxes(tonic::Request::new(super::WatchSandboxesRequest {})) + .instrument(span.clone()) + .await; + match result { + Ok(response) => { + let (metadata, inner, extensions) = response.into_parts(); + let stream: DriverWatchStream = Box::pin(TracedWatchStream::new(inner, span)); + Ok(tonic::Response::from_parts(metadata, stream, extensions)) + } + Err(status) => { + openshell_otel::record_grpc_status(&span, status.code()); + Err(status) + } + } + } } } @@ -763,9 +803,11 @@ impl ComputeRuntime { credential: credential.to_string(), }; self.driver - .call("driver.authenticate_sandbox", None, |driver| async move { - driver.authenticate_sandbox(Request::new(request)).await - }) + .call( + openshell_otel::rpc::AUTHENTICATE_SANDBOX, + None, + |driver| async move { driver.authenticate_sandbox(Request::new(request)).await }, + ) .await .map(|response| response.into_inner().sandbox_id) } @@ -793,11 +835,15 @@ impl ComputeRuntime { let workspace = workspace.to_string(); match self .driver - .call("driver.ensure_workspace", None, |driver| async move { - driver - .ensure_workspace(Request::new(EnsureWorkspaceRequest { workspace })) - .await - }) + .call( + openshell_otel::rpc::ENSURE_WORKSPACE, + None, + |driver| async move { + driver + .ensure_workspace(Request::new(EnsureWorkspaceRequest { workspace })) + .await + }, + ) .await { Ok(_) => Ok(()), @@ -810,11 +856,15 @@ impl ComputeRuntime { let workspace = workspace.to_string(); match self .driver - .call("driver.delete_workspace", None, |driver| async move { - driver - .delete_workspace(Request::new(DeleteWorkspaceRequest { workspace })) - .await - }) + .call( + openshell_otel::rpc::DELETE_WORKSPACE, + None, + |driver| async move { + driver + .delete_workspace(Request::new(DeleteWorkspaceRequest { workspace })) + .await + }, + ) .await { Ok(_) => Ok(()), @@ -828,7 +878,7 @@ impl ComputeRuntime { .map_err(|status| *status)?; self.driver .call( - "driver.validate_sandbox_create", + openshell_otel::rpc::VALIDATE_SANDBOX_CREATE, Some(sandbox.object_id()), |driver| async move { driver @@ -901,7 +951,7 @@ impl ComputeRuntime { match self .driver .call( - "driver.create_sandbox", + openshell_otel::rpc::CREATE_SANDBOX, Some(sandbox.object_id()), |driver| async move { driver @@ -1050,18 +1100,22 @@ impl ComputeRuntime { ) -> Result { let result = self .driver - .call("driver.stop_sandbox", Some(&sandbox_id), |driver| { - let sandbox_id = sandbox_id.clone(); - let sandbox_name = sandbox_name.clone(); - async move { - driver - .stop_sandbox(Request::new(StopSandboxRequest { - sandbox_id, - sandbox_name, - })) - .await - } - }) + .call( + openshell_otel::rpc::STOP_SANDBOX, + Some(&sandbox_id), + |driver| { + let sandbox_id = sandbox_id.clone(); + let sandbox_name = sandbox_name.clone(); + async move { + driver + .stop_sandbox(Request::new(StopSandboxRequest { + sandbox_id, + sandbox_name, + })) + .await + } + }, + ) .await; match result { @@ -1210,18 +1264,22 @@ impl ComputeRuntime { ) -> Result { let result = self .driver - .call("driver.start_sandbox", Some(&sandbox_id), |driver| { - let sandbox_id = sandbox_id.clone(); - let sandbox_name = sandbox_name.clone(); - async move { - driver - .start_sandbox(Request::new(StartSandboxRequest { - sandbox_id, - sandbox_name, - })) - .await - } - }) + .call( + openshell_otel::rpc::START_SANDBOX, + Some(&sandbox_id), + |driver| { + let sandbox_id = sandbox_id.clone(); + let sandbox_name = sandbox_name.clone(); + async move { + driver + .start_sandbox(Request::new(StartSandboxRequest { + sandbox_id, + sandbox_name, + })) + .await + } + }, + ) .await; match result { @@ -1505,7 +1563,7 @@ impl ComputeRuntime { let result = self .driver .call( - "driver.delete_sandbox", + openshell_otel::rpc::DELETE_SANDBOX, Some(transition.deleting.object_id()), |driver| { let sandbox_id = transition.deleting.object_id().to_string(); @@ -2067,18 +2125,22 @@ impl ComputeRuntime { let sandbox_name = sandbox.object_name().to_string(); match self .driver - .call("driver.stop_sandbox", Some(&sandbox_id), |driver| { - let sandbox_id = sandbox_id.clone(); - let sandbox_name = sandbox_name.clone(); - async move { - driver - .stop_sandbox(Request::new(StopSandboxRequest { - sandbox_id, - sandbox_name, - })) - .await - } - }) + .call( + openshell_otel::rpc::STOP_SANDBOX, + Some(&sandbox_id), + |driver| { + let sandbox_id = sandbox_id.clone(); + let sandbox_name = sandbox_name.clone(); + async move { + driver + .stop_sandbox(Request::new(StopSandboxRequest { + sandbox_id, + sandbox_name, + })) + .await + } + }, + ) .await { Ok(_) => { @@ -2175,18 +2237,22 @@ impl ComputeRuntime { let sandbox_name = sandbox.object_name().to_string(); match self .driver - .call("driver.start_sandbox", Some(&sandbox_id), |driver| { - let sandbox_id = sandbox_id.clone(); - let sandbox_name = sandbox_name.clone(); - async move { - driver - .start_sandbox(Request::new(StartSandboxRequest { - sandbox_id, - sandbox_name, - })) - .await - } - }) + .call( + openshell_otel::rpc::START_SANDBOX, + Some(&sandbox_id), + |driver| { + let sandbox_id = sandbox_id.clone(); + let sandbox_name = sandbox_name.clone(); + async move { + driver + .start_sandbox(Request::new(StartSandboxRequest { + sandbox_id, + sandbox_name, + })) + .await + } + }, + ) .await { Ok(_) => { @@ -2300,7 +2366,7 @@ impl ComputeRuntime { match self .driver .call( - "driver.stop_sandbox", + openshell_otel::rpc::STOP_SANDBOX, Some(&sandbox_id), |driver| async move { driver @@ -2347,7 +2413,7 @@ impl ComputeRuntime { if let Err(err) = self .driver .call( - "driver.start_sandbox", + openshell_otel::rpc::START_SANDBOX, Some(&sandbox_id), |driver| async move { driver @@ -2569,17 +2635,7 @@ impl ComputeRuntime { async fn watch_loop(self: Arc, mut cancel: watch::Receiver) { loop { - // Spans the stream open, not its lifetime: the future resolves - // once the driver accepts the watch. - let mut stream = match self - .driver - .call("driver.watch_sandboxes", None, |driver| async move { - driver - .watch_sandboxes(Request::new(WatchSandboxesRequest {})) - .await - }) - .await - { + let mut stream = match self.driver.watch().await { Ok(response) => response.into_inner(), Err(err) => { warn!(error = %err, "Compute driver watch stream failed to start"); @@ -2649,11 +2705,15 @@ impl ComputeRuntime { let sweep_started_at_ms = openshell_core::time::now_ms(); let backend_sandboxes = self .driver - .call("driver.list_sandboxes", None, |driver| async move { - driver - .list_sandboxes(Request::new(ListSandboxesRequest {})) - .await - }) + .call( + openshell_otel::rpc::LIST_SANDBOXES, + None, + |driver| async move { + driver + .list_sandboxes(Request::new(ListSandboxesRequest {})) + .await + }, + ) .await .map_err(|e| e.to_string()) .inspect_err(|_| crate::otel_tracing::mark_error(&tracing::Span::current()))? @@ -3281,18 +3341,22 @@ impl ComputeRuntime { async fn call_driver_delete_sandbox(&self, sandbox_id: &str, sandbox_name: &str) { let result = self .driver - .call("driver.delete_sandbox", Some(sandbox_id), |driver| { - let sandbox_id = sandbox_id.to_string(); - let sandbox_name = sandbox_name.to_string(); - async move { - driver - .delete_sandbox(Request::new(DeleteSandboxRequest { - sandbox_id, - sandbox_name, - })) - .await - } - }) + .call( + openshell_otel::rpc::DELETE_SANDBOX, + Some(sandbox_id), + |driver| { + let sandbox_id = sandbox_id.to_string(); + let sandbox_name = sandbox_name.to_string(); + async move { + driver + .delete_sandbox(Request::new(DeleteSandboxRequest { + sandbox_id, + sandbox_name, + })) + .await + } + }, + ) .await; if let Err(status) = result { @@ -3616,18 +3680,22 @@ impl ComputeRuntime { ) -> Result, String> { match self .driver - .call("driver.get_sandbox", Some(sandbox_id), |driver| { - let sandbox_id = sandbox_id.to_string(); - let sandbox_name = sandbox_name.to_string(); - async move { - driver - .get_sandbox(Request::new(GetSandboxRequest { - sandbox_id, - sandbox_name, - })) - .await - } - }) + .call( + openshell_otel::rpc::GET_SANDBOX, + Some(sandbox_id), + |driver| { + let sandbox_id = sandbox_id.to_string(); + let sandbox_name = sandbox_name.to_string(); + async move { + driver + .get_sandbox(Request::new(GetSandboxRequest { + sandbox_id, + sandbox_name, + })) + .await + } + }, + ) .await { Ok(response) => { @@ -6466,7 +6534,11 @@ mod tests { .instrument(tracing::info_span!("request")) .await; - let driver_span = traced.span_with("driver.create_sandbox", "sandbox.id", "sb-trace"); + let driver_span = traced.span_with( + "openshell.compute.v1.ComputeDriver/CreateSandbox", + "sandbox.id", + "sb-trace", + ); test_exporter::assert_has_parent(&driver_span); assert_eq!( test_exporter::attribute(&driver_span, "driver.name").as_deref(), @@ -6477,6 +6549,18 @@ mod tests { test_exporter::attribute(&driver_span, "sandbox.id").as_deref(), Some("sb-trace"), ); + assert_eq!( + test_exporter::attribute(&driver_span, "rpc.method").as_deref(), + Some("openshell.compute.v1.ComputeDriver/CreateSandbox"), + ); + assert!( + test_exporter::attribute(&driver_span, "rpc.service").is_none(), + "the current RPC semantic conventions integrate the service into rpc.method" + ); + assert_eq!( + test_exporter::attribute(&driver_span, "rpc.response.status_code").as_deref(), + Some("OK"), + ); assert_eq!( driver_span.span_kind, opentelemetry::trace::SpanKind::Client, @@ -6492,6 +6576,39 @@ mod tests { ); } + #[tokio::test] + async fn driver_watch_client_span_lives_until_the_stream_completes() { + use futures::StreamExt as _; + + use crate::otel_tracing::test_exporter; + + let traced = test_exporter::install_traced(); + let driver = TracedDriver::new(Arc::new(TestDriver::default()), "test-driver".to_string()); + let response = driver.watch().await.expect("watch opens"); + assert!( + traced + .finished_spans() + .iter() + .all(|span| span.name != "openshell.compute.v1.ComputeDriver/WatchSandboxes"), + "the client span must remain open while the response stream is alive" + ); + + let mut stream = response.into_inner(); + assert!(stream.next().await.is_none()); + drop(stream); + + let spans = traced.finished_spans(); + let span = spans + .iter() + .find(|span| span.name == "openshell.compute.v1.ComputeDriver/WatchSandboxes") + .expect("watch client span should finish with the stream"); + assert_eq!(span.span_kind, opentelemetry::trace::SpanKind::Client); + assert_eq!( + test_exporter::attribute(span, "rpc.response.status_code").as_deref(), + Some("OK"), + ); + } + /// A failing driver call must be visible as a failure in the trace, not /// just as a span that happens to be followed by nothing. #[tokio::test] @@ -6623,7 +6740,11 @@ mod tests { .instrument(tracing::info_span!("request")) .await; - let driver_span = traced.span_with("driver.create_sandbox", "sandbox.id", "sb-fail"); + let driver_span = traced.span_with( + "openshell.compute.v1.ComputeDriver/CreateSandbox", + "sandbox.id", + "sb-fail", + ); assert!( matches!( @@ -6634,8 +6755,8 @@ mod tests { driver_span.status ); assert_eq!( - test_exporter::attribute(&driver_span, "grpc.code").as_deref(), - Some("14"), + test_exporter::attribute(&driver_span, "rpc.response.status_code").as_deref(), + Some("UNAVAILABLE"), "the gRPC code names the cause without reading the message" ); } @@ -9635,7 +9756,7 @@ mod tests { .iter() .find(|root| { spans.iter().any(|span| { - span.name == "driver.list_sandboxes" + span.name == "openshell.compute.v1.ComputeDriver/ListSandboxes" && span.span_context.trace_id() == root.span_context.trace_id() }) && spans.iter().any(|span| { span.name.starts_with("store.") @@ -9648,7 +9769,7 @@ mod tests { let driver_span = spans .iter() .find(|span| { - span.name == "driver.list_sandboxes" + span.name == "openshell.compute.v1.ComputeDriver/ListSandboxes" && span.span_context.trace_id() == root.span_context.trace_id() }) .expect("the sweep records its driver call"); diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index 2acd34fc44..a8c8afdf08 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -1055,43 +1055,6 @@ pub enum ComputeDriverInstance { ManagedRemote(AcquiredRemoteDriverEndpoint), } -/// Type-erased tracing layer contributed by a compiled compute driver. -pub type ComputeDriverTracingLayer = - Box + Send + Sync>; - -/// Shutdown callback for resources owned by a compute-driver tracing layer. -pub type ComputeDriverTracingShutdown = - Box std::result::Result<(), String> + Send + Sync>; - -/// Optional process-wide tracing integration supplied by a compiled driver. -#[derive(Default)] -pub struct ComputeDriverTracingSetup { - layer: Option, - shutdown: Option, - error: Option, - target_prefix: Option<&'static str>, -} - -impl ComputeDriverTracingSetup { - #[must_use] - pub fn new( - layer: Option, - shutdown: Option, - error: Option, - target_prefix: Option<&'static str>, - ) -> Self { - Self { - layer, - shutdown, - error, - target_prefix, - } - } -} - -/// Factory for a compiled driver's optional tracing integration. -pub type ComputeDriverTracingFactory = fn(Option<&str>, Option<&str>) -> ComputeDriverTracingSetup; - /// Factory for a compute driver linked into a gateway binary. #[async_trait::async_trait] pub trait ComputeDriverFactory: Send + Sync { @@ -1109,7 +1072,7 @@ pub struct ComputeDriverRegistration { inherited_config_keys: &'static [&'static str], local_singleplayer: bool, supports_mtls_user_auth: bool, - tracing_setup: Option, + in_process_tracing: Option, } impl std::fmt::Debug for ComputeDriverRegistration { @@ -1142,7 +1105,7 @@ impl ComputeDriverRegistration { inherited_config_keys: &[], local_singleplayer: false, supports_mtls_user_auth: true, - tracing_setup: None, + in_process_tracing: None, }) } @@ -1175,10 +1138,13 @@ impl ComputeDriverRegistration { self } - /// Attach optional process-wide tracing for this compiled driver. + /// Attach process-wide tracing for this in-process compiled driver. #[must_use] - pub fn with_tracing_setup(mut self, setup: ComputeDriverTracingFactory) -> Self { - self.tracing_setup = Some(setup); + pub fn with_in_process_tracing( + mut self, + tracing: openshell_otel::ComputeDriverTracing, + ) -> Self { + self.in_process_tracing = Some(tracing); self } @@ -1191,6 +1157,11 @@ impl ComputeDriverRegistration { pub(crate) fn supports_mtls_user_auth(&self) -> bool { self.supports_mtls_user_auth } + + #[must_use] + pub fn in_process_tracing(&self) -> Option { + self.in_process_tracing + } } /// Registry of compute drivers compiled into this gateway binary. @@ -1259,22 +1230,17 @@ impl ComputeDriverRegistry { self.drivers.get(name) } - fn tracing_setup( + fn in_process_tracing( &self, selection: &ComputeDriverSelection, endpoint_overrides: &BTreeMap, - otlp_endpoint: Option<&str>, - gateway_name: Option<&str>, - ) -> ComputeDriverTracingSetup { + ) -> Option { let name = selection.name(); if endpoint_overrides.contains_key(name) { - return ComputeDriverTracingSetup::default(); + return None; } self.get(name) - .and_then(|registration| registration.tracing_setup) - .map_or_else(ComputeDriverTracingSetup::default, |setup| { - setup(otlp_endpoint, gateway_name) - }) + .and_then(ComputeDriverRegistration::in_process_tracing) } fn detect(&self) -> ComputeDriverDetection { diff --git a/crates/openshell-server/src/otel_tracing.rs b/crates/openshell-server/src/otel_tracing.rs index 16a94a7321..da12500050 100644 --- a/crates/openshell-server/src/otel_tracing.rs +++ b/crates/openshell-server/src/otel_tracing.rs @@ -51,6 +51,15 @@ impl<'a> GatewayResourceAttributes<'a> { compute_driver, } } + /// The configured gateway installation name, if any. + pub fn name(&self) -> Option<&'a str> { + self.name + } + + /// The configured compute-driver name, if any. + pub fn compute_driver(&self) -> Option<&'a str> { + self.compute_driver + } } fn trace_config<'cfg>( @@ -67,29 +76,14 @@ fn trace_config<'cfg>( ServiceName::Fixed, ); - let mut resource_attributes = Vec::new(); - if let Some(name) = gateway.name.map(str::trim).filter(|s| !s.is_empty()) { - resource_attributes.push(opentelemetry::KeyValue::new( - "openshell.gateway.name", - name.to_string(), - )); - } - if let Some(compute_driver) = gateway - .compute_driver - .map(str::trim) - .filter(|s| !s.is_empty()) - { - resource_attributes.push(opentelemetry::KeyValue::new( - "openshell.gateway.compute_driver", - compute_driver.to_string(), - )); - } - OtlpTraceConfig { endpoint: &cfg.endpoint, service_name, service_version: Some(openshell_core::VERSION), - resource_attributes, + resource_attributes: openshell_otel::gateway_resource_attributes( + gateway.name(), + gateway.compute_driver(), + ), } } @@ -135,15 +129,17 @@ pub fn provider_for( /// OpenTelemetry crates are excluded to prevent recursive export traffic. pub fn layer( provider: &SdkTracerProvider, - excluded_target_prefix: Option<&'static str>, + driver: Option, ) -> openshell_otel::TargetOtlpLayer where S: Subscriber + for<'span> LookupSpan<'span>, { - openshell_otel::layer_excluding_target_prefix( + openshell_otel::layer_excluding_target_prefixes( provider, INSTRUMENTATION_SCOPE, - excluded_target_prefix, + driver + .into_iter() + .flat_map(openshell_otel::ComputeDriverTracing::in_process_targets), ) } diff --git a/crates/openshell-server/src/tracing_setup.rs b/crates/openshell-server/src/tracing_setup.rs index 2bc583dac7..ed3f56c6e2 100644 --- a/crates/openshell-server/src/tracing_setup.rs +++ b/crates/openshell-server/src/tracing_setup.rs @@ -12,13 +12,12 @@ use tracing_subscriber::EnvFilter; use tracing_subscriber::prelude::*; use crate::config_file::OtlpConfig; -use crate::otel_tracing::GatewayResourceAttributes; +use crate::otel_tracing::{GatewayResourceAttributes, SetupError}; use crate::tracing_bus::TracingLogBus; -use crate::{ComputeDriverTracingSetup, ComputeDriverTracingShutdown}; pub struct TracingHandle { tracer_provider: Option, - compute_driver_shutdown: Option, + driver_tracer_provider: Option, } impl TracingHandle { @@ -28,10 +27,10 @@ impl TracingHandle { { tracing::warn!(error = %err, "OTLP tracer provider shutdown failed"); } - if let Some(shutdown) = &self.compute_driver_shutdown - && let Err(err) = shutdown() + if let Some(provider) = &self.driver_tracer_provider + && let Err(err) = provider.shutdown() { - tracing::warn!(error = %err, "Compute driver tracing shutdown failed"); + tracing::warn!(error = %err, "compute-driver OTLP tracer provider shutdown failed"); } } } @@ -40,34 +39,48 @@ pub fn install( env_filter: EnvFilter, tracing_log_bus: &TracingLogBus, otlp_config: Option<&OtlpConfig>, - compute_driver_tracing: ComputeDriverTracingSetup, + driver: Option, gateway: GatewayResourceAttributes<'_>, -) -> (TracingHandle, Option) { +) -> (TracingHandle, Option) { let (tracer_provider, setup_error) = crate::otel_tracing::provider_for(otlp_config, gateway); - let ComputeDriverTracingSetup { - layer, - shutdown, - error, - target_prefix, - } = compute_driver_tracing; + let driver_endpoint = driver + .is_some() + .then_some(otlp_config) + .flatten() + .map(|config| config.endpoint.as_str()); + let (driver_tracer_provider, driver_setup_error) = driver.map_or_else( + || (None, None), + |descriptor| { + descriptor.provider_for( + driver_endpoint, + openshell_core::VERSION, + gateway.name(), + gateway.compute_driver(), + ) + }, + ); tracing_subscriber::registry() - .with(layer) .with(env_filter) .with(tracing_subscriber::fmt::layer()) .with(tracing_log_bus.layer()) .with( tracer_provider .as_ref() - .map(|provider| crate::otel_tracing::layer(provider, target_prefix)), + .map(|provider| crate::otel_tracing::layer(provider, driver)), ) + .with(driver_tracer_provider.as_ref().map(|provider| { + driver + .expect("a driver provider requires a selected driver") + .in_process_layer(provider) + })) .init(); ( TracingHandle { tracer_provider, - compute_driver_shutdown: shutdown, + driver_tracer_provider, }, - setup_error.map(|error| error.to_string()).or(error), + setup_error.or(driver_setup_error), ) } diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 7553e7deb4..fa7d17a417 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -237,7 +237,7 @@ The OpenTelemetry SDK logs export failures after startup. Spans in a failed batc Only OpenTelemetry traces are exported. Inbound gRPC and HTTP requests produce server spans named for the RPC or HTTP method. Store and compute-driver operations appear as child spans. Internal reconciliation, credential-refresh, and driver-watch loops create operation roots for their store work because no inbound request supplies a parent. The gateway continues valid W3C `traceparent` context and starts a new trace when none is supplied. Request spans carry `method`, `path`, and the `request_id` that also appears in gateway logs. Health endpoint spans use DEBUG level and are not exported by the default INFO filter. -The gateway forwards the OTLP configuration, configured gateway name, and W3C trace context to managed external drivers. Built-in drivers also export their spans to the same collector through dedicated in-process providers. Driver spans retain the gateway trace context, use a distinct service name such as `openshell-driver-docker` or `openshell-driver-podman`, and carry the gateway name as the `openshell.gateway.name` resource attribute. Operator-run external drivers own their own telemetry configuration. +The gateway forwards the OTLP configuration, configured gateway name, configured compute driver, and W3C trace context to managed external drivers. Built-in drivers also export their spans to the same collector through dedicated in-process providers. Driver spans retain the gateway trace context, use a distinct service name such as `openshell-driver-docker` or `openshell-driver-podman`, and carry the same `openshell.gateway.name` and `openshell.gateway.compute_driver` resource attributes as gateway spans. Compute-driver client and server spans use the same fully qualified protobuf operation name, such as `openshell.compute.v1.ComputeDriver/CreateSandbox`, in both the span name and `rpc.method`. The service name and span kind distinguish each side. Backend-prefixed child spans identify implementation work. A streaming watch records a terminal status when observed; consumer teardown without a terminal status leaves the span status unset. Operator-run external drivers own their own telemetry configuration. For Helm deployments, set `server.otlp.endpoint` to render this table. The optional `server.otlp.serviceName` value overrides the gateway service name;