Merged
Conversation
Member
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Maddiaa0
force-pushed
the
md/02-17-chore_cleanup_libp2p_logger
branch
from
f835efc to
684c356
Compare
Maddiaa0
changed the base branch from
md/fix-e2e-p2p-version-warnings
to
graphite-base/12058
Maddiaa0
force-pushed
the
graphite-base/12058
branch
from
505e65e to
a150905
Compare
Maddiaa0
force-pushed
the
md/02-17-chore_cleanup_libp2p_logger
branch
from
684c356 to
fe8b42f
Compare
Maddiaa0
force-pushed
the
graphite-base/12058
branch
from
6e44305 to
fc8d49b
Compare
Maddiaa0
force-pushed
the
md/02-17-chore_cleanup_libp2p_logger
branch
from
fe8b42f to
646eadf
Compare
spalladino
reviewed
Feb 18, 2025
Contributor
spalladino
left a comment
spalladino
left a comment
There was a problem hiding this comment.
Looks good! Just a few minor changes.
| @@ -0,0 +1,48 @@ | |||
| import { type ComponentLogger, type Logger } from '@libp2p/interface'; | |||
Contributor
There was a problem hiding this comment.
We should add libp2p/interface to foundation's dev dependencies
| }; | ||
|
|
||
| return Object.assign(logFn, { | ||
| enabled: log.isLevelEnabled('debug'), |
Contributor
There was a problem hiding this comment.
I think I had already commented about whether this should be hardcoded, and you had replied, and I can't remember the answer.
Member
Author
There was a problem hiding this comment.
Pretty much that it is extremely noisy, and we only want to be able to see it whenever we are running with LOG_LEVEL=debug and below
| @@ -269,7 +268,7 @@ export class LibP2PService<T extends P2PClientType> extends WithTracer implement | |||
| }), | |||
| }, | |||
| // Fix the peer id in libp2p logs so we can see the source of the log | |||
Contributor
There was a problem hiding this comment.
Suggested change
| // Fix the peer id in libp2p logs so we can see the source of the log |
spalladino
approved these changes
Feb 18, 2025
TomAFrench
added a commit
that referenced
this pull request
Feb 19, 2025
* master: (245 commits) chore: Fix unbound CI variable on release image bootstrap (#12095) fix: dry run on grind (#12088) fix(spartan): eth-execution logging (#12094) fix: aws_handle_evict recovery & termination (#12086) chore: Use native acvm when available on orchestrator tests (#11560) refactor: function macros cleanup (#12066) refactor: remove `addNullifiedNote` from pxe (#11822) fix: `#[aztec]` macro warnings (#12038) refactor!: Notes implementing `Packable<N>` (#12004) chore(ops): add gcloud cli into devbox base image (#12082) fix: hotfix grinding fix: L1 deployment on reth (#12060) fix: Add missing bootstrap fast aliases (#12078) fix: hash_str caching (#12074) fix: Naive attempt to fix nightly deployments (#12079) fix: kind smoke (#12084) refactor!: nuking `NoteHeader` (#11942) fix: inject dockerhub creds (#12072) feat(docs): Note discovery concepts page (#11760) chore: cleanup libp2p logger (#12058) ...
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
2 participants
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.
Overview
moved into foundation
use pino directly
having fixed terms lead to requiring extra parsing to deal with %p etc. strings. We really do not want to do this extra parsing as it removes the
perforance benefits from using string replacement ( it does not do the substitution unless the log actually happens )
fixed terms have been removed ( only used in one place )
fixed logs were only added to aid in the p2p testbench, i will think of a different solution for this