-
Notifications
You must be signed in to change notification settings - Fork 96
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
Allow the logger to inject tags into the logs #145
Conversation
Test PASSed. |
return {"delta": self.delta(), | ||
"timestamp": self.timestamp(), | ||
"data": data} | ||
msg = {"delta": self.delta(), "timestamp": self.timestamp(), "data": data} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I think there's a risk here of tags stamping over some existing keys (delta
, timestamp
, data
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with your nesting suggestion, probably having something like 'tags' with msg = {"delta": self.delta(), "tags": .}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I actually reversed it - but then thought that might be desirable to overwrite. But probably not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a metadata
field, and always populate it, even if empty. tags
is a little arbitrary - metadata
fits nicer with data
. Potentially header
could be used here, too.
Test PASSed. |
Allow the logger to inject tags into the logs
Add a tags parameter to Base logger and use it in JSON logger. It's a string of tags separated by commas used to decorate the logs with. This results in something like:
generates:
This will be used to inject session data into the logs, namely
session_id
. There's some questions here on whether to nest these tags and metadata - we can update that as necessary./cc @mookerji