tintoy / seqlog

Python logging integration for Seq (https://getseq.net)
https://seqlog.readthedocs.io/
MIT License
17 stars 11 forks source link

convert args value of logging record to str #13

Closed snailkn closed 3 years ago

snailkn commented 6 years ago

Description

I use the seqlog in my app to save the python log information with a related c# api, but it raise some error when module sqlalchemy log some information. I debug the code and found there are some code in module sqlalchemy:

self.engine.logger.info(
                "%r",
                sql_util._repr_params(parameters, batches=10)
            )

and sql_util._repr_params(parameters, batches=10) is a custom object in module, then it raise a error in seqlog file structured_logging.py line 361

response = self.session.post(
                self.server_url,
                json=request_body,
                stream=True  # prevent '362'
            )

because of request_body is invalid dump to json

What I Did

I modify the code in file structured_logging.py line 386 from

log_props_shim[str(arg_index)] = arg

to

log_props_shim[str(arg_index)] = str(arg)

then the problem is sloved I hope you could solve this problem

tintoy commented 6 years ago

Hi - thanks for the detailed report! I'll take a look at this tomorrow morning :)

tintoy commented 6 years ago

Hmm - always converting to a string could work, but it means people can't log complex objects anymore. How about if we only convert to string when the object is not a dict, list, set, or intrinsic type?

tintoy commented 6 years ago

Relates to #7 - a custom JSONEncoder could also be a way of dealing with this :)

tintoy commented 6 years ago

It looks like the most "correct" way to handle this is with a custom JSON encoder (see PR #14 for details). You can create a class that derives from json.encoder.JSONEncoder and just calls str() on a value being serialised if it's one of the types of object you're having trouble with.

For help with extending json.encoder.JSONEncoder, have a look here under "Extending JSONEncoder".

tintoy commented 6 years ago

I'll try to publish a new version of the package sometime today but in the meanwhile I'd appreciate it if you could have a look at the PR to see if the API change makes sense to you.

piotrmaslanka commented 3 years ago

39 does exactly that. What do you think @snailkn ?