Skip to content

Commit

Permalink
Use correct semantic conventions for trace labels
Browse files Browse the repository at this point in the history
The OpenTelemetry spec defines the semantic conventions that HTTP services should use for the labels included in traces: https://opentelemetry.io/docs/specs/semconv/http/http-spans/

Previously, we were using an outdated version of this spec for the OpenCensus traces. This updates the labels to match the current spec.

The notable changes updates to the path for HTTP method and status code, the fields that include the URL parts, and more rigorously following the standard for propagating the Host header.

Signed-off-by: Scott Fleener <[email protected]>
  • Loading branch information
sfleen committed Dec 20, 2024
1 parent a1a5713 commit 005dd47
Showing 1 changed file with 32 additions and 13 deletions.
45 changes: 32 additions & 13 deletions linkerd/trace-context/src/service.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::{propagation, Span, SpanSink};
use futures::{future::Either, prelude::*};
use http::Uri;
use linkerd_stack::layer;
use std::{
collections::HashMap,
Expand Down Expand Up @@ -35,20 +36,35 @@ impl<K: Clone, S> TraceContext<K, S> {
}

fn request_labels<B>(req: &http::Request<B>) -> HashMap<&'static str, String> {
let mut labels = HashMap::with_capacity(5);
labels.insert("http.method", format!("{}", req.method()));
let path = req
.uri()
.path_and_query()
.map(|pq| pq.as_str().to_owned())
.unwrap_or_default();
labels.insert("http.path", path);
if let Some(authority) = req.uri().authority() {
labels.insert("http.authority", authority.as_str().to_string());
let mut labels = HashMap::with_capacity(6);
labels.insert("http.request.method", format!("{}", req.method()));
let url = req.uri();
if let Some(scheme) = url.scheme_str() {
labels.insert("url.scheme", scheme.to_string());

Check warning on line 43 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L39-L43

Added lines #L39 - L43 were not covered by tests
}
if let Some(host) = req.headers().get("host") {
labels.insert("url.path", url.path().to_string());
if let Some(query) = url.query() {
labels.insert("url.query", query.to_string());

Check warning on line 47 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L45-L47

Added lines #L45 - L47 were not covered by tests
}

// This is the order of precendence for host headers,
// see https://opentelemetry.io/docs/specs/semconv/http/http-spans/
let host_header = req

Check warning on line 52 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L52

Added line #L52 was not covered by tests
.headers()
.get("X-Forwarded-Host")
.or_else(|| req.headers().get(":authority"))
.or_else(|| req.headers().get("host"));

Check warning on line 56 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L55-L56

Added lines #L55 - L56 were not covered by tests

if let Some(host) = host_header {

Check warning on line 58 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L58

Added line #L58 was not covered by tests
if let Ok(host) = host.to_str() {
labels.insert("http.host", host.to_string());
if let Ok(uri) = host.parse::<Uri>() {
if let Some(host) = uri.host() {
labels.insert("server.address", host.to_string());

Check warning on line 62 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L60-L62

Added lines #L60 - L62 were not covered by tests
}
if let Some(port) = uri.port() {
labels.insert("server.port", port.to_string());

Check warning on line 65 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L64-L65

Added lines #L64 - L65 were not covered by tests
}
}
}
}
labels
Expand All @@ -58,7 +74,10 @@ impl<K: Clone, S> TraceContext<K, S> {
mut labels: HashMap<&'static str, String>,
rsp: &http::Response<B>,
) -> HashMap<&'static str, String> {
labels.insert("http.status_code", rsp.status().as_str().to_string());
labels.insert(

Check warning on line 77 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L77

Added line #L77 was not covered by tests
"http.response.status_code",
rsp.status().as_str().to_string(),

Check warning on line 79 in linkerd/trace-context/src/service.rs

View check run for this annotation

Codecov / codecov/patch

linkerd/trace-context/src/service.rs#L79

Added line #L79 was not covered by tests
);
labels
}
}
Expand Down

0 comments on commit 005dd47

Please sign in to comment.