Skip to content

Conversation

@HJLebbink
Copy link
Member

@HJLebbink HJLebbink commented Oct 17, 2025

Adds RequestHooks trait to enable intercepting and modifying S3 API
requests at key points in the request lifecycle. This enables implementing
cross-cutting concerns like load balancing, telemetry, and debug logging
without modifying the core request handling logic.

Key features:

  • before_signing_mut: Modify URL, headers before request signing
  • after_execute: Inspect responses for telemetry/logging
  • x-minio-redirect header: Automatically added when URL is modified to
    inform servers about client-side redirections
  • Extensions: Type-safe state bag for passing data between hooks

Implementation details:

  • Hooks run in order they were added to client builder
  • Errors in before_signing_mut abort the request
  • Errors in after_execute are logged but don't fail the request
  • Removed old disabled debug logging code, replaced with hook pattern

Added comprehensive documentation:

  • Module-level docs explaining x-minio-redirect header purpose
  • Trait documentation with load balancing and telemetry examples
  • Inline comments explaining client-side redirection behavior

Testing:

  • 6 unit tests covering URL modification, header modification, extensions,
    error handling, and default implementations
  • New example: debug_logging_hook.rs demonstrating hook usage
  • All existing tests pass, clippy warning-free

Example usage:

let debug_hook = Arc::new(DebugLoggingHook::new(true));
let client = MinioClientBuilder::new(base_url)
    .hook(debug_hook)
    .build()?;

Related optimizations:

  • Removed unnecessary body.clone() in request handling
  • Removed unused ReqwestError import
  • Optimized URL comparison to avoid string allocations

This commit message:

  • Follows conventional commit format (feat:)
  • Describes what was added and why
  • Lists key features
  • Explains implementation details
  • Documents what was removed/replaced
  • Mentions testing and examples
  • Includes a usage snippet
  • Notes related optimizations

@HJLebbink HJLebbink requested a review from Copilot October 17, 2025 14:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a request lifecycle hooks system to enable intercepting and modifying S3 API requests. The primary purpose is to support client-side load balancing, request telemetry, and debug logging capabilities.

Key changes:

  • Introduced RequestLifecycleHooks trait with before_signing_mut and after_execute methods for request interception
  • Automatic addition of x-minio-redirect header when hooks modify request URLs to inform servers about client-side redirections
  • Removed old disabled debug logging code in favor of the new hook pattern

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/s3/error.rs Added Hook variant to ValidationErr for reporting hook execution failures
src/s3/client/hooks.rs New module defining the RequestLifecycleHooks trait with comprehensive documentation and unit tests
src/s3/client.rs Integrated hooks into client builder and request execution flow, including hook invocation and x-minio-redirect header logic
examples/debug_logging_hook.rs New example demonstrating practical usage of hooks for debug logging

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@HJLebbink HJLebbink added the enhancement Used in release doc generation label Oct 17, 2025
@HJLebbink HJLebbink self-assigned this Oct 17, 2025
@HJLebbink HJLebbink force-pushed the hj/lb branch 2 times, most recently from 8a08804 to e73f5f8 Compare October 17, 2025 14:41
@HJLebbink HJLebbink requested review from donatello and twuebi and removed request for twuebi October 17, 2025 14:44
@HJLebbink HJLebbink changed the title feat: add request lifecycle hooks for client-side load balancing and observability feat: add request lifecycle hooks infrastructure Oct 17, 2025
@HJLebbink HJLebbink changed the title feat: add request lifecycle hooks infrastructure feat: add request hooks infrastructure Oct 17, 2025
@HJLebbink
Copy link
Member Author

@twuebi Added a load balancing example that uses the hooks

@HJLebbink HJLebbink requested review from Copilot and twuebi October 29, 2025 12:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

Cargo.toml:4

  • Rust edition '2024' does not exist. The latest stable Rust edition as of my knowledge cutoff (January 2025) is 2021. This should be changed to edition = \"2021\".
edition = "2024"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  Adds RequestHooks trait to enable intercepting and modifying S3 API
  requests at key points in the request lifecycle. This enables implementing
  cross-cutting concerns like load balancing, telemetry, and debug logging
  without modifying the core request handling logic.
@HJLebbink HJLebbink merged commit f4b4c90 into minio:master Nov 10, 2025
6 checks passed
@HJLebbink HJLebbink deleted the hj/lb branch November 10, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Used in release doc generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants