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

Jump to bottom

x/net/http2: When using an http2.Transport with http.Client, TLS-related httptrace callbacks are not invoked #52110

Open
hawkinsw opened this issue Apr 1, 2022 · 5 comments
Open

x/net/http2: When using an http2.Transport with http.Client, TLS-related httptrace callbacks are not invoked #52110

hawkinsw opened this issue Apr 1, 2022 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@hawkinsw
Copy link
Contributor

hawkinsw commented Apr 1, 2022

First, thank you so much for the awesome httptrace functionality! It's a lifesaver. Second, I have a patch that fixes the issue here -- I am submitting an Issue first because

Excluding very trivial changes, all contributions should be connected to an existing issue.

and I want to make sure that I am following best practices. And now, on with the report:

What version of Go are you using (go version)?

$ go version
go version devel go1.19-e72cd2de38 Sun Mar 27 02:50:33 2022 -0400 linux/amd64

Note: Same behavior seen with the "Release" versions of 1.18.

Does this issue reproduce with the latest release?

Yes (see above)

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/hawkinsw/.cache/go-build"
GOENV="/home/hawkinsw/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/hawkinsw/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/hawkinsw/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/hawkinsw/code/gosrc/goroot"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/hawkinsw/code/gosrc/goroot/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.19-e72cd2de38 Sun Mar 27 02:50:33 2022 -0400"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/hawkinsw/code/goresponsiveness/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3289694319=/tmp/go-build -gno-record-gcc-switches"

What did you do?

A fully buildable and executable program (with flags for eliciting expected and unexpected behavior) is available at

https://github.com/hawkinsw/bugs/tree/main/httptrace

$ go build ./repro.go

To get expected behavior,
$ ./repro

To get unexpected behavior,
$ ./repro -bug

All code (and rationale behind it) in the MRE is commented. I look forward to submitting my (simple) patch and contributing!

I hope that all this information is helpful!
Will
@hawkinsw hawkinsw mentioned this issue Apr 1, 2022
Open
@cherrymui
Copy link
Member

cc @neild

@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 5, 2022
@cherrymui cherrymui added this to the Unreleased milestone Apr 5, 2022
@hawkinsw
Copy link
Contributor Author

hawkinsw commented Apr 5, 2022

Thanks for updating the cc @cherrymui -- I am busy with ${DAY_JOB} for a few days and coming off an illness, but I have a patch ready and just need to get it properly formatted, etc. Thank you for your patience!

@gopherbot
Copy link

Change https://go.dev/cl/403114 mentions this issue: crypto/tls,net/http: consolidate TLS event logging

@gopherbot
Copy link

Change https://go.dev/cl/403077 mentions this issue: net/http, crypto/tls: centralize mgmt of TLS trace events

@hawkinsw
Copy link
Contributor Author

Hello @cherrymui and @neild -- I have a patch submitted that will address this issue: https://go-review.googlesource.com/c/go/+/403077

I'd love to get your feedback on how to make the patch acceptable. I'd love to be able to help the community! Thank you for everything!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@gopherbot @hawkinsw @cherrymui