bobbui / json-logging-python

Cloud-native distributed Python logging library to emit JSON log that can be easily indexed by logging infrastructure
Apache License 2.0
303 stars 62 forks source link

Implement extra properties from json-logging-py #78

Closed uda closed 3 years ago

uda commented 3 years ago

Addresses feature request #77

Taken from: https://github.com/sebest/json-logging-py/blob/master/jsonlogging.py#L40 Original work by Sebastien Estienne sebastien.estienne@gmail.com Distributed under the MIT license

bobbui commented 3 years ago

thanks for your contribution, there is one test is failing, can u take a look

alexhafner commented 3 years ago

basestring is a Python 2 type, in Python 3 there is only str. The code from json-logging will need to be updated to Python 3.

bobbui commented 3 years ago

ideally the this module is built to support both py2 and py3 . I see this

if sys.version_info < (3, 0):
    EASY_TYPES = (basestring, bool, dict, float, int, list, type(None))
else:
    RECORD_ATTR_SKIP_LIST.append('stack_info')
    EASY_TYPES = (str, bool, dict, float, int, list, type(None))

Isnt it supposed to be working for both py2 and py3

alexhafner commented 3 years ago

Ah, didn't see that you still support Python 2, as the test matrix is only targeting 3.7 to 3.9

In this case flake8 could be instructed to disable the check for this specific line, ie

if sys.version_info < (3, 0):
    EASY_TYPES = (basestring, bool, dict, float, int, list, type(None)) # noqa: F821
else:
    RECORD_ATTR_SKIP_LIST.append('stack_info')
    EASY_TYPES = (str, bool, dict, float, int, list, type(None))
bobbui commented 3 years ago

@uda Can u add suggested flake8 fix to get it passed

uda commented 3 years ago

Yeah, I can remove all Python 2 BC code or put comments to fix the flake8 test

bobbui commented 3 years ago

Yeah, I can remove all Python 2 BC code or put comments to fix the flake8 test

flake8 would be great

uda commented 3 years ago

I added a fix for the basestring definition in python 3, I've seen it in another library with backwards compatibility and flake8 tests where they didn't want to simply ignore F821. But when I run the second flake8 test it failed, but we'll see how it performs on github's pipeline

bobbui commented 3 years ago

looking good

bobbui commented 3 years ago

released https://pypi.org/project/json-logging/1.4.1rc0/