feat: logs filter by vladopajic · Pull Request #95 · logos-co/logos-libp2p-module

vladopajic · 2026-07-29T16:36:38Z

adds log filter
all tutorials disable filter to show clean output

closes: #79

gmelodie

reviewing as i was almost done before it got converted back

gmelodie · 2026-07-29T17:04:19Z

+    fprintf(stderr, "Failed to disable libp2p logs: %s\n",
+            logRes.error.c_str());
+    return 1;
+}


If we get rid of the #ifdef..#endif stuff from above:

Suggested change

}

node.setLogLevel(LogLevel::None);

hmm i don't see what #ifdef..#endif has to do with this code?

result from setLogLevel has to be checked because cbind returns result.

I guess the overall issue here is that set log level is creating ctx? Why is that. I would expect setLogLevel to never fail (i.e. do not create a context, require one and that's it).

This would keep us from doing

if (!logRes.success) { fprintf(stderr, "Failed to disable libp2p logs: %s\n", logRes.error.c_str()); return 1; }

every time

https://github.com/vacp2p/nim-libp2p/blob/a70c615c287a94e1134f45c05c18c338c7a3bc8f/cbind/libp2p.nim#L417

^ this is setLogLevel from cbind. that setLogLevel returns results, so this result has to be checked. this is what we are doing here when checking result.

pr #2899 makes setLogLevel a static function. this change requires that call sites continue to check the result, despite avoiding the context.

there is no validation when setting log lvl anymore, therefore no result (response).
validation is done on cbind side before libp2p node is created.
in logos module it will be impossible to set invalid value, as user will have to use LogLevel enum.

gmelodie · 2026-07-29T17:08:58Z

  inputs = {
    logos-module-builder.url = "github:logos-co/logos-module-builder";
-    libp2p.url = "github:vacp2p/nim-libp2p/fix/cbind/nim-ffi-bump";
+    libp2p.url = "github:vacp2p/nim-libp2p/master";


don't fix this to master, as changes can spill here unexpectedly

Suggested change

libp2p.url = "github:vacp2p/nim-libp2p/master";

libp2p.url = "github:vacp2p/nim-libp2p/fix/cbind/nim-ffi-bump";

or the commit that was merged in master

Suggested change

libp2p.url = "github:vacp2p/nim-libp2p/master";

libp2p.url = "github:vacp2p/nim-libp2p/a760255635529adbc2e8f198b8b53ba13396c4ec";

changes can't spill here unexpectedly as it is fixed to master, with rev a70c615c287a94e1134f45c05c18c338c7a3bc8f.

see flake.lock file

Yes! but when doing nix flake update to update some other dep this will also update nim-libp2p to latest master, which might not be what we want

why would it be okay for other deps and not for nim-libp2p dep?

Let's say we need to update logos core, but nim-libp2p has some breaking changes that have not been ported to cbind yet.

nim-libp2p has some breaking changes that have not been ported to cbind yet

could never happen. every push to nim-libp2p master has to pass CI. if nim-libp2p code brakes cbind CI that PR will not get through.

ether way, even when we assume that, that case can happen, those are exceptional cases. meaning that by default we want default behavior, which is nix flake update to updated everything. and exceptional cases are dealt with an exception. in this hypothetical case, one would still do nix flake update and manually rivert changes related to nim-libp2p.

vladopajic · 2026-07-29T17:46:15Z

hmmm i still see logs in tutorials... draft again

richard-ramos · 2026-07-29T18:39:41Z

+    if (!ctx) {
+        auto created = createContext();
+        if (!created.success) return created;
+    }
+    return callSync("Failed to set log level", [&](SyncPromise* p) {
+        return libp2p_ctx_set_log_level(ctx, static_cast<int64_t>(level),
+                                        &Libp2pModuleImpl::cbBool, p);


@gmelodie with nim-ffi, is it always required to have a context? I am wondering if we could just update nim-libp2p libp2pSetLogLevel function so it looks like:

proc libp2pSetLogLevel*(level: int): Future[Result[bool, string]] {.ffi.} = ## Changes the process-wide Chronicles runtime log level. if level < ord(low(chronicles.LogLevel)) or level > ord(high(chronicles.LogLevel)): return err("invalid log level: " & $level) when chronicles.runtimeFilteringEnabled: chronicles.setLogLevel(chronicles.LogLevel(level)) ok(true) else: err("Chronicles runtime filtering is disabled")

and that way you avoid allocating memory for context and also breaking createNode.

Yes it should be possible to just do that

yeah, this will make things better. i'll push this to nim-libp2p

LogLevel is now passed via config, avoiding need for context.

vladopajic · 2026-07-30T10:45:52Z

+In your own application, `LogLevel::Error` or `LogLevel::Fatal` is often a
+useful default. It keeps normal output quiet while still surfacing conditions
+that may indicate libp2p is misbehaving or that your integration code needs an
+adjustment. Some error logs describe remote-peer behavior, retries, or
+recoverable internal state, so they may not require any action from your side.


added this as a practical suggestion because users should not generally disable logs completely, especially while building or integrating an application.

still not fully sure this should be the final recommendation, because libp2p logs are not currently structured in a way that is consistently helpful to end users. there are cases where an error log may be written to stdout, but the condition is not necessarily actionable for the application. in some cases, the log level appears to be error mainly because the log statement is inside an except SomeError block.

second reason for adding this note is to raise the topic of improving (more accurately creating) a systematic approach to choosing log levels. runtime logging should be treated as a library feature with user-facing behavior. and now when we are not only a developers, but users as well, it may be easier for us to understand how to approach this.

this may also expand the scope of the commitment ift-ts:p2p:ift:2026q3-nimlibp2p-log-noise-reduction:logging-changes.

.... but it might be completely unnecessary in age of ai. but even with this assumption systematic approach would be useful to ai.

vladopajic · 2026-07-31T10:24:21Z

logs filter

0ba5750

vladopajic marked this pull request as draft July 29, 2026 17:03

fix

ff8fc9a

gmelodie reviewed Jul 29, 2026

View reviewed changes

log lvl is always here

8a20153

vladopajic marked this pull request as ready for review July 29, 2026 17:28

vladopajic requested review from gmelodie and richard-ramos July 29, 2026 17:30

update

bfd8117

vladopajic marked this pull request as draft July 29, 2026 17:46

vladopajic removed request for gmelodie and richard-ramos July 29, 2026 17:46

richard-ramos reviewed Jul 29, 2026

View reviewed changes

vladopajic added 5 commits July 30, 2026 11:29

update

aa2443e

comsetics

26712bf

exercies

e2bd897

add note

0b39b27

refresh

49036f6

vladopajic commented Jul 30, 2026

View reviewed changes

vladopajic added 9 commits July 30, 2026 12:57

fix

73a3a2f

update

216f578

update

69c3057

update deps

ff8efd7

update

79c2d5b

save log to context

f4f52eb

cfg log lvl

a9d2c90

update

5e8de48

update

d7134a6

vladopajic marked this pull request as ready for review July 31, 2026 10:15

vladopajic requested review from gmelodie and richard-ramos July 31, 2026 10:15

update

a6a57bd

gmelodie approved these changes Jul 31, 2026

View reviewed changes

Merge branch 'master' into disable-logs

a6442a2

vladopajic merged commit a31c40d into master Aug 1, 2026
10 checks passed

	libp2p.url = "github:vacp2p/nim-libp2p/master";
	libp2p.url = "github:vacp2p/nim-libp2p/fix/cbind/nim-ffi-bump";

	libp2p.url = "github:vacp2p/nim-libp2p/master";
	libp2p.url = "github:vacp2p/nim-libp2p/a760255635529adbc2e8f198b8b53ba13396c4ec";

Uh oh!

Conversation

vladopajic commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gmelodie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gmelodie Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladopajic commented Jul 29, 2026

Uh oh!

richard-ramos Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladopajic commented Jul 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vladopajic commented Jul 29, 2026 •

edited

Loading

gmelodie Jul 29, 2026 •

edited

Loading

richard-ramos Jul 29, 2026 •

edited

Loading