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 log level numbers #151

Merged
merged 4 commits into from
Feb 16, 2018
Merged

Add log level numbers #151

merged 4 commits into from
Feb 16, 2018

Conversation

goodspark
Copy link

@goodspark goodspark commented Feb 3, 2018

When combing through logs, I find it's sometimes helpful to filter by a range of log levels. Currently this must be done by checking log level strings. This adds the number so it's easier to do such a check:

With only log levels:
log_level in ("error", "warning", "critical")

With log level numbers:
log_level_number >= 30

@goodspark
Copy link
Author

Looks like style failures are from changes I haven't made. Would you prefer they be fixed in this PR or separately and then this branch merged/rebased?

Copy link
Owner

@hynek hynek left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, I’ve added a few wishes.

Additionally:

  • it needs a changelog entry.
  • it would be nice if you could use double quotes "-only in new code
  • yes, it would be nice if you could fix the style issues too, so I don’t have to fix and rebase your PR

thanks again!

docs/api.rst Outdated
@@ -187,6 +187,8 @@ API Reference

.. autofunction:: filter_by_level

.. autofunction:: add_log_level_number
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should go under add_log_level since that is still the main one.

@@ -58,6 +58,9 @@ Processors
:func:`~structlog.stdlib.add_logger_name`:
Adds the name of the logger to the event dictionary under the key ``logger``.

:func:`~structlog.stdlib.add_log_level_number`:
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above, maybe explain a bit more what a log level number is? Someone reading this who doesn’t know what a log level number is, will have not clue what this is support to be.

@@ -330,6 +330,14 @@ def filter_by_level(logger, name, event_dict):
raise DropEvent


def add_log_level_number(logger, method_name, event_dict):
"""
Adds the log level number to the event dict.
Copy link
Owner

Choose a reason for hiding this comment

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

Kind of as above: it should broadly explain what that is, maybe even link to the docs.

Also: Add not Adds please.

- Reorder new method's position in docs and code
- Fix unrelated isort issues
- Some more docs to explain what/how this works
- Use `"`s v `'`s
@codecov
Copy link

codecov bot commented Feb 5, 2018

Codecov Report

Merging #151 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #151   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     13           
  Lines         810    813    +3     
  Branches      105    105           
=====================================
+ Hits          810    813    +3
Impacted Files Coverage Δ
src/structlog/stdlib.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40f332f...3c3ead6. Read the comment docs.

Sam Park added 2 commits February 5, 2018 15:47
Doesn't work. Doesn't like `:meth:`

This reverts commit 2cb8bcb.
@goodspark
Copy link
Author

@hynek Take another look? I believe I've addressed issues

@hynek hynek merged commit a454cf8 into hynek:master Feb 16, 2018
@hynek
Copy link
Owner

hynek commented Feb 16, 2018

Thanks!

@goodspark goodspark deleted the log-level-number-processor branch February 16, 2018 07:31
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.

2 participants