Allow for configuring interceptors on SDK client #2669

ysaito1001 · 2023-05-03T17:57:55Z

We will park this PR as a draft due to this reason. We'll revisit and update this once we're getting close to integrate the orchestrator with the rest of the world.

Description

This PR allows users to pass-in interceptors to ConfigLoader, to SdkConfig. Client-level configured interceptors are eventually added to client_interceptors and operation-level interceptors to operation_interceptors, both of which are fields in the aws-smithy-runtime-api::client::interceptors::Interceptors.

SDK registers default set of (client-level & operation-level) interceptors, and the passed-in interceptors by a user will run after those default interceptors.

Admittedly, the code changes in the PR do not offer the ability to debug effectively when things go wrong with misconfigured interceptors (such as putting a wrong header value, leading to an unexpected request created). Adding traces could be an option. Open to suggestions/ideas.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

This commit adds newtypes `SharedInterceptor` and `AddOnlyInterceptors` around existing types so that they can be used outside of their defining crates without revealing the implementation details.

This commit allows users to pass their own interceptors to `ConfigLoader` and to `SdkConfig`. The docs for their methods are marked as `#[doc(hidden)]` until the orchestrator is fully functional.

This commit updates the codegen to register user-passed-in interceptors. Registration takes place at the client config level and the operation config level. The former is handled in `ServiceRuntimePlugin` and the latter in service config `Builder`.

This commit updates `sra_test` to verify interceptors can be configured at the client config level. It also adds a test to ensure interceptors registered run according to the pre-defined priority.

github-actions · 2023-05-03T18:31:35Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-05-03T19:18:35Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
Server Test Python (ignoring whitespace)
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

jdisanti

Nice work on this! I have mostly minor comments.

aws/rust-runtime/aws-config/src/lib.rs

aws/rust-runtime/aws-types/src/sdk_config.rs

...re/amazon/smithy/rust/codegen/client/smithy/customizations/InterceptorConfigCustomization.kt

...oftware/amazon/smithy/rust/codegen/client/smithy/generators/config/ServiceConfigGenerator.kt

rust-runtime/aws-smithy-runtime-api/src/client/interceptors.rs

rcoh · 2023-05-03T23:45:51Z

aws/rust-runtime/aws-config/Cargo.toml

@@ -26,6 +26,7 @@ aws-smithy-client = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-client", de
 aws-smithy-http = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-http" }
 aws-smithy-http-tower = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-http-tower" }
 aws-smithy-json = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-json" }
+aws-smithy-runtime-api = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-runtime-api" }


I thought we said we weren't going to add this dep

Hmm, if aws-config does not depend on aws-smithy-runtime-api, how does it know the type of Interceptor that's part of the method signature for .interceptor ?

