Skip to content

Commit

Permalink
Try not to panic when span lookups fail (#6695)
Browse files Browse the repository at this point in the history
  • Loading branch information
tninesling authored Jan 30, 2025
1 parent b3ee306 commit 675896f
Show file tree
Hide file tree
Showing 7 changed files with 270 additions and 265 deletions.
7 changes: 5 additions & 2 deletions apollo-router/src/plugins/telemetry/apollo_exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,11 @@ impl ApolloExporter {
let retries = if has_traces { 5 } else { 1 };

for i in 0..retries {
// We know these requests can be cloned
let task_req = req.try_clone().expect("requests must be clone-able");
let task_req = req.try_clone().ok_or_else(|| {
ApolloExportError::ServerError(
"Tried to clone a request that cannot be cloned".to_string(),
)
})?;
match self.client.execute(task_req).await {
Ok(v) => {
let status = v.status();
Expand Down
5 changes: 3 additions & 2 deletions apollo-router/src/plugins/telemetry/config_new/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,9 @@ pub(crate) trait DatadogId {
}
impl DatadogId for TraceId {
fn to_datadog(&self) -> String {
let bytes = &self.to_bytes()[std::mem::size_of::<u64>()..std::mem::size_of::<u128>()];
u64::from_be_bytes(bytes.try_into().unwrap()).to_string()
let mut bytes: [u8; 8] = Default::default();
bytes.copy_from_slice(&self.to_bytes()[8..16]);
u64::from_be_bytes(bytes).to_string()
}
}

Expand Down
50 changes: 20 additions & 30 deletions apollo-router/src/plugins/telemetry/dynamic_attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,16 @@ where
id: &tracing_core::span::Id,
ctx: Context<'_, S>,
) {
let span = ctx.span(id).expect("Span not found, this is a bug");
let mut extensions = span.extensions_mut();
if extensions.get_mut::<LogAttributes>().is_none() {
extensions.insert(LogAttributes::default());
}
if extensions.get_mut::<EventAttributes>().is_none() {
extensions.insert(EventAttributes::default());
if let Some(span) = ctx.span(id) {
let mut extensions = span.extensions_mut();
if extensions.get_mut::<LogAttributes>().is_none() {
extensions.insert(LogAttributes::default());
}
if extensions.get_mut::<EventAttributes>().is_none() {
extensions.insert(EventAttributes::default());
}
} else {
tracing::error!("Span not found, this is a bug");
}
}
}
Expand Down Expand Up @@ -134,30 +137,17 @@ impl SpanDynAttribute for ::tracing::Span {
let mut extensions = s.extensions_mut();
match extensions.get_mut::<OtelData>() {
Some(otel_data) => {
if otel_data.builder.attributes.is_none() {
otel_data.builder.attributes = Some(
attributes
.inspect(|attr| {
update_otel_data(
otel_data,
&attr.key,
&attr.value,
)
})
.collect(),
);
let attributes: Vec<KeyValue> = attributes
.inspect(|attr| {
update_otel_data(otel_data, &attr.key, &attr.value)
})
.collect();
if let Some(existing_attributes) =
otel_data.builder.attributes.as_mut()
{
existing_attributes.extend(attributes);
} else {
let attributes: Vec<KeyValue> = attributes
.inspect(|attr| {
update_otel_data(otel_data, &attr.key, &attr.value)
})
.collect();
otel_data
.builder
.attributes
.as_mut()
.unwrap()
.extend(attributes);
otel_data.builder.attributes = Some(attributes);
}
}
None => {
Expand Down
60 changes: 30 additions & 30 deletions apollo-router/src/plugins/telemetry/fmt_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,43 +106,43 @@ where
id: &tracing_core::span::Id,
ctx: Context<'_, S>,
) {
let span = ctx.span(id).expect("Span not found, this is a bug");
let mut visitor = FieldsVisitor::new(&self.excluded_attributes);
// We're checking if it's sampled to not add both attributes in OtelData and our LogAttributes
if !span.is_sampled() {
attrs.record(&mut visitor);
}
let mut extensions = span.extensions_mut();
if let Some(log_attrs) = extensions.get_mut::<LogAttributes>() {
log_attrs.extend(
visitor.values.into_iter().filter_map(|(k, v)| {
if let Some(span) = ctx.span(id) {
let mut visitor = FieldsVisitor::new(&self.excluded_attributes);
// We're checking if it's sampled to not add both attributes in OtelData and our LogAttributes
if !span.is_sampled() {
attrs.record(&mut visitor);
}
let mut extensions = span.extensions_mut();
if let Some(log_attrs) = extensions.get_mut::<LogAttributes>() {
log_attrs.extend(visitor.values.into_iter().filter_map(|(k, v)| {
Some(KeyValue::new(Key::new(k), v.maybe_to_otel_value()?))
}),
);
} else {
let mut fields = LogAttributes::default();
fields.extend(
visitor.values.into_iter().filter_map(|(k, v)| {
}));
} else {
let mut fields = LogAttributes::default();
fields.extend(visitor.values.into_iter().filter_map(|(k, v)| {
Some(KeyValue::new(Key::new(k), v.maybe_to_otel_value()?))
}),
);
extensions.insert(fields);
}));
extensions.insert(fields);
}
} else {
tracing::error!("Span not found, this is a bug");
}
}

fn on_record(&self, id: &Id, values: &Record<'_>, ctx: Context<'_, S>) {
let span = ctx.span(id).expect("Span not found, this is a bug");
let mut extensions = span.extensions_mut();
if let Some(fields) = extensions.get_mut::<LogAttributes>() {
let mut visitor = FieldsVisitor::new(&self.excluded_attributes);
values.record(&mut visitor);
fields.extend(
visitor.values.into_iter().filter_map(|(k, v)| {
if let Some(span) = ctx.span(id) {
let mut extensions = span.extensions_mut();
if let Some(fields) = extensions.get_mut::<LogAttributes>() {
let mut visitor = FieldsVisitor::new(&self.excluded_attributes);
values.record(&mut visitor);
fields.extend(visitor.values.into_iter().filter_map(|(k, v)| {
Some(KeyValue::new(Key::new(k), v.maybe_to_otel_value()?))
}),
);
}));
} else {
eprintln!("cannot access to LogAttributes, this is a bug");
}
} else {
eprintln!("cannot access to LogAttributes, this is a bug");
tracing::error!("Span not found, this is a bug");
}
}

Expand Down Expand Up @@ -431,7 +431,7 @@ connector:

impl std::fmt::Display for LogBuffer {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let content = String::from_utf8(self.0.lock().clone()).unwrap();
let content = String::from_utf8(self.0.lock().clone()).map_err(|_e| std::fmt::Error)?;

write!(f, "{content}")
}
Expand Down
Loading

0 comments on commit 675896f

Please sign in to comment.