Skip to content

Clean false errors in logs - #4538

Merged
kcp-ci-bot merged 1 commit into
kcp-dev:mainfrom
mjudeikis:quiet.proxy.errors
Nov 3, 2026
Merged

kcp-ci-bot merged 1 commit into
kcp-dev:mainfrom
mjudeikis:quiet.proxy.errors

Conversation

@mjudeikis

Copy link
Copy Markdown
Contributor

Summary

When running in production, there are quite a few false errors logged, not related to issues.
This is kinda grab-a-bag to clean up some of these. Error should mean error, not noise.

What Type of PR Is This?

/kind cleanup

Related Issue(s)

Fixes #

Release Notes

Cleanup false -possitive errors logging 

@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. dco-signoff: yes Indicates the PR's author has signed the DCO. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 2, 2026
@mjudeikis

Copy link
Copy Markdown
Contributor Author
Screenshot 2026-11-03 at 09 04 18

@mjudeikis

Copy link
Copy Markdown
Contributor Author

@mjudeikis-bot review this please

@mjudeikis-bot mjudeikis-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Two warnings and one note; no verified Critical.

Summary: the PR routes every httputil.ReverseProxy ErrorLog through a classifier that demotes stream-ending errors to V(4), and quiets 409 conflicts in the ClusterCachedResource controller.

Checked

  • Read beyond the diff: processNextWorkItem and process in clustercachedresources_controller.go (queue handling of the requeue bool), plus all ReverseProxy construction sites touched (proxy.go, mapping.go, localproxy.go, mountproxy.go, virtualresources/server.go, migratingworkspaces/builder/build.go, virtual/shared/proxy.go, server/config.go).
  • Correctness: the classifier is a substring match on the whole message, so any message containing context deadline exceeded/context canceled/broken pipe/connection reset by peer is demoted regardless of which side failed (finding 1). The conflict branch requeues without backoff (finding 2).
  • Concurrency: reverseProxyLogWriter is stateless and log.Logger serialises writes, so it is safe. The new requeue path can hot-loop (finding 2).
  • kcp specifics: not applicable. No logical-cluster scoping, authorizer, or shard routing changes. Auth stripping and Rewrite functions are untouched.
  • API/storage: not applicable. No CRD, codegen or feature-gate changes.
  • Security: error strings are logged as before, and no new data reaches logs. klog.Background() in localproxy.go and build.go drops request/cluster context from the logger, which is cosmetic.
  • Tests: TestReverseProxyErrorLog covers the classifier. It would fail without the change, since the function would not exist. The conflict-requeue branch has no test (finding 2).
  • Commands: go build ./pkg/... passed. go vet on pkg/logging, pkg/proxy, pkg/server/virtualresources and the clustercachedresources package was clean. go test ./pkg/logging ./pkg/proxy/... ./pkg/server/virtualresources passed. golangci-lint is not installed here, so I did not run it.
  • E2E: not run. No e2e covers log classification or the ClusterCachedResource conflict path, and the full suite needs more than ten minutes of setup.

Comment thread pkg/logging/errors.go Resolved
Comment thread pkg/logging/errors_test.go Resolved

@ntnn ntnn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm
/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 3, 2026
@kcp-ci-bot

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: 4bc5393cb24b8bc3dd091e7704f851625f3ca3ea

@kcp-ci-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ntnn

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 3, 2026
@kcp-ci-bot
kcp-ci-bot merged commit cc872e1 into kcp-dev:main Nov 3, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants