From 2b7a19ef54ee85d3c04ce5d40aef37a790696358 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 20 Apr 2023 13:53:49 -0700 Subject: [PATCH 1/9] Add a middleware vs. orchestrator performance comparison benchmark (#2593) ## Motivation and Context This PR adds a benchmark suite to the `aws-sdk-s3` test in `aws/sra-test` so that the performance of client middleware can be compared against the new orchestrator approach. The comparison is not yet apples to apples since the orchestrator isn't complete enough to be comparable, but this is a benchmark that we can check regularly as they become more comparable. Initially, the performance was really bad (43x slower than middleware), but that turned out to be a trivial mistake in orchestrator implementation that was causing some configuration JSON to get reparsed for every request. Fixing that makes the orchestrator only about 1.5x slower, and there is another trivial optimization that can be made to bring that down more. The benchmark can be run with the following: ```bash smithy-rs$ ./gradlew aws:sra-test:assemble $ cd aws/sra-test/integration-tests/aws-sdk-s3 $ cargo bench ``` ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../integration-tests/aws-sdk-s3/Cargo.toml | 32 +- .../benches/middleware_vs_orchestrator.rs | 285 ++++++++++++++++++ 2 files changed, 305 insertions(+), 12 deletions(-) create mode 100644 aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml b/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml index 51611304b2f..70aaa61a75c 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml +++ b/aws/sra-test/integration-tests/aws-sdk-s3/Cargo.toml @@ -6,20 +6,28 @@ publish = false # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -aws-credential-types = { path = "../../../sdk/build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] } -aws-http = { path = "../../../sdk/build/aws-sdk/sdk/aws-http" } -aws-runtime = { path = "../../../sdk/build/aws-sdk/sdk/aws-runtime" } -aws-sdk-s3 = { path = "../../../sdk/build/aws-sdk/sdk/s3/", features = ["test-util"] } -aws-sigv4 = { path = "../../../sdk/build/aws-sdk/sdk/aws-sigv4" } -aws-types = { path = "../../../sdk/build/aws-sdk/sdk/aws-types" } -aws-smithy-async = { path = "../../../sdk/build/aws-sdk/sdk/aws-smithy-async", features = ["rt-tokio"] } -aws-smithy-client = { path = "../../../sdk/build/aws-sdk/sdk/aws-smithy-client", features = ["test-util"] } -aws-smithy-types = { path = "../../../sdk/build/aws-sdk/sdk/aws-smithy-types" } -aws-smithy-http = { path = "../../../sdk/build/aws-sdk/sdk/aws-smithy-http" } -aws-smithy-runtime = { path = "../../../sdk/build/aws-sdk/sdk/aws-smithy-runtime", features = ["test-util"] } -aws-smithy-runtime-api = { path = "../../../sdk/build/aws-sdk/sdk/aws-smithy-runtime-api" } +aws-credential-types = { path = "../../../rust-runtime/aws-credential-types", features = ["test-util"] } +aws-http = { path = "../../../rust-runtime/aws-http" } +aws-runtime = { path = "../../../rust-runtime/aws-runtime" } +aws-sdk-s3 = { path = "../../build/sdk/aws-sdk-s3", features = ["test-util"] } +aws-sigv4 = { path = "../../../rust-runtime/aws-sigv4" } +aws-types = { path = "../../../rust-runtime/aws-types" } +aws-smithy-async = { path = "../../../../rust-runtime/aws-smithy-async", features = ["rt-tokio"] } +aws-smithy-client = { path = "../../../../rust-runtime/aws-smithy-client", features = ["test-util"] } +aws-smithy-types = { path = "../../../../rust-runtime/aws-smithy-types" } +aws-smithy-http = { path = "../../../../rust-runtime/aws-smithy-http" } +aws-smithy-runtime = { path = "../../../../rust-runtime/aws-smithy-runtime", features = ["test-util"] } +aws-smithy-runtime-api = { path = "../../../../rust-runtime/aws-smithy-runtime-api" } +criterion = { version = "0.4", features = ["async_tokio"] } tokio = { version = "1.23.1", features = ["macros", "test-util", "rt-multi-thread"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.15", features = ["env-filter", "json"] } http = "0.2.3" http-body = "0.4.5" + +[profile.release] +debug = 1 + +[[bench]] +name = "middleware_vs_orchestrator" +harness = false diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs new file mode 100644 index 00000000000..634553466aa --- /dev/null +++ b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs @@ -0,0 +1,285 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#[macro_use] +extern crate criterion; +use aws_sdk_s3 as s3; +use aws_smithy_client::erase::DynConnector; +use aws_smithy_client::test_connection::infallible_connection_fn; +use aws_smithy_runtime_api::type_erasure::TypedBox; +use criterion::Criterion; +use s3::operation::list_objects_v2::{ListObjectsV2Error, ListObjectsV2Input, ListObjectsV2Output}; + +async fn middleware(client: &s3::Client) { + client + .list_objects_v2() + .bucket("test-bucket") + .prefix("prefix~") + .send() + .await + .expect("successful execution"); +} + +async fn orchestrator(connector: &DynConnector) { + // TODO(enableNewSmithyRuntime): benchmark with `send_v2` directly once it works + let runtime_plugins = aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugins::new() + .with_client_plugin(orchestrator::ManualServiceRuntimePlugin(connector.clone())) + .with_operation_plugin(aws_sdk_s3::operation::list_objects_v2::ListObjectsV2::new()) + .with_operation_plugin(orchestrator::ManualOperationRuntimePlugin); + let input = ListObjectsV2Input::builder() + .bucket("test-bucket") + .prefix("prefix~") + .build() + .unwrap(); + let input = TypedBox::new(input).erase(); + let output = aws_smithy_runtime::client::orchestrator::invoke(input, &runtime_plugins) + .await + .map_err(|err| { + err.map_service_error(|err| { + TypedBox::::assume_from(err) + .expect("correct error type") + .unwrap() + }) + }) + .unwrap(); + TypedBox::::assume_from(output) + .expect("correct output type") + .unwrap(); +} + +fn test_connection() -> DynConnector { + infallible_connection_fn(|req| { + assert_eq!( + "https://test-bucket.s3.us-east-1.amazonaws.com/?list-type=2&prefix=prefix~", + req.uri().to_string() + ); + assert!(req.headers().contains_key("authorization")); + http::Response::builder() + .status(200) + .body( + r#" + + test-bucket + prefix~ + 1 + 1000 + false + + some-file.file + 2009-10-12T17:50:30.000Z + 434234 + STANDARD + + +"#, + ) + .unwrap() + }) +} + +fn middleware_bench(c: &mut Criterion) { + let conn = test_connection(); + let config = s3::Config::builder() + .credentials_provider(s3::config::Credentials::for_tests()) + .region(s3::config::Region::new("us-east-1")) + .http_connector(conn.clone()) + .build(); + let client = s3::Client::from_conf(config); + c.bench_function("middleware", move |b| { + b.to_async(tokio::runtime::Runtime::new().unwrap()) + .iter(|| async { middleware(&client).await }) + }); +} + +fn orchestrator_bench(c: &mut Criterion) { + let conn = test_connection(); + + c.bench_function("orchestrator", move |b| { + b.to_async(tokio::runtime::Runtime::new().unwrap()) + .iter(|| async { orchestrator(&conn).await }) + }); +} + +mod orchestrator { + use aws_credential_types::cache::{CredentialsCache, SharedCredentialsCache}; + use aws_credential_types::provider::SharedCredentialsProvider; + use aws_credential_types::Credentials; + use aws_http::user_agent::{ApiMetadata, AwsUserAgent}; + use aws_runtime::recursion_detection::RecursionDetectionInterceptor; + use aws_runtime::user_agent::UserAgentInterceptor; + use aws_sdk_s3::config::Region; + use aws_sdk_s3::operation::list_objects_v2::ListObjectsV2Input; + use aws_smithy_client::erase::DynConnector; + use aws_smithy_runtime::client::connections::adapter::DynConnectorAdapter; + use aws_smithy_runtime_api::client::endpoints::StaticUriEndpointResolver; + use aws_smithy_runtime_api::client::interceptors::{ + Interceptor, InterceptorContext, InterceptorError, Interceptors, + }; + use aws_smithy_runtime_api::client::orchestrator::{ + BoxError, ConfigBagAccessors, Connection, HttpRequest, HttpResponse, TraceProbe, + }; + use aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugin; + use aws_smithy_runtime_api::config_bag::ConfigBag; + use aws_types::region::SigningRegion; + use aws_types::SigningService; + use http::Uri; + use std::sync::Arc; + + pub struct ManualServiceRuntimePlugin(pub DynConnector); + + impl RuntimePlugin for ManualServiceRuntimePlugin { + fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { + let identity_resolvers = + aws_smithy_runtime_api::client::orchestrator::IdentityResolvers::builder() + .identity_resolver( + aws_runtime::auth::sigv4::SCHEME_ID, + aws_runtime::identity::credentials::CredentialsIdentityResolver::new( + SharedCredentialsCache::new(CredentialsCache::lazy().create_cache( + SharedCredentialsProvider::new(Credentials::for_tests()), + )), + ), + ) + .identity_resolver( + "anonymous", + aws_smithy_runtime_api::client::identity::AnonymousIdentityResolver::new(), + ) + .build(); + cfg.set_identity_resolvers(identity_resolvers); + + let http_auth_schemes = + aws_smithy_runtime_api::client::orchestrator::HttpAuthSchemes::builder() + .auth_scheme( + aws_runtime::auth::sigv4::SCHEME_ID, + aws_runtime::auth::sigv4::SigV4HttpAuthScheme::new(), + ) + .build(); + cfg.set_http_auth_schemes(http_auth_schemes); + + cfg.set_auth_option_resolver( + aws_smithy_runtime_api::client::auth::option_resolver::AuthOptionListResolver::new( + Vec::new(), + ), + ); + + //cfg.set_endpoint_resolver(DefaultEndpointResolver::new( + // aws_smithy_http::endpoint::SharedEndpointResolver::new( + // aws_sdk_s3::endpoint::DefaultResolver::new(), + // ), + //)); + cfg.set_endpoint_resolver(StaticUriEndpointResolver::uri(Uri::from_static( + "https://test-bucket.s3.us-east-1.amazonaws.com/", + ))); + + let params_builder = aws_sdk_s3::endpoint::Params::builder() + .set_region(Some("us-east-1".to_owned())) + .set_endpoint(Some("https://s3.us-east-1.amazonaws.com/".to_owned())); + cfg.put(params_builder); + + cfg.set_retry_strategy( + aws_smithy_runtime_api::client::retries::NeverRetryStrategy::new(), + ); + + let connection: Box = + Box::new(DynConnectorAdapter::new(self.0.clone())); + cfg.set_connection(connection); + + cfg.set_trace_probe({ + #[derive(Debug)] + struct StubTraceProbe; + impl TraceProbe for StubTraceProbe { + fn dispatch_events(&self) { + // no-op + } + } + StubTraceProbe + }); + + cfg.put(SigningService::from_static("s3")); + cfg.put(SigningRegion::from(Region::from_static("us-east-1"))); + + cfg.put(ApiMetadata::new("unused", "unused")); + cfg.put(AwsUserAgent::for_tests()); // Override the user agent with the test UA + cfg.get::>() + .expect("interceptors set") + .register_client_interceptor(Arc::new(UserAgentInterceptor::new()) as _) + .register_client_interceptor(Arc::new(RecursionDetectionInterceptor::new()) as _); + Ok(()) + } + } + + // This is a temporary operation runtime plugin until EndpointParamsInterceptor and + // EndpointParamsFinalizerInterceptor have been fully implemented, in which case + // `.with_operation_plugin(ManualOperationRuntimePlugin)` can be removed. + pub struct ManualOperationRuntimePlugin; + + impl RuntimePlugin for ManualOperationRuntimePlugin { + fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { + #[derive(Debug)] + struct ListObjectsV2EndpointParamsInterceptor; + impl Interceptor for ListObjectsV2EndpointParamsInterceptor { + fn read_before_execution( + &self, + context: &InterceptorContext, + cfg: &mut ConfigBag, + ) -> Result<(), BoxError> { + let input = context.input()?; + let input = input + .downcast_ref::() + .ok_or_else(|| InterceptorError::invalid_input_access())?; + let mut params_builder = cfg + .get::() + .ok_or(InterceptorError::read_before_execution( + "missing endpoint params builder", + ))? + .clone(); + params_builder = params_builder.set_bucket(input.bucket.clone()); + cfg.put(params_builder); + + Ok(()) + } + } + + #[derive(Debug)] + struct ListObjectsV2EndpointParamsFinalizerInterceptor; + impl Interceptor for ListObjectsV2EndpointParamsFinalizerInterceptor { + fn read_before_execution( + &self, + _context: &InterceptorContext, + cfg: &mut ConfigBag, + ) -> Result<(), BoxError> { + let params_builder = cfg + .get::() + .ok_or(InterceptorError::read_before_execution( + "missing endpoint params builder", + ))? + .clone(); + let params = params_builder + .build() + .map_err(InterceptorError::read_before_execution)?; + cfg.put( + aws_smithy_runtime_api::client::orchestrator::EndpointResolverParams::new( + params, + ), + ); + + Ok(()) + } + } + + cfg.get::>() + .expect("interceptors set") + .register_operation_interceptor( + Arc::new(ListObjectsV2EndpointParamsInterceptor) as _ + ) + .register_operation_interceptor(Arc::new( + ListObjectsV2EndpointParamsFinalizerInterceptor, + ) as _); + Ok(()) + } + } +} + +criterion_group!(benches, middleware_bench, orchestrator_bench); +criterion_main!(benches); From dd517bc868faec8d90d20890cf147f5fcff15736 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Fri, 21 Apr 2023 16:37:02 +0100 Subject: [PATCH 2/9] Fix `tracing-subscriber` missing `regex` features issue (#2618) ## Description See #2619. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- aws/rust-runtime/aws-credential-types/Cargo.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/aws/rust-runtime/aws-credential-types/Cargo.toml b/aws/rust-runtime/aws-credential-types/Cargo.toml index 8fad933c2aa..4a0b8a93907 100644 --- a/aws/rust-runtime/aws-credential-types/Cargo.toml +++ b/aws/rust-runtime/aws-credential-types/Cargo.toml @@ -28,6 +28,9 @@ env_logger = "0.9.0" tokio = { version = "1.23.1", features = ["full", "test-util", "rt"] } tracing-test = "0.2.1" +# TODO(https://github.com/awslabs/smithy-rs/issues/2619): Remove this +# workaround once the fixed is upstreamed. +regex = { version = "1.0", features = ["unicode-case", "unicode-perl"] } [package.metadata.docs.rs] all-features = true From 3d8c52a676f574ffbde12fb8b156e5b49406ad76 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 21 Apr 2023 12:16:28 -0500 Subject: [PATCH 3/9] Follow up on DefaultEndpointResolver in the orchestrator (#2592) ## Motivation and Context This PR incorporates post-merge feedback left in #2577. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito --- .../aws-sdk-s3/tests/sra_test.rs | 2 +- .../EndpointParamsInterceptorGenerator.kt | 11 +- .../ServiceRuntimePluginGenerator.kt | 2 +- .../aws-smithy-runtime-api/src/client.rs | 3 - .../src/client/endpoints.rs | 110 ------------------ .../src/client/interceptors/error.rs | 28 +++++ .../aws-smithy-runtime/external-types.toml | 2 +- .../src/client/orchestrator.rs | 3 +- .../src/client/orchestrator/endpoints.rs | 106 ++++++++++++++++- 9 files changed, 143 insertions(+), 124 deletions(-) delete mode 100644 rust-runtime/aws-smithy-runtime-api/src/client/endpoints.rs diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs index 2250e7927e0..ff6ed15ef13 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs @@ -17,7 +17,7 @@ use aws_sdk_s3::primitives::SdkBody; use aws_smithy_client::erase::DynConnector; use aws_smithy_client::test_connection::TestConnection; use aws_smithy_runtime::client::connections::adapter::DynConnectorAdapter; -use aws_smithy_runtime_api::client::endpoints::DefaultEndpointResolver; +use aws_smithy_runtime::client::orchestrator::endpoints::DefaultEndpointResolver; use aws_smithy_runtime_api::client::interceptors::{ Interceptor, InterceptorContext, InterceptorError, Interceptors, }; diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt index 8343bbc9bdd..0299eb21c76 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/generators/EndpointParamsInterceptorGenerator.kt @@ -32,6 +32,7 @@ class EndpointParamsInterceptorGenerator( arrayOf( "BoxError" to runtimeApi.resolve("client::runtime_plugin::BoxError"), "ConfigBag" to runtimeApi.resolve("config_bag::ConfigBag"), + "ContextAttachedError" to interceptors.resolve("error::ContextAttachedError"), "EndpointResolverParams" to orchestrator.resolve("EndpointResolverParams"), "HttpResponse" to orchestrator.resolve("HttpResponse"), "HttpRequest" to orchestrator.resolve("HttpRequest"), @@ -83,10 +84,10 @@ class EndpointParamsInterceptorGenerator( let input = context.input()?; let _input = input .downcast_ref::<${operationInput.name}>() - .ok_or_else(|| #{InterceptorError}::invalid_input_access())?; + .ok_or_else(|| "failed to downcast to ${operationInput.name}")?; let params_builder = cfg .get::<#{ParamsBuilder}>() - .ok_or(#{InterceptorError}::read_before_execution("missing endpoint params builder"))? + .ok_or_else(|| "missing endpoint params builder")? .clone(); ${"" /* TODO(EndpointResolver): Call setters on `params_builder` to update its fields by using values from `_input` */} cfg.put(params_builder); @@ -111,7 +112,7 @@ class EndpointParamsInterceptorGenerator( ) withBlockTemplate( "let endpoint_prefix = ", - ".map_err(#{InterceptorError}::read_before_execution)?;", + """.map_err(|err| #{ContextAttachedError}::new("endpoint prefix could not be built", err))?;""", *codegenScope, ) { endpointTraitBindings.render( @@ -130,11 +131,11 @@ class EndpointParamsInterceptorGenerator( let _ = context; let params_builder = cfg .get::<#{ParamsBuilder}>() - .ok_or(#{InterceptorError}::read_before_execution("missing endpoint params builder"))? + .ok_or_else(|| "missing endpoint params builder")? .clone(); let params = params_builder .build() - .map_err(#{InterceptorError}::read_before_execution)?; + .map_err(|err| #{ContextAttachedError}::new("endpoint params could not be built", err))?; cfg.put( #{EndpointResolverParams}::new(params) ); diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt index acb93bbfa73..8b70a847859 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/ServiceRuntimePluginGenerator.kt @@ -86,7 +86,7 @@ class ServiceRuntimePluginGenerator( "ConfigBagAccessors" to runtimeApi.resolve("client::orchestrator::ConfigBagAccessors"), "Connection" to runtimeApi.resolve("client::orchestrator::Connection"), "ConnectorSettings" to RuntimeType.smithyClient(rc).resolve("http_connector::ConnectorSettings"), - "DefaultEndpointResolver" to runtimeApi.resolve("client::endpoints::DefaultEndpointResolver"), + "DefaultEndpointResolver" to runtime.resolve("client::orchestrator::endpoints::DefaultEndpointResolver"), "DynConnectorAdapter" to runtime.resolve("client::connections::adapter::DynConnectorAdapter"), "HttpAuthSchemes" to runtimeApi.resolve("client::orchestrator::HttpAuthSchemes"), "IdentityResolvers" to runtimeApi.resolve("client::orchestrator::IdentityResolvers"), diff --git a/rust-runtime/aws-smithy-runtime-api/src/client.rs b/rust-runtime/aws-smithy-runtime-api/src/client.rs index c6724ff251c..192b97a5d4e 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client.rs @@ -21,8 +21,5 @@ pub mod retries; /// Runtime plugin type definitions. pub mod runtime_plugin; -/// Smithy endpoint resolution runtime plugins -pub mod endpoints; - /// Smithy auth runtime plugins pub mod auth; diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/endpoints.rs b/rust-runtime/aws-smithy-runtime-api/src/client/endpoints.rs deleted file mode 100644 index b9b4f4a512b..00000000000 --- a/rust-runtime/aws-smithy-runtime-api/src/client/endpoints.rs +++ /dev/null @@ -1,110 +0,0 @@ -/* - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -use crate::client::orchestrator::{ - BoxError, EndpointResolver, EndpointResolverParams, HttpRequest, -}; -use aws_smithy_http::endpoint::error::ResolveEndpointError; -use aws_smithy_http::endpoint::{ - apply_endpoint, EndpointPrefix, ResolveEndpoint, SharedEndpointResolver, -}; -use http::header::HeaderName; -use http::{HeaderValue, Uri}; -use std::fmt::Debug; -use std::str::FromStr; - -#[derive(Debug, Clone)] -pub struct StaticUriEndpointResolver { - endpoint: Uri, -} - -impl StaticUriEndpointResolver { - pub fn http_localhost(port: u16) -> Self { - Self { - endpoint: Uri::from_str(&format!("http://localhost:{port}")) - .expect("all u16 values are valid ports"), - } - } - - pub fn uri(endpoint: Uri) -> Self { - Self { endpoint } - } -} - -impl EndpointResolver for StaticUriEndpointResolver { - fn resolve_and_apply_endpoint( - &self, - _params: &EndpointResolverParams, - _endpoint_prefix: Option<&EndpointPrefix>, - request: &mut HttpRequest, - ) -> Result<(), BoxError> { - apply_endpoint(request.uri_mut(), &self.endpoint, None)?; - Ok(()) - } -} - -#[derive(Debug, Clone)] -pub struct DefaultEndpointResolver { - inner: SharedEndpointResolver, -} - -impl DefaultEndpointResolver { - pub fn new(resolve_endpoint: SharedEndpointResolver) -> Self { - Self { - inner: resolve_endpoint, - } - } -} - -impl EndpointResolver for DefaultEndpointResolver -where - Params: Debug + Send + Sync + 'static, -{ - fn resolve_and_apply_endpoint( - &self, - params: &EndpointResolverParams, - endpoint_prefix: Option<&EndpointPrefix>, - request: &mut HttpRequest, - ) -> Result<(), BoxError> { - let endpoint = match params.get::() { - Some(params) => self.inner.resolve_endpoint(params)?, - None => { - return Err(Box::new(ResolveEndpointError::message( - "params of expected type was not present", - ))); - } - }; - - let uri: Uri = endpoint.url().parse().map_err(|err| { - ResolveEndpointError::from_source("endpoint did not have a valid uri", err) - })?; - - apply_endpoint(request.uri_mut(), &uri, endpoint_prefix).map_err(|err| { - ResolveEndpointError::message(format!( - "failed to apply endpoint `{:?}` to request `{:?}`", - uri, request, - )) - .with_source(Some(err.into())) - })?; - - for (header_name, header_values) in endpoint.headers() { - request.headers_mut().remove(header_name); - for value in header_values { - request.headers_mut().insert( - HeaderName::from_str(header_name).map_err(|err| { - ResolveEndpointError::message("invalid header name") - .with_source(Some(err.into())) - })?, - HeaderValue::from_str(value).map_err(|err| { - ResolveEndpointError::message("invalid header value") - .with_source(Some(err.into())) - })?, - ); - } - } - - Ok(()) - } -} diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/error.rs b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/error.rs index e25b797aebb..8d8b8289715 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/error.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/interceptors/error.rs @@ -169,3 +169,31 @@ impl std::error::Error for InterceptorError { self.source.as_ref().map(|err| err.as_ref() as _) } } + +/// A convenience error that allows for adding additional `context` to `source` +#[derive(Debug)] +pub struct ContextAttachedError { + context: String, + source: Option, +} + +impl ContextAttachedError { + pub fn new(context: impl Into, source: impl Into) -> Self { + Self { + context: context.into(), + source: Some(source.into()), + } + } +} + +impl fmt::Display for ContextAttachedError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.context) + } +} + +impl std::error::Error for ContextAttachedError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.source.as_ref().map(|err| err.as_ref() as _) + } +} diff --git a/rust-runtime/aws-smithy-runtime/external-types.toml b/rust-runtime/aws-smithy-runtime/external-types.toml index 5ecb3a376bb..a8a47f10d99 100644 --- a/rust-runtime/aws-smithy-runtime/external-types.toml +++ b/rust-runtime/aws-smithy-runtime/external-types.toml @@ -6,5 +6,5 @@ allowed_external_types = [ "http::header::name::HeaderName", "http::request::Request", "http::response::Response", - "uri::Uri", + "http::uri::Uri", ] diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs index 217a0eaae7b..dff37ec7b0b 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator.rs @@ -18,7 +18,8 @@ use aws_smithy_runtime_api::config_bag::ConfigBag; use tracing::{debug_span, Instrument}; mod auth; -mod endpoints; +/// Defines types that implement a trait for endpoint resolution +pub mod endpoints; mod http; pub(self) mod phase; diff --git a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs index 5da7bfe5173..981ef3239cc 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/orchestrator/endpoints.rs @@ -3,12 +3,114 @@ * SPDX-License-Identifier: Apache-2.0 */ -use aws_smithy_http::endpoint::EndpointPrefix; +use aws_smithy_http::endpoint::error::ResolveEndpointError; +use aws_smithy_http::endpoint::{ + apply_endpoint, EndpointPrefix, ResolveEndpoint, SharedEndpointResolver, +}; use aws_smithy_runtime_api::client::interceptors::InterceptorContext; use aws_smithy_runtime_api::client::orchestrator::{ - BoxError, ConfigBagAccessors, HttpRequest, HttpResponse, + BoxError, ConfigBagAccessors, EndpointResolver, EndpointResolverParams, HttpRequest, + HttpResponse, }; use aws_smithy_runtime_api::config_bag::ConfigBag; +use http::header::HeaderName; +use http::{HeaderValue, Uri}; +use std::fmt::Debug; +use std::str::FromStr; + +#[derive(Debug, Clone)] +pub struct StaticUriEndpointResolver { + endpoint: Uri, +} + +impl StaticUriEndpointResolver { + pub fn http_localhost(port: u16) -> Self { + Self { + endpoint: Uri::from_str(&format!("http://localhost:{port}")) + .expect("all u16 values are valid ports"), + } + } + + pub fn uri(endpoint: Uri) -> Self { + Self { endpoint } + } +} + +impl EndpointResolver for StaticUriEndpointResolver { + fn resolve_and_apply_endpoint( + &self, + _params: &EndpointResolverParams, + _endpoint_prefix: Option<&EndpointPrefix>, + request: &mut HttpRequest, + ) -> Result<(), BoxError> { + apply_endpoint(request.uri_mut(), &self.endpoint, None)?; + Ok(()) + } +} + +#[derive(Debug, Clone)] +pub struct DefaultEndpointResolver { + inner: SharedEndpointResolver, +} + +impl DefaultEndpointResolver { + pub fn new(resolve_endpoint: SharedEndpointResolver) -> Self { + Self { + inner: resolve_endpoint, + } + } +} + +impl EndpointResolver for DefaultEndpointResolver +where + Params: Debug + Send + Sync + 'static, +{ + fn resolve_and_apply_endpoint( + &self, + params: &EndpointResolverParams, + endpoint_prefix: Option<&EndpointPrefix>, + request: &mut HttpRequest, + ) -> Result<(), BoxError> { + let endpoint = match params.get::() { + Some(params) => self.inner.resolve_endpoint(params)?, + None => { + return Err(Box::new(ResolveEndpointError::message( + "params of expected type was not present", + ))); + } + }; + + let uri: Uri = endpoint.url().parse().map_err(|err| { + ResolveEndpointError::from_source("endpoint did not have a valid uri", err) + })?; + + apply_endpoint(request.uri_mut(), &uri, endpoint_prefix).map_err(|err| { + ResolveEndpointError::message(format!( + "failed to apply endpoint `{:?}` to request `{:?}`", + uri, request, + )) + .with_source(Some(err.into())) + })?; + + for (header_name, header_values) in endpoint.headers() { + request.headers_mut().remove(header_name); + for value in header_values { + request.headers_mut().insert( + HeaderName::from_str(header_name).map_err(|err| { + ResolveEndpointError::message("invalid header name") + .with_source(Some(err.into())) + })?, + HeaderValue::from_str(value).map_err(|err| { + ResolveEndpointError::message("invalid header value") + .with_source(Some(err.into())) + })?, + ); + } + } + + Ok(()) + } +} pub(super) fn orchestrate_endpoint( ctx: &mut InterceptorContext, From 1a65a44e533896d9a950dc482a213fd6d8f8de97 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 21 Apr 2023 10:26:27 -0700 Subject: [PATCH 4/9] Upgrade MSRV to Rust 1.67.1 (#2611) _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .github/workflows/ci-pr.yml | 3 +++ .github/workflows/ci.yml | 2 +- .github/workflows/claim-crate-names.yml | 2 +- .github/workflows/pull-request-bot.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/update-sdk-next.yml | 2 +- CHANGELOG.next.toml | 12 ++++++++++++ .../aws-config/src/profile/parser/normalize.rs | 2 +- .../integration-tests/dynamodb/tests/endpoints.rs | 1 - .../amazon/smithy/rust/codegen/core/testutil/Rust.kt | 2 +- gradle.properties | 2 +- rust-runtime/aws-smithy-client/src/lib.rs | 2 +- rust-runtime/inlineable/src/endpoint_lib/arn.rs | 2 +- .../inlineable/src/endpoint_lib/parse_url.rs | 2 +- .../inlineable/src/endpoint_lib/partition.rs | 2 +- .../inlineable/src/endpoint_lib/substring.rs | 4 ++-- .../inlineable/src/endpoint_lib/uri_encode.rs | 4 ++-- rust-toolchain.toml | 2 +- tools/ci-build/Dockerfile | 2 +- tools/ci-build/smithy-rs-tool-common/src/git.rs | 4 ++-- 20 files changed, 35 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci-pr.yml b/.github/workflows/ci-pr.yml index 5b53c4dbf18..22f830e8bc8 100644 --- a/.github/workflows/ci-pr.yml +++ b/.github/workflows/ci-pr.yml @@ -111,6 +111,9 @@ jobs: semver-checks: name: check the semver status of this PR runs-on: smithy_ubuntu-latest_8-core + needs: + - save-docker-login-token + - acquire-base-image steps: - uses: actions/checkout@v3 with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c36d5ba08ca..54ccda52ff9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,7 @@ on: required: false env: - rust_version: 1.66.1 + rust_version: 1.67.1 rust_toolchain_components: clippy,rustfmt ENCRYPTED_DOCKER_PASSWORD: ${{ secrets.ENCRYPTED_DOCKER_PASSWORD }} DOCKER_LOGIN_TOKEN_PASSPHRASE: ${{ secrets.DOCKER_LOGIN_TOKEN_PASSPHRASE }} diff --git a/.github/workflows/claim-crate-names.yml b/.github/workflows/claim-crate-names.yml index ef24446bb74..a7019823c89 100644 --- a/.github/workflows/claim-crate-names.yml +++ b/.github/workflows/claim-crate-names.yml @@ -10,7 +10,7 @@ concurrency: cancel-in-progress: true env: - rust_version: 1.66.1 + rust_version: 1.67.1 name: Claim unpublished crate names on crates.io run-name: ${{ github.workflow }} diff --git a/.github/workflows/pull-request-bot.yml b/.github/workflows/pull-request-bot.yml index 221f520f1cb..aafeaf773c4 100644 --- a/.github/workflows/pull-request-bot.yml +++ b/.github/workflows/pull-request-bot.yml @@ -28,7 +28,7 @@ concurrency: env: java_version: 11 - rust_version: 1.66.1 + rust_version: 1.67.1 rust_toolchain_components: clippy,rustfmt apt_dependencies: libssl-dev gnuplot jq diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 52ddfdfe7a4..db3af0cb9c3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,7 +10,7 @@ concurrency: cancel-in-progress: true env: - rust_version: 1.66.1 + rust_version: 1.67.1 name: Release smithy-rs run-name: ${{ github.workflow }} ${{ inputs.semantic_version }} (${{ inputs.commit_sha }}) - ${{ inputs.dry_run && 'Dry run' || 'Production run' }} diff --git a/.github/workflows/update-sdk-next.yml b/.github/workflows/update-sdk-next.yml index c96b09b73f2..542abdde006 100644 --- a/.github/workflows/update-sdk-next.yml +++ b/.github/workflows/update-sdk-next.yml @@ -32,7 +32,7 @@ jobs: - name: Set up Rust uses: dtolnay/rust-toolchain@master with: - toolchain: 1.66.1 + toolchain: 1.67.1 - name: Delete old SDK run: | - name: Generate a fresh SDK diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index b9ca3ff1be1..c52faf5882c 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -144,3 +144,15 @@ message = "Add a sensitive method to `ParseHttpResponse`. When this returns true author = "rcoh" references = ["smithy-rs#2603"] meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" } + +[[aws-sdk-rust]] +message = "Update MSRV to Rust 1.67.1" +references = ["smithy-rs#2611"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "jdisanti" + +[[smithy-rs]] +message = "Update MSRV to Rust 1.67.1" +references = ["smithy-rs#2611"] +meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "all"} +author = "jdisanti" diff --git a/aws/rust-runtime/aws-config/src/profile/parser/normalize.rs b/aws/rust-runtime/aws-config/src/profile/parser/normalize.rs index 36b469b7291..5b3b5c1ea8c 100644 --- a/aws/rust-runtime/aws-config/src/profile/parser/normalize.rs +++ b/aws/rust-runtime/aws-config/src/profile/parser/normalize.rs @@ -113,7 +113,7 @@ pub(super) fn merge_in( } } -fn merge_into_base<'a>(target: &mut Profile, profile: HashMap<&str, Cow<'a, str>>) { +fn merge_into_base(target: &mut Profile, profile: HashMap<&str, Cow<'_, str>>) { for (k, v) in profile { match validate_identifier(k) { Ok(k) => { diff --git a/aws/sdk/integration-tests/dynamodb/tests/endpoints.rs b/aws/sdk/integration-tests/dynamodb/tests/endpoints.rs index 6a01fedefd2..25d65bb94ca 100644 --- a/aws/sdk/integration-tests/dynamodb/tests/endpoints.rs +++ b/aws/sdk/integration-tests/dynamodb/tests/endpoints.rs @@ -7,7 +7,6 @@ use aws_sdk_dynamodb::config::{self, Credentials, Region}; use aws_types::SdkConfig; use http::Uri; -#[track_caller] async fn expect_uri( conf: SdkConfig, uri: &'static str, diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt index 5f9e612ee76..4e0b0f67f98 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/testutil/Rust.kt @@ -109,7 +109,7 @@ object TestWorkspace { // help rust select the right version when we run cargo test // TODO(https://github.com/awslabs/smithy-rs/issues/2048): load this from the msrv property using a // method as we do for runtime crate versions - "[toolchain]\nchannel = \"1.66.1\"\n", + "[toolchain]\nchannel = \"1.67.1\"\n", ) // ensure there at least an empty lib.rs file to avoid broken crates newProject.resolve("src").mkdirs() diff --git a/gradle.properties b/gradle.properties index 8dbf0ddef9b..16f15110234 100644 --- a/gradle.properties +++ b/gradle.properties @@ -4,7 +4,7 @@ # # Rust MSRV (entered into the generated README) -rust.msrv=1.66.1 +rust.msrv=1.67.1 # To enable debug, swap out the two lines below. # When changing this value, be sure to run `./gradlew --stop` to kill the Gradle daemon. diff --git a/rust-runtime/aws-smithy-client/src/lib.rs b/rust-runtime/aws-smithy-client/src/lib.rs index 080e5ac9ebd..35065c84b4c 100644 --- a/rust-runtime/aws-smithy-client/src/lib.rs +++ b/rust-runtime/aws-smithy-client/src/lib.rs @@ -259,7 +259,7 @@ where > + Clone, { let _ = |o: static_tests::ValidTestOperation| { - let _ = self.call_raw(o); + drop(self.call_raw(o)); }; } } diff --git a/rust-runtime/inlineable/src/endpoint_lib/arn.rs b/rust-runtime/inlineable/src/endpoint_lib/arn.rs index a9b599e6323..0fd68d73bcf 100644 --- a/rust-runtime/inlineable/src/endpoint_lib/arn.rs +++ b/rust-runtime/inlineable/src/endpoint_lib/arn.rs @@ -89,7 +89,7 @@ impl<'a> Arn<'a> { } } -pub(crate) fn parse_arn<'a, 'b>(input: &'a str, e: &'b mut DiagnosticCollector) -> Option> { +pub(crate) fn parse_arn<'a>(input: &'a str, e: &mut DiagnosticCollector) -> Option> { e.capture(Arn::parse(input)) } diff --git a/rust-runtime/inlineable/src/endpoint_lib/parse_url.rs b/rust-runtime/inlineable/src/endpoint_lib/parse_url.rs index 5e437c9bfe3..03e07f3e417 100644 --- a/rust-runtime/inlineable/src/endpoint_lib/parse_url.rs +++ b/rust-runtime/inlineable/src/endpoint_lib/parse_url.rs @@ -45,7 +45,7 @@ impl<'a> Url<'a> { } } -pub(crate) fn parse_url<'a, 'b>(url: &'a str, e: &'b mut DiagnosticCollector) -> Option> { +pub(crate) fn parse_url<'a>(url: &'a str, e: &mut DiagnosticCollector) -> Option> { let raw = url; let uri: Uri = e.capture(url.parse())?; let url: ParsedUrl = e.capture(url.parse())?; diff --git a/rust-runtime/inlineable/src/endpoint_lib/partition.rs b/rust-runtime/inlineable/src/endpoint_lib/partition.rs index b9b4002ed11..02088d0f93b 100644 --- a/rust-runtime/inlineable/src/endpoint_lib/partition.rs +++ b/rust-runtime/inlineable/src/endpoint_lib/partition.rs @@ -458,7 +458,7 @@ mod test { use regex::Regex; use std::collections::HashMap; - fn resolve<'a, 'b>(resolver: &'a PartitionResolver, region: &'b str) -> Partition<'a> { + fn resolve<'a>(resolver: &'a PartitionResolver, region: &str) -> Partition<'a> { resolver .resolve_partition(region, &mut DiagnosticCollector::new()) .expect("could not resolve partition") diff --git a/rust-runtime/inlineable/src/endpoint_lib/substring.rs b/rust-runtime/inlineable/src/endpoint_lib/substring.rs index 8512810c4e0..27142e118ed 100644 --- a/rust-runtime/inlineable/src/endpoint_lib/substring.rs +++ b/rust-runtime/inlineable/src/endpoint_lib/substring.rs @@ -13,12 +13,12 @@ use crate::endpoint_lib::diagnostic::DiagnosticCollector; /// - When `reverse` is false, indexes are evaluated from the beginning of the string /// - When `reverse` is true, indexes are evaluated from the end of the string (however, the result /// will still be "forwards" and `start` MUST be less than `end`. -pub(crate) fn substring<'a, 'b>( +pub(crate) fn substring<'a>( input: &'a str, start: usize, stop: usize, reverse: bool, - e: &'b mut DiagnosticCollector, + e: &mut DiagnosticCollector, ) -> Option<&'a str> { if start >= stop { e.capture(Err("start > stop"))?; diff --git a/rust-runtime/inlineable/src/endpoint_lib/uri_encode.rs b/rust-runtime/inlineable/src/endpoint_lib/uri_encode.rs index 58c0ef5742e..de33264a9f4 100644 --- a/rust-runtime/inlineable/src/endpoint_lib/uri_encode.rs +++ b/rust-runtime/inlineable/src/endpoint_lib/uri_encode.rs @@ -39,9 +39,9 @@ pub(crate) const BASE_SET: &AsciiSet = &CONTROLS .add(b'\\'); // Returns `Option` for forwards compatibility -pub(crate) fn uri_encode<'a, 'b>( +pub(crate) fn uri_encode<'a>( s: &'a str, - _e: &'b mut DiagnosticCollector, + _e: &mut DiagnosticCollector, ) -> std::borrow::Cow<'a, str> { utf8_percent_encode(s, BASE_SET).into() } diff --git a/rust-toolchain.toml b/rust-toolchain.toml index f701aa5354e..588ffd57885 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,2 +1,2 @@ [toolchain] -channel = "1.66.1" +channel = "1.67.1" diff --git a/tools/ci-build/Dockerfile b/tools/ci-build/Dockerfile index 0e64f746a9f..2bce2b0ec9f 100644 --- a/tools/ci-build/Dockerfile +++ b/tools/ci-build/Dockerfile @@ -6,7 +6,7 @@ # This is the base Docker build image used by CI ARG base_image=public.ecr.aws/amazonlinux/amazonlinux:2 -ARG rust_stable_version=1.66.1 +ARG rust_stable_version=1.67.1 ARG rust_nightly_version=nightly-2022-11-16 FROM ${base_image} AS bare_base_image diff --git a/tools/ci-build/smithy-rs-tool-common/src/git.rs b/tools/ci-build/smithy-rs-tool-common/src/git.rs index b1b34aa263b..c370529c913 100644 --- a/tools/ci-build/smithy-rs-tool-common/src/git.rs +++ b/tools/ci-build/smithy-rs-tool-common/src/git.rs @@ -104,11 +104,11 @@ pub trait Git: Send + Sync { /// Returns a list of commit hashes in reverse chronological order starting with /// `start_inclusive_revision` and ending before `end_exclusive_revision`. Both of /// these arguments can be any kind of Git revision (e.g., HEAD, HEAD~2, commit hash, etc). - fn rev_list<'a>( + fn rev_list( &self, start_inclusive_revision: &str, end_exclusive_revision: &str, - path: Option<&'a Path>, + path: Option<&Path>, ) -> Result>; /// Returns information about a given revision. From 44ece8e0f92e9d72c47f14d28fe519f0f7344e08 Mon Sep 17 00:00:00 2001 From: Harry Barber <106155934+hlbarber@users.noreply.github.com> Date: Fri, 21 Apr 2023 18:37:39 +0100 Subject: [PATCH 5/9] Prevent build stalling with overlapping projections (#2602) ## Motivation and Context Smithy CLI runs projections concurrently, for some currently unknown reason this causes the build to stall. ## Description - Switch `defaultRustMetadata` in `BaseSymbolMetadataProvider` from a `val` to a `fun`. - Remove unused `RustType::TestModule` companion object. ## Notes A deadlock occurs when the client/server plugins both attempt to grab file locks. While this fixes this deadlock it is unknown whether there still exist conditions where it can happen. ## Testing 1. Checkout https://github.com/crisidev/smithy-rs-pokemon-service/commit/d276bb95bbd7bf5c5f029953b8f16cecfa8af24f, run `./gradlew assemble`, and observe the build halting. 2. Run the following commands to switch to this branch: ```bash git submodule update --init --recursive --remote cd smithy-rs git checkout harryb/remove-build-deadlock cd .. ./gradlew assemble ``` 3. Observe the build succeeding. --- .../smithy/rust/codegen/core/rustlang/RustType.kt | 9 --------- .../codegen/core/smithy/SymbolMetadataProvider.kt | 12 ++++++------ 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt index 56f0b63c52b..79dde7de12c 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustType.kt @@ -422,15 +422,6 @@ data class RustMetadata( fun hasDebugDerive(): Boolean { return derives.contains(RuntimeType.Debug) } - - companion object { - val TestModule = RustMetadata( - visibility = Visibility.PRIVATE, - additionalAttributes = listOf( - Attribute.CfgTest, - ), - ) - } } data class Argument(val argument: String, val value: String, val type: String) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt index 880ac0510ad..d78818601cd 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/SymbolMetadataProvider.kt @@ -137,13 +137,13 @@ class BaseSymbolMetadataProvider( // Only the server subproject uses these, so we provide a sane and conservative default implementation here so that // the rest of symbol metadata providers can just delegate to it. - private val defaultRustMetadata = RustMetadata(visibility = Visibility.PRIVATE) + private fun defaultRustMetadata() = RustMetadata(visibility = Visibility.PRIVATE) - override fun listMeta(listShape: ListShape) = defaultRustMetadata - override fun mapMeta(mapShape: MapShape) = defaultRustMetadata - override fun stringMeta(stringShape: StringShape) = defaultRustMetadata - override fun numberMeta(numberShape: NumberShape) = defaultRustMetadata - override fun blobMeta(blobShape: BlobShape) = defaultRustMetadata + override fun listMeta(listShape: ListShape) = defaultRustMetadata() + override fun mapMeta(mapShape: MapShape) = defaultRustMetadata() + override fun stringMeta(stringShape: StringShape) = defaultRustMetadata() + override fun numberMeta(numberShape: NumberShape) = defaultRustMetadata() + override fun blobMeta(blobShape: BlobShape) = defaultRustMetadata() } private const val META_KEY = "meta" From 982319ff5fc9947937dd6d41ca0532abe5098fab Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 21 Apr 2023 12:44:49 -0500 Subject: [PATCH 6/9] Switch to rust-toolchain@master in "Update GitHub Pages" (#2613) ## Motivation and Context The previous attempt https://github.com/awslabs/smithy-rs/pull/2610 did not work for properly running the workflow for `Update GitHub Pages`. This PR reverts it and uses a different fix to address the issue. ## Description Possibly with [this](https://users.rust-lang.org/t/sparse-registry-breaking-my-ci-and-i-dont-understand-why/89976) and `dtolnaly/rust-toolchain` having fixed a couple of issues [related to the sparse registry](https://github.com/dtolnay/rust-toolchain/issues?q=is%3Aissue+is%3Aclosed+sparse) recently, this PR will use `dtolnaly/rust-toolchain@master` rather than `stable` (most CIs in this repository have been using `master`). ## Testing The workflow in question has been verified to run successfully with this change ([link](https://github.com/awslabs/smithy-rs/actions/runs/4758820036)). ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito Co-authored-by: John DiSanti --- .github/workflows/github-pages.yml | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/.github/workflows/github-pages.yml b/.github/workflows/github-pages.yml index 809955c7a07..9e0f52d9b66 100644 --- a/.github/workflows/github-pages.yml +++ b/.github/workflows/github-pages.yml @@ -1,11 +1,5 @@ on: workflow_dispatch: - inputs: - opt-in-to-sparse-registry: - description: Enable/disable CARGO_UNSTABLE_SPARSE_REGISTRY (https://blog.rust-lang.org/2022/06/22/sparse-registry-testing.html) - required: true - type: boolean - default: false push: branches: [main] paths: @@ -13,6 +7,9 @@ on: name: Update GitHub Pages +env: + rust_version: 1.67.1 + # Allow only one doc pages build to run at a time for the entire smithy-rs repo concurrency: group: github-pages-yml @@ -26,11 +23,12 @@ jobs: uses: actions/checkout@v3 with: persist-credentials: false - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@master + with: + toolchain: ${{ env.rust_version }} - name: Generate docs env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - CARGO_UNSTABLE_SPARSE_REGISTRY: ${{ inputs.opt-in-to-sparse-registry }} run: | git config --local user.name "AWS SDK Rust Bot" git config --local user.email "aws-sdk-rust-primary@amazon.com" From 07dd8d467fa51f9260b02cf271e10d30894008c0 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Fri, 21 Apr 2023 15:25:18 -0500 Subject: [PATCH 7/9] Create CredentialsCache and DefaultResolver only once in test/bench (#2620) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation and Context Addresses https://github.com/awslabs/smithy-rs/pull/2593#discussion_r1170703265 and https://github.com/awslabs/smithy-rs/pull/2593#discussion_r1170703694. Also ports the changes made in #2592. ## Description Prior to this PR, `sra_manual_test` and the bench `middleware_vs_orchestrator` created `CredentialsCache` and `aws_sdk_s3::endpoint::DefaultResolver` every time a request for `ListObjectsV2` was dispatched. This is not the case in production where those two types are created once during the construction of a service config ([here](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/s3/src/config.rs#L623-L625) and [here](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/s3/src/config.rs#L635-L652)) and reused for subsequent request dispatches. The PR will make `sra_manual_test` and `middleware_vs_orchestrator` do the same, creating `CredentialsCache` and `aws_sdk_s3::endpoint::DefaultResolver` only once before the test/benchmark starts running and reusing them thereafter. The change will help bring the performance for `orchestrator` closer to that for `middleware`. - In the `main` branch with `DefaultEndpointResolver` commented in and `StaticUriEndpointResolver` commented out: ``` middleware time: [20.924 µs 20.943 µs 20.964 µs] change: [-1.0107% -0.7357% -0.4827%] (p = 0.00 < 0.05) Change within noise threshold. Found 7 outliers among 100 measurements (7.00%) 1 (1.00%) high mild 6 (6.00%) high severe orchestrator time: [933.68 µs 940.11 µs 945.82 µs] change: [+2735.7% +2754.5% +2770.9%] (p = 0.00 < 0.05) Performance has regressed. Found 17 outliers among 100 measurements (17.00%) 14 (14.00%) low mild 2 (2.00%) high mild 1 (1.00%) high severe ``` - With the change in this PR: ``` middleware time: [21.161 µs 21.194 µs 21.232 µs] change: [-0.8973% -0.6397% -0.3758%] (p = 0.00 < 0.05) Change within noise threshold. Found 8 outliers among 100 measurements (8.00%) 5 (5.00%) high mild 3 (3.00%) high severe orchestrator time: [56.038 µs 56.182 µs 56.349 µs] change: [-0.7921% -0.5552% -0.3157%] (p = 0.00 < 0.05) Change within noise threshold. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) high mild 3 (3.00%) high severe ``` ## Testing Executed the following without any errors: ``` ➜ smithy-rs git:(ysaito/create-creds-cache-and-ep-resolver-only-once) ✗ ./gradlew aws:sra-test:assemble ➜ aws-sdk-s3 git:(ysaito/create-creds-cache-and-ep-resolver-only-once) cargo t ➜ aws-sdk-s3 git:(ysaito/create-creds-cache-and-ep-resolver-only-once) cargo bench ``` ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Yuki Saito --- .../benches/middleware_vs_orchestrator.rs | 73 +++++++++++-------- .../aws-sdk-s3/tests/sra_test.rs | 61 +++++++++------- 2 files changed, 77 insertions(+), 57 deletions(-) diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs index 634553466aa..417b99c77b3 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/benches/middleware_vs_orchestrator.rs @@ -5,9 +5,13 @@ #[macro_use] extern crate criterion; +use aws_credential_types::cache::{CredentialsCache, SharedCredentialsCache}; +use aws_credential_types::provider::SharedCredentialsProvider; +use aws_credential_types::Credentials; use aws_sdk_s3 as s3; use aws_smithy_client::erase::DynConnector; use aws_smithy_client::test_connection::infallible_connection_fn; +use aws_smithy_http::endpoint::SharedEndpointResolver; use aws_smithy_runtime_api::type_erasure::TypedBox; use criterion::Criterion; use s3::operation::list_objects_v2::{ListObjectsV2Error, ListObjectsV2Input, ListObjectsV2Output}; @@ -22,10 +26,20 @@ async fn middleware(client: &s3::Client) { .expect("successful execution"); } -async fn orchestrator(connector: &DynConnector) { +async fn orchestrator( + connector: &DynConnector, + endpoint_resolver: SharedEndpointResolver, + credentials_cache: SharedCredentialsCache, +) { + let service_runtime_plugin = orchestrator::ManualServiceRuntimePlugin { + connector: connector.clone(), + endpoint_resolver: endpoint_resolver.clone(), + credentials_cache: credentials_cache.clone(), + }; + // TODO(enableNewSmithyRuntime): benchmark with `send_v2` directly once it works let runtime_plugins = aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugins::new() - .with_client_plugin(orchestrator::ManualServiceRuntimePlugin(connector.clone())) + .with_client_plugin(service_runtime_plugin) .with_operation_plugin(aws_sdk_s3::operation::list_objects_v2::ListObjectsV2::new()) .with_operation_plugin(orchestrator::ManualOperationRuntimePlugin); let input = ListObjectsV2Input::builder() @@ -95,25 +109,32 @@ fn middleware_bench(c: &mut Criterion) { fn orchestrator_bench(c: &mut Criterion) { let conn = test_connection(); + let endpoint_resolver = SharedEndpointResolver::new(s3::endpoint::DefaultResolver::new()); + let credentials_cache = SharedCredentialsCache::new( + CredentialsCache::lazy() + .create_cache(SharedCredentialsProvider::new(Credentials::for_tests())), + ); c.bench_function("orchestrator", move |b| { b.to_async(tokio::runtime::Runtime::new().unwrap()) - .iter(|| async { orchestrator(&conn).await }) + .iter(|| async { + orchestrator(&conn, endpoint_resolver.clone(), credentials_cache.clone()).await + }) }); } mod orchestrator { - use aws_credential_types::cache::{CredentialsCache, SharedCredentialsCache}; - use aws_credential_types::provider::SharedCredentialsProvider; - use aws_credential_types::Credentials; + use aws_credential_types::cache::SharedCredentialsCache; use aws_http::user_agent::{ApiMetadata, AwsUserAgent}; use aws_runtime::recursion_detection::RecursionDetectionInterceptor; use aws_runtime::user_agent::UserAgentInterceptor; use aws_sdk_s3::config::Region; + use aws_sdk_s3::endpoint::Params; use aws_sdk_s3::operation::list_objects_v2::ListObjectsV2Input; use aws_smithy_client::erase::DynConnector; + use aws_smithy_http::endpoint::SharedEndpointResolver; use aws_smithy_runtime::client::connections::adapter::DynConnectorAdapter; - use aws_smithy_runtime_api::client::endpoints::StaticUriEndpointResolver; + use aws_smithy_runtime::client::orchestrator::endpoints::DefaultEndpointResolver; use aws_smithy_runtime_api::client::interceptors::{ Interceptor, InterceptorContext, InterceptorError, Interceptors, }; @@ -124,10 +145,13 @@ mod orchestrator { use aws_smithy_runtime_api::config_bag::ConfigBag; use aws_types::region::SigningRegion; use aws_types::SigningService; - use http::Uri; use std::sync::Arc; - pub struct ManualServiceRuntimePlugin(pub DynConnector); + pub struct ManualServiceRuntimePlugin { + pub connector: DynConnector, + pub endpoint_resolver: SharedEndpointResolver, + pub credentials_cache: SharedCredentialsCache, + } impl RuntimePlugin for ManualServiceRuntimePlugin { fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { @@ -136,9 +160,7 @@ mod orchestrator { .identity_resolver( aws_runtime::auth::sigv4::SCHEME_ID, aws_runtime::identity::credentials::CredentialsIdentityResolver::new( - SharedCredentialsCache::new(CredentialsCache::lazy().create_cache( - SharedCredentialsProvider::new(Credentials::for_tests()), - )), + self.credentials_cache.clone(), ), ) .identity_resolver( @@ -163,14 +185,7 @@ mod orchestrator { ), ); - //cfg.set_endpoint_resolver(DefaultEndpointResolver::new( - // aws_smithy_http::endpoint::SharedEndpointResolver::new( - // aws_sdk_s3::endpoint::DefaultResolver::new(), - // ), - //)); - cfg.set_endpoint_resolver(StaticUriEndpointResolver::uri(Uri::from_static( - "https://test-bucket.s3.us-east-1.amazonaws.com/", - ))); + cfg.set_endpoint_resolver(DefaultEndpointResolver::new(self.endpoint_resolver.clone())); let params_builder = aws_sdk_s3::endpoint::Params::builder() .set_region(Some("us-east-1".to_owned())) @@ -182,7 +197,7 @@ mod orchestrator { ); let connection: Box = - Box::new(DynConnectorAdapter::new(self.0.clone())); + Box::new(DynConnectorAdapter::new(self.connector.clone())); cfg.set_connection(connection); cfg.set_trace_probe({ @@ -227,12 +242,10 @@ mod orchestrator { let input = context.input()?; let input = input .downcast_ref::() - .ok_or_else(|| InterceptorError::invalid_input_access())?; + .ok_or_else(|| "failed to downcast to ListObjectsV2Input")?; let mut params_builder = cfg .get::() - .ok_or(InterceptorError::read_before_execution( - "missing endpoint params builder", - ))? + .ok_or_else(|| "missing endpoint params builder")? .clone(); params_builder = params_builder.set_bucket(input.bucket.clone()); cfg.put(params_builder); @@ -251,13 +264,11 @@ mod orchestrator { ) -> Result<(), BoxError> { let params_builder = cfg .get::() - .ok_or(InterceptorError::read_before_execution( - "missing endpoint params builder", - ))? + .ok_or_else(|| "missing endpoint params builder")? .clone(); - let params = params_builder - .build() - .map_err(InterceptorError::read_before_execution)?; + let params = params_builder.build().map_err(|err| { + ContextAttachedError::new("endpoint params could not be built", err) + })?; cfg.put( aws_smithy_runtime_api::client::orchestrator::EndpointResolverParams::new( params, diff --git a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs index ff6ed15ef13..77d576edfe9 100644 --- a/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs +++ b/aws/sra-test/integration-tests/aws-sdk-s3/tests/sra_test.rs @@ -10,17 +10,18 @@ use aws_runtime::auth::sigv4::SigV4OperationSigningConfig; use aws_runtime::recursion_detection::RecursionDetectionInterceptor; use aws_runtime::user_agent::UserAgentInterceptor; use aws_sdk_s3::config::{Credentials, Region}; +use aws_sdk_s3::endpoint::Params; use aws_sdk_s3::operation::list_objects_v2::{ ListObjectsV2Error, ListObjectsV2Input, ListObjectsV2Output, }; use aws_sdk_s3::primitives::SdkBody; use aws_smithy_client::erase::DynConnector; use aws_smithy_client::test_connection::TestConnection; +use aws_smithy_http::endpoint::SharedEndpointResolver; use aws_smithy_runtime::client::connections::adapter::DynConnectorAdapter; use aws_smithy_runtime::client::orchestrator::endpoints::DefaultEndpointResolver; -use aws_smithy_runtime_api::client::interceptors::{ - Interceptor, InterceptorContext, InterceptorError, Interceptors, -}; +use aws_smithy_runtime_api::client::interceptors::error::ContextAttachedError; +use aws_smithy_runtime_api::client::interceptors::{Interceptor, InterceptorContext, Interceptors}; use aws_smithy_runtime_api::client::orchestrator::{ BoxError, ConfigBagAccessors, Connection, HttpRequest, HttpResponse, TraceProbe, }; @@ -74,7 +75,11 @@ async fn sra_test() { async fn sra_manual_test() { tracing_subscriber::fmt::init(); - struct ManualServiceRuntimePlugin(TestConnection<&'static str>); + struct ManualServiceRuntimePlugin { + connector: TestConnection<&'static str>, + endpoint_resolver: SharedEndpointResolver, + credentials_cache: SharedCredentialsCache, + } impl RuntimePlugin for ManualServiceRuntimePlugin { fn configure(&self, cfg: &mut ConfigBag) -> Result<(), BoxError> { @@ -83,9 +88,7 @@ async fn sra_manual_test() { .identity_resolver( aws_runtime::auth::sigv4::SCHEME_ID, aws_runtime::identity::credentials::CredentialsIdentityResolver::new( - SharedCredentialsCache::new(CredentialsCache::lazy().create_cache( - SharedCredentialsProvider::new(Credentials::for_tests()), - )), + self.credentials_cache.clone(), ), ) .identity_resolver( @@ -110,13 +113,9 @@ async fn sra_manual_test() { ), ); - cfg.set_endpoint_resolver(DefaultEndpointResolver::new( - aws_smithy_http::endpoint::SharedEndpointResolver::new( - aws_sdk_s3::endpoint::DefaultResolver::new(), - ), - )); + cfg.set_endpoint_resolver(DefaultEndpointResolver::new(self.endpoint_resolver.clone())); - let params_builder = aws_sdk_s3::endpoint::Params::builder() + let params_builder = Params::builder() .set_region(Some("us-east-1".to_owned())) .set_endpoint(Some("https://s3.us-east-1.amazonaws.com/".to_owned())); cfg.put(params_builder); @@ -125,8 +124,9 @@ async fn sra_manual_test() { aws_smithy_runtime_api::client::retries::NeverRetryStrategy::new(), ); - let connection: Box = - Box::new(DynConnectorAdapter::new(DynConnector::new(self.0.clone()))); + let connection: Box = Box::new(DynConnectorAdapter::new( + DynConnector::new(self.connector.clone()), + )); cfg.set_connection(connection); cfg.set_trace_probe({ @@ -189,12 +189,10 @@ async fn sra_manual_test() { let input = context.input()?; let input = input .downcast_ref::() - .ok_or_else(|| InterceptorError::invalid_input_access())?; + .ok_or_else(|| "failed to downcast to ListObjectsV2Input")?; let mut params_builder = cfg .get::() - .ok_or(InterceptorError::read_before_execution( - "missing endpoint params builder", - ))? + .ok_or_else(|| "missing endpoint params builder")? .clone(); params_builder = params_builder.set_bucket(input.bucket.clone()); cfg.put(params_builder); @@ -213,13 +211,11 @@ async fn sra_manual_test() { ) -> Result<(), BoxError> { let params_builder = cfg .get::() - .ok_or(InterceptorError::read_before_execution( - "missing endpoint params builder", - ))? + .ok_or_else(|| "missing endpoint params builder")? .clone(); - let params = params_builder - .build() - .map_err(InterceptorError::read_before_execution)?; + let params = params_builder.build().map_err(|err| { + ContextAttachedError::new("endpoint params could not be built", err) + })?; cfg.put( aws_smithy_runtime_api::client::orchestrator::EndpointResolverParams::new( params, @@ -265,8 +261,21 @@ async fn sra_manual_test() { "#).unwrap(), )]); + let endpoint_resolver = + SharedEndpointResolver::new(aws_sdk_s3::endpoint::DefaultResolver::new()); + let credentials_cache = SharedCredentialsCache::new( + CredentialsCache::lazy() + .create_cache(SharedCredentialsProvider::new(Credentials::for_tests())), + ); + + let service_runtime_plugin = ManualServiceRuntimePlugin { + connector: conn.clone(), + endpoint_resolver, + credentials_cache, + }; + let runtime_plugins = aws_smithy_runtime_api::client::runtime_plugin::RuntimePlugins::new() - .with_client_plugin(ManualServiceRuntimePlugin(conn.clone())) + .with_client_plugin(service_runtime_plugin) .with_operation_plugin(aws_sdk_s3::operation::list_objects_v2::ListObjectsV2::new()) .with_operation_plugin(ManualOperationRuntimePlugin); From 2de6815c103af5b0b85f2e6e65a7b0d478fe0cb2 Mon Sep 17 00:00:00 2001 From: Julian Antonielli Date: Mon, 24 Apr 2023 11:25:27 +0100 Subject: [PATCH 8/9] Add `AlbHealthCheckLayer` (#2540) ## Motivation and Context Services often need the ability to report health status via health checks (see [ALB Health Checks](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/target-group-health-checks.html)). This PR adds a simple layer that allows configuring your service to respond to these health check requests. ## Description Adds `AlbHealthCheckLayer`, and `AlbHealthCheckService`. ## Testing Added this layer to the `pokemon-service` binary, and added an integration test for it. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- CHANGELOG.next.toml | 27 ++- examples/pokemon-service/Cargo.toml | 3 + examples/pokemon-service/src/main.rs | 15 +- examples/pokemon-service/tests/common/mod.rs | 4 + examples/pokemon-service/tests/simple.rs | 7 + .../src/plugin/alb_health_check.rs | 176 ++++++++++++++++++ .../aws-smithy-http-server/src/plugin/mod.rs | 1 + .../src/plugin/pipeline.rs | 7 + 8 files changed, 231 insertions(+), 9 deletions(-) create mode 100644 rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index c52faf5882c..a03141576ab 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -11,16 +11,31 @@ # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} # author = "rcoh" +[[smithy-rs]] +message = """ +Implement layer for servers to handle [ALB health checks](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/target-group-health-checks.html). +Take a look at `aws_smithy_http_server::plugin::alb_health_check` to learn about it. +""" +references = ["smithy-rs#2540"] +meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "server" } +author = "jjant" + +[[smithy-rs]] +message = "Implement `PluginPipeline::http_layer` which allows you to apply a `tower::Layer` to all operations." +references = ["smithy-rs#2540"] +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server" } +author = "jjant" + [[aws-sdk-rust]] -message = "Implement std::error::Error#source() properly for the service meta Error enum" +message = "Implement std::error::Error#source() properly for the service meta Error enum." references = ["aws-sdk-rust#784"] meta = { "breaking" = false, "tada" = false, "bug" = false } author = "abusch" [[smithy-rs]] -message = "Implement std::error::Error#source() properly for the service meta Error enum" +message = "Implement std::error::Error#source() properly for the service meta Error enum." references = ["aws-sdk-rust#784"] -meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"} +meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" } author = "abusch" [[aws-sdk-rust]] @@ -32,7 +47,7 @@ author = "jdisanti" [[smithy-rs]] message = "The outputs for event stream operations now implement the `Sync` auto-trait." references = ["smithy-rs#2496"] -meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "all"} +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "all" } author = "jdisanti" [[aws-sdk-rust]] @@ -44,7 +59,7 @@ author = "eduardomourar" [[smithy-rs]] message = "Clients now compile for the `wasm32-unknown-unknown` and `wasm32-wasi` targets when no default features are enabled. WebAssembly is not officially supported yet, but this is a great first step towards it!" references = ["smithy-rs#2254"] -meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "client"} +meta = { "breaking" = false, "tada" = true, "bug" = false, "target" = "client" } author = "eduardomourar" [[smithy-rs]] @@ -56,7 +71,7 @@ author = "jdisanti" [[smithy-rs]] message = "Streaming operations now emit the request ID at the `debug` log level like their non-streaming counterparts." references = ["smithy-rs#2495"] -meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client"} +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" } author = "jdisanti" [[smithy-rs]] diff --git a/examples/pokemon-service/Cargo.toml b/examples/pokemon-service/Cargo.toml index 30839509e51..8324ffcc8c2 100644 --- a/examples/pokemon-service/Cargo.toml +++ b/examples/pokemon-service/Cargo.toml @@ -24,6 +24,9 @@ async-stream = "0.3" rand = "0.8.5" serial_test = "1.0.0" +# We use hyper client in tests +hyper = {version = "0.14.25", features = ["server", "client"] } + # This dependency is only required for testing the `pokemon-service-tls` program. hyper-rustls = { version = "0.23.2", features = ["http2"] } diff --git a/examples/pokemon-service/src/main.rs b/examples/pokemon-service/src/main.rs index b62118c816a..7a865abfafd 100644 --- a/examples/pokemon-service/src/main.rs +++ b/examples/pokemon-service/src/main.rs @@ -8,11 +8,15 @@ mod plugin; use std::{net::SocketAddr, sync::Arc}; use aws_smithy_http_server::{ - extension::OperationExtensionExt, instrumentation::InstrumentExt, plugin::PluginPipeline, - request::request_id::ServerRequestIdProviderLayer, AddExtensionLayer, + extension::OperationExtensionExt, + instrumentation::InstrumentExt, + plugin::{alb_health_check::AlbHealthCheckLayer, PluginPipeline}, + request::request_id::ServerRequestIdProviderLayer, + AddExtensionLayer, }; use clap::Parser; +use hyper::StatusCode; use plugin::PrintExt; use pokemon_service::{ @@ -47,7 +51,12 @@ pub async fn main() { // `Response::extensions`, or infer routing failure when it's missing. .insert_operation_extension() // Adds `tracing` spans and events to the request lifecycle. - .instrument(); + .instrument() + // Handle `/ping` health check requests. + .http_layer(AlbHealthCheckLayer::from_handler("/ping", |_req| async { + StatusCode::OK + })); + let app = PokemonService::builder_with_plugins(plugins) // Build a registry containing implementations to all the operations in the service. These // are async functions or async closures that take as input the operation's input and diff --git a/examples/pokemon-service/tests/common/mod.rs b/examples/pokemon-service/tests/common/mod.rs index 0cc34064df8..d10430bfbd7 100644 --- a/examples/pokemon-service/tests/common/mod.rs +++ b/examples/pokemon-service/tests/common/mod.rs @@ -23,6 +23,10 @@ pub async fn run_server() -> ChildDrop { ChildDrop(child) } +pub fn base_url() -> String { + format!("http://{DEFAULT_ADDRESS}:{DEFAULT_PORT}") +} + pub fn client() -> Client> { let authority = Authority::from_str(&format!("{DEFAULT_ADDRESS}:{DEFAULT_PORT}")) .expect("could not parse authority"); diff --git a/examples/pokemon-service/tests/simple.rs b/examples/pokemon-service/tests/simple.rs index 1c86d3cf109..af03bfee30d 100644 --- a/examples/pokemon-service/tests/simple.rs +++ b/examples/pokemon-service/tests/simple.rs @@ -81,4 +81,11 @@ async fn simple_integration_test() { let service_statistics_out = client.get_server_statistics().send().await.unwrap(); assert_eq!(2, service_statistics_out.calls_count.unwrap()); + + let hyper_client = hyper::Client::new(); + let health_check_url = format!("{}/ping", common::base_url()); + let health_check_url = hyper::Uri::try_from(health_check_url).unwrap(); + let result = hyper_client.get(health_check_url).await.unwrap(); + + assert_eq!(result.status(), 200); } diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs b/rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs new file mode 100644 index 00000000000..33932ddd7db --- /dev/null +++ b/rust-runtime/aws-smithy-http-server/src/plugin/alb_health_check.rs @@ -0,0 +1,176 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +//! Middleware for handling [ALB health +//! checks](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/target-group-health-checks.html). +//! +//! # Example +//! +//! ```no_run +//! # use aws_smithy_http_server::{body, plugin::{PluginPipeline, alb_health_check::AlbHealthCheckLayer}}; +//! # use hyper::{Body, Response, StatusCode}; +//! let plugins = PluginPipeline::new() +//! // Handle all `/ping` health check requests by returning a `200 OK`. +//! .http_layer(AlbHealthCheckLayer::from_handler("/ping", |_req| async { +//! StatusCode::OK +//! })); +//! +//! ``` + +use std::borrow::Cow; +use std::convert::Infallible; +use std::task::{Context, Poll}; + +use futures_util::{Future, FutureExt}; +use http::StatusCode; +use hyper::{Body, Request, Response}; +use pin_project_lite::pin_project; +use tower::{service_fn, util::Oneshot, Layer, Service, ServiceExt}; + +use crate::body::BoxBody; + +use super::either::EitherProj; +use super::Either; + +/// A [`tower::Layer`] used to apply [`AlbHealthCheckService`]. +#[derive(Clone, Debug)] +pub struct AlbHealthCheckLayer { + health_check_uri: Cow<'static, str>, + health_check_handler: HealthCheckHandler, +} + +impl AlbHealthCheckLayer<()> { + /// Handle health check requests at `health_check_uri` with the specified handler. + pub fn from_handler, H: Fn(Request) -> HandlerFuture + Clone>( + health_check_uri: impl Into>, + health_check_handler: H, + ) -> AlbHealthCheckLayer< + impl Service< + Request, + Response = StatusCode, + Error = Infallible, + Future = impl Future>, + > + Clone, + > { + let service = service_fn(move |req| health_check_handler(req).map(Ok)); + + AlbHealthCheckLayer::new(health_check_uri, service) + } + + /// Handle health check requests at `health_check_uri` with the specified service. + pub fn new, Response = StatusCode>>( + health_check_uri: impl Into>, + health_check_handler: H, + ) -> AlbHealthCheckLayer { + AlbHealthCheckLayer { + health_check_uri: health_check_uri.into(), + health_check_handler, + } + } +} + +impl Layer for AlbHealthCheckLayer { + type Service = AlbHealthCheckService; + + fn layer(&self, inner: S) -> Self::Service { + AlbHealthCheckService { + inner, + layer: self.clone(), + } + } +} + +/// A middleware [`Service`] responsible for handling health check requests. +#[derive(Clone, Debug)] +pub struct AlbHealthCheckService { + inner: S, + layer: AlbHealthCheckLayer, +} + +impl Service> for AlbHealthCheckService +where + S: Service, Response = Response> + Clone, + S::Future: std::marker::Send + 'static, + H: Service, Response = StatusCode, Error = Infallible> + Clone, +{ + type Response = S::Response; + + type Error = S::Error; + + type Future = AlbHealthCheckFuture; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + // The check that the service is ready is done by `Oneshot` below. + Poll::Ready(Ok(())) + } + + fn call(&mut self, req: Request) -> Self::Future { + if req.uri() == self.layer.health_check_uri.as_ref() { + let clone = self.layer.health_check_handler.clone(); + let service = std::mem::replace(&mut self.layer.health_check_handler, clone); + let handler_future = service.oneshot(req); + + AlbHealthCheckFuture::handler_future(handler_future) + } else { + let clone = self.inner.clone(); + let service = std::mem::replace(&mut self.inner, clone); + let service_future = service.oneshot(req); + + AlbHealthCheckFuture::service_future(service_future) + } + } +} + +type HealthCheckFutureInner = Either>, Oneshot>>; + +pin_project! { + /// Future for [`AlbHealthCheckService`]. + pub struct AlbHealthCheckFuture, Response = StatusCode>, S: Service>> { + #[pin] + inner: HealthCheckFutureInner + } +} + +impl, Response = StatusCode>, S: Service>> AlbHealthCheckFuture { + fn handler_future(handler_future: Oneshot>) -> Self { + Self { + inner: Either::Left { value: handler_future }, + } + } + + fn service_future(service_future: Oneshot>) -> Self { + Self { + inner: Either::Right { value: service_future }, + } + } +} + +impl< + H: Service, Response = StatusCode, Error = Infallible>, + S: Service, Response = Response>, + > Future for AlbHealthCheckFuture +{ + type Output = Result; + + fn poll(self: std::pin::Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let either_proj = self.project().inner.project(); + + match either_proj { + EitherProj::Left { value } => { + let polled: Poll = value.poll(cx).map(|res| { + res.map(|status_code| { + Response::builder() + .status(status_code) + .body(crate::body::empty()) + .unwrap() + }) + .map_err(|never| match never {}) + }); + polled + } + EitherProj::Right { value } => value.poll(cx), + } + } +} diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs b/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs index 72db50cbb4b..ab3c3857208 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/mod.rs @@ -118,6 +118,7 @@ //! ``` //! +pub mod alb_health_check; mod closure; mod either; mod filter; diff --git a/rust-runtime/aws-smithy-http-server/src/plugin/pipeline.rs b/rust-runtime/aws-smithy-http-server/src/plugin/pipeline.rs index 7b390fc903a..2a956922e46 100644 --- a/rust-runtime/aws-smithy-http-server/src/plugin/pipeline.rs +++ b/rust-runtime/aws-smithy-http-server/src/plugin/pipeline.rs @@ -6,6 +6,8 @@ use crate::operation::Operation; use crate::plugin::{IdentityPlugin, Plugin, PluginStack}; +use super::HttpLayer; + /// A wrapper struct for composing [`Plugin`]s. /// It is used as input for the `builder_with_plugins` method on the generate service struct /// (e.g. `PokemonService::builder_with_plugins`). @@ -168,6 +170,11 @@ impl

PluginPipeline

{ pub fn push(self, new_plugin: NewPlugin) -> PluginPipeline> { PluginPipeline(PluginStack::new(new_plugin, self.0)) } + + /// Applies a single [`tower::Layer`] to all operations _before_ they are deserialized. + pub fn http_layer(self, layer: L) -> PluginPipeline, P>> { + PluginPipeline(PluginStack::new(HttpLayer(layer), self.0)) + } } impl Plugin for PluginPipeline From 2ebbfad498f428b945cba453ef6cd464d05d0ce2 Mon Sep 17 00:00:00 2001 From: Tim McNamara Date: Mon, 24 Apr 2023 23:40:04 +1200 Subject: [PATCH 9/9] Expand example documentation (#2570) ## Motivation and Context The current README for the example services is fairly spartan. ## Description I've expanded the README, specifically: - Clean up some markup - Document pre-reqs. - Document all make targets. - Avoid the $BINARY because it looks like a bash variable. - Expand intro ## Testing n/a ## Checklist n/a ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: Matteo Bigoi <1781140+crisidev@users.noreply.github.com> Co-authored-by: david-perez --- examples/README.md | 79 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/examples/README.md b/examples/README.md index 7f087384557..a82327bc64e 100644 --- a/examples/README.md +++ b/examples/README.md @@ -1,17 +1,35 @@ # Smithy Rust Server SDK examples -This folder contains an example services showcasing the service framework capabilities and to run benchmarks. +This folder contains some example services showcasing Smithy Rust Server SDK, +also known as the Rust service framework, capabilities and to run benchmarks. -- `/pokemon-service`, a HTTP server implementation demonstrating [middleware](https://awslabs.github.io/smithy-rs/design/server/middleware.html) -and [extractors](https://awslabs.github.io/smithy-rs/design/server/from_parts.html). -- `/pokemon-service-tls`, a minimal HTTPS server implementation. -- `/pokemon-service-lambda`, a minimal Lambda deployment. +Three server implementations are available: -The `/{binary}/tests` folders are integration tests involving the generated clients. +- `/pokemon-service`, a HTTP server demonstrating [middleware] and [extractors]. +- `/pokemon-service-tls`, a HTTPS server. This server can do + its own TLS negotiation, rather than relying on a load balancer. +- `/pokemon-service-lambda`, a server that can be deployed onto AWS Lambda. -## Build +These servers, and their clients, are generated using smithy-rs. You're invited +to benchmark the performance of these servers to see whether smithy-rs might be +a suitable choice for implementing your web service. -Since this example requires both the server and client SDK to be code-generated +[middleware]: https://awslabs.github.io/smithy-rs/design/server/middleware.html +[extractors]: https://awslabs.github.io/smithy-rs/design/server/from_parts.html + + +## Pre-requisites + +You will need install Java 11 to run the smithy-rs code generator and an +installation of Rust, including `cargo`, to compile the generated code. + +(Optional) The [Cargo Lambda](https://cargo-lambda.info/) sub-command for +`cargo` is required to support the AWS Lambda integration. + + +## Building + +Since these examples require both the server and client SDK to be code-generated from their [model](/codegen-server-test/model/pokemon.smithy), a Makefile is provided to build and run the service. Just run `make` to prepare the first build. @@ -19,31 +37,52 @@ build. Once the example has been built successfully the first time, idiomatic `cargo` can be used directly. -`make distclean` can be used for a complete cleanup of all artefacts. +### Make targets: + +- `codegen`: generates the Pokémon service crates (default) +- `build`: compiles the generated client and server +- `clean`: deletes build artifacts +- `clippy`: lints the code +- `distclean`: delete generated code and build artifacts +- `doc-open`: builds and opens the rustdoc documentation +- `lambda_invoke`: invokes a running server +- `lambda_watch`: runs the service on an emulated AWS Lambda environment +- `run`: runs the Pokémon service +- `test`: runs integration and unit tests + -## Run +## Running services -To run a binary use +To run one of the three server implementations locally, provide the appropriate +service name to the `--bin` flag: ```bash -cargo run -p $BINARY +cargo run --bin pokemon-service[(-lambda|-tls)] ``` -CLI arguments can be passed to the servers, use +CLI arguments can be passed to the server binaries by adding them after `--`. +For example, to see a service's help information, use the following: ```bash -cargo run -p $BINARY -- --help +cargo run --bin -- --help ``` -for more information. +## Testing -## Test +The `/pokemon-test*/tests` folders provide integration tests involving the +generated clients. -`cargo test` can be used to spawn a service and run some simple integration -tests against it. Use `-p $BINARY` to filter by package. +They can be invoked with `cargo test`. This will spawn each service in turn +and run some integration tests against it. Use `-p ` to filter by +package. More info can be found in the `tests` folder of each package. -## Benchmarks -Please see [BENCHMARKS.md](/examples/BENCHMARKS.md). +## Benchmarking + +Servers running locally (see "Running services") can be benchmarked with any +load testing tool, such as Artillery or `wrk`. + +Please see [BENCHMARKS.md](/examples/BENCHMARKS.md) for benchmarking results +produced by the smithy-rs team.