fix: keep exporter diagnostics local and classify rename failures
This commit is contained in:
4 files changed
+174
-9
No files matched your search
+7
-2
@@ -88,8 +88,13 @@ Explicit filters must be valid and nonempty; changing them requires a restart.
|
|||||||
Without explicit filters, `RUST_LOG=warn` suppresses informational spans and logs.
|
Without explicit filters, `RUST_LOG=warn` suppresses informational spans and logs.
|
||||||
Use `wakey_agent=info,wakey=info,wakey_core=info,opentelemetry=warn` for the agent and
|
Use `wakey_agent=info,wakey=info,wakey_core=info,opentelemetry=warn` for the agent and
|
||||||
`wakey_control_plane=info,wakey_core=info,opentelemetry=warn` for CC. Exporter warnings
|
`wakey_control_plane=info,wakey_core=info,opentelemetry=warn` for CC. Exporter warnings
|
||||||
remain local to prevent export-failure feedback loops. Increasing verbosity exports the
|
remain local to prevent export-failure feedback loops. Both OTLP logs and traces
|
||||||
existing debug events too; inspect them before enabling it broadly.
|
exclude OpenTelemetry SDK targets and the `h2`, `hyper`, `hyper_util`, `tonic`
|
||||||
|
and `tower` crate targets, even under broad debug filters. These transport
|
||||||
|
crates also serve application traffic: their diagnostics stay console-only,
|
||||||
|
while Wakey's command/storage/relay spans still export. Console filters remain
|
||||||
|
independent. Increasing verbosity exports application debug events too; inspect
|
||||||
|
them before enabling it broadly.
|
||||||
|
|
||||||
## Signals
|
## Signals
|
||||||
|
|
||||||
|
|||||||
@@ -35,6 +35,15 @@ pub async fn rename_known_device(
|
|||||||
AxumPath(device_id): AxumPath<String>,
|
AxumPath(device_id): AxumPath<String>,
|
||||||
Json(req): Json<RenameKnownDeviceRequest>,
|
Json(req): Json<RenameKnownDeviceRequest>,
|
||||||
) -> Result<impl IntoResponse, ApiError> {
|
) -> Result<impl IntoResponse, ApiError> {
|
||||||
|
// The store validates too, but checking here distinguishes bad input from
|
||||||
|
// storage failures without interpreting anyhow error messages.
|
||||||
|
if req.display_name.trim().is_empty() {
|
||||||
|
return Err(ApiError::new(
|
||||||
|
StatusCode::BAD_REQUEST,
|
||||||
|
"rename_known_device_failed",
|
||||||
|
"display_name must not be empty",
|
||||||
|
));
|
||||||
|
}
|
||||||
match state
|
match state
|
||||||
.store
|
.store
|
||||||
.rename_known_device(&device_id, &req.display_name)
|
.rename_known_device(&device_id, &req.display_name)
|
||||||
@@ -49,7 +58,7 @@ pub async fn rename_known_device(
|
|||||||
Err(err) => {
|
Err(err) => {
|
||||||
warn!(error = %err, "failed to rename known device");
|
warn!(error = %err, "failed to rename known device");
|
||||||
Err(ApiError::new(
|
Err(ApiError::new(
|
||||||
StatusCode::BAD_REQUEST,
|
StatusCode::INTERNAL_SERVER_ERROR,
|
||||||
"rename_known_device_failed",
|
"rename_known_device_failed",
|
||||||
err.to_string(),
|
err.to_string(),
|
||||||
))
|
))
|
||||||
@@ -294,3 +303,112 @@ fn known_device_response(device: crate::state::KnownDevice) -> KnownDeviceRespon
|
|||||||
.collect(),
|
.collect(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use std::collections::HashMap;
|
||||||
|
use std::sync::Arc;
|
||||||
|
use std::time::Duration;
|
||||||
|
use tokio::sync::{Mutex, RwLock};
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn rename_distinguishes_invalid_names_missing_devices_and_storage_failures() {
|
||||||
|
let dir = std::env::temp_dir().join(format!("wakey-rename-api-{}", uuid::Uuid::new_v4()));
|
||||||
|
let store = Arc::new(
|
||||||
|
crate::state::Store::load_or_init(
|
||||||
|
&dir.join("state.sqlite3"),
|
||||||
|
vec![],
|
||||||
|
Duration::from_secs(60),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap(),
|
||||||
|
);
|
||||||
|
let state = AppState {
|
||||||
|
store: store.clone(),
|
||||||
|
sessions: Arc::new(RwLock::new(HashMap::new())),
|
||||||
|
pending: Arc::new(Mutex::new(HashMap::new())),
|
||||||
|
public_url: "http://localhost".into(),
|
||||||
|
command_timeout: Duration::from_secs(30),
|
||||||
|
enroll_token_ttl: Duration::from_secs(60),
|
||||||
|
terminals: crate::runtime::terminals::TerminalRegistry::new(),
|
||||||
|
alerts: crate::api::AlertsHub::new(),
|
||||||
|
fleet: Arc::new(crate::api::FleetCache::default()),
|
||||||
|
};
|
||||||
|
let device = store
|
||||||
|
.create_known_device(KnownDeviceInput {
|
||||||
|
display_name: "before".into(),
|
||||||
|
pinned: false,
|
||||||
|
notes: None,
|
||||||
|
identifiers: vec![],
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let rename = |id: String, name: &str| {
|
||||||
|
rename_known_device(
|
||||||
|
State(state.clone()),
|
||||||
|
AxumPath(id),
|
||||||
|
Json(RenameKnownDeviceRequest {
|
||||||
|
display_name: name.into(),
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
};
|
||||||
|
let response = rename(device.device_id.clone(), " after ")
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.into_response();
|
||||||
|
assert_eq!(response.status(), StatusCode::OK);
|
||||||
|
assert_eq!(
|
||||||
|
store
|
||||||
|
.get_known_device(&device.device_id)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.unwrap()
|
||||||
|
.display_name,
|
||||||
|
"after"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
rename("missing".into(), "valid")
|
||||||
|
.await
|
||||||
|
.err()
|
||||||
|
.unwrap()
|
||||||
|
.status,
|
||||||
|
StatusCode::NOT_FOUND
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
rename(device.device_id.clone(), " \t\n ")
|
||||||
|
.await
|
||||||
|
.err()
|
||||||
|
.unwrap()
|
||||||
|
.status,
|
||||||
|
StatusCode::BAD_REQUEST
|
||||||
|
);
|
||||||
|
// A deterministic SQLite failure, not a clock-sensitive lock timeout.
|
||||||
|
let mut tx = store.begin_write().await.unwrap();
|
||||||
|
sqlx::query("CREATE TRIGGER reject_rename BEFORE UPDATE ON known_devices BEGIN SELECT RAISE(ABORT, 'test rename failure'); END")
|
||||||
|
.execute(&mut *tx).await.unwrap();
|
||||||
|
tx.commit().await.unwrap();
|
||||||
|
let failure = rename(device.device_id.clone(), "valid")
|
||||||
|
.await
|
||||||
|
.err()
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(failure.status, StatusCode::INTERNAL_SERVER_ERROR);
|
||||||
|
assert_eq!(failure.code, "rename_known_device_failed");
|
||||||
|
assert_eq!(
|
||||||
|
store
|
||||||
|
.get_known_device(&device.device_id)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.unwrap()
|
||||||
|
.display_name,
|
||||||
|
"after"
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
rename(device.device_id, " ").await.err().unwrap().status,
|
||||||
|
StatusCode::BAD_REQUEST
|
||||||
|
);
|
||||||
|
drop(state);
|
||||||
|
drop(store);
|
||||||
|
std::fs::remove_dir_all(dir).unwrap();
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -181,14 +181,14 @@ pub fn init(
|
|||||||
let _ = METRICS.set(Instruments::new());
|
let _ = METRICS.set(Instruments::new());
|
||||||
let otel = tracing_opentelemetry::layer()
|
let otel = tracing_opentelemetry::layer()
|
||||||
.with_tracer(traces.tracer(service.to_owned()))
|
.with_tracer(traces.tracer(service.to_owned()))
|
||||||
.with_filter(trace_filter);
|
.with_filter(trace_filter)
|
||||||
// Exporter failures must remain local, never feed back into the failing exporter.
|
.with_filter(tracing_subscriber::filter::filter_fn(not_exporter_internal));
|
||||||
|
// Apply the same exclusion to both OTLP layers. Console filtering stays
|
||||||
|
// independent so exporter and transport diagnostics remain available.
|
||||||
let logs_layer =
|
let logs_layer =
|
||||||
opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge::new(&logs)
|
opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge::new(&logs)
|
||||||
.with_filter(log_filter)
|
.with_filter(log_filter)
|
||||||
.with_filter(tracing_subscriber::filter::filter_fn(|metadata| {
|
.with_filter(tracing_subscriber::filter::filter_fn(not_exporter_internal));
|
||||||
!metadata.target().starts_with("opentelemetry")
|
|
||||||
}));
|
|
||||||
let subscriber = tracing_subscriber::registry().with(otel).with(logs_layer);
|
let subscriber = tracing_subscriber::registry().with(otel).with(logs_layer);
|
||||||
if config.json_logs {
|
if config.json_logs {
|
||||||
subscriber
|
subscriber
|
||||||
@@ -235,6 +235,17 @@ pub fn init(
|
|||||||
Ok(guard)
|
Ok(guard)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Exporting the exporter's own diagnostics can create a perpetual feedback
|
||||||
|
/// loop. Transport targets are shared with application traffic, so exclude
|
||||||
|
/// them from OTLP altogether while retaining console diagnostics.
|
||||||
|
fn not_exporter_internal(metadata: &tracing::Metadata<'_>) -> bool {
|
||||||
|
let target = metadata.target();
|
||||||
|
let root = target.split("::").next().unwrap_or(target);
|
||||||
|
!(root == "opentelemetry"
|
||||||
|
|| root.starts_with("opentelemetry_")
|
||||||
|
|| matches!(root, "h2" | "hyper" | "hyper_util" | "tonic" | "tower"))
|
||||||
|
}
|
||||||
|
|
||||||
fn signal_filter(
|
fn signal_filter(
|
||||||
explicit: Option<&str>,
|
explicit: Option<&str>,
|
||||||
rust_log: Option<&str>,
|
rust_log: Option<&str>,
|
||||||
|
|||||||
@@ -238,7 +238,7 @@ async fn exports_all_signals_and_preserves_remote_command_parent() {
|
|||||||
json_logs: false,
|
json_logs: false,
|
||||||
console_filter: Some("info".into()),
|
console_filter: Some("info".into()),
|
||||||
otlp_log_filter: Some("debug".into()),
|
otlp_log_filter: Some("debug".into()),
|
||||||
trace_filter: Some("info".into()),
|
trace_filter: Some("info,opentelemetry_sdk=debug,h2=debug,hyper=debug,hyper_util=debug,tonic=debug,tower=debug".into()),
|
||||||
},
|
},
|
||||||
"unused",
|
"unused",
|
||||||
"info",
|
"info",
|
||||||
@@ -295,6 +295,20 @@ async fn exports_all_signals_and_preserves_remote_command_parent() {
|
|||||||
let hidden = tracing::debug_span!("debug_span_not_exported");
|
let hidden = tracing::debug_span!("debug_span_not_exported");
|
||||||
drop(hidden);
|
drop(hidden);
|
||||||
tracing::warn!(target: "opentelemetry_sdk", "synthetic exporter failure remains local");
|
tracing::warn!(target: "opentelemetry_sdk", "synthetic exporter failure remains local");
|
||||||
|
macro_rules! transport_activity {
|
||||||
|
($target:literal) => {{
|
||||||
|
let span = tracing::debug_span!(target: $target, "exporter_transport_span");
|
||||||
|
span.in_scope(|| tracing::debug!(target: $target, "synthetic exporter transport event"));
|
||||||
|
}};
|
||||||
|
}
|
||||||
|
transport_activity!("opentelemetry_sdk");
|
||||||
|
transport_activity!("h2::proto");
|
||||||
|
transport_activity!("hyper::client");
|
||||||
|
transport_activity!("hyper_util::client");
|
||||||
|
transport_activity!("tonic::transport");
|
||||||
|
transport_activity!("tower::buffer");
|
||||||
|
// A similarly named application target is not transport infrastructure.
|
||||||
|
tracing::info!(target: "tower_device", "application target remains exported");
|
||||||
let invalid_parent = tracing::info_span!("invalid_remote_parent");
|
let invalid_parent = tracing::info_span!("invalid_remote_parent");
|
||||||
telemetry::set_remote_parent(
|
telemetry::set_remote_parent(
|
||||||
&invalid_parent,
|
&invalid_parent,
|
||||||
@@ -332,6 +346,12 @@ async fn exports_all_signals_and_preserves_remote_command_parent() {
|
|||||||
assert!(invalid_parent.parent_span_id.is_empty());
|
assert!(invalid_parent.parent_span_id.is_empty());
|
||||||
assert_ne!(parent.trace_id, invalid_parent.trace_id);
|
assert_ne!(parent.trace_id, invalid_parent.trace_id);
|
||||||
let logs = collector.logs.lock().unwrap();
|
let logs = collector.logs.lock().unwrap();
|
||||||
|
assert!(
|
||||||
|
!spans
|
||||||
|
.iter()
|
||||||
|
.any(|span| span.name == "exporter_transport_span"),
|
||||||
|
"exporter transport spans must not feed back into OTLP"
|
||||||
|
);
|
||||||
assert!(
|
assert!(
|
||||||
!spans
|
!spans
|
||||||
.iter()
|
.iter()
|
||||||
@@ -343,6 +363,17 @@ async fn exports_all_signals_and_preserves_remote_command_parent() {
|
|||||||
.flat_map(|r| &r.scope_logs)
|
.flat_map(|r| &r.scope_logs)
|
||||||
.flat_map(|s| &s.log_records)
|
.flat_map(|s| &s.log_records)
|
||||||
.collect();
|
.collect();
|
||||||
|
assert!(
|
||||||
|
!records
|
||||||
|
.iter()
|
||||||
|
.any(|r| format!("{:?}", r.body).contains("synthetic exporter transport event")),
|
||||||
|
"exporter transport events must not feed back into OTLP"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
records
|
||||||
|
.iter()
|
||||||
|
.any(|r| format!("{:?}", r.body).contains("application target remains exported"))
|
||||||
|
);
|
||||||
assert!(
|
assert!(
|
||||||
records
|
records
|
||||||
.iter()
|
.iter()
|
||||||
|
|||||||
Reference in new issue
Block a user