-
Notifications
You must be signed in to change notification settings - Fork 34
refactor: Migrate from log to tracing #458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughReplaced all log crate macro imports/usages with tracing equivalents across the codebase. Removed the log dependency from Cargo.toml and dropped the tracing-log feature from tracing-subscriber. No control-flow or public API changes; only logging backend/import adjustments. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
4f04c4a
to
0296ca1
Compare
Signed-off-by: Dylan Kilkenny <[email protected]>
0296ca1
to
1554d53
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/domain/relayer/solana/rpc/methods/sign_transaction.rs (1)
115-118
: Don’t log full signatures.
info!("Transaction signed successfully with signature: {}", result.signature);
exposes the full signature. Log success without the raw signature or emit a short prefix.Apply this diff:
- info!( - "Transaction signed successfully with signature: {}", - result.signature - ); + info!("Transaction signed successfully.");Optionally, emit a short prefix:
- info!("Transaction signed successfully."); + info!("Transaction signed successfully (sig_prefix={})", + &result.signature.chars().take(8).collect::<String>());src/services/google_cloud_kms/mod.rs (3)
328-341
: Do not log PEM material (sensitive).
debug!("PEM ...: {}", pem_str);
leaks public key PEMs; paired with other logs this can aid key misuse. Log a short fingerprint instead.- debug!("PEM solana: {}", pem_str); + // Avoid logging PEM. Log a stable fingerprint for troubleshooting. + let fp = sha2::Sha256::digest(pem_str.as_bytes()); + debug!("PEM solana fingerprint (sha256, first8): {:02x?}", &fp[..8]);Apply the same redaction to the EVM branch.
355-399
: Replace printlns and redact KMS payload/response.
print!
is noisy and bypasses subscriber filtering.- Full request/response bodies may include signatures and identifiers; avoid dumping them.
- print!("KMS asymmetricSign body: {}", body); + debug!("KMS asymmetricSign body: {{ name: ****, digest/data: <redacted> }}"); ... - debug!("KMS asymmetricSign response: {}", resp); + debug!("KMS asymmetricSign response: {{ signature_len_b64: {} }}", + resp.get("signature").and_then(|v| v.as_str()).map(|s| s.len()).unwrap_or(0));Repeat the same redaction in
sign_evm
.
260-267
: Inconsistent pre-hash for EVM signing.
sign_bytes_evm
uses keccak256, whilesign_evm
uses SHA‑256. That yields different signatures for the same payload and will break Ethereum semantics. Unify on a single path (keccak pre-hash) and reuse one method.- async fn sign_evm(&self, message: &[u8]) -> GoogleCloudKmsResult<Vec<u8>> { - let base_url = self.get_base_url(); - let key_path = self.get_key_path(); - let url = format!("{}/v1/{}:asymmetricSign", base_url, key_path,); - debug!("KMS asymmetricSign URL: {}", url); - - let hash = Sha256::digest(message); - let digest = base64_encode(&hash); - - let body = serde_json::json!({ - "name": key_path, - "digest": { - "sha256": digest - } - }); - - print!("KMS asymmetricSign body: {}", body); - - let resp = self.kms_post(&url, &body).await?; - let signature = resp - .get("signature") - .and_then(|v| v.as_str()) - .ok_or_else(|| GoogleCloudKmsError::MissingField("signature".to_string()))?; - - debug!("KMS asymmetricSign response: {}", resp); - let signature_b64 = - base64_decode(signature).map_err(|e| GoogleCloudKmsError::ParseError(e.to_string()))?; - debug!("Signature b64 decoded: {:?}", signature_b64); - Ok(signature_b64) - } + async fn sign_evm(&self, message: &[u8]) -> GoogleCloudKmsResult<Vec<u8>> { + // Reuse the keccak-based path for Ethereum semantics. + self.sign_bytes_evm(message).await + }If you need both variants, split APIs explicitly:
sign_evm_keccak
andsign_evm_sha256
.Also applies to: 371-379
src/services/gas/evm_gas_price.rs (1)
1-16
: Install a log→tracing bridge or confirm no deps uselog
.
- Findings: tracing-subscriber is in Cargo.toml (line 23); Cargo.lock contains transitive "tracing-log" and "env_logger"; no use of tracing_log/LogTracer::init() or other tracing-log bridge in src; no direct
log::...
macros found.- Action: either re-enable the log-to-tracing bridge or call tracing_log::LogTracer::init() early (e.g., in src/main.rs before other startup logs), or verify all transitive deps emit via tracing instead of log.
src/main.rs (1)
70-75
: Install a log->tracing bridge (tracing_log::LogTracer) or confirm nolog
users.
setup_logging (src/logging/mod.rs) configures tracing_subscriber but does not call tracing_log::LogTracer::init(); a repo search found no directlog
usage, but transitive deps may still emit vialog
. Either call tracing_log::LogTracer::init() early (e.g., at the top of setup_logging) to forwardlog
macros totracing
, or verify & document that no dependencies uselog
.
🧹 Nitpick comments (29)
src/domain/transaction/stellar/prepare/operations.rs (1)
52-55
: Prefer structured fields in logs for better tracing.Use fields instead of string interpolation to improve searchability and JSON output.
- info!( - "Using sequence number {} for operations transaction {}", - sequence_i64, tx.id - ); + info!( + sequence = sequence_i64, + tx_id = %tx.id, + "Using sequence number for operations transaction" + );src/utils/service_info_log.rs (1)
3-3
: LGTM: switched to tracing::info.Optional: consider structured fields (service_name, version, profile, cwd, etc.) instead of interpolated strings to make logs machine-friendly.
src/services/provider/solana/mod.rs (1)
297-301
: Use structured fields in debug log.This improves filtering and keeps message stable across refactors.
- tracing::debug!( - "Starting RPC operation '{}' with timeout: {}s", - operation_name, - self.timeout_seconds.as_secs() - ); + tracing::debug!( + operation = %operation_name, + timeout_secs = self.timeout_seconds.as_secs(), + "Starting RPC operation" + );src/domain/transaction/stellar/prepare/common.rs (1)
8-8
: LGTM: tracing::{info,warn} import.Optional: switch messages with variables (e.g., fee updates) to structured fields.
src/domain/relayer/solana/dex/jupiter_swap.rs (1)
22-22
: Reduce info-level verbosity; avoid logging full encoded transactions at info.Dumping params/quotes/entire swap responses at info can be noisy. Keep “confirmation start/finish” at info; move parameter/quote/tx body logs to debug with concise fields.
- info!("Executing Jupiter swap: {:?}", params); + tracing::debug!(?params, "Executing Jupiter swap"); - info!("Received quote: {:?}", quote); + tracing::debug!(in_amount = quote.in_amount, out_amount = quote.out_amount, "Received quote"); - info!("Received swap transaction: {:?}", swap_tx); + tracing::debug!( + tx_len = swap_tx.swap_transaction.len(), + last_valid_block_height = swap_tx.last_valid_block_height, + "Received swap transaction" + ); - info!("Waiting for transaction confirmation: {}", signature); + info!(%signature, "Waiting for transaction confirmation"); - info!("Transaction confirmed: {}", signature); + info!(%signature, "Transaction confirmed");src/services/signer/solana/local_signer.rs (1)
19-19
: Remove unused import.
tracing::info
isn’t used in this module; drop the import to avoid warnings.-use tracing::info;
src/repositories/transaction/transaction_redis.rs (1)
204-218
: Optional: use structured fields (tracing) for better filtering.Emitting tx_id/nonce/status as fields improves correlation and avoids string formatting costs when disabled.
- debug!("Updating indexes for transaction {}", tx.id); + debug!(tx_id = %tx.id, "Updating indexes for transaction"); ... - debug!("Added nonce index for tx {} with nonce {}", tx.id, nonce); + debug!(tx_id = %tx.id, nonce, "Added nonce index"); ... - debug!( - "Removing old status index for tx {} (status: {} -> {})", - tx.id, old.status, tx.status - ); + debug!(tx_id = %tx.id, old_status = ?old.status, new_status = ?tx.status, "Removing old status index"); ... - debug!( - "Removing old nonce index for tx {} (nonce: {} -> {:?})", - tx.id, old_nonce, new_nonce - ); + debug!(tx_id = %tx.id, old_nonce = old_nonce, new_nonce = ?new_nonce, "Removing old nonce index"); - debug!("Successfully updated indexes for transaction {}", tx.id); + debug!(tx_id = %tx.id, "Successfully updated indexes");Also applies to: 225-241, 254-255
src/domain/transaction/stellar/prepare/mod.rs (1)
38-49
: Optional: add spans with tx_id/relayer_id.
tracing
’sinstrument
helps tie logs for a transaction together.- pub async fn prepare_transaction_impl( + #[tracing::instrument(skip(self, tx), fields(tx_id = %tx.id, relayer_id = %self.relayer().id))] + pub async fn prepare_transaction_impl( &self, tx: TransactionRepoModel, ) -> Result<TransactionRepoModel, TransactionError> { ... - async fn handle_prepare_failure( + #[tracing::instrument(skip(self, tx, error), fields(tx_id = %tx.id, relayer_id = %self.relayer().id))] + async fn handle_prepare_failure( &self, tx: TransactionRepoModel, error: TransactionError, ) -> Result<TransactionRepoModel, TransactionError> {Also applies to: 141-150
src/domain/transaction/stellar/submit.rs (1)
31-45
: Optional: add spans and structured fields.Improve observability during submit/retry flows.
- pub async fn submit_transaction_impl( + #[tracing::instrument(skip(self, tx), fields(tx_id = %tx.id, relayer_id = %self.relayer().id))] + pub async fn submit_transaction_impl( &self, tx: TransactionRepoModel, ) -> Result<TransactionRepoModel, TransactionError> { ... - async fn handle_submit_failure( + #[tracing::instrument(skip(self, tx, error), fields(tx_id = %tx.id, relayer_id = %self.relayer().id))] + async fn handle_submit_failure( &self, tx: TransactionRepoModel, error: TransactionError, ) -> Result<TransactionRepoModel, TransactionError> {Also consider structured logs for key values:
- info!("Submitting Stellar transaction: {:?}", tx.id); + info!(tx_id = %tx.id, "Submitting Stellar transaction");Also applies to: 97-106
src/domain/relayer/evm/evm_relayer.rs (1)
150-158
: Optional: structured fields and instrumentation.Add fields for relayer and counts; add a span for
initialize_relayer
.- debug!( - "Relayer: {} - On-chain nonce: {}, Transaction counter nonce: {}", - self.relayer.id, on_chain_nonce, transaction_counter_nonce - ); + debug!(relayer_id = %self.relayer.id, on_chain_nonce, transaction_counter_nonce, "Nonce sources"); - info!("Setting nonce: {} for relayer: {}", nonce, self.relayer.id); + info!(relayer_id = %self.relayer.id, nonce, "Setting nonce"); - info!( - "Processing {} pending transactions for relayer: {}", - transaction_count, self.relayer.id - ); + info!(relayer_id = %self.relayer.id, count = transaction_count, "Processing pending transactions"); - info!("Completed processing pending transactions for relayer {}: {} queued for cancellation, {} failed to queue", - self.relayer.id, cancelled_transaction_ids.len(), failed_transaction_ids.len()); + info!(relayer_id = %self.relayer.id, queued = cancelled_transaction_ids.len(), failed = failed_transaction_ids.len(), "Completed pending transaction processing"); - async fn initialize_relayer(&self) -> Result<(), RelayerError> { - info!("Initializing relayer: {}", self.relayer.id); + #[tracing::instrument(skip(self), fields(relayer_id = %self.relayer.id))] + async fn initialize_relayer(&self) -> Result<(), RelayerError> { + info!("Initializing relayer");Also applies to: 369-371, 398-405, 513-558
src/domain/relayer/stellar/stellar_relayer.rs (1)
46-46
: Replace println! with tracing for consistency.Two println! calls remain in production paths. Use tracing to keep logs consistent and structured.
Apply this diff (outside the shown hunk):
- println!("Stellar delete_pending_transactions..."); + info!("Stellar delete_pending_transactions...");- println!("Stellar rpc..."); + info!("Stellar rpc...");src/services/signer/stellar/local_signer.rs (1)
38-38
: Remove unused import.
info
isn’t used in this file; avoid lint warnings.-use tracing::info;
src/jobs/handlers/transaction_submission_handler.rs (1)
12-12
: LGTM: tracing import updated.Optional: if
job.data
can be large/sensitive, consider structured fields instead of full debug dump.src/jobs/handlers/transaction_request_handler.rs (1)
29-33
: Prefer structured tracing fields over formatted logsLeverage tracing’s fields to emit one structured event instead of multiple formatted lines.
- info!("Handling transaction request: {:?}", job.data); - info!("Attempt: {:?}", attempt); - info!("Worker: {:?}", worker); - info!("Task ID: {:?}", task_id); - info!("Context: {:?}", ctx); + info!( + job = ?job.data, + attempt = ?attempt, + worker = ?worker, + task_id = %task_id, + ctx = ?ctx, + "Handling transaction request" + );src/services/vault/mod.rs (1)
278-279
: Avoid logging full cryptographic signaturesLogging the entire Vault signature can leak sensitive material into logs. Emit minimal metadata instead (e.g., length) with structured fields.
- debug!("vault_signature_str: {}", vault_signature_str); + debug!( + key = %key_name, + sig_len = %vault_signature_str.len(), + "Vault returned signature" + );src/services/signer/evm/turnkey_signer.rs (1)
23-23
: Remove misleading inline commentThe comment mentions
FutureExt::boxed
, which isn’t imported or used here.-use tracing::{debug, info}; // Import FutureExt to enable the `boxed` method +use tracing::{debug, info};src/services/gas/evm_gas_price.rs (1)
271-279
: Consider adding tracing instrumentation on hot paths.Add an instrument attribute to capture errors and attach key fields; minimal surface change.
- async fn estimate_gas(&self, tx_data: &EvmTransactionData) -> Result<u64, TransactionError> { + #[tracing::instrument(level = "info", skip(self, tx_data), err)] + async fn estimate_gas(&self, tx_data: &EvmTransactionData) -> Result<u64, TransactionError> {src/domain/transaction/solana/status.rs (1)
128-129
: Replace println! with tracing to avoid stdout noise.
Use the existing tracing import and structured field.- println!("solana_status: {:?}", solana_status); + debug!(?solana_status, "solana status");src/jobs/handlers/transaction_status_handler.rs (2)
21-28
: Optional: instrument the handler for better traces.
Adds span context without behavior change.-pub async fn transaction_status_handler( +#[tracing::instrument(skip(state))] +pub async fn transaction_status_handler(
40-44
: Optional: instrument the inner function as well.-async fn handle_request( +#[tracing::instrument(skip(state))] +async fn handle_request(src/main.rs (1)
150-151
: Nit: prefer consistent macro style.
Sinceuse tracing::info;
is imported, useinfo!
instead of fully-qualified calls.- tracing::info!("Metrics server enabled, starting metrics server..."); + info!("Metrics server enabled, starting metrics server..."); @@ - tracing::info!("Metrics server disabled"); + info!("Metrics server disabled");Also applies to: 171-172
src/jobs/handlers/notification_handler.rs (1)
29-35
: Optional: instrument the handler.-pub async fn notification_handler( +#[tracing::instrument(skip(context))] +pub async fn notification_handler(src/jobs/handlers/mod.rs (1)
31-39
: Optional: bump log levels for failures/abort.
Use warn! for retriable failures and error! when aborting. No behavior change, better signal.-use tracing::info; +use tracing::{info, warn, error}; @@ - if result.is_ok() { - info!("{} request handled successfully", job_type); - return Ok(()); - } - info!("{} request failed: {:?}", job_type, result); + if result.is_ok() { + info!("{} request handled successfully", job_type); + return Ok(()); + } + warn!("{} request failed: {:?}", job_type, result); @@ - if attempt.current() >= max_attempts { - info!("Max attempts ({}) reached, failing job", max_attempts); + if attempt.current() >= max_attempts { + error!("Max attempts ({}) reached, failing job", max_attempts);src/services/provider/retry.rs (3)
215-248
: Prefer structured tracing fields and a span around the call.Switch these string-formatted debug!s to structured events so filters and log backends can key off fields. Also consider an instrument span to tie all child logs to a call.
Example refactor within this hunk:
- tracing::debug!( - "Starting RPC call '{}' with max_retries={}, max_failovers={}, available_providers={}", - operation_name, - config.max_retries, - max_failovers, - total_providers - ); + tracing::debug!( + operation = %operation_name, + max_retries = config.max_retries, + max_failovers, + available_providers = total_providers, + "Starting RPC call", + );Optional (outside this hunk) to add a span:
#[tracing::instrument( level = "debug", skip(selector, is_retriable_error, should_mark_provider_failed, provider_initializer, operation, config), fields(operation = %operation_name) )]Also applies to: 262-268
277-314
: Warn logs: emit fields, not formatted strings.These warn!s would be more queryable with fields (error, provider_url, failover idx). Keep the human string, but add fields.
- tracing::warn!( - "All {} retry attempts failed for provider '{}' on operation '{}'. Error: {}. Marking as failed and switching to next provider (failover {}/{})...", - config.max_retries, - provider_url, - operation_name, - last_error.as_ref().unwrap(), - failover_count + 1, - max_failovers - ); + tracing::warn!( + provider = %provider_url, + operation = %operation_name, + retries = config.max_retries, + next_failover = failover_count + 1, + max_failovers, + error = %last_error.as_ref().unwrap(), + "Retries exhausted; marking provider failed and failing over", + );
336-336
: Consistently structure all events in hot paths.Apply the same field-first pattern to error at completion, provider init warnings, per-attempt success/fail, and retry-delay logs to make downstream aggregation/alerting robust.
Example tweaks:
- tracing::error!("{}", error_message); + tracing::error!(operation = %operation_name, total_attempts, failovers = failover_count, error = %error_message, "RPC call failed");- tracing::debug!( - "Retrying RPC call '{}' with provider '{}' after {:?} delay (attempt {}/{})", - operation_name, provider_url, delay, current_attempt_idx + 2, config.max_retries - ); + tracing::debug!( + operation = %operation_name, + provider = %provider_url, + ?delay, + next_attempt = current_attempt_idx + 2, + max_retries = config.max_retries, + "Retrying RPC call", + );Also applies to: 357-359, 364-369, 405-412, 419-430, 438-441, 452-458
src/domain/transaction/evm/utils.rs (1)
37-41
: LGTM on warn! migration; consider structured fields.No functional changes; optionally include fields like
chain_id
,provider
, orerr = %e
for better filtering.Also applies to: 95-96, 107-108
src/models/transaction/repository.rs (1)
762-762
: Use consistent tracing style across the codebase.Importing
use tracing::info;
locally works; consider using fully qualifiedtracing::info!
(as elsewhere) to keep style uniform, or add structured fields:fee = updated_fee
.src/models/transaction/stellar/conversion.rs (1)
85-87
: LGTM on tracing migration; add fields for observability.Keep the strings, but add structured fields (e.g., indicate presence/length of simulation data, include error display) for better searchability.
Also applies to: 91-94
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (70)
Cargo.toml
(1 hunks)src/bootstrap/config_processor.rs
(1 hunks)src/bootstrap/initialize_app_state.rs
(1 hunks)src/bootstrap/initialize_relayers.rs
(1 hunks)src/bootstrap/initialize_workers.rs
(1 hunks)src/domain/relayer/evm/evm_relayer.rs
(1 hunks)src/domain/relayer/solana/dex/jupiter_swap.rs
(1 hunks)src/domain/relayer/solana/dex/jupiter_ultra.rs
(1 hunks)src/domain/relayer/solana/rpc/handler.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/fee_estimate.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/get_supported_tokens.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/prepare_transaction.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/sign_transaction.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/transfer_transaction.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/utils.rs
(1 hunks)src/domain/relayer/solana/rpc/methods/validations.rs
(1 hunks)src/domain/relayer/solana/rpc/mod.rs
(1 hunks)src/domain/relayer/solana/solana_relayer.rs
(1 hunks)src/domain/relayer/solana/token.rs
(1 hunks)src/domain/relayer/stellar/stellar_relayer.rs
(1 hunks)src/domain/transaction/evm/evm_transaction.rs
(1 hunks)src/domain/transaction/evm/status.rs
(1 hunks)src/domain/transaction/evm/utils.rs
(3 hunks)src/domain/transaction/solana/solana_transaction.rs
(1 hunks)src/domain/transaction/solana/status.rs
(1 hunks)src/domain/transaction/stellar/prepare/common.rs
(1 hunks)src/domain/transaction/stellar/prepare/mod.rs
(1 hunks)src/domain/transaction/stellar/prepare/operations.rs
(1 hunks)src/domain/transaction/stellar/prepare/unsigned_xdr.rs
(1 hunks)src/domain/transaction/stellar/status.rs
(1 hunks)src/domain/transaction/stellar/stellar_transaction.rs
(1 hunks)src/domain/transaction/stellar/submit.rs
(1 hunks)src/domain/transaction/stellar/utils.rs
(1 hunks)src/jobs/handlers/mod.rs
(1 hunks)src/jobs/handlers/notification_handler.rs
(1 hunks)src/jobs/handlers/solana_swap_request_handler.rs
(1 hunks)src/jobs/handlers/transaction_cleanup_handler.rs
(1 hunks)src/jobs/handlers/transaction_request_handler.rs
(1 hunks)src/jobs/handlers/transaction_status_handler.rs
(1 hunks)src/jobs/handlers/transaction_submission_handler.rs
(1 hunks)src/jobs/job_producer.rs
(1 hunks)src/jobs/queue.rs
(1 hunks)src/main.rs
(3 hunks)src/models/transaction/repository.rs
(1 hunks)src/models/transaction/stellar/conversion.rs
(1 hunks)src/repositories/network/network_redis.rs
(1 hunks)src/repositories/notification/notification_redis.rs
(1 hunks)src/repositories/plugin/plugin_redis.rs
(1 hunks)src/repositories/redis_base.rs
(1 hunks)src/repositories/relayer/relayer_redis.rs
(1 hunks)src/repositories/signer/signer_redis.rs
(1 hunks)src/repositories/transaction/transaction_redis.rs
(1 hunks)src/repositories/transaction_counter/transaction_counter_redis.rs
(1 hunks)src/services/gas/cache.rs
(1 hunks)src/services/gas/evm_gas_price.rs
(1 hunks)src/services/google_cloud_kms/mod.rs
(1 hunks)src/services/provider/evm/mod.rs
(1 hunks)src/services/provider/retry.rs
(12 hunks)src/services/provider/solana/mod.rs
(1 hunks)src/services/signer/evm/cdp_signer.rs
(1 hunks)src/services/signer/evm/turnkey_signer.rs
(1 hunks)src/services/signer/solana/google_cloud_kms_signer.rs
(1 hunks)src/services/signer/solana/local_signer.rs
(1 hunks)src/services/signer/solana/turnkey_signer.rs
(1 hunks)src/services/signer/solana/vault_transit_signer.rs
(1 hunks)src/services/signer/stellar/local_signer.rs
(1 hunks)src/services/turnkey/mod.rs
(1 hunks)src/services/vault/mod.rs
(1 hunks)src/utils/service_info_log.rs
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: dylankilkenny
PR: OpenZeppelin/openzeppelin-relayer#455
File: Cargo.toml:23-27
Timestamp: 2025-09-10T12:57:16.515Z
Learning: In the OpenZeppelin relayer project, the log-to-tracing bridge is handled automatically by tracing-subscriber and additional refactoring in child PRs removes the need for explicit bridging setup.
📚 Learning: 2025-09-10T12:57:16.515Z
Learnt from: dylankilkenny
PR: OpenZeppelin/openzeppelin-relayer#455
File: Cargo.toml:23-27
Timestamp: 2025-09-10T12:57:16.515Z
Learning: In the OpenZeppelin relayer project, the log-to-tracing bridge is handled automatically by tracing-subscriber and additional refactoring in child PRs removes the need for explicit bridging setup.
Applied to files:
Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: msrv
- GitHub Check: clippy
- GitHub Check: Analyze (rust)
🔇 Additional comments (53)
src/domain/relayer/solana/rpc/mod.rs (1)
10-10
: LGTM: migrated to tracing::error.Import swap is correct and consistent with the PR goal.
src/domain/transaction/stellar/prepare/operations.rs (1)
4-4
: LGTM: switched to tracing::info.No behavior change; aligns with the migration objective.
src/domain/transaction/stellar/prepare/unsigned_xdr.rs (1)
6-6
: LGTM: switched to tracing::info.Change is consistent with the migration.
src/services/provider/solana/mod.rs (1)
47-57
: Verify log→tracing bridge is initialized (LogTracer / tracing-log)Verification run returned no matches — unable to confirm a LogTracer or tracing-subscriber initialization. Manually verify a log→tracing bridge is set up early in startup (e.g., call tracing_log::LogTracer::init() in main.rs or ensure tracing-subscriber is initialized/feature-enabled) so crates emitting log::*(e.g., Solana stack) are routed into tracing.
src/repositories/transaction/transaction_redis.rs (1)
16-16
: LGTM: migrated to tracing.Import switch to
tracing::{debug, error, warn}
is correct; call sites already use these macros.src/services/signer/solana/google_cloud_kms_signer.rs (1)
17-17
: LGTM: tracing import.Switch to
tracing
is consistent with repo-wide migration.src/domain/transaction/stellar/prepare/mod.rs (1)
12-12
: LGTM: tracing import.Matches usage of
info!
/warn!
throughout.src/bootstrap/config_processor.rs (2)
21-21
: LGTM: tracing import.No functional changes; consistent with migration.
21-21
: Verify: did we intentionally drop log→tracing bridging?Repo scan: no direct
log
imports/macros found; Cargo.lock containslog v0.4.28
(pulled by a dependency). If you did not intend to silence log-emitting deps, re-enable bridging (e.g. tracing_log::LogTracer::init() early in bootstrap) or enable the subscriber'stracing-log
feature.src/domain/transaction/stellar/utils.rs (1)
5-5
: LGTM: tracing import.
info!
usage matches.src/domain/transaction/solana/solana_transaction.rs (1)
9-9
: LGTM: tracing import.No behavior change; consistent with other modules.
src/domain/transaction/stellar/submit.rs (1)
6-6
: LGTM: tracing import.
info!
/warn!
calls align with import.src/domain/relayer/evm/evm_relayer.rs (1)
51-51
: LGTM: tracing import.
debug!/info!/warn!
usage aligns.src/bootstrap/initialize_app_state.rs (1)
19-19
: LGTM on switching to tracing::warn.No functional changes; import is correct.
src/jobs/job_producer.rs (1)
22-22
: LGTM on tracing import.Import set covers the macros used in this module.
src/jobs/queue.rs (1)
15-15
: LGTM on tracing import.Matches the
error!
usage in connection handling.src/domain/relayer/solana/rpc/handler.rs (1)
20-20
: LGTM on tracing import.Unqualified
info!
will resolve correctly.src/domain/relayer/solana/rpc/methods/validations.rs (1)
25-25
: LGTM: tracing migration here is correct.Import swap to
tracing::info
matches the PR objective.src/repositories/transaction_counter/transaction_counter_redis.rs (1)
16-16
: LGTM: switched totracing::debug
.No behavior change; consistent with repo-wide migration.
src/services/gas/cache.rs (1)
20-20
: LGTM: moved totracing::info
.All good.
src/services/turnkey/mod.rs (1)
35-35
: LGTM: tracing import updated.Change is mechanical and correct.
src/domain/transaction/stellar/status.rs (1)
8-8
: LGTM: migrated totracing::{info, warn}
.No further action.
src/domain/relayer/solana/rpc/methods/sign_transaction.rs (1)
23-23
: Repo-wide sanity check for lingeringlog::
usage and missingtracing
macro imports — re-run (previous attempt failed)Previous run produced '/dev/fd' errors and "No files were searched". Re-run the corrected script below and paste the output.
#!/usr/bin/env bash # Avoid process-substitution (/dev/fd) — use a pipe-based approach and fixed-string searches. # 1) Find direct uses of the `log` crate macros rg -n --hidden --no-ignore --glob '!target/**' -F -e 'log::trace!(' -e 'log::debug!(' -e 'log::info!(' -e 'log::warn!(' -e 'log::error!(' || true # 2) Find unqualified macro calls (error!/warn!/debug!/info!) and flag files that lack tracing imports/qualifiers for macro in error warn debug info; do echo "---- scanning for missing import of $macro! ----" rg -l --hidden --no-ignore --glob '!target/**' -F -e "${macro}!(" | while IFS= read -r file; do # skip if file explicitly qualifies or imports the tracing macro if rg -q --hidden --no-ignore -F -e "tracing::${macro}!(" "$file"; then continue fi if rg -q --hidden --no-ignore -F -e "use tracing::${macro}" "$file"; then continue fi # if file imports a tracing group (use tracing::{...}) and also contains the macro name, treat as OK if rg -q --hidden --no-ignore -e 'use\s+tracing::\{' "$file" && rg -q --hidden --no-ignore -w -F -e "${macro}" "$file"; then continue fi echo "Potential missing import in: $file" done donesrc/domain/relayer/solana/token.rs (1)
15-15
: LGTM: migrated to tracingImport switch to tracing is correct and consistent with the PR goals.
src/domain/relayer/solana/dex/jupiter_ultra.rs (1)
21-21
: LGTM: migrated to tracingImport switch aligns with repo-wide migration; call sites remain unchanged.
src/jobs/handlers/transaction_cleanup_handler.rs (1)
12-12
: LGTM: migrated to tracingImport set covers all used levels (debug/info/warn/error) in this module.
src/services/signer/evm/cdp_signer.rs (1)
23-23
: LGTM: migrated to tracingImport switch is consistent with the PR; no functional changes.
src/repositories/plugin/plugin_redis.rs (1)
11-11
: Swap to tracing macros looks good.Import migration is correct; call sites already use debug!/warn!/error!.
src/services/gas/evm_gas_price.rs (1)
12-12
: Import migration to tracing is correct.Nothing else to change here.
src/repositories/network/network_redis.rs (1)
17-17
: Tracing import change looks good.No behavior change in this module.
src/jobs/handlers/solana_swap_request_handler.rs (1)
10-10
: Good: switched to tracing::info.Works with existing setup_job_tracing! span.
src/repositories/signer/signer_redis.rs (1)
11-11
: Tracing import migration LGTM.No further action needed here.
src/domain/relayer/solana/rpc/methods/get_supported_tokens.rs (1)
12-12
: Import swap to tracing is correct.No API or flow changes.
src/services/google_cloud_kms/mod.rs (1)
31-31
: Import migration to tracing is fine.Proceeding with additional KMS-specific notes below.
src/repositories/relayer/relayer_redis.rs (1)
12-12
: Tracing import change looks good.No functional impact here.
src/domain/transaction/stellar/stellar_transaction.rs (1)
24-24
: LGTM: switched to tracing::info.Import aligns with usage in this module; no further changes needed.
src/services/provider/evm/mod.rs (1)
259-263
: LGTM: use fully-qualified tracing::debug!.No import required; consistent with local style.
src/bootstrap/initialize_workers.rs (1)
20-20
: LGTM: tracing import updated.
info!
/error!
usage matches the import.src/bootstrap/initialize_relayers.rs (1)
15-15
: LGTM: tracing::info migration.No other changes required here.
src/domain/relayer/solana/solana_relayer.rs (1)
39-39
: LGTM: tracing imports cover error/info/warn.Matches macro usage across the file.
src/domain/relayer/solana/rpc/methods/fee_estimate.rs (1)
28-28
: LGTM: switched to tracing::info.Consistent with the only logging macro used in this file.
src/services/signer/solana/turnkey_signer.rs (1)
17-17
: LGTM: tracing import added.No functional changes.
src/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rs (1)
25-25
: Confirm repo-wide tracing/log migrationNo matches found for leftover
log::
usage or bare logging macros withouttracing
imports in a quick re-scan; this looks clean.src/domain/transaction/solana/status.rs (1)
9-9
: Tracing import migration looks good.src/jobs/handlers/transaction_status_handler.rs (1)
12-12
: Switched to tracing::info — good.src/domain/transaction/evm/evm_transaction.rs (1)
11-11
: Tracing import migration looks good.src/main.rs (1)
45-45
: Tracing import in main is correct.src/domain/transaction/evm/status.rs (1)
8-8
: Tracing import migration looks good.src/jobs/handlers/notification_handler.rs (1)
10-10
: Tracing import migration looks good.src/jobs/handlers/mod.rs (1)
7-7
: Tracing import migration looks good.src/repositories/redis_base.rs (1)
9-9
: Tracing import migration looks good.src/repositories/notification/notification_redis.rs (1)
11-11
: LGTM on the macro import swap.Importing tracing’s debug/error/warn is consistent with the migration; no behavior change.
Cargo.toml (1)
23-23
: Verify whether you still need a log→tracing bridgeScan: no in-repo
log::...!
macros or anytracing_log::LogTracer
/ subscriber init found; root Cargo.toml containstracing-subscriber
and Cargo.lock includestracing-log
andlog
transitively.
Action: if any upstream deps still emit vialog
, addtracing-log = "0.2"
and initialize early (e.g. in src/main.rs:tracing_log::LogTracer::init().ok();
); if all inbound deps emittracing
, no bridge is needed.
use color_eyre::Result; | ||
use log::warn; | ||
use std::sync::Arc; | ||
use tracing::warn; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Quick repo-wide sanity check for logging migration (script included).
Verify no stray log macros/println! remain and that all unqualified tracing macros are imported.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Leftover log crate macro usage:"
rg -nP --type=rust '\blog::\w+!' || true
echo -e "\n2) println! in src (non-tests):"
rg -nP --type=rust -g 'src/**' -g '!**/tests/**' '\bprintln!\s*\(' || true
echo -e "\n3) Files using unqualified tracing macros without importing them:"
for macro in error warn info debug trace; do
echo "---- checking $macro! ----"
for f in $(rg -nP --type=rust "\\b${macro}!\\s*\\(" -l | sort -u); do
# OK if fully-qualified macro is used
if rg -qPn 'tracing::'"$macro"'!\s*\(' "$f"; then
continue
fi
# OK if the macro is imported from tracing
if rg -qPn 'use\s+tracing::\s*{\s*[^}]*\b'"$macro"'\b[^}]*}\s*;' "$f" || rg -qPn 'use\s+tracing::\s*'"$macro"'\s*;' "$f"; then
continue
fi
echo "Missing import for tracing::$macro in $f"
done
done
Length of output: 1893
Action required: complete logging migration — remove println! and add missing tracing imports
Found stray println! calls and unqualified tracing macros missing imports.
-
println! occurrences (replace with appropriate tracing macros):
src/utils/secp256k.rs:50
src/services/plugins/socket.rs:150
src/models/signer/mod.rs:883
src/domain/transaction/solana/status.rs:128
src/domain/transaction/evm/price_calculator.rs:1007
src/domain/relayer/stellar/stellar_relayer.rs:337, 364
src/domain/relayer/solana/rpc/methods/get_features_enabled.rs:76
src/domain/relayer/solana/rpc/methods/prepare_transaction.rs:683
src/domain/relayer/solana/rpc/methods/utils.rs:2596–2597 -
Missing import for tracing::error (unqualified error! used in):
src/domain/relayer/solana/rpc/methods/prepare_transaction.rs
src/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rs
src/domain/relayer/solana/rpc/methods/sign_transaction.rs
src/domain/relayer/solana/rpc/methods/transfer_transaction.rs
src/domain/relayer/solana/rpc/methods/utils.rs
Fix: replace println! with the appropriate tracing macro (info/warn/debug/etc.) and either add the missing imports (e.g., add use tracing::error; or use a grouped import) or fully-qualify calls (tracing::error!).
🤖 Prompt for AI Agents
In src/bootstrap/initialize_app_state.rs around line 19, the file currently
imports only tracing::warn but the codebase migration expects use of tracing
macros instead of println! and some modules call error! unqualified; update the
imports to include the needed tracing macros (e.g., add use tracing::{error,
warn, info, debug}; or at minimum use tracing::error;) so error! is resolved,
and then go through the listed files replacing println! with the appropriate
tracing macro (info!/warn!/debug! etc.) or fully-qualify calls as
tracing::error! where applicable.
transaction::Transaction, | ||
}; | ||
use std::str::FromStr; | ||
use tracing::info; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix compile error: import tracing::error for error! macro used below.
error!
is invoked in this file but not imported; build will fail.
Apply this diff:
-use tracing::info;
+use tracing::{info, error};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
use tracing::info; | |
use tracing::{info, error}; |
🤖 Prompt for AI Agents
In src/domain/relayer/solana/rpc/methods/prepare_transaction.rs around line 31,
the file uses the error! macro but only imports tracing::info; add the missing
tracing::error import by updating the use line to import error as well (e.g.,
use tracing::{info, error};) so the error! macro compiles.
use futures::try_join; | ||
use log::info; | ||
use solana_sdk::{pubkey::Pubkey, transaction::Transaction}; | ||
use tracing::info; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix: missing tracing import for error! macro (build breaks).
This file uses error!()
in multiple places but only imports tracing::info
. Add error
to the import list.
-use tracing::info;
+use tracing::{info, error};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
use tracing::info; | |
use tracing::{info, error}; |
🤖 Prompt for AI Agents
In src/domain/relayer/solana/rpc/methods/sign_and_send_transaction.rs around
line 25, the file uses the error!() macro but only imports tracing::info; update
the tracing import to include the error macro (i.e., import error alongside
info) so the error! calls compile and the build no longer breaks.
use futures::try_join; | ||
use log::info; | ||
use solana_sdk::{pubkey::Pubkey, transaction::Transaction}; | ||
use tracing::info; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Build break: error!
macro not imported.
This file uses error!
but only imports tracing::info
. Bring error
into scope (or qualify calls).
Apply this diff:
-use tracing::info;
+use tracing::{error, info};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
use tracing::info; | |
use tracing::{error, info}; |
🤖 Prompt for AI Agents
In src/domain/relayer/solana/rpc/methods/sign_transaction.rs around line 23, the
code calls the error! macro but only imports tracing::info; update the import to
bring error into scope (e.g., import tracing::error or use tracing::{info,
error}) so error! is available, or alternatively qualify error! calls with
tracing::error!; adjust the use statement accordingly and re-run the build.
|
||
use log::info; | ||
use solana_sdk::{hash::Hash, pubkey::Pubkey, transaction::Transaction}; | ||
use tracing::info; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix missing error!
import (compile error)
This file calls error!
but only imports tracing::info
. Without importing error
(or prefixing tracing::error!
), this won’t compile.
-use tracing::info;
+use tracing::{info, error};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
use tracing::info; | |
use tracing::{info, error}; |
🤖 Prompt for AI Agents
In src/domain/relayer/solana/rpc/methods/transfer_transaction.rs around line 30,
the file uses the error! macro but only imports tracing::info; add the missing
import by importing tracing::error (or import tracing::{info, error}) so the
error! macro is available, or alternatively prefix calls with tracing::error!
where used; update the use line accordingly to fix the compile error.
transaction::Transaction, | ||
}; | ||
use solana_system_interface::program; | ||
use tracing::debug; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix compile error: import tracing::error for error! usages.
This file calls error!
multiple times but only imports debug
.
Apply this diff:
-use tracing::debug;
+use tracing::{debug, error};
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
use tracing::debug; | |
use tracing::{debug, error}; |
🤖 Prompt for AI Agents
In src/domain/relayer/solana/rpc/methods/utils.rs around line 41, the file
currently only imports tracing::debug but uses error! macros, causing a compile
error; update the imports to include the error macro (e.g., add use
tracing::error; or replace the single import with use tracing::{debug, error};)
so error! invocations compile.
signer::{SeedDerivable, Signer as SolanaSigner}, | ||
transaction::Transaction, | ||
}; | ||
use tracing::{debug, info}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid logging raw signatures.
debug!("vault_signature_str: {}", vault_signature_str);
(Line 102) can leak signing material. Prefer length-only or a short prefix.
Apply this diff:
- debug!("vault_signature_str: {}", vault_signature_str);
+ // Do not log raw signatures; they are sensitive
+ debug!("vault signature received (len={})", vault_signature_str.len());
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
use tracing::{debug, info}; | |
// Do not log raw signatures; they are sensitive | |
debug!("vault signature received (len={})", vault_signature_str.len()); |
🤖 Prompt for AI Agents
In src/services/signer/solana/vault_transit_signer.rs (referenced imports at
line 17 and the debug log at ~line 102), stop logging the full raw signature
string; replace the debug!("vault_signature_str: {}", vault_signature_str) with
a safe message that does not emit the entire signature — e.g., log only the
signature length and a short prefix or masked prefix (first N chars) or an
indication like "<redacted>" plus length, ensuring the full vault_signature_str
is never printed.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #458 +/- ##
=======================================
- Coverage 93.0% 92.9% -0.1%
=======================================
Files 225 225
Lines 77270 77271 +1
=======================================
- Hits 71862 71827 -35
- Misses 5408 5444 +36
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@dylankilkenny is this PR relevant or most of its changes was merged by other PRs? |
Summary
log
crate from Cargo.tomllog::*
imports withtracing::*
Testing Process
Checklist
Summary by CodeRabbit