aws / aws/aws-sdk-java-v2

Allow modifying SdkHttpFullRequest before every retry attempt

Open
#1,741 2 comments 0 reactions 0 assignees View on GitHub
feature-request p3
Dominant language
Java
Stars
2.6k
Forks
1k
Avg merge
2d 9h
Merged PRs (30d)
51

Description

## Describe the Feature

Currently, an `ExecutionInterceptor` can implement `modifyHttpRequest` to rewrite attributes of the HTTP request, such as headers. This is important for some systems like distributed tracing which need to populate headers with tracing-related information.

Unfortunately, right now, it is not possible to populate headers at the granularity of retries. Since for tracing we want to model each attempt as a request (they are at the HTTP level), we need to be able to do this mutation per retry but currently it doesn't seem possible.

## Is your Feature Request related to a problem?

I ran into this while debugging https://github.com/openzipkin/zipkin-aws/issues/158 Zipkin would like to populate headers per retry but currently cannot.

## Proposed Solution

A couple of approaches might be

#### Add `ExecutionAttributes` to `ExecutableHttpRequest`.

Then, instrumentation like Brave could wrap `SdkHttpClient` to do a final mutation before delegating to an SDK client
1) `ExecutionInterceptor.beforeTransmission` - create a new span to model the transmission attempt and put it in `ExecutionAttributes`
2) `SdkHttpClient.prepareRequest` - reads `ExecutableHttpRequest.executionAttributes` and reads the span, and populates the headers with its propagation information, and delegates to an actual HTTP client.
3) `ExecutionInterceptor.afterTransmission` - clears span from `ExecutionAttributes` and finishes it

It's a bit unfortunate this means registering instrumentation requires setting two configuration options, both an `ExecutionInterceptor` and wrapping an `SdkHttpClient`, and it needs to be implemented for `SdkAsyncHttpClient` too, but it's livable.

#### Add another lifecycle method for modifying the request just before transmission.

This does not have as much of a duplication issue as the previous solution, but conceptually the interceptor becomes harder to understand as statements like `This includes modifications made by other interceptors and the message signature. ` stop being correct.

One thing I'm not sure of with both approaches is whether it's ok to add headers after the signature has been computed. I think it just means those headers won't be part of the signature but am not sure if this is valid.

## Describe alternatives you've considered

We can model the entire request across retries as a single request, but this reduces the usefulness of the tracing data to users.

## Additional Context

- [X] I may be able to implement this feature request

Happy to implement if an approach seems reasonable.

## Your Environment

* AWS Java SDK version used: 2.10.73
* JDK version used: Doesn't matter
* Operating System and version: Doesn't matter

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.