Skip to content

feat(logs): Add Monolog channel to compiled log attributes#2029

Open
melkamar wants to merge 1 commit intogetsentry:masterfrom
melkamar:patch-1
Open

feat(logs): Add Monolog channel to compiled log attributes#2029
melkamar wants to merge 1 commit intogetsentry:masterfrom
melkamar:patch-1

Conversation

@melkamar
Copy link

Description

Adds channel name to the log metadata.

The channel is a relevant piece of metadata of a log entry. This change includes the channel in the recorded attributes, but remains backwards compatible and will not overwrite existing extra or context fields named 'channel'.

I considered making this an opt-in configuration of the Handler in the constructor, but since this is backwards compatible, I figured that's not necessary.

The channel is a relevant piece of metadata of a log entry.
This change includes the channel in the recorded attributes,
but remains backwards compatible and will not overwrite
existing extra or context fields named 'channel'.
@Litarnus
Copy link
Contributor

Hi @melkamar! Can you tell us why you need the channel name in the log attributes? Adding it to every log message would increase the number of bytes per message, so we are a bit reluctant to add it without proper reason

@melkamar
Copy link
Author

@Litarnus sure, the reason is that I may want to filter for messages sent to a certain channel. For example, if some troubleshooting issues related to async processing, I may want to narrow my search to the messenger channel only to see the Symfony messenger logs.

In our application, we use dozens of different channels. I may want to drill down to a specific one. That's what we currently do in Elastic/Kibana and it's very useful. Having the same in Sentry, along with all the other app data, would be great.

I do understand the reluctance of adding data. I also understand that this is truly pointless for use cases where someone uses a single channel (e.g. app) to send the logs to. So, could this be an opt-in?

@Litarnus
Copy link
Contributor

Thanks for the explanation, I can see that it makes a lot of sense to have that option in your setup.

So, could this be an opt-in?

Yes, I think that would be best. I'm happy to review the PR if you change it to opt-in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants