Cranker: Logging Improvements for Loki - #276
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes cranker logging to be more Loki-friendly by moving toward single-line, key=value-style log messages and improving timestamp formatting for easier ingestion and ordering in log aggregators.
Changes:
- Updated cranker runtime logs to structured-ish
key=valuemessages and addedwarn!level where appropriate. - Improved env_logger timestamp formatting to include millisecond precision.
- Reduced severity/noise for a common metrics emission condition by switching from
error!towarn!.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crankers/src/vault_handler.rs | Converts vault cranking logs to key=value style and adds warn-level retry logging. |
| crankers/src/metrics.rs | Downgrades a missing-account condition from error to warn while continuing metrics emission for other vaults. |
| crankers/src/bin/main.rs | Makes startup/config logging single-line and enables millisecond timestamps in env_logger for better log aggregation. |
Comments suppressed due to low confidence (2)
crankers/src/metrics.rs:110
- The code builds a
Resultjust to checkis_err()and thenunwrap(); this is more complex than necessary and pays the cost of allocating an error message even on the success path. Prefer matching onOptionfrom the map lookup directly.
let try_st_deposit_account = st_ata_map
.get(&get_associated_token_address(
address,
&vault.supported_mint,
))
.ok_or_else(|| anyhow::anyhow!("ST deposit account not found in map"));
if try_st_deposit_account.is_err() {
warn!(
"Skipping vault metrics, ST deposit account not found vault={}",
address
);
continue;
}
let st_deposit_account = try_st_deposit_account.unwrap();
crankers/src/vault_handler.rs:162
- When the retry limit is hit, the function logs the final RPC error but returns an
anyhow!error without the underlying cause. Returning the terminal error (with context) makes upstream logs/metrics more actionable.
if retries >= MAX_RETRIES {
error!(
"Transaction failed permanently retries={}: {:?}",
MAX_RETRIES, err
);
}
}
Err(anyhow::anyhow!(
"Transaction failed after {} retries",
MAX_RETRIES
))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
ebatsell
approved these changes
Jul 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.