From 01d3428fb3d54390de9e2fadf653f26082869c91 Mon Sep 17 00:00:00 2001 From: Brit Myers Date: Wed, 15 Jan 2025 17:04:25 -0500 Subject: [PATCH] use si.error.message instead of error in spans When we do something like `error=?err` the `error` attribute in Honeycomb is mapped to a bool, so we lose the context. By sending the error struct to `si.error.message` we get more context. This would have been helpful when troubleshooting this week, so I tried to update them everywhere for consistency sake. --- lib/cyclone-server/src/execution.rs | 10 +++++----- lib/cyclone-server/src/handlers.rs | 6 +++--- lib/naxum/src/middleware/ack/maintain_progress.rs | 4 ++-- lib/naxum/src/serve.rs | 2 +- lib/pinga-server/src/handlers.rs | 2 +- lib/si-data-pg/src/lib.rs | 2 +- lib/si-pool-noodle/src/instance/cyclone/local_http.rs | 6 +++--- lib/veritech-client/src/lib.rs | 6 +++--- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/cyclone-server/src/execution.rs b/lib/cyclone-server/src/execution.rs index 1c5dcebef2..187976ec06 100644 --- a/lib/cyclone-server/src/execution.rs +++ b/lib/cyclone-server/src/execution.rs @@ -390,22 +390,22 @@ where // 2/3 steps errored so warn about the lower priority error and return the highest // priority (Ok(_), Err(err), Err(shutdown)) => { - warn!(error = ?shutdown, "failed to shutdown child cleanly"); + warn!(si.error.message = ?shutdown, "failed to shutdown child cleanly"); Err(err) } (Err(err), Ok(_), Err(shutdown)) => { - warn!(error = ?shutdown, "failed to shutdown child cleanly"); + warn!(si.error.message = ?shutdown, "failed to shutdown child cleanly"); Err(err) } (Err(err), Err(closed), Ok(_)) => { - warn!(error = ?closed, "failed to cleanly close websocket"); + warn!(si.error.message = ?closed, "failed to cleanly close websocket"); Err(err) } // All steps failed so warn about the lower priorities and return the highest priority (Err(err), Err(closed), Err(shutdown)) => { - warn!(error = ?shutdown, "failed to shutdown child cleanly"); - warn!(error = ?closed, "failed to cleanly close websocket"); + warn!(si.error.message = ?shutdown, "failed to shutdown child cleanly"); + warn!(si.error.message = ?closed, "failed to cleanly close websocket"); Err(err) } } diff --git a/lib/cyclone-server/src/handlers.rs b/lib/cyclone-server/src/handlers.rs index 17de084526..4b1ec4ee77 100644 --- a/lib/cyclone-server/src/handlers.rs +++ b/lib/cyclone-server/src/handlers.rs @@ -289,7 +289,7 @@ async fn handle_socket( match execution.start(&mut socket).await { Ok(started) => started, Err(err) => { - warn!(error = ?err, "failed to start protocol"); + warn!(si.error.message = ?err, "failed to start protocol"); request_span.record_err(&err); if let Err(err) = fail_to_process(socket, "failed to start protocol", success_marker).await @@ -307,7 +307,7 @@ async fn handle_socket( let proto = match proto.process(&mut socket).await { Ok(processed) => processed, Err(err) => { - warn!(error = ?err, "failed to process protocol"); + warn!(si.error.message = ?err, "failed to process protocol"); request_span.record_err(&err); if let Err(err) = fail_to_process( socket, @@ -327,7 +327,7 @@ async fn handle_socket( }; if let Err(err) = proto.finish(socket).await { request_span.record_err(&err); - warn!(error = ?err, "failed to finish protocol"); + warn!(si.error.message = ?err, "failed to finish protocol"); return; } diff --git a/lib/naxum/src/middleware/ack/maintain_progress.rs b/lib/naxum/src/middleware/ack/maintain_progress.rs index be2e6caeb2..28a33a9172 100644 --- a/lib/naxum/src/middleware/ack/maintain_progress.rs +++ b/lib/naxum/src/middleware/ack/maintain_progress.rs @@ -30,7 +30,7 @@ impl MaintainProgressTask { trace!(task = Self::NAME, "running task"); debug!(task = Self::NAME, "first ack message"); if let Err(err) = self.acker.ack_with(jetstream::AckKind::Progress).await { - warn!(error = ?err, "failed initial ack"); + warn!(si.error.message = ?err, "failed initial ack"); } loop { @@ -42,7 +42,7 @@ impl MaintainProgressTask { _ = self.interval.tick() => { debug!(task = Self::NAME, "acking message with progress"); if let Err(err) = self.acker.ack_with(jetstream::AckKind::Progress).await { - warn!(error = ?err, "failed to ack with progress"); + warn!(si.error.message = ?err, "failed to ack with progress"); } } } diff --git a/lib/naxum/src/serve.rs b/lib/naxum/src/serve.rs index 62f8883f50..f0ff64836c 100644 --- a/lib/naxum/src/serve.rs +++ b/lib/naxum/src/serve.rs @@ -183,7 +183,7 @@ where Some(Err(err)) => { // TODO(fnichol): this level might need to be `trace!()`, just // unclear at the moment - warn!(error = ?err, "failed to read next message from stream"); + warn!(si.error.message = ?err, "failed to read next message from stream"); failed_count += 1; continue; }, diff --git a/lib/pinga-server/src/handlers.rs b/lib/pinga-server/src/handlers.rs index ef5c30ccee..e168019fc1 100644 --- a/lib/pinga-server/src/handlers.rs +++ b/lib/pinga-server/src/handlers.rs @@ -152,7 +152,7 @@ async fn execute_job( } Err(err) => { error!( - error = ?err, + si.error.message = ?err, job.invocation_id = %id, job.instance = metadata.instance_id(), "job execution failed" diff --git a/lib/si-data-pg/src/lib.rs b/lib/si-data-pg/src/lib.rs index 6beee0f018..dbfaa2a8ce 100644 --- a/lib/si-data-pg/src/lib.rs +++ b/lib/si-data-pg/src/lib.rs @@ -390,7 +390,7 @@ impl PgPool { )] pub async fn test_connection(&self) -> PgPoolResult<()> { let conn = self.pool.get().await.si_inspect_err( - |err| warn!(error = %err, "failed to get test database connection from pool"), + |err| warn!(si.error.message = %err, "failed to get test database connection from pool"), )?; debug!("connected to database"); let row = conn diff --git a/lib/si-pool-noodle/src/instance/cyclone/local_http.rs b/lib/si-pool-noodle/src/instance/cyclone/local_http.rs index bd767b9595..5782600bca 100644 --- a/lib/si-pool-noodle/src/instance/cyclone/local_http.rs +++ b/lib/si-pool-noodle/src/instance/cyclone/local_http.rs @@ -423,7 +423,7 @@ async fn watch_task( _ = Pin::new(&mut shutdown_rx) => { trace!("watch task received shutdown"); if let Err(err) = watch_progress.stop().await { - warn!(error = ?err, "failed to cleanly close the watch session"); + warn!(si.error.message = ?err, "failed to cleanly close the watch session"); } break; } @@ -435,9 +435,9 @@ async fn watch_task( // An error occurred on the stream. We are going to treat this as catastrophic // and end the watch. Some(Err(err)) => { - warn!(error = ?err, "error on watch stream"); + warn!(si.error.message = ?err, "error on watch stream"); if let Err(err) = watch_progress.stop().await { - warn!(error = ?err, "failed to cleanly close the watch session"); + warn!(si.error.message = ?err, "failed to cleanly close the watch session"); } break } diff --git a/lib/veritech-client/src/lib.rs b/lib/veritech-client/src/lib.rs index f8ac746717..daac8e4888 100644 --- a/lib/veritech-client/src/lib.rs +++ b/lib/veritech-client/src/lib.rs @@ -359,11 +359,11 @@ async fn forward_output_task( Ok(output) => { output.process_span.follows_from(&request_span); if let Err(err) = output_tx.send(output.payload).await { - warn!(error = ?err, "output forwarder failed to send message on channel"); + warn!(si.error.message = ?err, "output forwarder failed to send message on channel"); } } Err(err) => { - warn!(error = ?err, "output forwarder received an error on its subscriber") + warn!(si.error.message = ?err, "output forwarder received an error on its subscriber") } } } @@ -372,6 +372,6 @@ async fn forward_output_task( } } if let Err(err) = output_subscriber.unsubscribe_after(0).await { - warn!(error = ?err, "error when unsubscribing from output subscriber"); + warn!(si.error.message = ?err, "error when unsubscribing from output subscriber"); } }