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

net/http: add GODEBUG=http1debug #18733

Open
bradfitz opened this issue Jan 20, 2017 · 15 comments
Open

net/http: add GODEBUG=http1debug #18733

bradfitz opened this issue Jan 20, 2017 · 15 comments
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted
Milestone

Comments

@bradfitz
Copy link
Contributor

The GODEBUG knob for http2 debugging (e.g. GODEBUG=http2debug=2) has proven to be very useful and popular.

Unfortunately it only works for HTTP/2.

There is code in the net/http package to do a similar thing for HTTP/1, but it's not accessible via an environment variable. Should we make it accessible with GODEBUG=http1debug=...?

@rsc
Copy link
Contributor

rsc commented Jan 23, 2017

Seems OK to me (Go1.9Early)

@rsc rsc modified the milestones: Go1.9Early, Proposal Jan 23, 2017
@rsc rsc changed the title proposal: net/http: add GODEBUG=http1debug ? net/http: add GODEBUG=http1debug Jan 23, 2017
@Sajmani Sajmani self-assigned this Feb 7, 2017
@gopherbot
Copy link

CL https://golang.org/cl/36548 mentions this issue.

@Sajmani
Copy link
Contributor

Sajmani commented Feb 8, 2017

I've updated the commit message in my CL with sample debug output. This is a first cut; I'm happy to adjust the output, content, leveling, etc. as needed.

@bradfitz
Copy link
Contributor Author

One of the nice things about http2 is that it has a per-request "streamid" which shows up on all request and response lines, making it easy to line things up. Unfortunately, the stream id is unique only to a connection, though, so it's not a unique identifier for debugging, but generally works.

What I'd like to see for http1 is for each conn to get its own incrementing number, and each request on a conn to also get its own incrementing number, and then make sure each line about data going out or in for a request says either the relevant conn=%d or conn=%d,req=%d pair.

@Sajmani
Copy link
Contributor

Sajmani commented Feb 10, 2017

@Sajmani Sajmani removed their assignment Feb 22, 2017
@Sajmani
Copy link
Contributor

Sajmani commented Feb 22, 2017

Unfortunately I've run out of time to contribute to this issue, so unassigning myself.

@meirf
Copy link
Contributor

meirf commented Apr 20, 2017

Requirements gleaned from this issue and the CL herein:

  1. Enable debugging via environment variables GODEBUG=http1debug=1 (connections) and GODEBUG=http1debug=2 (connections and requests)
  2. "make sure each line about data going out or in for a request says either the relevant conn=%d or conn=%d,req=%d pair"
  3. Log at the wire level, i.e. before the data has been interpreted by go.

Question:
@bradfitz comments in the CL:

... "type loggingConn" in server.go
I wonder if we should just use that instead, extended a bit.
When debugging is enabled, we wrap things in *loggingConn and extend it to tell it the state of things ...

Are you saying logging should be done on the server side or that we should use a similar paradigm on the client where we wrap things in a logging connection? HTTP/2 debug logging seems to be implemented on the client side only so the latter seems plausible. Or perhaps the former is correct since you're saying there's no other way to achieve requirement 3.

(I would like to work on this.)

@bradfitz
Copy link
Contributor Author

Let's start with just http1 Transport (client) logging, and not server logging. I just meant that a net.Conn wrapper type (like loggingConn) might be helpful in implementing this. But feel free to ignore that too. Do whatever's easiest implementation-wise.

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9Early May 3, 2017
@bradfitz bradfitz added FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. labels May 3, 2017
@meirf
Copy link
Contributor

meirf commented May 10, 2017

I've had a couple speed bumps (transport code took a while to digest/accounting for pipelining when determining request id). Hopefully will have a first draft within a week.

@gopherbot
Copy link

CL https://golang.org/cl/43534 mentions this issue.

@odeke-em

This comment was marked as resolved.

@Sajmani
Copy link
Contributor

Sajmani commented Aug 24, 2017

Sorry about that; Google docs limits access to golang.org email addresses. I've published the doc to the web for read-only access: https://docs.google.com/document/d/e/2PACX-1vQrY8OCy56DwAO9vTqqBPMm8tL4zDJakFABJ9fRyHhwFtzMvQLwB1jjfK1t04lDietfwtyPheXWvAxM/pub

@rsc rsc modified the milestones: Go1.10, Go1.11 Nov 22, 2017
@bradfitz bradfitz removed this from the Go1.11 milestone May 18, 2018
@dol

This comment was marked as duplicate.

@irsl

This comment was marked as duplicate.

@TingxinLi

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest help wanted NeedsFix The path to resolution is known, but the work has not been done. Proposal-Accepted
Projects
None yet
Development

No branches or pull requests

9 participants