Skip to content

refactor(CapabilitiesManager): log slow capabilities in a single message - #58971

Merged
szaimen merged 2 commits into
masterfrom
enh/noid/slow-caps-debug-mode
Jun 16, 2026
Merged

refactor(CapabilitiesManager): log slow capabilities in a single message#58971
szaimen merged 2 commits into
masterfrom
enh/noid/slow-caps-debug-mode

Conversation

@szaimen

@szaimenszaimen commented Mar 16, 2026•
edited
Loading

Copy link
Copy Markdown
Contributor

Instead of logging one message per slow capability (and only in debug
mode), collect all slow capabilities and emit a single log entry with
all timings, using the highest applicable log level.

Signed-off-by: Simon L. szaimen@e.mail.de
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

@szaimenszaimen added this to the Nextcloud 34 milestone Mar 16, 2026
@szaimenszaimen added bug 2. developing Work in progress labels Mar 16, 2026
@szaimen
szaimenforce-pushed the enh/noid/slow-caps-debug-mode branch from e31f263 to 76b8024CompareMarch 16, 2026 10:59
@szaimenszaimen changed the title fix(CapabilitiesManager): do not log slow capabilities if debug mode is not enabledfix(CapabilitiesManager): only check execution time if debug mode is enabledMar 16, 2026
@szaimen
szaimenforce-pushed the enh/noid/slow-caps-debug-mode branch 4 times, most recently from d284eaa to 89aa1edCompareMarch 16, 2026 11:38
@szaimenszaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 16, 2026
@szaimen
szaimen marked this pull request as ready for review March 16, 2026 13:50
@szaimen
szaimen requested a review from a team as a code ownerMarch 16, 2026 13:50
@szaimen
szaimen requested review from ArtificialOwl, CarlSchwan, come-nc, icewind1991, miaulalala and salmart-dev and removed request for a teamMarch 16, 2026 13:50
@szaimenszaimen changed the title fix(CapabilitiesManager): only check execution time if debug mode is enabledfix(CapabilitiesManager): only log execution time if debug mode is enabledMar 16, 2026
@come-nc

come-nc commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

The point of the log line was for users to report the bug to the application developers I believe. Is it happening a lot?

@szaimen

szaimen commented Mar 17, 2026

Copy link
Copy Markdown
ContributorAuthor

The point of the log line was for users to report the bug to the application developers I believe. Is it happening a lot?

Unfortunately if the system is under high load, this reporting feature leads to a lot of false-positives if the response time increases due to the high load. So I would rather only enable it in debug mode. Added the reasoning to the PR description.

miaulalala
miaulalala requested changes Mar 17, 2026
View reviewed changes
Comment threadlib/private/CapabilitiesManager.php Outdated
@szaimen
szaimen requested a review from miaulalalaMarch 19, 2026 10:08
@szaimen
szaimenforce-pushed the enh/noid/slow-caps-debug-mode branch 3 times, most recently from 25b9cbe to 3f68ab4CompareMarch 23, 2026 21:07
@nickvergessen

nickvergessen commented Mar 24, 2026

Copy link
Copy Markdown
Member

Unfortunately if the system is under high load, this reporting feature leads to a lot of false-positives if the response time increases due to the high load. So I would rather only enable it in debug mode. Added the reasoning to the PR description.

I think it's wrong to not log when its not debug. Then no one will ever see this as it's no longer being logged on prod and locally most data sets and connections are quick anyway.
Maybe we need a better mechanism in general then, so it's only "accumulating" and then reporting.

@szaimen

szaimen commented Mar 24, 2026

Copy link
Copy Markdown
ContributorAuthor

Maybe we need a better mechanism in general then, so it's only "accumulating" and then reporting.

This sounds generally like a good idea. How could this technically be implemented?

@nextcloud-botnextcloud-bot mentioned this pull request May 27, 2026
Merged
@nextcloud-botnextcloud-bot mentioned this pull request Jun 4, 2026
Closed
@skjnldsvskjnldsv mentioned this pull request Jun 4, 2026
Merged
This was referenced Jun 5, 2026
Merged
Merged
@sorbaugh

sorbaugh commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@szaimen since this PR reads like a different approach was agreed upon, I'm unsure if this should be kept open or not?

@szaimen

szaimen commented Jun 11, 2026

Copy link
Copy Markdown
ContributorAuthor

@szaimen since this PR reads like a different approach was agreed upon, I'm unsure if this should be kept open or not?

I didnt get an answer for #58971 (comment) yet

@nickvergessen

nickvergessen commented Jun 12, 2026

Copy link
Copy Markdown
Member

How could this technically be implemented?

I have no real idea, but basically we could collect all slow capabilities and then log a single message with all the times and the highest applicable level. Then it's at least only 1 log.

szaimenand others added 2 commits June 15, 2026 18:37
@szaimen@miaulalala
fix(CapabilitiesManager): only check execution time if debug mode is …
3e6d2dc
…enabled
Signed-off-by: Simon L. <szaimen@e.mail.de>
Co-Authored-By: Anna <anna@nextcloud.com>
@szaimen@claude
refactor(CapabilitiesManager): log slow capabilities in a single message
3881d9b
Instead of logging one message per slow capability (and only in debug
mode), collect all slow capabilities and emit a single log entry with
all timings, using the highest applicable log level.
Signed-off-by: Simon L. <szaimen@e.mail.de>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Simon L. <szaimen@e.mail.de>
@szaimen
szaimenforce-pushed the enh/noid/slow-caps-debug-mode branch from 3f68ab4 to 3881d9bCompareJune 15, 2026 16:43
@szaimen

szaimen commented Jun 15, 2026

Copy link
Copy Markdown
ContributorAuthor

How could this technically be implemented?

I have no real idea, but basically we could collect all slow capabilities and then log a single message with all the times and the highest applicable level. Then it's at least only 1 log.

See 3881d9b

@szaimenszaimen changed the title fix(CapabilitiesManager): only log execution time if debug mode is enabledrefactor(CapabilitiesManager): log slow capabilities in a single messageJun 15, 2026
@szaimen
szaimen merged commit e9eac64 into masterJun 16, 2026
205 of 207 checks passed
@szaimen
szaimen deleted the enh/noid/slow-caps-debug-mode branch June 16, 2026 08:14
@nextcloud-botnextcloud-bot mentioned this pull request Aug 11, 2026
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen approved these changes
@miaulalalamiaulalalamiaulalala approved these changes
@come-nccome-nccome-nc approved these changes
@ArtificialOwlArtificialOwlAwaiting requested review from ArtificialOwlArtificialOwl is a code owner automatically assigned from nextcloud/server-backend
@icewind1991icewind1991Awaiting requested review from icewind1991icewind1991 is a code owner automatically assigned from nextcloud/server-backend
@salmart-devsalmart-devAwaiting requested review from salmart-devsalmart-dev is a code owner automatically assigned from nextcloud/server-backend
@CarlSchwanCarlSchwanAwaiting requested review from CarlSchwan

Assignees

No one assigned

Labels

3. to reviewWaiting for reviewsAI assistedbackport-requestbug

Projects

None yet

Milestone

Nextcloud 35

Development

Successfully merging this pull request may close these issues.

8 participants

@szaimen@come-nc@nickvergessen@sorbaugh@miaulalala@joshtrichards@susnux@nextcloud-bot