logging: wire klog backend, but only output request throttling logs#5419
logging: wire klog backend, but only output request throttling logs#5419negz merged 1 commit intocrossplane:masterfrom
Conversation
0bf32da to
e982faa
Compare
jbw976
left a comment
There was a problem hiding this comment.
@sttts would it be possible for you to provide more details in the PR description that show what the logs look like with this change? besides helping understand the general experience, I'm also hoping to see how these logs will be able to positively identify slowdowns and if there's anything the person reading the logs can do about it.
|
@jbw976 added the logging lines from client-go that now pass. |
|
As an example, @turkenh would have seen something like in crossplane-contrib/provider-kubernetes#203 like crazy because client-side throttling was aggressively set to 10 requests per second. |
@sttts I am not sure, my understanding was we were rate-limiting ourself even before making client side requests 🤔 |
|
Ok, controller rate limiting is even before, yes. |
|
For background: there are 3 levels of throttling usually:
In Crossplane afaik request rps is 5x Max-reconcile-rate. Any of these 3 can make things slow if load is high enough. |
negz
left a comment
There was a problem hiding this comment.
This seems reasonable. Do you want to go straight to adding it to crossplane-runtime pkg/logging rather than proceeding here?
If you want to proceed here, see comments. There's also some linter errors to address.
WRT the log output - it will use the same format as the rest of our log lines, right? e.g. If we set our logger to use structured logging these would be structured too? I expect that to be the case from the code but it's not clear from the log lines in the description.
FYI I retroactively documented our client-side rate limiters in #5415. I was thinking debug logging and emitting metrics when the workqueue and/or rate-limiting |
|
Would go directly to crossplane-runtime. Will open it there. |
c1db3bf to
9c36e62
Compare
9c36e62 to
6e85902
Compare
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@upbound.io>
6e85902 to
eb9fe61
Compare
Description of your changes
Vendoring and wiring crossplane/crossplane-runtime#673.
I have:
make reviewableto ensure this PR is ready for review.[ ] Added or updated unit tests.[ ] Added or updated e2e tests.[ ] Linked a PR or a docs tracking issue to document this change.[ ] Addedbackport release-x.ylabels to auto-backport this PR.Need help with this checklist? See the cheat sheet.