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.

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

emit an event when a message is logged #38490

Merged
merged 1 commit into from
Jun 14, 2023
Merged

emit an event when a message is logged #38490

merged 1 commit into from
Jun 14, 2023

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented May 26, 2023 •
edited
Loading

Allow apps to do their own handling, notably this is emitted even if it would be normally filtered by log level

Needed for nextcloud/logreader#873

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label May 26, 2023
@icewind1991 icewind1991 added this to the Nextcloud 28 milestone May 26, 2023
@icewind1991 icewind1991 requested review from a team, ArtificialOwl, blizzz and nfebe and removed request for a team May 26, 2023 17:05
@icewind1991 icewind1991 mentioned this pull request May 26, 2023
Merged
1 task
@icewind1991 icewind1991 marked this pull request as ready for review May 26, 2023 17:06
@icewind1991 icewind1991 force-pushed the log-dispatch-event branch 4 times, most recently from 3507df8 to 3d72315 Compare June 5, 2023 15:36
@icewind1991 icewind1991 requested a review from come-nc June 9, 2023 17:38
come-nc
come-nc reviewed Jun 12, 2023
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Performance-wise, is that not too much overhead?

lib/private/EventDispatcher/EventDispatcher.php Show resolved Hide resolved
lib/private/Files/Cache/Scanner.php Outdated Show resolved Hide resolved
lib/public/Log/BeforeMessageLoggedEvent.php Outdated Show resolved Hide resolved
@icewind1991
Copy link
Member Author

Performance-wise, is that not too much overhead?

From my understanding the event dispatcher is well optimized for the "no listeners" case.

@icewind1991
emit an event when a message is logged
d9fa7b1 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 merged commit e6959bd into master Jun 14, 2023
@icewind1991 icewind1991 deleted the log-dispatch-event branch June 14, 2023 11:17
@icewind1991
Copy link
Member Author

/backport to stable27

@icewind1991
Copy link
Member Author

/backport to stable26

This was referenced Jun 14, 2023
Merged
Merged
@ChristophWurst ChristophWurst added enhancement pending documentation This pull request needs an associated documentation update labels Jun 15, 2023
@ChristophWurst
Copy link
Member

New API -> docs

@nickvergessen nickvergessen mentioned this pull request Aug 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@nfebe nfebe nfebe approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@blizzz blizzz Awaiting requested review from blizzz blizzz was automatically assigned from nextcloud/server-backend

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement pending documentation This pull request needs an associated documentation update
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.
4 participants
@icewind1991 @ChristophWurst @nfebe @come-nc