Skip to content
This repository has been archived by the owner on Jun 21, 2023. It is now read-only.

Add logger name to default message #13

Closed
wants to merge 5 commits into from

Conversation

vlasy
Copy link
Contributor

@vlasy vlasy commented Feb 26, 2019

Automatically put logger name before the message, when using default (stackdriver optimized) logger.

Before:
Some message

Now:
[Database] Some message

@@ -13,6 +13,9 @@ class StackDriverFormatStream extends Transform {
// tslint:disable-next-line:function-name
public _transform(chunk: any, _encoding: string, callback: (error?: Error | undefined, data?: any) => void) {
const obj = JSON.parse(chunk);
if (obj.name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, why to choose such a generic prop name? I find it pretty common to use name in many objects that are logged directly.

@grissius whats your opinion on this?

Copy link
Contributor Author

@vlasy vlasy Feb 28, 2019

Choose a reason for hiding this comment

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

Actually, this is not something new. The name field is used for the logger name since the beginning, so this PR changes nothing for that matter.

But you are right, that this might be problematic. I will try to fix that in future PRs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not event sure how to fix it. Know about it just from the discussion, PR comment and commented file. But I would not consider it a grave issue until it proves problematic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be in the logger property, but never in a message payload, this is new. So far there were only pkgVersion and level properties added, but never assumed anything from the payload it self.

Besides, is it really necessary to add it to the root of the message? You can add the loggerNameproperty to the payload, as you do with the pkgVersion for instance. It wouldnt be visible in the log message directly, but you can query Stackdriver for it 💡

@vlasy vlasy closed this Mar 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants