Skip to content

[Logging] Please advise people to use reset() for long running processes #15086

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.

Sign up for GitHub

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

Jump to bottom
Closed
Seldaek opened this issue Mar 11, 2021 · 8 comments · Fixed by #16609
Closed

[Logging] Please advise people to use reset() for long running processes #15086

Seldaek opened this issue Mar 11, 2021 · 8 comments · Fixed by #16609
Labels
Stalled

Comments

@Seldaek
Copy link
Member

Seldaek commented Mar 11, 2021

See https://twitter.com/seldaek/status/1370115718499422208 for details.

The latest recipe has a buffer limit now so it's less of an issue I guess, but still reset is cleaner.
@Seldaek Seldaek changed the title [Logging] Please advice people to use reset() for long running processes [Logging] Please advise people to use reset() for long running processes Mar 11, 2021
@kbond
Copy link
Member

kbond commented Mar 11, 2021

Is this an issue for messenger workers? Should reset be called between jobs (WorkerMessageHandledEvent event)?

@Seldaek
Copy link
Member Author

Seldaek commented Mar 11, 2021

I'd say yes that would probably be a good idea. The one trade-off is that you lose context in case you have an issue that is triggered by something that happened in a previous job, but those are rather rare occurrences.

@Seldaek
Copy link
Member Author

Seldaek commented Mar 11, 2021

In my worker implementations I typically do a reset() before starting processing the first job (so that any logging happening while spinning up a worker is cleared) and then after every job processed.

@kbond kbond mentioned this issue Mar 11, 2021
Closed
@kbond
Copy link
Member

kbond commented Apr 18, 2021 •
edited
Loading

symfony/symfony#40761 solves this I believe.

@Seldaek
Copy link
Member Author

Seldaek commented Apr 18, 2021

Not really. I mean it solves it for messenger workers, but pretty much every project I work on has custom built long term processes, so I suspect this is a fairly common thing people do, and the logging docs should still mention reset() IMO.

94noni added a commit to 94noni/symfony-docs that referenced this issue Mar 16, 2022
@94noni
[Logging] Document the Monolog reset() for long running processes
a34c28d 
Close symfony#15086
@94noni 94noni mentioned this issue Mar 16, 2022
Merged
@carsonbot
Copy link
Collaborator

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@carsonbot
Copy link
Collaborator

Friendly ping? Should this still be open? I will close if I don't hear anything.

@Seldaek
Copy link
Member Author

Seldaek commented May 3, 2022

Can be closed as #16609 is enough to keep track of this.

@Seldaek Seldaek closed this as completed May 3, 2022
OskarStark added a commit that referenced this issue Mar 8, 2023
@OskarStark
minor #16609 [Logging] Document the Monolog reset() method for long…
59b5689 
… running processes (94noni)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[Logging] Document the Monolog `reset()` method for long running processes

Close #15086

Commits
-------

d949325 [Logging] Document the Monolog `reset()` method for long running processes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Stalled
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Logging] Document the Monolog reset() method for long running processes 94noni/symfony-docs
3 participants
@kbond @Seldaek @carsonbot