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

http: forwarding transport failure details for CONNECT and ALPN connections #16975

Merged
merged 2 commits into from
Jun 21, 2021

Conversation

alyssawilk
Copy link
Contributor

@alyssawilk alyssawilk commented Jun 14, 2021

Previously, L7 connections forwarded over TCP didn't get connection failure details (auto_http before codec assignment, CONNECT requests during the entire lifetime) but now they should.

Risk Level: low
Testing: unit test.
Docs Changes: n/a
Release Notes: n/a
Fixes #/16881

zuercher
zuercher previously approved these changes Jun 14, 2021
Copy link
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

Looks good! Is the plan to eventually remove the original conn pool or should we circle back and add failure reasons there?

@alyssawilk
Copy link
Contributor Author

yeah, we plan on removing it after the standard 6 months but it's not used for the aforementioned code paths (CONNECT/ALPN) in any case.

Signed-off-by: Alyssa Wilk <[email protected]>
@alyssawilk
Copy link
Contributor Author

(removing the bonus unrelated integration test as I broke it somewhere along the way and don't want to reflog it =P)

@alyssawilk alyssawilk merged commit 62ca8bd into envoyproxy:main Jun 21, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
…ctions (envoyproxy#16975)

Previously, L7 connections forwarded over TCP didn't get connection failure details (auto_http before codec assignment, CONNECT requests during the entire lifetime) but now they should.

Risk Level: low
Testing: unit test.
Docs Changes: n/a
Release Notes: n/a
Fixes #/16881

Signed-off-by: Alyssa Wilk <[email protected]>
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.

2 participants