From dde907758e659280b8bdefd2bc169e11ad800d04 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 19 Mar 2024 16:38:06 +0100 Subject: [PATCH] Use OTEL semantic conventions constants for most attributes --- Cargo.lock | 2 + crates/cli/src/server.rs | 35 ++++++------ crates/handlers/src/graphql/mod.rs | 7 ++- crates/http/Cargo.toml | 1 + crates/http/src/client.rs | 6 +- crates/http/src/layers/client.rs | 55 ++++++++++--------- crates/storage-pg/Cargo.toml | 1 + crates/storage-pg/src/oauth2/client.rs | 11 ++-- crates/storage-pg/src/tracing.rs | 3 +- .../src/upstream_oauth2/provider.rs | 5 +- crates/storage-pg/src/user/email.rs | 3 +- 11 files changed, 72 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c5f37f57..0380941f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3239,6 +3239,7 @@ dependencies = [ "hyper-rustls", "mas-tower", "opentelemetry", + "opentelemetry-semantic-conventions", "rustls 0.22.2", "rustls-platform-verifier", "serde", @@ -3538,6 +3539,7 @@ dependencies = [ "mas-jose", "mas-storage", "oauth2-types", + "opentelemetry-semantic-conventions", "rand 0.8.5", "rand_chacha 0.3.1", "sea-query", diff --git a/crates/cli/src/server.rs b/crates/cli/src/server.rs index 3e43a73e..4ff3e4b2 100644 --- a/crates/cli/src/server.rs +++ b/crates/cli/src/server.rs @@ -42,7 +42,7 @@ use opentelemetry::{Key, KeyValue}; use opentelemetry_http::HeaderExtractor; use opentelemetry_semantic_conventions::trace::{ HTTP_REQUEST_METHOD, HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE, NETWORK_PROTOCOL_NAME, - NETWORK_PROTOCOL_VERSION, URL_SCHEME, + NETWORK_PROTOCOL_VERSION, URL_PATH, URL_QUERY, URL_SCHEME, USER_AGENT_ORIGINAL, }; use rustls::ServerConfig; use sentry_tower::{NewSentryLayer, SentryHttpLayer}; @@ -120,30 +120,31 @@ fn make_http_span(req: &Request) -> Span { "otel.kind" = "server", "otel.name" = span_name, "otel.status_code" = tracing::field::Empty, - "network.protocol.name" = "http", - "network.protocol.version" = otel_net_protocol_version(req), - "http.method" = method, - "http.route" = tracing::field::Empty, - "http.response.status_code" = tracing::field::Empty, - "url.path" = req.uri().path(), - "url.query" = tracing::field::Empty, - "url.scheme" = otel_url_scheme(req), - "user_agent.original" = tracing::field::Empty, + { NETWORK_PROTOCOL_NAME } = "http", + { NETWORK_PROTOCOL_VERSION } = otel_net_protocol_version(req), + { HTTP_REQUEST_METHOD } = method, + { HTTP_ROUTE } = tracing::field::Empty, + { HTTP_RESPONSE_STATUS_CODE } = tracing::field::Empty, + { URL_PATH } = req.uri().path(), + { URL_QUERY } = tracing::field::Empty, + { URL_SCHEME } = otel_url_scheme(req), + { USER_AGENT_ORIGINAL } = tracing::field::Empty, ); if let Some(route) = route.as_ref() { - span.record("http.route", route); + span.record(HTTP_ROUTE, route); } if let Some(query) = req.uri().query() { - span.record("url.query", query); + span.record(URL_QUERY, query); } - if let Some(user_agent) = req.headers().get(USER_AGENT) { - span.record( - "user_agent.original", - user_agent.to_str().unwrap_or("INVALID"), - ); + if let Some(user_agent) = req + .headers() + .get(USER_AGENT) + .and_then(|ua| ua.to_str().ok()) + { + span.record(USER_AGENT_ORIGINAL, user_agent); } // Extract the parent span context from the request headers diff --git a/crates/handlers/src/graphql/mod.rs b/crates/handlers/src/graphql/mod.rs index fb395424..7306435a 100644 --- a/crates/handlers/src/graphql/mod.rs +++ b/crates/handlers/src/graphql/mod.rs @@ -39,6 +39,7 @@ use mas_storage::{ BoxClock, BoxRepository, BoxRng, Clock, Repository, RepositoryError, SystemClock, }; use mas_storage_pg::PgRepository; +use opentelemetry_semantic_conventions::trace::{GRAPHQL_DOCUMENT, GRAPHQL_OPERATION_NAME}; use rand::{thread_rng, SeedableRng}; use rand_chacha::ChaChaRng; use sqlx::PgPool; @@ -112,13 +113,13 @@ fn span_for_graphql_request(request: &async_graphql::Request) -> tracing::Span { "GraphQL operation", "otel.name" = tracing::field::Empty, "otel.kind" = "server", - "graphql.document" = request.query, - "graphql.operation.name" = tracing::field::Empty, + { GRAPHQL_DOCUMENT } = request.query, + { GRAPHQL_OPERATION_NAME } = tracing::field::Empty, ); if let Some(name) = &request.operation_name { span.record("otel.name", name); - span.record("graphql.operation.name", name); + span.record(GRAPHQL_OPERATION_NAME, name); } span diff --git a/crates/http/Cargo.toml b/crates/http/Cargo.toml index a6207b7d..24de6871 100644 --- a/crates/http/Cargo.toml +++ b/crates/http/Cargo.toml @@ -20,6 +20,7 @@ http-body.workspace = true hyper.workspace = true hyper-rustls = { workspace = true, optional = true } opentelemetry.workspace = true +opentelemetry-semantic-conventions.workspace = true rustls = { workspace = true, optional = true } rustls-platform-verifier = { workspace = true, optional = true } serde.workspace = true diff --git a/crates/http/src/client.rs b/crates/http/src/client.rs index c5b77d1c..a24b0692 100644 --- a/crates/http/src/client.rs +++ b/crates/http/src/client.rs @@ -22,6 +22,7 @@ use mas_tower::{ DurationRecorderLayer, DurationRecorderService, FnWrapper, InFlightCounterLayer, InFlightCounterService, TraceLayer, TraceService, }; +use opentelemetry_semantic_conventions::trace::SERVER_ADDRESS; use tower::Layer; use tracing::Span; @@ -54,9 +55,10 @@ where let trace_layer = TraceLayer::from_fn( (|request: &Name| { tracing::info_span!( - "dns.resolve", + "dns.lookup", "otel.kind" = "client", - "net.host.name" = %request, + { SERVER_ADDRESS } = %request, + ) }) as fn(&Name) -> Span, ); diff --git a/crates/http/src/layers/client.rs b/crates/http/src/layers/client.rs index 19ace214..a3f51eb1 100644 --- a/crates/http/src/layers/client.rs +++ b/crates/http/src/layers/client.rs @@ -23,6 +23,11 @@ use mas_tower::{ TraceLayer, TraceService, }; use opentelemetry::KeyValue; +use opentelemetry_semantic_conventions::trace::{ + CLIENT_ADDRESS, CLIENT_PORT, HTTP_REQUEST_BODY_SIZE, HTTP_REQUEST_METHOD, + HTTP_RESPONSE_BODY_SIZE, HTTP_RESPONSE_STATUS_CODE, NETWORK_PROTOCOL_NAME, NETWORK_TRANSPORT, + NETWORK_TYPE, SERVER_ADDRESS, SERVER_PORT, URL_FULL, USER_AGENT_ORIGINAL, +}; use tower::{ limit::{ConcurrencyLimit, GlobalConcurrencyLimitLayer}, Layer, @@ -69,27 +74,25 @@ impl MakeSpan> for MakeSpanForRequest { .typed_get::() .map(tracing::field::display); let content_length = headers.typed_get().map(|ContentLength(len)| len); - let net_sock_peer_name = request.uri().host(); let category = self.category.unwrap_or("UNSET"); tracing::info_span!( "http.client.request", "otel.kind" = "client", "otel.status_code" = tracing::field::Empty, - "http.method" = %request.method(), - "http.url" = %request.uri(), - "http.status_code" = tracing::field::Empty, - "http.host" = host, - "http.request_content_length" = content_length, - "http.response_content_length" = tracing::field::Empty, - "net.transport" = "ip_tcp", - "net.sock.family" = tracing::field::Empty, - "net.sock.peer.name" = net_sock_peer_name, - "net.sock.peer.addr" = tracing::field::Empty, - "net.sock.peer.port" = tracing::field::Empty, - "net.sock.host.addr" = tracing::field::Empty, - "net.sock.host.port" = tracing::field::Empty, - "user_agent.original" = user_agent, + { HTTP_REQUEST_METHOD } = %request.method(), + { URL_FULL } = %request.uri(), + { HTTP_RESPONSE_STATUS_CODE } = tracing::field::Empty, + { SERVER_ADDRESS } = host, + { HTTP_REQUEST_BODY_SIZE } = content_length, + { HTTP_RESPONSE_BODY_SIZE } = tracing::field::Empty, + { NETWORK_TRANSPORT } = "tcp", + { NETWORK_TYPE } = tracing::field::Empty, + { SERVER_ADDRESS } = tracing::field::Empty, + { SERVER_PORT } = tracing::field::Empty, + { CLIENT_ADDRESS } = tracing::field::Empty, + { CLIENT_PORT } = tracing::field::Empty, + { USER_AGENT_ORIGINAL } = user_agent, "rust.error" = tracing::field::Empty, "mas.category" = category, ) @@ -102,22 +105,22 @@ pub struct EnrichSpanOnResponse; impl EnrichSpan> for EnrichSpanOnResponse { fn enrich_span(&self, span: &Span, response: &Response) { span.record("otel.status_code", "OK"); - span.record("http.status_code", response.status().as_u16()); + span.record(HTTP_RESPONSE_STATUS_CODE, response.status().as_u16()); if let Some(ContentLength(content_length)) = response.headers().typed_get() { - span.record("http.response_content_length", content_length); + span.record(HTTP_RESPONSE_BODY_SIZE, content_length); } if let Some(http_info) = response.extensions().get::() { let local = http_info.local_addr(); let remote = http_info.remote_addr(); - let family = if local.is_ipv4() { "inet" } else { "inet6" }; - span.record("net.sock.family", family); - span.record("net.sock.peer.addr", remote.ip().to_string()); - span.record("net.sock.peer.port", remote.port()); - span.record("net.sock.host.addr", local.ip().to_string()); - span.record("net.sock.host.port", local.port()); + let family = if local.is_ipv4() { "ipv4" } else { "ipv6" }; + span.record(NETWORK_TYPE, family); + span.record(CLIENT_ADDRESS, remote.ip().to_string()); + span.record(CLIENT_PORT, remote.port()); + span.record(SERVER_ADDRESS, local.ip().to_string()); + span.record(SERVER_PORT, local.port()); } else { tracing::warn!("No HttpInfo injected in response extensions"); } @@ -149,8 +152,8 @@ where type Iter<'a> = std::array::IntoIter; fn attributes<'a>(&'a self, t: &'a Request) -> Self::Iter<'a> { [ - KeyValue::new("http.request.method", t.method().as_str().to_owned()), - KeyValue::new("network.protocol.name", "http"), + KeyValue::new(HTTP_REQUEST_METHOD, t.method().as_str().to_owned()), + KeyValue::new(NETWORK_PROTOCOL_NAME, "http"), KeyValue::new("mas.category", self.category.unwrap_or("UNSET")), ] .into_iter() @@ -167,7 +170,7 @@ where type Iter<'a> = std::iter::Once; fn attributes<'a>(&'a self, t: &'a Response) -> Self::Iter<'a> { std::iter::once(KeyValue::new( - "http.response.status_code", + HTTP_RESPONSE_STATUS_CODE, i64::from(t.status().as_u16()), )) } diff --git a/crates/storage-pg/Cargo.toml b/crates/storage-pg/Cargo.toml index 868dc26f..9df774d4 100644 --- a/crates/storage-pg/Cargo.toml +++ b/crates/storage-pg/Cargo.toml @@ -22,6 +22,7 @@ serde_json.workspace = true thiserror.workspace = true tracing.workspace = true futures-util = "0.3.30" +opentelemetry-semantic-conventions.workspace = true rand.workspace = true rand_chacha = "0.3.1" diff --git a/crates/storage-pg/src/oauth2/client.rs b/crates/storage-pg/src/oauth2/client.rs index c0da7974..e73d3754 100644 --- a/crates/storage-pg/src/oauth2/client.rs +++ b/crates/storage-pg/src/oauth2/client.rs @@ -31,6 +31,7 @@ use oauth2_types::{ requests::GrantType, scope::{Scope, ScopeToken}, }; +use opentelemetry_semantic_conventions::trace::DB_STATEMENT; use rand::RngCore; use sqlx::PgConnection; use tracing::{info_span, Instrument}; @@ -786,7 +787,7 @@ impl<'c> OAuth2ClientRepository for PgOAuth2ClientRepository<'c> { { let span = info_span!( "db.oauth2_client.delete_by_id.authorization_grants", - db.statement = tracing::field::Empty, + { DB_STATEMENT } = tracing::field::Empty, ); sqlx::query!( @@ -806,7 +807,7 @@ impl<'c> OAuth2ClientRepository for PgOAuth2ClientRepository<'c> { { let span = info_span!( "db.oauth2_client.delete_by_id.consents", - db.statement = tracing::field::Empty, + { DB_STATEMENT } = tracing::field::Empty, ); sqlx::query!( @@ -826,7 +827,7 @@ impl<'c> OAuth2ClientRepository for PgOAuth2ClientRepository<'c> { { let span = info_span!( "db.oauth2_client.delete_by_id.access_tokens", - db.statement = tracing::field::Empty, + { DB_STATEMENT } = tracing::field::Empty, ); sqlx::query!( @@ -849,7 +850,7 @@ impl<'c> OAuth2ClientRepository for PgOAuth2ClientRepository<'c> { { let span = info_span!( "db.oauth2_client.delete_by_id.refresh_tokens", - db.statement = tracing::field::Empty, + { DB_STATEMENT } = tracing::field::Empty, ); sqlx::query!( @@ -872,7 +873,7 @@ impl<'c> OAuth2ClientRepository for PgOAuth2ClientRepository<'c> { { let span = info_span!( "db.oauth2_client.delete_by_id.sessions", - db.statement = tracing::field::Empty, + { DB_STATEMENT } = tracing::field::Empty, ); sqlx::query!( diff --git a/crates/storage-pg/src/tracing.rs b/crates/storage-pg/src/tracing.rs index 853b5d9d..2925f8cc 100644 --- a/crates/storage-pg/src/tracing.rs +++ b/crates/storage-pg/src/tracing.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use opentelemetry_semantic_conventions::trace::DB_STATEMENT; use tracing::Span; /// An extension trait for [`sqlx::Execute`] that records the SQL statement as @@ -34,7 +35,7 @@ where DB: sqlx::Database, { fn record(self, span: &Span) -> Self { - span.record("db.statement", self.sql()); + span.record(DB_STATEMENT, self.sql()); self } } diff --git a/crates/storage-pg/src/upstream_oauth2/provider.rs b/crates/storage-pg/src/upstream_oauth2/provider.rs index a7ae86cd..dcd6e202 100644 --- a/crates/storage-pg/src/upstream_oauth2/provider.rs +++ b/crates/storage-pg/src/upstream_oauth2/provider.rs @@ -21,6 +21,7 @@ use mas_storage::{ }, Clock, Page, Pagination, }; +use opentelemetry_semantic_conventions::trace::DB_STATEMENT; use rand::RngCore; use sea_query::{enum_def, Expr, PostgresQueryBuilder, Query}; use sea_query_binder::SqlxBinder; @@ -333,7 +334,7 @@ impl<'c> UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<' let span = info_span!( "db.oauth2_client.delete_by_id.authorization_sessions", upstream_oauth_provider.id = %id, - db.statement = tracing::field::Empty, + { DB_STATEMENT } = tracing::field::Empty, ); sqlx::query!( r#" @@ -354,7 +355,7 @@ impl<'c> UpstreamOAuthProviderRepository for PgUpstreamOAuthProviderRepository<' let span = info_span!( "db.oauth2_client.delete_by_id.links", upstream_oauth_provider.id = %id, - db.statement = tracing::field::Empty, + { DB_STATEMENT } = tracing::field::Empty, ); sqlx::query!( r#" diff --git a/crates/storage-pg/src/user/email.rs b/crates/storage-pg/src/user/email.rs index 10b73527..28cd3b66 100644 --- a/crates/storage-pg/src/user/email.rs +++ b/crates/storage-pg/src/user/email.rs @@ -19,6 +19,7 @@ use mas_storage::{ user::{UserEmailFilter, UserEmailRepository}, Clock, Page, Pagination, }; +use opentelemetry_semantic_conventions::trace::DB_STATEMENT; use rand::RngCore; use sea_query::{enum_def, Expr, PostgresQueryBuilder, Query}; use sea_query_binder::SqlxBinder; @@ -382,7 +383,7 @@ impl<'c> UserEmailRepository for PgUserEmailRepository<'c> { async fn remove(&mut self, user_email: UserEmail) -> Result<(), Self::Error> { let span = info_span!( "db.user_email.remove.codes", - db.statement = tracing::field::Empty + { DB_STATEMENT } = tracing::field::Empty ); sqlx::query!( r#"