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

Add namespace-specific levels support to logger #22619

Merged
merged 8 commits into from
May 19, 2024

Conversation

Nerivec
Copy link
Collaborator

@Nerivec Nerivec commented May 13, 2024

Adds the ability to alter levels for specific namespaces.
Also:

  • Bring "MQTT publish" lines back to info level and add z2m:mqtt namespace for whole MQTT class, to allow changing the level for that namespace specifically
  • Replace warn for warning (with automatic remap if warn still used in config) to match syslog levels used
  • Use settings for logger-related values throughout the code instead of repeating them
  • Short-circuit winston if determined level won't end up logging anything (performance gain since all transport levels are synced, no need to go through that logic)
  • Remove json option from File Transport, not supported (stale value?)

Example use:

advanced:
  log_namespaced_levels:
    z2m:mqtt: warning
    zh:ember:uart:ash: info

TODO

  • frontend support for new setting (object with unknown string keys mapped to log level values)
  • docs

@Nerivec Nerivec force-pushed the logger-namespaced-levels branch from f1e4b20 to c6ae45f Compare May 13, 2024 20:53
@Nerivec
Copy link
Collaborator Author

Nerivec commented May 13, 2024

@sjorge This, in combination with the debug-level regex should provide good customization for the logs. Let me know if you have any suggestions. With a bit of refactoring on the logger, and the fact this only looks up levels by key (namespace), it ends up way faster than the old calls in "filtered" scenario (noop), and about the same performance for "not filtered" scenario, so I think we're good on that end.
This setting should be more user-friendly, and the regex for debug more tailored for devs to filter out any which way they want.

@sjorge
Copy link
Contributor

sjorge commented May 13, 2024

  • Bring "MQTT publish" lines back to info level and add z2m:mqtt namespace for whole MQTT class, to allow changing the level for that namespace specifically

<3

This setting should be more user-friendly, and the regex for debug more tailored for devs to filter out any which way they want.

I started using this when that got implemented, it's working well for my needs. (I still have most info I care about and I can easily restart z2m when I need the info I filter out)

I'll try the other way when this gets added though, as I do indeed filter out 2 namespaces and nothing else.

Also I think your example in the PR comment is wrong? Looking at the diff should it not be:

advanced:
  log_namespaced_levels:
    'z2m:mqtt': warning
    'zh:ember:uart:ash': info

@Nerivec
Copy link
Collaborator Author

Nerivec commented May 13, 2024

Good catch, forgot a step on the ladder!

The MQTT publish stuff is really dividing, between users who seem to use it all the time, and users who had gone to warn level to avoid the spamming it does in large network/fast-update devices (and can represent a massive amount of MQTT traffic). This should provide a way for both to be happy.

@Koenkk
Copy link
Owner

Koenkk commented May 14, 2024

Created a PR for the docs. @Nerivec please check if all OK and move out of draft, then I can merge it.

@martyzeq
Copy link

looks promising, thank you guys !

@Nerivec Nerivec marked this pull request as ready for review May 15, 2024 19:48
@Koenkk Koenkk merged commit 2eec6a4 into Koenkk:dev May 19, 2024
11 checks passed
@Koenkk Koenkk mentioned this pull request May 19, 2024
@Nerivec Nerivec deleted the logger-namespaced-levels branch May 19, 2024 14:03
@zen2
Copy link

zen2 commented May 27, 2024

As many, I miss MQTT messages in logs and it seems this new namespaces filtering can bring best of two world.

Anyway I wonder why we do not use "Notice" level because "Info" level should represent normal messages for current operations (like MQTT ones) and "Notice" only relevant ones. I mean that we could put back MQTT messages in "info" level and put actual "info" level in "notice". This way it is easy to choose which log level is needed quickly.

Note that advanced namespaces filtering is really a good stuff too and it can be complementary with a notice level especially to debug a particular topic. As it is filtering it should be use as advanced log configuration.

So to sum up, I propose this:

  • move actual info level to a notice level
  • move mqtt messages in info level
  • keep namespaces filtering as advanced options

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.

5 participants