Skip to content
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

Add ActiveSupport tagged logging support #1344

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

tombruijn
Copy link
Member

@tombruijn tombruijn commented Dec 4, 2024

Don't use thread vars in logger

To temporarily set the attributes in the logger instance, use instance variables instead of Thread variables.

I don't know why Thread variables are used here. I don't think it's needed.

Add ActiveSupport tagged logging support

Our logger did not support Rails ActiveSupport tagged logging yet. This was because our logger didn't listen to the tagged, push_tags and pop_tags methods.

Implement these methods to have the logger that wraps our logger call these methods to set tags.

The format is the same as Rails formats it: [My tag value] I decide not to set these as attributes, because they're only values. They have no key and the value can be anything and change frequently.

@tombruijn tombruijn self-assigned this Dec 4, 2024
To temporarily set the attributes in the logger instance, use
instance variables instead of Thread variables.

I don't know why Thread variables are used here. I don't think it's
needed.
@tombruijn tombruijn marked this pull request as ready for review December 5, 2024 09:26
Our logger did not support Rails ActiveSupport tagged logging yet.
This was because our logger didn't listen to the `tagged`, `push_tags`
and `pop_tags` methods.

Implement these methods to have the logger that wraps our logger call
these methods to set tags.

The format is the same as Rails formats it: `[My tag value]`
I decide not to set these as attributes, because they're only values.
They have no key and the value can be anything and change frequently.
@backlog-helper

This comment has been minimized.

1 similar comment
@backlog-helper
Copy link


This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

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

Successfully merging this pull request may close these issues.

2 participants