Skip to content
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

feat: add logging to DIP components #630

Merged
merged 14 commits into from
Apr 29, 2024
Merged

feat: add logging to DIP components #630

merged 14 commits into from
Apr 29, 2024

Conversation

ntn-x2
Copy link
Member

@ntn-x2 ntn-x2 commented Apr 23, 2024

Fixes https://github.com/KILTprotocol/ticket/issues/3128.

I introduced different new logging targets. For the deposit storage and relay store pallet, which are not technically part of the DIP protocol, the target is a standalone one. For the other things, meaning DIP pallets and DIP provider/consumer components, the log targets have the following structure: "dip::<provider>|<consumer>::<type_name>". So filtering provider logs would require RUST_LOG=dip::provider=info, for instance. Most of the logs are info level, with some error where we return Internal errors or errors coalesced into a u8 value, and a couple of trace for the less important stuff to show.

@ntn-x2 ntn-x2 requested a review from Ad96el April 23, 2024 09:26
@ntn-x2 ntn-x2 self-assigned this Apr 23, 2024
Ad96el
Ad96el previously approved these changes Apr 23, 2024
Copy link
Member

@Ad96el Ad96el left a comment

Choose a reason for hiding this comment

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

Nice changes.

Two picky comments. Would be charming if some logs are more consistent. Some start with Failed and some others have the failed in the middle of the sentence, but nothing which should block this PR to be merged.

I assume the integration-tests are currently failing because we are in an intermediate state. I tried to fix them and I get the error Transport("NoChannel").

These errors are probably resolved, once we accept the channel request from HydraDx. Nevertheless, some changes are required. The KILT token registration is no longer necessary. I have fixed them already and will create a PR, once the referendum is through.

@ntn-x2 ntn-x2 enabled auto-merge (squash) April 23, 2024 13:26
Ad96el
Ad96el previously approved these changes Apr 23, 2024
@ntn-x2 ntn-x2 merged commit 9a69512 into develop Apr 29, 2024
1 of 2 checks passed
@ntn-x2 ntn-x2 deleted the aa/dip-logging branch April 29, 2024 05:38
Ad96el pushed a commit that referenced this pull request Aug 20, 2024
Fixes KILTprotocol/ticket#3128.

I introduced different new logging targets. For the deposit storage and
relay store pallet, which are not technically part of the DIP protocol,
the target is a standalone one. For the other things, meaning DIP
pallets and DIP provider/consumer components, the log targets have the
following structure: `"dip::<provider>|<consumer>::<type_name>"`. So
filtering provider logs would require `RUST_LOG=dip::provider=info`, for
instance. Most of the logs are `info` level, with some `error` where we
return `Internal` errors or errors coalesced into a `u8` value, and a
couple of `trace` for the less important stuff to show.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants