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 · 12 comments

Comments

Projects
None yet
6 participants
@bradfitz
Member

bradfitz commented Jan 20, 2017

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

This comment has been minimized.

Contributor

rsc commented Jan 23, 2017

Seems OK to me (Go1.9Early)

@rsc rsc added Proposal-Accepted and removed Proposal labels Jan 23, 2017

@rsc rsc modified the milestones: Go1.9Early, Proposal Jan 23, 2017

@rsc rsc changed the title from proposal: net/http: add GODEBUG=http1debug ? to net/http: add GODEBUG=http1debug Jan 23, 2017

@Sajmani Sajmani self-assigned this Feb 7, 2017

@gopherbot

This comment has been minimized.

gopherbot commented Feb 8, 2017

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

@Sajmani

This comment has been minimized.

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

This comment has been minimized.

Member

bradfitz commented Feb 10, 2017

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

This comment has been minimized.

Contributor

Sajmani commented Feb 10, 2017

@Sajmani Sajmani removed their assignment Feb 22, 2017

@Sajmani

This comment has been minimized.

Contributor

Sajmani commented Feb 22, 2017

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

@meirf

This comment has been minimized.

Member

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

This comment has been minimized.

Member

bradfitz commented Apr 21, 2017

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.

@meirf

This comment has been minimized.

Member

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

This comment has been minimized.

gopherbot commented May 17, 2017

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

@odeke-em

This comment has been minimized.

Member

odeke-em commented Aug 20, 2017

@Sajmani unfortunately the doc that you mentioned in #18733 (comment) is not public
screen shot 2017-08-19 at 6 44 48 pm

If it is top secret, no biggie but otherwise it might be relevant for this issue.

@Sajmani

This comment has been minimized.

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

@bradfitz bradfitz added this to the Unplanned milestone May 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment