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

fix: log-level being ignored #1272

Merged
merged 1 commit into from
Oct 18, 2022
Merged

fix: log-level being ignored #1272

merged 1 commit into from
Oct 18, 2022

Conversation

alrevuelta
Copy link
Contributor

Closes #1197

Problem:

  • Without -d:chronicles_runtime_filtering:on flag setLogLevel does not work.
  • setLogLevel only works with levels less verbose than -d:chronicles_log_level=xxx, i.e. setLogLevel(TRACE) won't work if -d:chronicles_log_level=INFO.
  • More info here.

Solution:

  • Add -d:chronicles_runtime_filtering:on when compiling all binaries
  • Set -d:chronicles_log_level=TRACE to all nwaku app and wakunode2

Now the following works as expected. Note that default is INFO.

$ ./build/wakunode2 --log-level=TRACE
$ ./build/wakunode2 --log-level=DEBUG
$ ./build/wakunode2 --log-level=WARN

@status-im-auto
Copy link
Collaborator

status-im-auto commented Oct 17, 2022

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ cd8931e #1 2022-10-17 15:38:41 ~18 min linux 📦bin
✔️ cd8931e #1 2022-10-17 15:38:41 ~18 min macos 📦bin
✔️ e4fb43a #2 2022-10-18 18:51:11 ~16 min linux 📦bin
✔️ e4fb43a #2 2022-10-18 18:55:19 ~20 min macos 📦bin

Copy link
Contributor

@LNSD LNSD left a comment

Choose a reason for hiding this comment

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

LGTM ✅

Copy link
Contributor

@jakubgs jakubgs left a comment

Choose a reason for hiding this comment

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

Thanks.

Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

LGTM. One thing to consider is that the default log level almost everywhere will now be INFO, whereas previously it would have been DEBUG after compilation. We may want to change the default for fleet deployments to DEBUG so that we can continue using it as before: https://github.com/status-im/infra-role-nim-waku/blob/master/defaults/main.yml#L19

@jakubgs
Copy link
Contributor

jakubgs commented Oct 18, 2022

@jm-clius actually we already explicitly set log level to info:

 > a all -a 'grep log-level /docker/nim-waku/docker-compose.yml'
node-01.do-ams3.wakuv2.prod | CHANGED | rc=0 >>
      --log-level=info
node-01.gc-us-central1-a.wakuv2.prod | CHANGED | rc=0 >>
      --log-level=info
node-01.ac-cn-hongkong-c.wakuv2.prod | CHANGED | rc=0 >>
      --log-level=info
node-01.do-ams3.wakuv2.test | CHANGED | rc=0 >>
      --log-level=info
node-01.gc-us-central1-a.wakuv2.test | CHANGED | rc=0 >>
      --log-level=info
node-01.ac-cn-hongkong-c.wakuv2.test | CHANGED | rc=0 >>
      --log-level=info
node-01.do-ams3.status.test | CHANGED | rc=0 >>
      --log-level=info
node-01.gc-us-central1-a.status.test | CHANGED | rc=0 >>
      --log-level=info
node-01.ac-cn-hongkong-c.status.test | CHANGED | rc=0 >>
      --log-level=info

But we can always change that.

@jm-clius
Copy link
Contributor

actually we already explicitly set log level to info:

Yeah, indeed (info explicitly set for fleet deployments and it's now the default for everyone running nodes without explicit configuration). Since we're using logging quite extensively for debugging on the fleets, we should change the log level to DEBUG. Other node operators may also expect debug level logs, so we'll have to inform them that default behaviour has changed.

@jakubgs
Copy link
Contributor

jakubgs commented Oct 18, 2022

Okay, done:

jakubgs added a commit to status-im/infra-status-legacy that referenced this pull request Oct 18, 2022
Previously the log level flag was ignored:
waku-org/nwaku#1272

Signed-off-by: Jakub Sokołowski <jakub@status.im>
jakubgs added a commit to status-im/infra-nim-waku that referenced this pull request Oct 18, 2022
Previously the log level flag was ignored:
waku-org/nwaku#1272

Signed-off-by: Jakub Sokołowski <jakub@status.im>
@alrevuelta alrevuelta merged commit df6d215 into master Oct 18, 2022
@alrevuelta alrevuelta deleted the fix-log-level branch October 18, 2022 23:03
LNSD pushed a commit that referenced this pull request Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: --log-level flag appears to be ignored
5 participants