joaodrp / gelf-formatter

GELF formatter for the Python standard library logging module.
MIT License
9 stars 4 forks source link

feat: allow additional reserved fields #1

Closed girstenbrei closed 4 years ago

girstenbrei commented 4 years ago

Hi João, really nice formatter you have got there going! Short, concise, well setup, exactly what I needed!

I would like to add a small piece of functionality. The GelfFormatter already allows with allowed_reserved_attrs to re-allow attributes. I use this regularily, but in some cases I would like to add attributes to that list, in essence filtering them from the output. My use case is e.g. my applications logs its configuration on startup but i would like to filter fields like 'secret' or 'key', just to be sure not to disclose any secrets in my logs. I am aware that filtering like this is not perfect (e.g. dumping nested dicts would only filter one level) but IMHO that would be out of scope for a logging formatter. I think this solution is in scope, as the user should be allowed to configure RESERVED_ATTRS in both directions.

Thank your for your work!

Greets, Christoph

codecov[bot] commented 4 years ago

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #1   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         108    109    +1     
  Branches        8      8           
=====================================
+ Hits          108    109    +1
Impacted Files Coverage Δ
gelfformatter/formatter.py 100% <100%> (ø) :arrow_up:
tests/test_formatter.py 100% <100%> (ø) :arrow_up:

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 59b82c7...a13318b. Read the comment docs.

joaodrp commented 4 years ago

@girstenbrei thanks, this is going to be a useful feature! 🙂

Please see my comments above and let me know what you think.

Once we agree on the changes and how they're reflected on the API, it would be great if you could update the README on this PR as well, to document the new functionality.

girstenbrei commented 4 years ago

Hey @joaodrp, thanks for your advice! I integrated the changed variable name and the dynamic computation of excluded fields in the format method. Please, feel free to read the additions to the Readme and comment on all the changes!

Greets, Chris

joaodrp commented 4 years ago

Thanks @girstenbrei, it looks great now. Happy to merge this. I'll raise an issue to add support for nested ignored fields in future.