Skip to content
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

feat(telemetry): add the ability to override span name with otel.name attribute #5365

Merged
merged 8 commits into from
Jun 10, 2024
Merged
16 changes: 16 additions & 0 deletions .changesets/feat_bnjjj_feat_otel_name_attribute.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
### Add the ability to override span name with otel.name attribute ([Issue #5261](https://github.com/apollographql/router/issues/5261))

bnjjj marked this conversation as resolved.
Show resolved Hide resolved
It gives you the ability to override the span name by using custom telemetry with any [selectors](https://www.apollographql.com/docs/router/configuration/telemetry/instrumentation/selectors/) you want just by setting the `otel.name` attribute.

Example:

```yaml
telemetry:
instrumentation:
spans:
router:
otel.name:
static: router # Override the span name to router
```

By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/5365
14 changes: 14 additions & 0 deletions apollo-router/src/plugins/telemetry/config_new/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,16 @@ mod test {
value: Arc::new(Default::default()),
},
);
spans.attributes.custom.insert(
"otel.name".to_string(),
Conditional {
selector: RouterSelector::StaticField {
r#static: String::from("new_name").into(),
},
condition: None,
value: Arc::new(Default::default()),
},
);
let values = spans.attributes.on_response(
&router::Response::fake_builder()
.header("my-header", "test_val")
Expand All @@ -556,6 +566,10 @@ mod test {
assert!(values
.iter()
.any(|key_val| key_val.key == opentelemetry::Key::from_static_str("test")));

assert!(values.iter().any(|key_val| key_val.key
== opentelemetry::Key::from_static_str("otel.name")
&& key_val.value == opentelemetry::Value::String(String::from("new_name").into())));
bnjjj marked this conversation as resolved.
Show resolved Hide resolved
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion apollo-router/src/plugins/telemetry/dynamic_attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ impl SpanDynAttribute for ::tracing::Span {

fn update_otel_data(otel_data: &mut OtelData, key: &Key, value: &opentelemetry::Value) {
match key.as_str() {
SPAN_NAME_FIELD => otel_data.builder.name = format!("{:?}", value).into(),
SPAN_NAME_FIELD => otel_data.forced_span_name = Some(value.to_string()),
SPAN_KIND_FIELD => otel_data.builder.span_kind = str_to_span_kind(&value.as_str()),
SPAN_STATUS_CODE_FIELD => otel_data.forced_status = str_to_status(&value.as_str()).into(),
SPAN_STATUS_MESSAGE_FIELD => {
Expand Down
44 changes: 44 additions & 0 deletions apollo-router/src/plugins/telemetry/otel/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,7 @@ where
parent_cx,
event_attributes: None,
forced_status: None,
forced_span_name: None,
});
}

Expand Down Expand Up @@ -884,6 +885,7 @@ where
mut builder,
parent_cx,
forced_status,
forced_span_name,
..
}) = extensions.remove::<OtelData>()
{
Expand All @@ -903,6 +905,9 @@ where
if let Some(forced_status) = forced_status {
builder.status = forced_status;
}
if let Some(forced_span_name) = forced_span_name {
builder.name = forced_span_name.into();
}

// Assign end time, build and start span, drop span to export
builder
Expand Down Expand Up @@ -964,8 +969,11 @@ mod tests {
use opentelemetry::trace::TraceFlags;
use opentelemetry::StringValue;
use tracing_subscriber::prelude::*;
use tracing_subscriber::Registry;

use super::*;
use crate::plugins::telemetry::dynamic_attribute::SpanDynAttribute;
use crate::plugins::telemetry::reload::SampledSpan;

#[derive(Debug, Clone)]
struct TestTracer(Arc<Mutex<Option<OtelData>>>);
Expand Down Expand Up @@ -993,6 +1001,7 @@ mod tests {
parent_cx: parent_cx.clone(),
event_attributes: None,
forced_status: None,
forced_span_name: None,
});
noop::NoopSpan::new()
}
Expand Down Expand Up @@ -1089,6 +1098,41 @@ mod tests {
assert_eq!(recorded_name, Some(dynamic_name.into()))
}

#[test]
fn forced_dynamic_span_names() {
let dynamic_name = "GET http://example.com".to_string();
let forced_dynamic_name = "OVERRIDE GET http://example.com".to_string();
let tracer = TestTracer(Arc::new(Mutex::new(None)));
let subscriber = tracing_subscriber::registry().with(layer().with_tracer(tracer.clone()));

tracing::subscriber::with_default(subscriber, || {
let span = tracing::debug_span!("static_name", otel.name = dynamic_name.as_str());
span.with_subscriber(move |(id, dispatch)| {
if let Some(reg) = dispatch.downcast_ref::<Registry>() {
match reg.span(id) {
Some(s) => {
s.extensions_mut().insert(SampledSpan {});
}
None => panic!("should not happen"),
}
}
});
let _entered = span.enter();
span.set_span_dyn_attribute(
Key::from_static_str("otel.name"),
opentelemetry::Value::String(forced_dynamic_name.clone().into()),
);
});

let recorded_name = tracer
.0
.lock()
.unwrap()
.as_ref()
.map(|b| b.builder.name.clone());
assert_eq!(recorded_name, Some(Cow::Owned(forced_dynamic_name)))
}

#[test]
fn span_kind() {
let tracer = TestTracer(Arc::new(Mutex::new(None)));
Expand Down
3 changes: 3 additions & 0 deletions apollo-router/src/plugins/telemetry/otel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,7 @@ pub(crate) struct OtelData {

/// Forced status in case it's coming from the custom attributes
pub(crate) forced_status: Option<opentelemetry::trace::Status>,

/// Forced span name in case it's coming from the custom attributes
pub(crate) forced_span_name: Option<String>,
}
2 changes: 2 additions & 0 deletions apollo-router/src/plugins/telemetry/otel/tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ mod tests {
parent_cx,
event_attributes: None,
forced_status: None,
forced_span_name: None,
});
let span = cx.span();
let span_context = span.span_context();
Expand Down Expand Up @@ -232,6 +233,7 @@ mod tests {
parent_cx,
event_attributes: None,
forced_status: None,
forced_span_name: None,
});

assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion apollo-router/src/plugins/telemetry/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ where
}
}

struct SampledSpan;
pub(crate) struct SampledSpan;

pub(crate) trait IsSampled {
fn is_sampled(&self) -> bool;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,20 @@ The `mode` option will be defaulted to `spec_compliant` in a future release, and
By default spans are marked in error only if the http status code is different than 200. If you want to mark a span in error for other reason you can override the `otel.status_code` attribute which is responsible to mark a span in error or not.
If it's in error then `otel.status_code` = `error`, if not it will be `ok`.

Here is an example if you want to mark the `router` and `supergraph` span in error if you have a graphql error in the payload.
## Span name
bnjjj marked this conversation as resolved.
Show resolved Hide resolved

bnjjj marked this conversation as resolved.
Show resolved Hide resolved
If you want to change the name of spans we're creating for each services you can override this value by setting the `otel.name` attribute using any selectors you want.

Here is an example if you want to mark the `router` and `supergraph` span in error if you have a graphql error in the payload and you want to enforce the `router` span name to be `graphql_router`.

```yaml title="router.yaml"
telemetry:
instrumentation:
spans:
router:
attributes:
otel.name:
static: graphql_router # Override the span name to graphql_router
bnjjj marked this conversation as resolved.
Show resolved Hide resolved
otel.status_code:
static: error
condition:
Expand Down
Loading