Skip to content

Commit

Permalink
Clean some logs (#455)
Browse files Browse the repository at this point in the history
## 📝 Summary

This format of logs
```
error!(?err, "Error while receiving CL SEE event, ignoring");
```
is better than
```
error!("Error while receiving CL SEE event: {:?}, ignoring", err);
```

because when logged to json in the first case error will be in a
different structure field and message fields would be the same for
different errors. This makes log dashboards more readable and flexible.

## 💡 Motivation and Context

<!--- (Optional) Why is this change required? What problem does it
solve? Remove this section if not applicable. -->

---

## ✅ I have completed the following steps:

* [ ] Run `make lint`
* [ ] Run `make test`
* [ ] Added tests (if applicable)
  • Loading branch information
dvush authored Feb 28, 2025
1 parent d82e9da commit 70e3962
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 34 deletions.
4 changes: 2 additions & 2 deletions crates/rbuilder/src/backtest/backtest_build_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ fn spawn_block_fetcher(
let mut blocks = match historical_data_storage.read_blocks(blocks).await {
Ok(res) => res,
Err(err) => {
warn!("Failed to read blocks from storage: {:?}", err);
warn!(?err, "Failed to read blocks from storage");
return;
}
};
Expand All @@ -428,7 +428,7 @@ fn spawn_block_fetcher(
match sender.send(blocks).await {
Ok(_) => {}
Err(err) => {
warn!("Failed to send blocks to processing: {:?}", err);
warn!(?err, "Failed to send blocks to processing");
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,7 @@ impl PrioritizedOrderStore {
}
}
let retain_order = valid && valid_nonces > 0;
tracing::trace!(
"invalidated order: {:?}, retain: {}",
order_id,
retain_order
);
tracing::trace!(order = ?order_id, retain_order, "invalidated order");
if retain_order {
self.insert_order(order);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl<SinkType: SimulatedOrderSink> MultiBackrunManager<SinkType> {
self.regenerate_multi_order();
return Some(order.sim_order);
}
error!("sorted order not found for {:?}", key);
error!(?key, "sorted order not found");
None
}

Expand All @@ -197,10 +197,7 @@ impl<SinkType: SimulatedOrderSink> MultiBackrunManager<SinkType> {
}
let merged_order = self.merge_orders();
if merged_order.is_none() {
error!(
"Failed to generate order for user bundle {:?}",
self.user_bundle_hash
);
error!(bundle_hash = ?self.user_bundle_hash, "Failed to generate order for user bundle");
return;
}
let merged_order = merged_order.unwrap();
Expand Down Expand Up @@ -274,7 +271,7 @@ impl<SinkType: SimulatedOrderSink> ShareBundleMerger<SinkType> {
return None;
};
let first_item = sbundle.inner_bundle.body.first().or_else(|| {
error!("Empty sbundle {:?}", sbundle);
error!(?sbundle, "Empty sbundle");
None
})?;

Expand Down
16 changes: 6 additions & 10 deletions crates/rbuilder/src/building/sim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,9 @@ where
if !nonce.optional {
// this order will never be valid
trace!(
id = order.id().to_string(),
"Dropping order because of nonce: {:?}",
nonce
order = ?order.id(),
?nonce,
"Dropping order because of nonce"
);
return Ok(OrderNonceState::Invalid);
} else {
Expand Down Expand Up @@ -372,8 +372,8 @@ where
OrderSimResult::Failed(err) => {
trace!(
order = sim_task.order.id().to_string(),
"Order simulation failed: {:?}",
err
?err,
"Order simulation failed"
);
sim_errors.push(err);
continue;
Expand Down Expand Up @@ -445,11 +445,7 @@ pub fn simulate_order_using_fork<Tracer: SimulationTracer>(
blob_gas_used += res.blob_gas_used;
}
Err(err) => {
tracing::trace!(
"failed to simulate parent order, id: {:?}, err: {:?}",
parent.id(),
err
);
tracing::trace!(parent_order = ?parent.id(), ?err, "failed to simulate parent order");
return Ok(OrderSimResult::Failed(err));
}
}
Expand Down
10 changes: 5 additions & 5 deletions crates/rbuilder/src/live_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ where
if blocklist.contains(&payload.fee_recipient()) {
warn!(
slot = payload.slot(),
"Fee recipient is in blocklist: {:?}",
payload.fee_recipient()
fee_recipient = ?payload.fee_recipient(),
"Fee recipient is in blocklist"
);
continue;
}
Expand Down Expand Up @@ -247,7 +247,7 @@ where
{
Ok(header) => header,
Err(err) => {
warn!(parent_hash = ?payload.parent_block_hash(),"Failed to get parent header for new slot: {:?}", err);
warn!(parent_hash = ?payload.parent_block_hash(), ?err, "Failed to get parent header for new slot");
continue;
}
}
Expand All @@ -262,7 +262,7 @@ where

// notify the order pool that there is a new header
if let Err(err) = header_sender.send(parent_header.clone()).await {
warn!("Failed to send header to builder pool: {:?}", err);
warn!(?err, "Failed to send header to builder pool");
}

inc_active_slots();
Expand Down Expand Up @@ -299,7 +299,7 @@ where
for handle in inner_jobs_handles {
handle
.await
.map_err(|err| warn!("Job handle await error: {:?}", err))
.map_err(|err| warn!(?err, "Job handle await error"))
.unwrap_or_default();
}
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/rbuilder/src/live_builder/order_input/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ where
handle
.await
.map_err(|err| {
tracing::error!("Error while waiting for OrderPoolJobs to finish: {:?}", err)
tracing::error!(?err, "Error while waiting for OrderPoolJobs to finish")
})
.unwrap_or_default();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl CLPayloadSource {
}
}
Err(err) => {
error!("Error while receiving CL SEE event: {:?}, ignoring", err);
error!(?err, "Error while receiving CL SEE event, ignoring");
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions crates/rbuilder/src/utils/error_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ pub async fn spawn_error_storage_writer(
if let Err(err) = storage.write_error_event(&event).await {
warn!(
category = event.category,
"Error writing error event to storage: {:?}", err
?err,
"Error writing error event to storage"
);
}
} else {
Expand Down Expand Up @@ -79,14 +80,14 @@ pub fn store_error_event<T: serde::Serialize>(category: &str, error: &str, paylo
let payload_json = match serde_json::to_string(&payload) {
Ok(res) => res,
Err(err) => {
error!("Error serializing error payload: {:?}", err);
error!(?err, "Error serializing error payload");
return;
}
};
if payload_json.len() > MAX_PAYLOAD_SIZE_BYTES {
error!(
"Error payload is too large, not storing error event. Payload size: {}",
payload_json.len()
payload_size = payload_json.len(),
"Error payload is too large, not storing error event"
);
return;
}
Expand Down

0 comments on commit 70e3962

Please sign in to comment.