Turns out that having a dependency from aws-config (or aws-types) onto aws-smithy-runtime-api makes its hands tied to aws-smithy-runtime-api when it comes to breaking changes. In other words, it becomes more difficult to introduce breaking changes in aws-smithy-runtime-api because they are translated to breaking changes in aws-config (we can only do so during a release that's allowed to introduce breaking changes but that makes development of aws-smithy-runtime-api less nimble).

rcoh · 2023-05-03T23:46:07Z

aws/rust-runtime/aws-types/Cargo.toml

@@ -17,6 +17,7 @@ aws-smithy-async = { path = "../../../rust-runtime/aws-smithy-async" }
 aws-smithy-types = { path = "../../../rust-runtime/aws-smithy-types" }
 aws-smithy-client = { path = "../../../rust-runtime/aws-smithy-client" }
 aws-smithy-http = { path = "../../../rust-runtime/aws-smithy-http" }
+aws-smithy-runtime-api = { path = "../../../rust-runtime/aws-smithy-runtime-api" }


Co-authored-by: John DiSanti <jdisanti@amazon.com>

…egen/client/smithy/customizations/InterceptorConfigCustomization.kt Co-authored-by: John DiSanti <jdisanti@amazon.com>

…egen/client/smithy/generators/config/ServiceConfigGenerator.kt Co-authored-by: John DiSanti <jdisanti@amazon.com>

…ps://github.com/awslabs/smithy-rs into ysaito/configure-interceptors-at-various-levels

This commit addresses #2669 (comment)

github-actions · 2023-05-05T19:23:04Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

This commit addresses the following discussions: #2669 (comment) #2669 (comment)

github-actions · 2023-05-05T20:21:41Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

This commit addresses the following discussions: #2669 (comment) #2669 (comment)

github-actions · 2023-05-05T22:22:26Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-05-06T04:36:46Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2023-05-06T05:39:15Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

This commit addresses #2669 (comment)

ysaito1001 added 4 commits May 3, 2023 09:57

Add newtypes for a shared interceptor and its collection

9d0e481

This commit adds newtypes `SharedInterceptor` and `AddOnlyInterceptors` around existing types so that they can be used outside of their defining crates without revealing the implementation details.

Configure interceptors in ConfigLoader and SdkConfig

97f0f0c

This commit allows users to pass their own interceptors to `ConfigLoader` and to `SdkConfig`. The docs for their methods are marked as `#[doc(hidden)]` until the orchestrator is fully functional.

Add tests to verify configuring interceptors

85c313a

This commit updates `sra_test` to verify interceptors can be configured at the client config level. It also adds a test to ensure interceptors registered run according to the pre-defined priority.

ysaito1001 requested a review from a team as a code owner May 3, 2023 17:57

ysaito1001 added the needs-sdk-review label May 3, 2023

Address test failures in CI

fb62dd3

jdisanti reviewed May 3, 2023

View reviewed changes

rcoh reviewed May 3, 2023

View reviewed changes

ysaito1001 and others added 11 commits May 5, 2023 12:33

Merge branch 'main' into ysaito/configure-interceptors-at-various-levels

b4d9054

Update aws/rust-runtime/aws-config/src/lib.rs

ac1c69c

Co-authored-by: John DiSanti <jdisanti@amazon.com>

Update aws/rust-runtime/aws-config/src/lib.rs

4d3d390

Co-authored-by: John DiSanti <jdisanti@amazon.com>

Update aws/rust-runtime/aws-config/src/lib.rs

462a355

Co-authored-by: John DiSanti <jdisanti@amazon.com>

Update codegen-client/src/main/kotlin/software/amazon/smithy/rust/cod…

f011bd9

…egen/client/smithy/customizations/InterceptorConfigCustomization.kt Co-authored-by: John DiSanti <jdisanti@amazon.com>

Update codegen-client/src/main/kotlin/software/amazon/smithy/rust/cod…

1baba61

…egen/client/smithy/customizations/InterceptorConfigCustomization.kt Co-authored-by: John DiSanti <jdisanti@amazon.com>

Update codegen-client/src/main/kotlin/software/amazon/smithy/rust/cod…

2c9167d

…egen/client/smithy/generators/config/ServiceConfigGenerator.kt Co-authored-by: John DiSanti <jdisanti@amazon.com>

Merge branch 'ysaito/configure-interceptors-at-various-levels' of htt…

c68c761

…ps://github.com/awslabs/smithy-rs into ysaito/configure-interceptors-at-various-levels

Make the getter interceptors #[doc(hidden)]

32f7334

This commit addresses #2669 (comment)

Drop # from #[doc(hidden)] in comments for codegen

da7a306

Rename AddOnlyInterceptors to InterceptorRegistrar

8c1fc01

This commit addresses #2669 (comment)

Rename set_interceptor and add set_interceptors

ba26b98

This commit addresses the following discussions: #2669 (comment) #2669 (comment)

Replace doc example with one modifying request URI

5583680

This commit addresses the following discussions: #2669 (comment) #2669 (comment)

Fix clippy warning needless_borrow

02cdf15

Add SharedInterceptor to approved types in aws-types

e8df08b

ysaito1001 requested a review from jdisanti May 8, 2023 15:13

ysaito1001 marked this pull request as draft May 8, 2023 17:36

ysaito1001 mentioned this pull request May 8, 2023

Integrate orchestrator with the rest of the world #2679

Closed

1 task

ysaito1001 added a commit that referenced this pull request May 12, 2023

Exclude aws runtime crates from interceptor config

1d834d0

This commit addresses #2669 (comment)

ysaito1001 changed the title ~~Allow for configuring interceptors at various config levels~~ Allow for configuring interceptors on SDK client May 12, 2023

Merge branch 'main' into ysaito/configure-interceptors-at-various-levels

e9a45fa

ysaito1001 removed the needs-sdk-review label May 12, 2023

rcoh closed this Oct 18, 2023

ysaito1001 deleted the ysaito/configure-interceptors-at-various-levels branch January 24, 2024 22:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow for configuring interceptors on SDK client #2669

Allow for configuring interceptors on SDK client #2669

ysaito1001 commented May 3, 2023 •

edited

Loading

github-actions bot commented May 3, 2023

github-actions bot commented May 3, 2023

jdisanti left a comment

rcoh May 3, 2023

ysaito1001 May 4, 2023

ysaito1001 May 8, 2023

rcoh May 3, 2023

github-actions bot commented May 5, 2023

github-actions bot commented May 5, 2023

github-actions bot commented May 5, 2023

github-actions bot commented May 6, 2023

github-actions bot commented May 6, 2023

Allow for configuring interceptors on SDK client #2669

Allow for configuring interceptors on SDK client #2669

Conversation

ysaito1001 commented May 3, 2023 • edited Loading

Description

github-actions bot commented May 3, 2023

github-actions bot commented May 3, 2023

jdisanti left a comment

Choose a reason for hiding this comment

rcoh May 3, 2023

Choose a reason for hiding this comment

ysaito1001 May 4, 2023

Choose a reason for hiding this comment

ysaito1001 May 8, 2023

Choose a reason for hiding this comment

rcoh May 3, 2023

Choose a reason for hiding this comment

github-actions bot commented May 5, 2023

github-actions bot commented May 5, 2023

github-actions bot commented May 5, 2023

github-actions bot commented May 6, 2023

github-actions bot commented May 6, 2023

ysaito1001 commented May 3, 2023 •

edited

Loading