Skip to content

[*] release logrus.Entry immediately in BrokerHook.Fire()#1303

Merged
pashagolub merged 2 commits intomasterfrom
fix-log-broker-hook-entry-usage
Mar 16, 2026
Merged

[*] release logrus.Entry immediately in BrokerHook.Fire()#1303
pashagolub merged 2 commits intomasterfrom
fix-log-broker-hook-entry-usage

Conversation

@pashagolub
Copy link
Copy Markdown
Collaborator

@pashagolub pashagolub commented Mar 16, 2026

Format synchronously in Fire() while the entry is owned by the calling goroutine, send MessageType through the channel, and send() delivers out without logrus.Entry copy and without lastError channel

Format synchronously in `Fire()` while the entry is owned by the calling
goroutine, send `MessageType` through the channel, and `send()` delivers
out without `logrus.Entry` copy and without `lastError` channel
@pashagolub pashagolub self-assigned this Mar 16, 2026
@pashagolub pashagolub added the refactoring Something done as it should've been done from the start label Mar 16, 2026
@coveralls
Copy link
Copy Markdown

coveralls commented Mar 16, 2026

Pull Request Test Coverage Report for Build 23142039709

Details

  • 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 83.699%

Totals Coverage Status
Change from base Build 23139655786: 0.6%
Covered Lines: 4580
Relevant Lines: 5472

💛 - Coveralls

@pashagolub pashagolub force-pushed the fix-log-broker-hook-entry-usage branch from a403c2c to d07dfcd Compare March 16, 2026 11:45
@pashagolub pashagolub merged commit deb3973 into master Mar 16, 2026
9 checks passed
@pashagolub pashagolub deleted the fix-log-broker-hook-entry-usage branch March 16, 2026 11:56
@Mazen050
Copy link
Copy Markdown
Contributor

Nice fix, moving the formatter into Fire() while the entry is still owned is much cleaner than my entry copy fix. I'll keep this in mind for future PRs.

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

Labels

refactoring Something done as it should've been done from the start

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants