Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add AlbHealthCheckLayer #2540

Merged
merged 44 commits into from
Apr 24, 2023
Merged

Add AlbHealthCheckLayer #2540

merged 44 commits into from
Apr 24, 2023

Conversation

jjant
Copy link
Contributor

@jjant jjant commented Apr 4, 2023

Motivation and Context

Services often need the ability to report health status via health checks (see ALB Health Checks).
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.

@smithy-lang smithy-lang deleted a comment from github-actions bot Apr 4, 2023
@jjant jjant requested a review from david-perez April 5, 2023 09:04
@jjant jjant force-pushed the jjant/add-ping-layer branch from ca83a2c to 8f6ec3d Compare April 13, 2023 10:17
Comment on lines 58 to 79
pin_project! {
/// A future that converts `F` into a compatible `S::Future`.
pub struct MappedHandlerFuture<R, S, F> {
#[pin]
inner: F,
pd: PhantomData<fn(R) -> S>,
}
}

impl<R, S, F> MappedHandlerFuture<R, S, F> {
fn new(inner: F) -> MappedHandlerFuture<R, S, F> {
Self { inner, pd: PhantomData }
}
}

impl<R, S: Service<R>, F: Future<Output = S::Response>> Future for MappedHandlerFuture<R, S, F> {
type Output = Result<S::Response, S::Error>;

fn poll(self: std::pin::Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
self.project().inner.poll(cx).map(Ok)
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a simpler way to do this? I basically just want to map HandlerFuture from a Future<Output = Response<BoxBody>> to a Future<Output = Result<Response<BoxBody>, S::Error>>.

I tried using futures_util::future::Map but I ran into a compiler error when trying to write the type in line 92 as (expected function pointer but got function item):

type Future = Either<
  Map<HandlerFuture, fn(Response<BoxBody) -> Result<Response<BoxBody>, S::Error>>,
  Oneshot<S, Request<Body>>
>;

@jjant jjant marked this pull request as ready for review April 13, 2023 10:21
@jjant jjant requested a review from a team as a code owner April 13, 2023 10:21
@smithy-lang smithy-lang deleted a comment from github-actions bot Apr 20, 2023
@jjant jjant requested a review from david-perez April 20, 2023 10:13
@jjant jjant changed the title Add CheckHealthLayer Add AlbHealthCheckLayer Apr 20, 2023
Add `AlbHealthCheckLayer::new_fn`

Make service return a `StatusCode`
@smithy-lang smithy-lang deleted a comment from github-actions bot Apr 21, 2023
@smithy-lang smithy-lang deleted a comment from github-actions bot Apr 21, 2023
@smithy-lang smithy-lang deleted a comment from github-actions bot Apr 21, 2023
@smithy-lang smithy-lang deleted a comment from github-actions bot Apr 21, 2023
@smithy-lang smithy-lang deleted a comment from github-actions bot Apr 21, 2023
@github-actions
Copy link

A new generated diff is ready to view.

  • No codegen difference in the AWS SDK
  • No codegen difference in the Client Test
  • 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.

Copy link
Contributor

@david-perez david-perez left a comment

Choose a reason for hiding this comment

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

Very nice.


impl AlbHealthCheckLayer<()> {
/// Handle health check requests at `health_check_uri` with the specified handler.
pub fn from_handler<HandlerFuture: Future<Output = StatusCode>, H: Fn(Request<Body>) -> HandlerFuture + Clone>(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: these bounds would read nicer in a where clause.

impl<H, S> Service<Request<Body>> for AlbHealthCheckService<H, S>
where
S: Service<Request<Body>, Response = Response<BoxBody>> + Clone,
S::Future: std::marker::Send + 'static,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
S::Future: std::marker::Send + 'static,
S::Future: Send + 'static,

Send is in the prelude.

@jjant jjant added this pull request to the merge queue Apr 24, 2023
Merged via the queue into main with commit 2de6815 Apr 24, 2023
@jjant jjant deleted the jjant/add-ping-layer branch April 24, 2023 10:52
unexge pushed a commit that referenced this pull request Apr 24, 2023
## 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._
rcoh pushed a commit that referenced this pull request Apr 24, 2023
## 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._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants