Skip to content

Add request URI to API error debug logs#1973

Open
AkashKumar7902 wants to merge 1 commit into
kube-rs:mainfrom
AkashKumar7902:issue-949-error-uri
Open

Add request URI to API error debug logs#1973
AkashKumar7902 wants to merge 1 commit into
kube-rs:mainfrom
AkashKumar7902:issue-949-error-uri

Conversation

@AkashKumar7902

Copy link
Copy Markdown

Motivation

Closes #949.

When the API server returns a 4xx or 5xx response, the client debug log currently prints the parsed or reconstructed Status without the request target. That makes missing API-resource errors harder to diagnose.

Solution

  • Preserve the configured cluster base URI on default clients for error-debug context.
  • Include uri: ... in the parsed and reconstructed unsuccessful API debug messages.
  • Keep the returned Error::Api(Status) value unchanged.

Validation:

  • cargo test -p kube-client error_uri --lib
  • cargo test -p kube-client --lib
  • cargo test -p kube-client --doc
  • cargo clippy -p kube-client --lib -- -D warnings
  • cargo +nightly fmt --all -- --check
  • git diff --check

@codecov

codecov Bot commented Apr 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.9%. Comparing base (67e4671) to head (f15bf03).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
kube-client/src/client/builder.rs 91.7% 1 Missing ⚠️
kube-client/src/client/mod.rs 95.7% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1973     +/-   ##
=======================================
+ Coverage   75.6%   75.9%   +0.4%     
=======================================
  Files         89      89             
  Lines       8666    8774    +108     
=======================================
+ Hits        6544    6654    +110     
+ Misses      2122    2120      -2     
Files with missing lines Coverage Δ
kube-client/src/client/middleware/base_uri.rs 91.2% <100.0%> (ø)
kube-client/src/client/middleware/mod.rs 93.4% <ø> (ø)
kube-client/src/client/builder.rs 71.4% <91.7%> (+1.7%) ⬆️
kube-client/src/client/mod.rs 84.1% <95.7%> (+6.5%) ⬆️

... and 9 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Akash Kumar <meakash7902@gmail.com>
@AkashKumar7902

Copy link
Copy Markdown
Author

Resolved the merge conflicts by rebasing onto current main; the conflict was only in the test module, so I kept both the upstream PartialObjectMeta header tests and this PR’s request-URI debug tests. Local validation: git diff --check. I could not run cargo/rustfmt locally because this environment does not have the Rust toolchain installed, so CI remains the source of truth for the Rust checks.

@doxxx93

doxxx93 commented May 27, 2026

Copy link
Copy Markdown
Member

Thanks for fix,

request_error_uri(...) runs on every request, but the resulting request_uri is only ever used when self.send(...) comes back as an error. On the happy path (which is request_text for basically every typed API call) we still pay for the set_base_uri rebuild, a format! + Uri::build allocation, and then immediately throw it away. Would it make more sense to do the reconstruction inside handle_api_errors, so it only happens on 4xx/5xx?

Curious what others think though.

@clux clux 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.

comment on similarity with a system we already have. i appreciate the intent here though, would be good to get this aligned 👍

Comment on lines +69 to +76
fn with_base_uri(self, base_uri: Uri) -> Self {
ClientBuilder {
service: self.service,
default_ns: self.default_ns,
base_uri: Some(base_uri),
valid_until: self.valid_until,
}
}

@clux clux May 27, 2026

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.

This does leave a gap in custom clients (see custom_ examples), and it's a bit awkward to have to make this method to be public just for debug logs.

Does it make sense to hook into the existing BaseUri system we have for tower for this same pupose (we support rancher style cluster url there already) rather than have a special system for it just for logs?

@goodbsw

goodbsw commented Jun 10, 2026

Copy link
Copy Markdown

Hi @AkashKumar7902! I was working on a parallel take for #949 and @doxxx93 kindly pointed me to your PR.

First off, brilliant work on utilizing the BaseUri layer via request.extensions()—this completely resolves the subtle URI truncation bugs in Rancher/proxy multi-tenant environments while brilliantly avoiding breaking changes on Error::Api.

Looking through your implementation in kube-client/src/client/middleware/base_uri.rs, I noticed we could make the error diagnosis even more robust for production debugging:

  1. Explicit Wrapper Type for Extensions: Instead of inserting a raw http::Uri directly into req.extensions_mut(), it might be safer to define a small internal tuple struct like struct ResolvedBaseUri(pub Uri);. This prevents potential type collisions with other tower middlewares that might also inject a raw Uri into request extensions.
  2. Granular Logging Placement: When extracting the mapped URI within the client request error path, adding an explicit tracing::debug! or anyhow::Context-like message tracking exactly when a request drops out before/after hitting the BaseUri layer would give SREs precise pipeline visibility.

I'd love to collaborate and help you polish this up if you need any assistance with tests or refactoring.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unhelpful 404 error for missing API resources

4 participants