search

aws / aws-sdk-java-v2

The official AWS SDK for Java - Version 2
Apache License 2.0
2.14k stars 816 forks source link

Upgrade apache-httpclient to httpclient5 which uses SLF4J #2831

Open frehov opened 2 years ago

frehov commented 2 years ago

Describe the feature

I'd like to request an upgrade of the apache httpclient to httpclient5 which uses SLF4J as a logging-facade instead of having to install bridge handlers to redirect log output from commons-logging.

Is your Feature Request related to a problem?

It's not directly a problem, but the issue is that the apache httpclient is leaking a dependency on commons-logging which has to be bridged by either jul-to-slf4j or jcl-over-slf4j which is an inconvenience for downstream libraries.

This feature request is opened in relation to https://github.com/micronaut-projects/micronaut-aws/issues/1203

but it also seems it's a cause of frustration here as well https://github.com/quarkusio/quarkus/issues/4528

Proposed Solution

My proposed solution would be to migrate from httpclient 4.5 til 5.1 following the migration guide set forward at the apache site https://hc.apache.org/httpcomponents-client-5.1.x/migration-guide/index.html

Describe alternatives you've considered

There is a possbility for any downstream library that use dependencies from the aws-sdk-v2 to include bridge handlers to redirect the logs from the httpclient, but imho the solution should be done at the root and trickle down, simplifying library maintainers jobs by not having to actively exclude the commons-logging and installing bridge handlers.

Acknowledge

AWS Java SDK version used

2.16.104 and later

JDK version used

11

Operating System and version

Linux

debora-ito commented 2 years ago

@frehov apologies for the long silence here. Marking this for review from the team.

debora-ito commented 2 years ago

Hi @frehov thank you for your patience.

Discussed it with the team, and apparently we tried to upgrade before but httpclient5 introduced breaking changes, and the work needed to make the changes on the SDK was not prioritized when compared to other tasks. We currently have no plans to make the upgrade.

Let us know if you have more questions.

github-actions[bot] commented 2 years ago

It looks like this issue has not been active for more than five days. In the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please add a comment to prevent automatic closure, or if the issue is already closed please feel free to reopen it.

lazystone commented 6 months ago

@debora-ito could you reopen or re-evaluate this? Last release of Apache http client v4 was released in 2022 and unmaintained since then.

debora-ito commented 6 months ago

@lazystone I'll raise this for discussion with the team, as it looks like httpcore 4 is at EOL - httpclient 4 will probably follow the same path. Will post an update when we get to a conclusion.

joebancf commented 5 months ago

I'd just like to add some weight for the team discussion. httpcore v4 doesn't work with virtual threads and there doesn't seem to be a plan to backport the fix to v4. Therefore the AWS SDK is not compatible with virtual threads until the switch to v5 is made.

To be clear, virtual threads do technically work but they are pinned to their carrier threads due to the use of the synchronized keyword in httpcore. This makes them practically unusable in production.

PankajSAgarwal commented 5 months ago

Is there any plan to support aws sdk2 java with httpclient5 @debora-ito ?

debora-ito commented 5 months ago

We are considering migrating to httpclient5 and httpcore5, since 4 is at EOL. We need to do research on how to make the change in a backwards compatible way, and how to prioritize this task against other tasks we need to work on. Added a task in our backlog, we don't have a timeline to share right now.

lazystone commented 2 months ago

If this ticket is still on your backlog, then I suggest to prioritize #1447 before this one. #1447 solves future compatibility and Virtual Threads usage at the same time.

rhermes62 commented 2 months ago

Until the virtual thread problem is addressed, thoughts on potentially updating documentation (maybe here) to highlight which clients are and are not compatible with virtual threads? Or maybe a higher level "Support Running In Virtual Threads" issue that can track until the SDK is compatible with running in virtual threads?

Background: our team just had a outage due to not knowing the AWS SDK's underlying HTTP client was incompatible with virtual threads. Pre-production stages did not have enough traffic to simulate the errors that result from issues like this until it was unfortunately too late. Given that this issue does not present well until at scale, it might be worth more visibility outside of tracking in this issue and #1447. We had done quick searches of virtual threads and saw this and #1447, but unfortunately assumed they were casual mentions of virtual threads and not that the SDK didn't support virtual threads.

millems commented 3 weeks ago

@rhermes62 Sorry this bit you. We're tracking an action item to document our state of support for virtual threads without pinning. We also have a desire to support HTTP clients (e.g. Apache 5.x) that do not have thread pinning issues at some point in the future.