From 82c36384530a464ccbf176972fbd3e4bdab29e6a Mon Sep 17 00:00:00 2001 From: Caleb Schoepp Date: Mon, 23 Sep 2024 11:46:35 -0600 Subject: [PATCH 1/4] Fix comment Signed-off-by: Caleb Schoepp --- crates/telemetry/src/traces.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/telemetry/src/traces.rs b/crates/telemetry/src/traces.rs index 926d355c3..e2f34e024 100644 --- a/crates/telemetry/src/traces.rs +++ b/crates/telemetry/src/traces.rs @@ -34,10 +34,7 @@ pub(crate) fn otel_tracing_layer LookupSpan<'span>>( ], ); - // This will configure the exporter based on the OTEL_EXPORTER_* environment variables. We - // currently default to using the HTTP exporter but in the future we could select off of the - // combination of OTEL_EXPORTER_OTLP_PROTOCOL and OTEL_EXPORTER_OTLP_TRACES_PROTOCOL to - // determine whether we should use http/protobuf or grpc. + // This will configure the exporter based on the OTEL_EXPORTER_* environment variables. let exporter: SpanExporterBuilder = match OtlpProtocol::traces_protocol_from_env() { OtlpProtocol::Grpc => opentelemetry_otlp::new_exporter().tonic().into(), OtlpProtocol::HttpProtobuf => opentelemetry_otlp::new_exporter().http().into(), From 9f56929aee6cc8f4840fa41f2a0581f3afef3668 Mon Sep 17 00:00:00 2001 From: Caleb Schoepp Date: Mon, 23 Sep 2024 13:24:28 -0600 Subject: [PATCH 2/4] Fix metrics and tracing providers not being set globally This is relied on by other parts of spin-telemetry Signed-off-by: Caleb Schoepp --- crates/telemetry/src/metrics.rs | 3 +++ crates/telemetry/src/traces.rs | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/telemetry/src/metrics.rs b/crates/telemetry/src/metrics.rs index 200a786a2..58e62826f 100644 --- a/crates/telemetry/src/metrics.rs +++ b/crates/telemetry/src/metrics.rs @@ -1,6 +1,7 @@ use std::time::Duration; use anyhow::{bail, Result}; +use opentelemetry::global; use opentelemetry_otlp::MetricsExporterBuilder; use opentelemetry_sdk::{ metrics::{ @@ -57,6 +58,8 @@ pub(crate) fn otel_metrics_layer LookupSpan<'span>>( .with_resource(resource) .build(); + global::set_meter_provider(meter_provider.clone()); + Ok(MetricsLayer::new(meter_provider)) } diff --git a/crates/telemetry/src/traces.rs b/crates/telemetry/src/traces.rs index e2f34e024..213684fe9 100644 --- a/crates/telemetry/src/traces.rs +++ b/crates/telemetry/src/traces.rs @@ -1,7 +1,7 @@ use std::time::Duration; use anyhow::bail; -use opentelemetry::trace::TracerProvider; +use opentelemetry::{global, trace::TracerProvider}; use opentelemetry_otlp::SpanExporterBuilder; use opentelemetry_sdk::{ resource::{EnvResourceDetector, TelemetryResourceDetector}, @@ -35,7 +35,7 @@ pub(crate) fn otel_tracing_layer LookupSpan<'span>>( ); // This will configure the exporter based on the OTEL_EXPORTER_* environment variables. - let exporter: SpanExporterBuilder = match OtlpProtocol::traces_protocol_from_env() { + let exporter_builder: SpanExporterBuilder = match OtlpProtocol::traces_protocol_from_env() { OtlpProtocol::Grpc => opentelemetry_otlp::new_exporter().tonic().into(), OtlpProtocol::HttpProtobuf => opentelemetry_otlp::new_exporter().http().into(), OtlpProtocol::HttpJson => bail!("http/json OTLP protocol is not supported"), @@ -43,10 +43,12 @@ pub(crate) fn otel_tracing_layer LookupSpan<'span>>( let tracer_provider = opentelemetry_otlp::new_pipeline() .tracing() - .with_exporter(exporter) + .with_exporter(exporter_builder) .with_trace_config(opentelemetry_sdk::trace::Config::default().with_resource(resource)) .install_batch(opentelemetry_sdk::runtime::Tokio)?; + global::set_tracer_provider(tracer_provider.clone()); + let env_filter = match EnvFilter::try_from_env("SPIN_OTEL_TRACING_LEVEL") { Ok(filter) => filter, // If it isn't set or it fails to parse default to info From 61435060415881927d76304d571b6144a0165e2d Mon Sep 17 00:00:00 2001 From: Caleb Schoepp Date: Mon, 23 Sep 2024 13:37:04 -0600 Subject: [PATCH 3/4] Remove values from OTel traces to not expose PII Signed-off-by: Caleb Schoepp --- crates/factor-outbound-mysql/src/host.rs | 4 ++-- crates/factor-outbound-pg/src/host.rs | 4 ++-- crates/factor-sqlite/src/host.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/factor-outbound-mysql/src/host.rs b/crates/factor-outbound-mysql/src/host.rs index b28a340a8..5fbe7246e 100644 --- a/crates/factor-outbound-mysql/src/host.rs +++ b/crates/factor-outbound-mysql/src/host.rs @@ -52,7 +52,7 @@ impl v2::HostConnection for InstanceState { self.open_connection(&address).await } - #[instrument(name = "spin_outbound_mysql.execute", skip(self, connection), err(level = Level::INFO), fields(otel.kind = "client", db.system = "mysql", otel.name = statement))] + #[instrument(name = "spin_outbound_mysql.execute", skip(self, connection, params), err(level = Level::INFO), fields(otel.kind = "client", db.system = "mysql", otel.name = statement))] async fn execute( &mut self, connection: Resource, @@ -66,7 +66,7 @@ impl v2::HostConnection for InstanceState { .await?) } - #[instrument(name = "spin_outbound_mysql.query", skip(self, connection), err(level = Level::INFO), fields(otel.kind = "client", db.system = "mysql", otel.name = statement))] + #[instrument(name = "spin_outbound_mysql.query", skip(self, connection, params), err(level = Level::INFO), fields(otel.kind = "client", db.system = "mysql", otel.name = statement))] async fn query( &mut self, connection: Resource, diff --git a/crates/factor-outbound-pg/src/host.rs b/crates/factor-outbound-pg/src/host.rs index 1f7be3570..0c387e6e4 100644 --- a/crates/factor-outbound-pg/src/host.rs +++ b/crates/factor-outbound-pg/src/host.rs @@ -77,7 +77,7 @@ impl v2::HostConnection for InstanceState { self.open_connection(&address).await } - #[instrument(name = "spin_outbound_pg.execute", skip(self, connection), err(level = Level::INFO), fields(otel.kind = "client", db.system = "postgresql", otel.name = statement))] + #[instrument(name = "spin_outbound_pg.execute", skip(self, connection, params), err(level = Level::INFO), fields(otel.kind = "client", db.system = "postgresql", otel.name = statement))] async fn execute( &mut self, connection: Resource, @@ -91,7 +91,7 @@ impl v2::HostConnection for InstanceState { .await?) } - #[instrument(name = "spin_outbound_pg.query", skip(self, connection), err(level = Level::INFO), fields(otel.kind = "client", db.system = "postgresql", otel.name = statement))] + #[instrument(name = "spin_outbound_pg.query", skip(self, connection, params), err(level = Level::INFO), fields(otel.kind = "client", db.system = "postgresql", otel.name = statement))] async fn query( &mut self, connection: Resource, diff --git a/crates/factor-sqlite/src/host.rs b/crates/factor-sqlite/src/host.rs index 9deef168a..9b9a151cf 100644 --- a/crates/factor-sqlite/src/host.rs +++ b/crates/factor-sqlite/src/host.rs @@ -83,7 +83,7 @@ impl v2::HostConnection for InstanceState { .map(Resource::new_own) } - #[instrument(name = "spin_sqlite.execute", skip(self, connection), err(level = Level::INFO), fields(otel.kind = "client", db.system = "sqlite", otel.name = query, sqlite.backend = Empty))] + #[instrument(name = "spin_sqlite.execute", skip(self, connection, parameters), err(level = Level::INFO), fields(otel.kind = "client", db.system = "sqlite", otel.name = query, sqlite.backend = Empty))] async fn execute( &mut self, connection: Resource, From 7ee25794b46c6d1b96c1dddbb8613cd5478e0b11 Mon Sep 17 00:00:00 2001 From: Caleb Schoepp Date: Wed, 25 Sep 2024 13:48:43 -0600 Subject: [PATCH 4/4] Hide addresses in telemetry Signed-off-by: Caleb Schoepp --- crates/factor-key-value/src/host.rs | 8 ++++---- crates/factor-outbound-mysql/src/host.rs | 5 ++++- crates/factor-outbound-networking/src/lib.rs | 20 ++++++++++++++++++++ crates/factor-outbound-pg/src/host.rs | 5 ++++- crates/factor-outbound-redis/src/host.rs | 5 ++++- 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/crates/factor-key-value/src/host.rs b/crates/factor-key-value/src/host.rs index 3a28c115a..4a9753a02 100644 --- a/crates/factor-key-value/src/host.rs +++ b/crates/factor-key-value/src/host.rs @@ -85,7 +85,7 @@ impl key_value::HostStore for KeyValueDispatch { .await) } - #[instrument(name = "spin_key_value.get", skip(self, store), err(level = Level::INFO), fields(otel.kind = "client"))] + #[instrument(name = "spin_key_value.get", skip(self, store, key), err(level = Level::INFO), fields(otel.kind = "client"))] async fn get( &mut self, store: Resource, @@ -95,7 +95,7 @@ impl key_value::HostStore for KeyValueDispatch { Ok(store.get(&key).await) } - #[instrument(name = "spin_key_value.set", skip(self, store, value), err(level = Level::INFO), fields(otel.kind = "client"))] + #[instrument(name = "spin_key_value.set", skip(self, store, key, value), err(level = Level::INFO), fields(otel.kind = "client"))] async fn set( &mut self, store: Resource, @@ -106,7 +106,7 @@ impl key_value::HostStore for KeyValueDispatch { Ok(store.set(&key, &value).await) } - #[instrument(name = "spin_key_value.delete", skip(self, store), err(level = Level::INFO), fields(otel.kind = "client"))] + #[instrument(name = "spin_key_value.delete", skip(self, store, key), err(level = Level::INFO), fields(otel.kind = "client"))] async fn delete( &mut self, store: Resource, @@ -116,7 +116,7 @@ impl key_value::HostStore for KeyValueDispatch { Ok(store.delete(&key).await) } - #[instrument(name = "spin_key_value.exists", skip(self, store), err(level = Level::INFO), fields(otel.kind = "client"))] + #[instrument(name = "spin_key_value.exists", skip(self, store, key), err(level = Level::INFO), fields(otel.kind = "client"))] async fn exists( &mut self, store: Resource, diff --git a/crates/factor-outbound-mysql/src/host.rs b/crates/factor-outbound-mysql/src/host.rs index 5fbe7246e..b5f92b9a0 100644 --- a/crates/factor-outbound-mysql/src/host.rs +++ b/crates/factor-outbound-mysql/src/host.rs @@ -5,6 +5,7 @@ use spin_world::v1::mysql as v1; use spin_world::v2::mysql::{self as v2, Connection}; use spin_world::v2::rdbms_types as v2_types; use spin_world::v2::rdbms_types::ParameterValue; +use tracing::field::Empty; use tracing::{instrument, Level}; use crate::client::Client; @@ -38,8 +39,10 @@ impl v2::Host for InstanceState {} #[async_trait] impl v2::HostConnection for InstanceState { - #[instrument(name = "spin_outbound_mysql.open_connection", skip(self), err(level = Level::INFO), fields(otel.kind = "client", db.system = "mysql"))] + #[instrument(name = "spin_outbound_mysql.open", skip(self, address), err(level = Level::INFO), fields(otel.kind = "client", db.system = "mysql", db.address = Empty, server.port = Empty, db.namespace = Empty))] async fn open(&mut self, address: String) -> Result, v2::Error> { + spin_factor_outbound_networking::record_address_fields(&address); + if !self .is_address_allowed(&address) .await diff --git a/crates/factor-outbound-networking/src/lib.rs b/crates/factor-outbound-networking/src/lib.rs index 25d8085c1..330d1d098 100644 --- a/crates/factor-outbound-networking/src/lib.rs +++ b/crates/factor-outbound-networking/src/lib.rs @@ -21,6 +21,7 @@ pub use config::{ }; pub use runtime_config::ComponentTlsConfigs; +use url::Url; pub type SharedFutureResult = Shared, Arc>>>; @@ -248,3 +249,22 @@ impl DisallowedHostHandler for F { self(scheme, authority); } } + +/// Records the address host, port, and database as fields on the current tracing span. +/// +/// This should only be called from within a function that has been instrumented with a span. +/// +/// The following fields must be pre-declared as empty on the span or they will not show up. +/// ``` +/// use tracing::field::Empty; +/// #[tracing::instrument(fields(db.address = Empty, server.port = Empty, db.namespace = Empty))] +/// fn open() {} +/// ``` +pub fn record_address_fields(address: &str) { + if let Ok(url) = Url::parse(address) { + let span = tracing::Span::current(); + span.record("db.address", url.host_str().unwrap_or_default()); + span.record("server.port", url.port().unwrap_or_default()); + span.record("db.namespace", url.path().trim_start_matches('/')); + } +} diff --git a/crates/factor-outbound-pg/src/host.rs b/crates/factor-outbound-pg/src/host.rs index 0c387e6e4..9250e1196 100644 --- a/crates/factor-outbound-pg/src/host.rs +++ b/crates/factor-outbound-pg/src/host.rs @@ -5,6 +5,7 @@ use spin_world::v1::rdbms_types as v1_types; use spin_world::v2::postgres::{self as v2, Connection}; use spin_world::v2::rdbms_types; use spin_world::v2::rdbms_types::{ParameterValue, RowSet}; +use tracing::field::Empty; use tracing::instrument; use tracing::Level; @@ -63,8 +64,10 @@ impl v2::Host for InstanceState {} #[async_trait] impl v2::HostConnection for InstanceState { - #[instrument(name = "spin_outbound_pg.open_connection", skip(self), err(level = Level::INFO), fields(otel.kind = "client", db.system = "postgresql"))] + #[instrument(name = "spin_outbound_pg.open", skip(self, address), err(level = Level::INFO), fields(otel.kind = "client", db.system = "postgresql", db.address = Empty, server.port = Empty, db.namespace = Empty))] async fn open(&mut self, address: String) -> Result, v2::Error> { + spin_factor_outbound_networking::record_address_fields(&address); + if !self .is_address_allowed(&address) .await diff --git a/crates/factor-outbound-redis/src/host.rs b/crates/factor-outbound-redis/src/host.rs index 112fb5f87..4fbc57318 100644 --- a/crates/factor-outbound-redis/src/host.rs +++ b/crates/factor-outbound-redis/src/host.rs @@ -6,6 +6,7 @@ use spin_world::v1::{redis as v1, redis_types}; use spin_world::v2::redis::{ self as v2, Connection as RedisConnection, Error, RedisParameter, RedisResult, }; +use tracing::field::Empty; use tracing::{instrument, Level}; pub struct InstanceState { @@ -53,8 +54,10 @@ impl v2::Host for crate::InstanceState { #[async_trait] impl v2::HostConnection for crate::InstanceState { - #[instrument(name = "spin_outbound_redis.open_connection", skip(self), err(level = Level::INFO), fields(otel.kind = "client", db.system = "redis"))] + #[instrument(name = "spin_outbound_redis.open_connection", skip(self, address), err(level = Level::INFO), fields(otel.kind = "client", db.system = "redis", db.address = Empty, server.port = Empty, db.namespace = Empty))] async fn open(&mut self, address: String) -> Result, Error> { + spin_factor_outbound_networking::record_address_fields(&address); + if !self .is_address_allowed(&address) .await