diff --git a/AGENTS.md b/AGENTS.md index ce7fc7cd63..540a8d9207 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,6 +37,7 @@ These pipelines connect skills into end-to-end workflows. Individual skill files | `crates/openshell-bootstrap/` | Gateway metadata | Gateway registration metadata, auth token storage, mTLS bundle storage | | `crates/openshell-gateway-interceptors/` | Gateway interceptors | Intercepts and transforms configured gRPC requests at the gateway routing boundary | | `crates/openshell-ocsf/` | OCSF logging | OCSF v1.7.0 event types, builders, shorthand/JSONL formatters, tracing layers | +| `crates/openshell-otel/` | OpenTelemetry support | Shared OTLP trace provider, resource, and tracing-layer construction | | `crates/openshell-core/` | Shared core | Common types, configuration, error handling | | `crates/openshell-sdk/` | Shared client SDK | Async Rust gateway client (gRPC transport, TLS, OIDC refresh, edge tunnel); consumed by CLI, TUI, and `@openshell/sdk` | | `crates/openshell-providers/` | Provider management | Credential provider backends | diff --git a/Cargo.lock b/Cargo.lock index cde49c6935..717f3260a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4045,6 +4045,21 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "openshell-otel" +version = "0.0.0" +dependencies = [ + "http 1.4.0", + "opentelemetry", + "opentelemetry-otlp", + "opentelemetry_sdk", + "thiserror 2.0.18", + "tokio", + "tracing", + "tracing-opentelemetry", + "tracing-subscriber", +] + [[package]] name = "openshell-policy" version = "0.0.0" @@ -4196,6 +4211,7 @@ dependencies = [ "openshell-driver-podman", "openshell-gateway-interceptors", "openshell-ocsf", + "openshell-otel", "openshell-policy", "openshell-prover", "openshell-providers", @@ -4203,7 +4219,6 @@ dependencies = [ "openshell-supervisor-middleware", "openshell-supervisor-middleware-builtins", "opentelemetry", - "opentelemetry-otlp", "opentelemetry-proto", "opentelemetry_sdk", "petname", diff --git a/architecture/gateway.md b/architecture/gateway.md index 9469161477..56899983a2 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -615,7 +615,8 @@ replace the existing logging paths. export: the table's presence is the on-switch, and `OTEL_EXPORTER_OTLP_ENDPOINT` is ignored so enablement has a single source. TOML decides whether and where to export; the SDK's `OTEL_*` variables tune how. Transport is OTLP over gRPC -only. +only. Shared provider, resource, and tracing-layer construction lives in +`openshell-otel`. Span emission requires no per-handler instrumentation. The `tower_http` `TraceLayer` in `multiplex.rs` opens a span per inbound request, and that span diff --git a/crates/openshell-otel/Cargo.toml b/crates/openshell-otel/Cargo.toml new file mode 100644 index 0000000000..bdf42630af --- /dev/null +++ b/crates/openshell-otel/Cargo.toml @@ -0,0 +1,27 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +[package] +name = "openshell-otel" +description = "Shared OpenTelemetry trace export support for OpenShell services" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +repository.workspace = true + +[dependencies] +http = { workspace = true } +opentelemetry = { workspace = true } +opentelemetry_sdk = { workspace = true } +opentelemetry-otlp = { workspace = true } +thiserror = { workspace = true } +tracing = { workspace = true } +tracing-opentelemetry = { workspace = true } +tracing-subscriber = { workspace = true } + +[dev-dependencies] +tokio = { workspace = true } + +[lints] +workspace = true diff --git a/crates/openshell-otel/src/lib.rs b/crates/openshell-otel/src/lib.rs new file mode 100644 index 0000000000..ae6f503ad7 --- /dev/null +++ b/crates/openshell-otel/src/lib.rs @@ -0,0 +1,198 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Shared OpenTelemetry trace export support for `OpenShell` services. + +use opentelemetry::KeyValue; +use opentelemetry::trace::TracerProvider as _; +use opentelemetry_otlp::{SpanExporter, WithExportConfig}; +use opentelemetry_sdk::Resource; +use opentelemetry_sdk::trace::SdkTracer; +pub use opentelemetry_sdk::trace::SdkTracerProvider; +use tracing::Subscriber; +use tracing_opentelemetry::OpenTelemetryLayer; +use tracing_subscriber::Layer as _; +use tracing_subscriber::registry::LookupSpan; + +const SDK_UNKNOWN_SERVICE_PREFIX: &str = "unknown_service"; + +/// How a process chooses its OpenTelemetry `service.name`. +#[derive(Debug, Clone, Copy)] +pub enum ServiceName<'a> { + /// Always use this name, overriding `OTEL_SERVICE_NAME`. + Fixed(&'a str), + /// Use `OTEL_SERVICE_NAME` when set, otherwise use this default. + EnvironmentOr(&'a str), +} + +/// Inputs for an OTLP/gRPC trace provider. +#[derive(Debug, Clone)] +pub struct OtlpTraceConfig<'a> { + pub endpoint: &'a str, + pub service_name: ServiceName<'a>, + pub service_version: Option<&'a str>, + pub resource_attributes: Vec, +} + +/// Failure to construct an OTLP trace provider. +#[derive(Debug, thiserror::Error)] +pub enum SetupError { + #[error("OTLP endpoint is empty")] + EmptyEndpoint, + + #[error("invalid OTLP endpoint {endpoint:?}: {source}")] + InvalidEndpoint { + endpoint: String, + source: http::uri::InvalidUri, + }, + + #[error("failed to build the OTLP span exporter: {0}")] + Exporter(#[from] opentelemetry_otlp::ExporterBuildError), +} + +fn resource_attributes(config: &OtlpTraceConfig<'_>) -> Vec { + let mut attributes = config.resource_attributes.clone(); + if let Some(version) = config + .service_version + .map(str::trim) + .filter(|version| !version.is_empty()) + { + attributes.push(KeyValue::new("service.version", version.to_string())); + } + attributes +} + +/// Build the OpenTelemetry resource for a trace provider configuration. +#[must_use] +pub fn resource_for(config: &OtlpTraceConfig<'_>) -> Resource { + let attributes = resource_attributes(config); + match config.service_name { + ServiceName::Fixed(name) => Resource::builder() + .with_service_name(name.trim().to_string()) + .with_attributes(attributes) + .build(), + ServiceName::EnvironmentOr(default) => { + let detected = Resource::builder() + .with_attributes(attributes.clone()) + .build(); + if detected + .get(&opentelemetry::Key::from_static_str("service.name")) + .is_some_and(|value| !value.to_string().starts_with(SDK_UNKNOWN_SERVICE_PREFIX)) + { + detected + } else { + Resource::builder() + .with_service_name(default.trim().to_string()) + .with_attributes(attributes) + .build() + } + } + } +} + +/// Build an OTLP/gRPC trace provider. +pub fn build_provider(config: &OtlpTraceConfig<'_>) -> Result { + let endpoint = config.endpoint.trim(); + if endpoint.is_empty() { + return Err(SetupError::EmptyEndpoint); + } + endpoint + .parse::() + .map_err(|source| SetupError::InvalidEndpoint { + endpoint: endpoint.to_string(), + source, + })?; + + let exporter = SpanExporter::builder() + .with_tonic() + .with_endpoint(endpoint) + .build()?; + + Ok(SdkTracerProvider::builder() + .with_batch_exporter(exporter) + .with_resource(resource_for(config)) + .build()) +} + +/// Build the provider for an optional OTLP configuration. +/// +/// Telemetry setup failures disable export and remain available for the caller +/// to report after its tracing subscriber is installed. +#[must_use] +pub fn provider_for( + config: Option>, +) -> (Option, Option) { + match config.as_ref().map(build_provider) { + None => (None, None), + Some(Ok(provider)) => (Some(provider), None), + Some(Err(error)) => (None, Some(error)), + } +} + +/// Filtered OpenTelemetry layer returned by [`layer`]. +pub type OtlpLayer = tracing_subscriber::filter::Filtered< + OpenTelemetryLayer, + tracing_subscriber::filter::FilterFn, + S, +>; + +/// Build a tracing layer that exports spans and excludes exporter callsites. +pub fn layer(provider: &SdkTracerProvider, instrumentation_scope: &'static str) -> OtlpLayer +where + S: Subscriber + for<'span> LookupSpan<'span>, +{ + tracing_opentelemetry::layer() + .with_tracer(provider.tracer(instrumentation_scope)) + .with_filter(tracing_subscriber::filter::filter_fn(|metadata| { + metadata.is_span() && !metadata.target().starts_with("opentelemetry") + })) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resource_uses_fixed_service_identity_and_custom_attributes() { + let resource = resource_for(&OtlpTraceConfig { + endpoint: "http://127.0.0.1:4317", + service_name: ServiceName::Fixed("openshell-driver-vm"), + service_version: Some("1.2.3"), + resource_attributes: vec![KeyValue::new("openshell.gateway.name", "vm-dev")], + }); + + assert_eq!( + resource + .get(&opentelemetry::Key::from_static_str("service.name")) + .map(|value| value.to_string()), + Some("openshell-driver-vm".to_string()) + ); + assert_eq!( + resource + .get(&opentelemetry::Key::from_static_str("service.version")) + .map(|value| value.to_string()), + Some("1.2.3".to_string()) + ); + assert_eq!( + resource + .get(&opentelemetry::Key::from_static_str( + "openshell.gateway.name", + )) + .map(|value| value.to_string()), + Some("vm-dev".to_string()) + ); + } + + #[tokio::test] + async fn malformed_endpoint_disables_export_with_a_reportable_error() { + let (provider, error) = provider_for(Some(OtlpTraceConfig { + endpoint: "definitely not a url", + service_name: ServiceName::Fixed("test-service"), + service_version: None, + resource_attributes: Vec::new(), + })); + + assert!(provider.is_none()); + assert!(matches!(error, Some(SetupError::InvalidEndpoint { .. }))); + } +} diff --git a/crates/openshell-server/Cargo.toml b/crates/openshell-server/Cargo.toml index 06970735ad..e22db21e32 100644 --- a/crates/openshell-server/Cargo.toml +++ b/crates/openshell-server/Cargo.toml @@ -22,6 +22,7 @@ openshell-driver-kubernetes = { path = "../openshell-driver-kubernetes" } openshell-driver-podman = { path = "../openshell-driver-podman" } openshell-gateway-interceptors = { path = "../openshell-gateway-interceptors" } openshell-ocsf = { path = "../openshell-ocsf" } +openshell-otel = { path = "../openshell-otel" } openshell-policy = { path = "../openshell-policy" } openshell-prover = { path = "../openshell-prover" } openshell-providers = { path = "../openshell-providers" } @@ -72,7 +73,6 @@ tracing-subscriber = { workspace = true } # OpenTelemetry (OTLP trace export, opt-in via [openshell.gateway.otlp]) opentelemetry = { workspace = true } opentelemetry_sdk = { workspace = true } -opentelemetry-otlp = { workspace = true } tracing-opentelemetry = { workspace = true } # Metrics diff --git a/crates/openshell-server/src/otel_tracing.rs b/crates/openshell-server/src/otel_tracing.rs index b22f7a8dc8..dd143f22b4 100644 --- a/crates/openshell-server/src/otel_tracing.rs +++ b/crates/openshell-server/src/otel_tracing.rs @@ -21,14 +21,12 @@ //! Only traces are exported. Logs and metrics have their own surfaces (OCSF //! JSONL and the Prometheus `/metrics` endpoint). -use opentelemetry::KeyValue; -use opentelemetry::trace::TracerProvider as _; -use opentelemetry_otlp::{SpanExporter, WithExportConfig}; +pub use openshell_otel::SetupError; +use openshell_otel::{OtlpTraceConfig, ServiceName}; +#[cfg(test)] use opentelemetry_sdk::Resource; -use opentelemetry_sdk::trace::{SdkTracer, SdkTracerProvider}; +use opentelemetry_sdk::trace::SdkTracerProvider; use tracing::Subscriber; -use tracing_opentelemetry::OpenTelemetryLayer; -use tracing_subscriber::Layer as _; use tracing_subscriber::registry::LookupSpan; use crate::config_file::OtlpConfig; @@ -39,67 +37,28 @@ const DEFAULT_SERVICE_NAME: &str = "openshell-gateway"; /// Instrumentation scope recorded on spans this gateway emits. const INSTRUMENTATION_SCOPE: &str = "openshell-gateway"; -/// Prefix of the placeholder `service.name` the SDK's own detector supplies -/// when nothing else set one. The full value is `unknown_service:`. -/// Used to tell "operator configured nothing" apart from "operator set -/// `OTEL_SERVICE_NAME`". -const SDK_UNKNOWN_SERVICE_PREFIX: &str = "unknown_service"; - -/// Failure to construct the OTLP export pipeline from gateway config. -/// -/// Every variant is a configuration error, surfaced at startup. Collector -/// *reachability* is deliberately not an error here — the batch exporter -/// connects lazily so a down collector never blocks the gateway from serving. -#[derive(Debug, thiserror::Error)] -pub enum SetupError { - #[error("otlp endpoint is empty; set [openshell.gateway.otlp].endpoint")] - EmptyEndpoint, - - #[error("invalid otlp endpoint {endpoint:?}: {source}")] - InvalidEndpoint { - endpoint: String, - source: http::uri::InvalidUri, - }, - - #[error("failed to build the otlp span exporter: {0}")] - Exporter(#[from] opentelemetry_otlp::ExporterBuildError), -} - -/// Build the `OTel` resource describing this gateway process. -/// -/// `Resource::builder` also runs the SDK's env detector, so -/// `OTEL_RESOURCE_ATTRIBUTES` and `OTEL_SERVICE_NAME` are merged in on top of -/// these defaults. -fn build_resource(cfg: &OtlpConfig) -> Resource { - let version = KeyValue::new("service.version", openshell_core::VERSION); - - if let Some(name) = cfg +fn trace_config(cfg: &OtlpConfig) -> OtlpTraceConfig<'_> { + let service_name = cfg .service_name .as_deref() .map(str::trim) .filter(|s| !s.is_empty()) - { - return Resource::builder() - .with_service_name(name.to_string()) - .with_attribute(version) - .build(); - } - - // No name configured: prefer whatever the env detector found, and fall - // back to our own default only if it found nothing. Setting the default - // unconditionally would mask `OTEL_SERVICE_NAME`. - let detected = Resource::builder().with_attribute(version.clone()).build(); - if detected - .get(&opentelemetry::Key::from_static_str("service.name")) - .is_some_and(|v| !v.to_string().starts_with(SDK_UNKNOWN_SERVICE_PREFIX)) - { - return detected; - } - - Resource::builder() - .with_service_name(DEFAULT_SERVICE_NAME) - .with_attribute(version) - .build() + .map_or( + ServiceName::EnvironmentOr(DEFAULT_SERVICE_NAME), + ServiceName::Fixed, + ); + + OtlpTraceConfig { + endpoint: &cfg.endpoint, + service_name, + service_version: Some(openshell_core::VERSION), + resource_attributes: Vec::new(), + } +} + +#[cfg(test)] +fn build_resource(cfg: &OtlpConfig) -> Resource { + openshell_otel::resource_for(&trace_config(cfg)) } /// Build a tracer provider exporting over OTLP/gRPC to the configured endpoint. @@ -110,29 +69,9 @@ fn build_resource(cfg: &OtlpConfig) -> Resource { /// /// The sampler and span limits are left at the SDK's defaults, which are /// themselves resolved from `OTEL_*` env vars (see the module docs). +#[cfg(test)] fn build_provider(cfg: &OtlpConfig) -> Result { - let endpoint = cfg.endpoint.trim(); - if endpoint.is_empty() { - return Err(SetupError::EmptyEndpoint); - } - // Validate up front: the exporter defers connection, so without this a - // typo'd endpoint would surface only as export failures at runtime. - endpoint - .parse::() - .map_err(|source| SetupError::InvalidEndpoint { - endpoint: endpoint.to_string(), - source, - })?; - - let exporter = SpanExporter::builder() - .with_tonic() - .with_endpoint(endpoint) - .build()?; - - Ok(SdkTracerProvider::builder() - .with_batch_exporter(exporter) - .with_resource(build_resource(cfg)) - .build()) + openshell_otel::build_provider(&trace_config(cfg)) } /// Resolve the tracer provider for a gateway config file's optional @@ -144,32 +83,18 @@ fn build_provider(cfg: &OtlpConfig) -> Result { /// The error is returned rather than logged because the provider is built /// before the subscriber it attaches to, so logging here would go nowhere. pub fn provider_for(cfg: Option<&OtlpConfig>) -> (Option, Option) { - match cfg.map(build_provider) { - None => (None, None), - Some(Ok(provider)) => (Some(provider), None), - Some(Err(err)) => (None, Some(err)), - } + openshell_otel::provider_for(cfg.map(trace_config)) } /// Build the `tracing` layer that forwards spans to `provider`. /// /// Events stay on the gateway's logging layers. Spans emitted by the /// OpenTelemetry crates are excluded to prevent recursive export traffic. -pub fn layer( - provider: &SdkTracerProvider, -) -> tracing_subscriber::filter::Filtered< - OpenTelemetryLayer, - tracing_subscriber::filter::FilterFn, - S, -> +pub fn layer(provider: &SdkTracerProvider) -> openshell_otel::OtlpLayer where S: Subscriber + for<'span> LookupSpan<'span>, { - tracing_opentelemetry::layer() - .with_tracer(provider.tracer(INSTRUMENTATION_SCOPE)) - .with_filter(tracing_subscriber::filter::filter_fn(|meta| { - meta.is_span() && !is_opentelemetry_target(meta.target()) - })) + openshell_otel::layer(provider, INSTRUMENTATION_SCOPE) } /// Mark `span` as failed. @@ -180,14 +105,6 @@ pub fn mark_error(span: &tracing::Span) { span.record("otel.status_code", "ERROR"); } -/// Whether `target` belongs to the OpenTelemetry crates themselves. -/// -/// OpenTelemetry crates use their crate name as the target (`opentelemetry`, -/// `opentelemetry_sdk`, `opentelemetry-otlp`), so a prefix match covers them. -fn is_opentelemetry_target(target: &str) -> bool { - target.starts_with("opentelemetry") -} - /// In-process OTLP/gRPC trace collector, for tests that need to assert what /// the gateway actually put on the wire. #[cfg(test)]