Otel: dedupe LLM span error/status + traceparent helpers (#130 follow-up)
Addresses the P2 refactoring follow-ups parked on issue #130: - otel::with_traceparent(builder, &SpanContext) — one shared W3C traceparent injector. Replaces the 4-line inline header loop in llamacpp (3x) and openrouter (3x) and removes ollama's private inject_traceparent method (4 call sites now use the shared helper). Takes &SpanContext (not &dyn Span) because the Span trait is not dyn compatible in opentelemetry 0.31. - otel::record_http_failure(span, status, message) and otel::set_http_status(span, status) — collapse the repeated 'set http.status_code attribute + set_status(Status::error)' boilerplate across all three clients (~10 sites). Net -45 lines. The remaining if-let-Some(span.as_mut()) guards (token attributes, parse-failure status) stay inline rather than growing a SpanHandle wrapper type for a pattern that's already readable.
This commit is contained in:
1 parent
696d9fa8f3
commit
10ed84d7b4
4 files changed
+108
-153
No files matched your search
+29
-64
@@ -2,7 +2,7 @@ use anyhow::Context as _;
|
||||
use anyhow::Result;
|
||||
use async_trait::async_trait;
|
||||
use chrono::NaiveDate;
|
||||
use opentelemetry::trace::{Span, Status, TraceContextExt};
|
||||
use opentelemetry::trace::{Span, Status};
|
||||
use opentelemetry::{Context, KeyValue};
|
||||
use reqwest::Client;
|
||||
use serde::{Deserialize, Serialize};
|
||||
@@ -12,7 +12,6 @@ use std::sync::{Arc, Mutex};
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use crate::ai::llm_client::{LlmClient, LlmStreamEvent};
|
||||
use crate::otel::traceparent_headers;
|
||||
use futures::stream::{BoxStream, StreamExt};
|
||||
|
||||
// Re-export shared types so existing `crate::ai::ollama::{...}` imports
|
||||
@@ -392,7 +391,7 @@ impl OllamaClient {
|
||||
.post(format!("{}/api/generate", url))
|
||||
.json(&request);
|
||||
if let Some(s) = span.as_mut() {
|
||||
request_builder = self.inject_traceparent(request_builder, s);
|
||||
request_builder = crate::otel::with_traceparent(request_builder, s.span_context());
|
||||
}
|
||||
let response = match request_builder.send().await {
|
||||
Ok(r) => r,
|
||||
@@ -406,10 +405,11 @@ impl OllamaClient {
|
||||
|
||||
if !response.status().is_success() {
|
||||
let status = response.status();
|
||||
if let Some(s) = span.as_mut() {
|
||||
s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64));
|
||||
s.set_status(Status::error(format!("Ollama request failed: {status}")));
|
||||
}
|
||||
crate::otel::record_http_failure(
|
||||
span.as_deref_mut(),
|
||||
status.as_u16(),
|
||||
&format!("Ollama request failed: {status}"),
|
||||
);
|
||||
let error_body = response.text().await.unwrap_or_default();
|
||||
return Err(anyhow::anyhow!(
|
||||
"Ollama request failed: {} - {}",
|
||||
@@ -417,12 +417,7 @@ impl OllamaClient {
|
||||
error_body
|
||||
));
|
||||
}
|
||||
if let Some(s) = span.as_mut() {
|
||||
s.set_attribute(KeyValue::new(
|
||||
"http.status_code",
|
||||
response.status().as_u16() as i64,
|
||||
));
|
||||
}
|
||||
crate::otel::set_http_status(span.as_deref_mut(), response.status().as_u16());
|
||||
|
||||
let result: OllamaResponse = match response.json().await {
|
||||
Ok(r) => r,
|
||||
@@ -451,22 +446,6 @@ impl OllamaClient {
|
||||
Ok(result.response)
|
||||
}
|
||||
|
||||
/// Attach the W3C traceparent header to an outgoing Ollama request so
|
||||
/// a server that understands it can parent its own spans onto ours.
|
||||
fn inject_traceparent(
|
||||
&self,
|
||||
builder: reqwest::RequestBuilder,
|
||||
span: &opentelemetry::global::BoxedSpan,
|
||||
) -> reqwest::RequestBuilder {
|
||||
let mut builder = builder;
|
||||
for (k, v) in traceparent_headers(
|
||||
&Context::new().with_remote_span_context(span.span_context().clone()),
|
||||
) {
|
||||
builder = builder.header(k, v);
|
||||
}
|
||||
builder
|
||||
}
|
||||
|
||||
/// Start the `llm.request` span for an outgoing Ollama call.
|
||||
fn start_request_span(
|
||||
&self,
|
||||
@@ -899,7 +878,7 @@ Analyze the image and use specific details from both the visual content and the
|
||||
|
||||
let response = {
|
||||
let mut rb = self.client.post(&url).json(&request_body);
|
||||
rb = self.inject_traceparent(rb, &span);
|
||||
rb = crate::otel::with_traceparent(rb, span.span_context());
|
||||
match rb.send().await {
|
||||
Ok(r) => r,
|
||||
Err(e) => {
|
||||
@@ -912,10 +891,11 @@ Analyze the image and use specific details from both the visual content and the
|
||||
|
||||
if !response.status().is_success() {
|
||||
let status = response.status();
|
||||
span.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64));
|
||||
span.set_status(Status::error(format!(
|
||||
"Ollama stream request failed: {status}"
|
||||
)));
|
||||
crate::otel::record_http_failure(
|
||||
Some(&mut span),
|
||||
status.as_u16(),
|
||||
&format!("Ollama stream request failed: {status}"),
|
||||
);
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
anyhow::bail!(
|
||||
"Ollama stream request failed with status {}: {}",
|
||||
@@ -923,10 +903,7 @@ Analyze the image and use specific details from both the visual content and the
|
||||
body
|
||||
);
|
||||
}
|
||||
span.set_attribute(KeyValue::new(
|
||||
"http.status_code",
|
||||
response.status().as_u16() as i64,
|
||||
));
|
||||
crate::otel::set_http_status(Some(&mut span), response.status().as_u16());
|
||||
|
||||
// Ollama streams NDJSON: each line is a full `OllamaStreamChunk`.
|
||||
// We buffer partial lines across chunks from the byte stream.
|
||||
@@ -1064,7 +1041,7 @@ Analyze the image and use specific details from both the visual content and the
|
||||
let response = {
|
||||
let mut rb = self.client.post(&url).json(&request_body);
|
||||
if let Some(s) = span.as_mut() {
|
||||
rb = self.inject_traceparent(rb, s);
|
||||
rb = crate::otel::with_traceparent(rb, s.span_context());
|
||||
}
|
||||
match rb.send().await {
|
||||
Ok(r) => r,
|
||||
@@ -1080,12 +1057,11 @@ Analyze the image and use specific details from both the visual content and the
|
||||
|
||||
if !response.status().is_success() {
|
||||
let status = response.status();
|
||||
if let Some(s) = span.as_mut() {
|
||||
s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64));
|
||||
s.set_status(Status::error(format!(
|
||||
"Ollama chat request failed: {status}"
|
||||
)));
|
||||
}
|
||||
crate::otel::record_http_failure(
|
||||
span.as_deref_mut(),
|
||||
status.as_u16(),
|
||||
&format!("Ollama chat request failed: {status}"),
|
||||
);
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
// warn, not error — the outer `chat_with_tools` may recover via
|
||||
// the fallback server. When both fail, the outer layer emits the
|
||||
@@ -1102,12 +1078,7 @@ Analyze the image and use specific details from both the visual content and the
|
||||
body
|
||||
);
|
||||
}
|
||||
if let Some(s) = span.as_mut() {
|
||||
s.set_attribute(KeyValue::new(
|
||||
"http.status_code",
|
||||
response.status().as_u16() as i64,
|
||||
));
|
||||
}
|
||||
crate::otel::set_http_status(span.as_deref_mut(), response.status().as_u16());
|
||||
|
||||
let chat_response: OllamaChatResponse = match response.json().await {
|
||||
Ok(r) => r,
|
||||
@@ -1268,7 +1239,7 @@ Analyze the image and use specific details from both the visual content and the
|
||||
.post(format!("{}/api/embed", url))
|
||||
.json(&request);
|
||||
if let Some(s) = span.as_mut() {
|
||||
rb = self.inject_traceparent(rb, s);
|
||||
rb = crate::otel::with_traceparent(rb, s.span_context());
|
||||
}
|
||||
match rb.send().await {
|
||||
Ok(r) => r,
|
||||
@@ -1283,12 +1254,11 @@ Analyze the image and use specific details from both the visual content and the
|
||||
|
||||
if !response.status().is_success() {
|
||||
let status = response.status();
|
||||
if let Some(s) = span.as_mut() {
|
||||
s.set_attribute(KeyValue::new("http.status_code", status.as_u16() as i64));
|
||||
s.set_status(Status::error(format!(
|
||||
"Ollama batch embedding request failed: {status}"
|
||||
)));
|
||||
}
|
||||
crate::otel::record_http_failure(
|
||||
span.as_deref_mut(),
|
||||
status.as_u16(),
|
||||
&format!("Ollama batch embedding request failed: {status}"),
|
||||
);
|
||||
let error_body = response.text().await.unwrap_or_default();
|
||||
return Err(anyhow::anyhow!(
|
||||
"Ollama batch embedding request failed: {} - {}",
|
||||
@@ -1296,12 +1266,7 @@ Analyze the image and use specific details from both the visual content and the
|
||||
error_body
|
||||
));
|
||||
}
|
||||
if let Some(s) = span.as_mut() {
|
||||
s.set_attribute(KeyValue::new(
|
||||
"http.status_code",
|
||||
response.status().as_u16() as i64,
|
||||
));
|
||||
}
|
||||
crate::otel::set_http_status(span.as_deref_mut(), response.status().as_u16());
|
||||
|
||||
let result: OllamaEmbedResponse = match response.json().await {
|
||||
Ok(r) => r,
|
||||
|
||||
Reference in new issue
Block a user