Skip to content

refactor(stackable-telemetry): Use semantic convention constants #1055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ opentelemetry = "0.29.1"
opentelemetry_sdk = { version = "0.29.0", features = ["rt-tokio"] }
opentelemetry-appender-tracing = "0.29.1"
opentelemetry-otlp = "0.29.0"
# opentelemetry-semantic-conventions = "0.28.0"
opentelemetry-semantic-conventions = "0.29.0"
p256 = { version = "0.13.2", features = ["ecdsa"] }
paste = "1.0.15"
pin-project = "1.1.5"
Expand Down
2 changes: 2 additions & 0 deletions crates/stackable-telemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.

### Changed

- Use constants from `opentelemetry-semantic-conventions` instead of hard-coded strings ([#1055]).
- Bump `opentelemetry` and related crates to `0.29.x` and `tracing-opentelemetry` to `0.30.0` ([#1021]).

### Fixed
Expand All @@ -14,6 +15,7 @@ All notable changes to this project will be documented in this file.

[#1021]: https://github.com/stackabletech/operator-rs/pull/1021
[#1026]: https://github.com/stackabletech/operator-rs/pull/1026
[#1055]: https://github.com/stackabletech/operator-rs/pull/1055

## [0.6.0] - 2025-04-14

Expand Down
2 changes: 1 addition & 1 deletion crates/stackable-telemetry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ futures-util.workspace = true
opentelemetry = { workspace = true, features = ["logs"] }
opentelemetry-appender-tracing.workspace = true
opentelemetry-otlp = { workspace = true, features = ["grpc-tonic", "gzip-tonic", "logs"] }
# opentelemetry-semantic-conventions.workspace = true
opentelemetry-semantic-conventions.workspace = true
opentelemetry_sdk = { workspace = true, features = ["logs", "rt-tokio", "spec_unstable_logs_enabled"] }
pin-project.workspace = true
snafu.workspace = true
Expand Down
75 changes: 47 additions & 28 deletions crates/stackable-telemetry/src/instrumentation/axum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@ use opentelemetry::{
Context,
trace::{SpanKind, TraceContextExt},
};
use opentelemetry_semantic_conventions::{
attribute::{OTEL_STATUS_CODE, OTEL_STATUS_DESCRIPTION},
trace::{
CLIENT_ADDRESS, CLIENT_PORT, HTTP_REQUEST_HEADER, HTTP_REQUEST_METHOD,
HTTP_RESPONSE_STATUS_CODE, HTTP_ROUTE, SERVER_ADDRESS, SERVER_PORT, URL_PATH, URL_QUERY,
URL_SCHEME, USER_AGENT_ORIGINAL,
},
};
use pin_project::pin_project;
use snafu::{ResultExt, Snafu};
use tower::{Layer, Service};
Expand All @@ -41,6 +49,14 @@ const X_FORWARDED_HOST_HEADER_KEY: &str = "X-Forwarded-Host";
const DEFAULT_HTTPS_PORT: u16 = 443;
const DEFAULT_HTTP_PORT: u16 = 80;

// NOTE (@Techassi): These constants are defined here because they are private in
// the tracing-opentelemetry crate.
const OTEL_NAME: &str = "otel.name";
const OTEL_KIND: &str = "otel.kind";

const OTEL_TRACE_ID_FROM: &str = "opentelemetry.trace_id.from";
const OTEL_TRACE_ID_TO: &str = "opentelemetry.trace_id.to";

/// A Tower [`Layer`][1] which decorates [`TraceService`].
///
/// ### Example with Axum
Expand Down Expand Up @@ -414,21 +430,23 @@ impl SpanExt for Span {

let span = tracing::trace_span!(
"HTTP request",
"otel.name" = span_name,
"otel.kind" = ?SpanKind::Server,
"otel.status_code" = Empty,
"otel.status_message" = Empty,
"http.request.method" = http_method,
"http.response.status_code" = Empty,
"http.route" = Empty,
"url.path" = url.path(),
"url.query" = url.query(),
"url.scheme" = url.scheme_str().unwrap_or_default(),
"user_agent.original" = Empty,
"server.address" = Empty,
"server.port" = Empty,
"client.address" = Empty,
"client.port" = Empty,
{ OTEL_NAME } = span_name,
{ OTEL_KIND } = ?SpanKind::Server,
{ OTEL_STATUS_CODE } = Empty,
// The current tracing-opentelemetry version still uses the old semantic convention
// See https://github.com/tokio-rs/tracing-opentelemetry/pull/209
{ OTEL_STATUS_DESCRIPTION } = Empty,
{ HTTP_REQUEST_METHOD } = http_method,
{ HTTP_RESPONSE_STATUS_CODE } = Empty,
{ HTTP_ROUTE } = Empty,
{ URL_PATH } = url.path(),
{ URL_QUERY } = url.query(),
{ URL_SCHEME } = url.scheme_str().unwrap_or_default(),
{ USER_AGENT_ORIGINAL } = Empty,
{ SERVER_ADDRESS } = Empty,
{ SERVER_PORT } = Empty,
{ CLIENT_ADDRESS } = Empty,
{ CLIENT_PORT } = Empty,
// TODO (@Techassi): Add network.protocol.version
);

Expand Down Expand Up @@ -462,8 +480,8 @@ impl SpanExt for Span {

if new_span_context != current_span_context {
tracing::trace!(
opentelemetry.trace_id.from = ?current_span_context.trace_id(),
opentelemetry.trace_id.to = ?new_span_context.trace_id(),
{ OTEL_TRACE_ID_FROM } = ?current_span_context.trace_id(),
{ OTEL_TRACE_ID_TO } = ?new_span_context.trace_id(),
"set parent span context based on context extracted from request headers"
);

Expand All @@ -473,7 +491,7 @@ impl SpanExt for Span {
}

if let Some(user_agent) = req.user_agent() {
span.record("user_agent.original", user_agent);
span.record(USER_AGENT_ORIGINAL, user_agent);
}

// Setting server.address and server.port
Expand All @@ -483,21 +501,21 @@ impl SpanExt for Span {
// NOTE (@Techassi): We cast to i64, because otherwise the field
// will NOT be recorded as a number but as a string. This is likely
// an issue in the tracing-opentelemetry crate.
span.record("server.address", host)
.record("server.port", port as i64);
span.record(SERVER_ADDRESS, host)
.record(SERVER_PORT, port as i64);
}

// Setting fields according to the HTTP server semantic conventions
// See https://opentelemetry.io/docs/specs/semconv/http/http-spans/#http-server-semantic-conventions

if let Some(client_socket_address) = req.client_socket_address() {
span.record("client.address", client_socket_address.ip().to_string());
span.record(CLIENT_ADDRESS, client_socket_address.ip().to_string());

if opt_in {
// NOTE (@Techassi): We cast to i64, because otherwise the field
// will NOT be recorded as a number but as a string. This is
// likely an issue in the tracing-opentelemetry crate.
span.record("client.port", client_socket_address.port() as i64);
span.record(CLIENT_PORT, client_socket_address.port() as i64);
}
}

Expand All @@ -511,7 +529,7 @@ impl SpanExt for Span {
// See: https://github.com/tokio-rs/tracing/issues/1343

if let Some(http_route) = req.matched_path() {
span.record("http.route", http_route.as_str());
span.record(HTTP_ROUTE, http_route.as_str());
}

span
Expand All @@ -525,7 +543,7 @@ impl SpanExt for Span {
// header_name.as_str() always returns lowercase strings and thus we
// don't need to call to_lowercase on it.
let header_name = header_name.as_str();
let field_name = format!("http.request.header.{header_name}");
let field_name = format!("{HTTP_REQUEST_HEADER}.{header_name}");

self.record(
field_name.as_str(),
Expand All @@ -540,15 +558,15 @@ impl SpanExt for Span {
// NOTE (@Techassi): We cast to i64, because otherwise the field will
// NOT be recorded as a number but as a string. This is likely an issue
// in the tracing-opentelemetry crate.
self.record("http.response.status_code", status_code.as_u16() as i64);
self.record(HTTP_RESPONSE_STATUS_CODE, status_code.as_u16() as i64);

// Only set the span status to "Error" when we encountered an server
// error. See:
//
// - https://opentelemetry.io/docs/specs/semconv/http/http-spans/#status
// - https://github.com/open-telemetry/opentelemetry-specification/blob/v1.26.0/specification/trace/api.md#set-status
if status_code.is_server_error() {
self.record("otel.status_code", "Error");
self.record(OTEL_STATUS_CODE, "Error");
// NOTE (@Techassi): Can we add a status_description here as well?
}

Expand All @@ -561,8 +579,9 @@ impl SpanExt for Span {
E: std::error::Error,
{
// NOTE (@Techassi): This field might get renamed: https://github.com/tokio-rs/tracing-opentelemetry/issues/115
self.record("otel.status_code", "Error")
.record("otel.status_message", error.to_string());
// NOTE (@Techassi): It got renamed, a fixed version of tracing-opentelemetry is not available yet
self.record(OTEL_STATUS_CODE, "Error")
.record(OTEL_STATUS_DESCRIPTION, error.to_string());
}
}

Expand Down