Skip to content

Conversation

@raymondkfcheung
Copy link
Contributor

@raymondkfcheung raymondkfcheung commented Jul 21, 2025

This PR replaces log with tracing instrumentation in Snowbridge-related modules by providing structured logging.

Partially addresses #9211
Similar to #8732

Key Features

  • Consistent targets: All components use predictable log targets
  • Structured fields: Uses ?variable/%variable syntax for automatic Debug/Display formatting
  • Zero runtime impact: No behavioural changes, only observability improvements

@raymondkfcheung raymondkfcheung self-assigned this Jul 21, 2025
@raymondkfcheung raymondkfcheung marked this pull request as draft July 21, 2025 14:15
@raymondkfcheung raymondkfcheung added the T15-bridges This PR/Issue is related to bridges. label Jul 21, 2025
@paritytech-review-bot paritytech-review-bot bot requested a review from a team July 21, 2025 14:16
@raymondkfcheung raymondkfcheung changed the title Ray bridge tracing Replace log with tracing on Bridge-related modules Jul 21, 2025
@raymondkfcheung raymondkfcheung marked this pull request as draft August 5, 2025 09:11
@raymondkfcheung raymondkfcheung marked this pull request as ready for review August 5, 2025 15:25
crates:
- name: snowbridge-core
bump: minor
- name: snowbridge-ethereum
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is snowbridge-ethereum needed here ? I don't see any changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@@ -0,0 +1,30 @@
title: Replace `log` with `tracing` on Snowbridge-related modules
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With regards to most of the crates here you're removing a dependency log and adding new one tracing. Should we consider this major level change due to removal of log ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. Previous PRs were marked as minor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor or even patch are fine

@paritytech-review-bot paritytech-review-bot bot requested a review from a team August 7, 2025 11:37
let message = Self::prepare(message)?;

log::trace!(target: LOG_TARGET, "prepared message: {:?}", message);
tracing::trace!(target: LOG_TARGET, ?message, "prepared message");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need a blank space after prepared message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so.

Comment on lines +22 to +26
bump: patch
- name: snowbridge-pallet-system-v2
bump: patch
- name: snowbridge-runtime-common
bump: minor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dq: how do you distinguish between minor and patch in terms of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the link, I was not aware of it.
I looked at Validate prdoc for ... step in the link that you sent and checked the corresponding yml file with job definition; it uses check-prdoc.py underneath.

So I checked both: the script and yml job but could not find a piece of code responsible for bump validation.
Did I miss anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually, it shall call prdoc.

github-merge-queue bot pushed a commit that referenced this pull request Nov 10, 2025
This PR replaces `log` with `tracing` instrumentation on Bridge Relay
related modules by providing structured logging.

Partially addresses #9211
Similar to #9279

## Key Features
- **Consistent targets**: All components use predictable log targets
- **Structured fields**: Uses `?variable`/`%variable` syntax for
automatic `Debug`/`Display` formatting
- **Zero runtime impact**: No behavioural changes, only observability
improvements

---------

Co-authored-by: Andrii <[email protected]>
@@ -0,0 +1,30 @@
title: Replace `log` with `tracing` on Snowbridge-related modules
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor or even patch are fine

@raymondkfcheung raymondkfcheung added this pull request to the merge queue Nov 11, 2025
Merged via the queue into master with commit c1c5245 Nov 11, 2025
251 of 252 checks passed
@raymondkfcheung raymondkfcheung deleted the ray-bridge-tracing branch November 11, 2025 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. T15-bridges This PR/Issue is related to bridges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants