Skip to content

fix(CapabilitiesManager): only log execution time if debug mode is enabled#58971

Open
szaimen wants to merge 1 commit intomasterfrom
enh/noid/slow-caps-debug-mode
Open

fix(CapabilitiesManager): only log execution time if debug mode is enabled#58971
szaimen wants to merge 1 commit intomasterfrom
enh/noid/slow-caps-debug-mode

Conversation

@szaimen
Copy link
Copy Markdown
Contributor

@szaimen szaimen commented Mar 16, 2026 •
edited
Loading

  • Reason: do not spam logs with messages the admin cannot do anything about: if the system is under high load, the reporting feature leads to a lot of false-positives if the response time increases.
  • This is best reviewed like this: https://github.com/nextcloud/server/pull/58971/changes?w=1

@szaimen szaimen added this to the Nextcloud 34 milestone Mar 16, 2026
@szaimen szaimen added bug 2. developing Work in progress labels Mar 16, 2026
@szaimen szaimen force-pushed the enh/noid/slow-caps-debug-mode branch from e31f263 to 76b8024 Compare March 16, 2026 10:59
@szaimen szaimen changed the title fix(CapabilitiesManager): do not log slow capabilities if debug mode is not enabled fix(CapabilitiesManager): only check execution time if debug mode is enabled Mar 16, 2026
@szaimen szaimen force-pushed the enh/noid/slow-caps-debug-mode branch 4 times, most recently from d284eaa to 89aa1ed Compare March 16, 2026 11:38
@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 16, 2026
@szaimen
Copy link
Copy Markdown
Contributor Author

szaimen commented Mar 16, 2026

/backport to stable33

@szaimen
Copy link
Copy Markdown
Contributor Author

szaimen commented Mar 16, 2026

/backport to stable32

@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 owner March 16, 2026 13:50
@szaimen szaimen requested review from ArtificialOwl, CarlSchwan, come-nc, icewind1991, miaulalala and salmart-dev and removed request for a team March 16, 2026 13:50
@szaimen szaimen changed the title fix(CapabilitiesManager): only check execution time if debug mode is enabled fix(CapabilitiesManager): only log execution time if debug mode is enabled Mar 16, 2026
@come-nc
Copy link
Copy Markdown
Contributor

come-nc commented Mar 17, 2026

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
Copy link
Copy Markdown
Contributor Author

szaimen commented Mar 17, 2026

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
@szaimen szaimen requested a review from miaulalala March 19, 2026 10:08
@szaimen szaimen force-pushed the enh/noid/slow-caps-debug-mode branch 2 times, most recently from ccd554f to 25b9cbe Compare March 19, 2026 10:17
@szaimen @miaulalala
fix(CapabilitiesManager): only check execution time if debug mode is …
3f68ab4 
…enabled

Signed-off-by: Simon L. <szaimen@e.mail.de>
Co-Authored-By: Anna <anna@nextcloud.com>
@szaimen szaimen force-pushed the enh/noid/slow-caps-debug-mode branch from 25b9cbe to 3f68ab4 Compare March 23, 2026 21:07
@nickvergessen
Copy link
Copy Markdown
Member

nickvergessen commented Mar 24, 2026

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
Copy link
Copy Markdown
Contributor Author

szaimen commented Mar 24, 2026

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?

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

Reviewers

@miaulalala miaulalala miaulalala approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl is a code owner automatically assigned from nextcloud/server-backend

@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 is a code owner automatically assigned from nextcloud/server-backend

@salmart-dev salmart-dev Awaiting requested review from salmart-dev salmart-dev is a code owner automatically assigned from nextcloud/server-backend

@come-nc come-nc Awaiting requested review from come-nc come-nc is a code owner automatically assigned from nextcloud/server-backend

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

3. to review Waiting for reviews backport-request bug

Projects

None yet

Milestone

Nextcloud 34

Development

Successfully merging this pull request may close these issues.

4 participants

@szaimen @come-nc @nickvergessen @miaulalala