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-debug shows incorrect "Accept-Encoding" #1042

Open
kkeranen opened this issue Jun 6, 2019 · 7 comments
Open

--http-debug shows incorrect "Accept-Encoding" #1042

kkeranen opened this issue Jun 6, 2019 · 7 comments
Labels

Comments

@kkeranen
Copy link

kkeranen commented Jun 6, 2019

When not explicitly initializing Accept-Encoding field in HTTP request header, k6 or Go runtime implicitly adds Accept-Encoding: identity, which is fine. But not fine at all is that --http-debug option of k6 shows Accept-Encoding: gzip. I have wasted dozens of hours on this problem because I used to trust k6 debug info. A lot of this wasted time went just being able to get a proof by Fiddler, which I was finally able to get working with k6.

My related opening at community.k6.io.

Possibly related issue

@na-- na-- added the bug label Jun 6, 2019
@na--
Copy link
Member

na-- commented Jun 6, 2019

Sorry for the frustration with this, unfortunately you are completely right again, this is a bug. Here's a demo script that confirms it:

import http from "k6/http";

export default function () {
    let resp = http.get("http://httpbin.org/anything")
    console.log(resp.body)
}

Running that script with k6 0.24.0 and the --http-debug CLI flag produces something like this:

Request:
GET /anything HTTP/1.1
Host: httpbin.org
User-Agent: k6/0.24.0 (https://k6.io/)
Accept-Encoding: gzip

But the console.log() message shows that httpbin.org receives only the following headers:

"headers": {
    "Host": "httpbin.org", 
    "User-Agent": "k6/0.24.0 (https://k6.io/)"
}, 

I still don't see what's causing the issue. We may have to open an upstream Golang bug report for this, since k6 is simply using the httputil.DumpRequestOut() Go helper function 😕

@kkeranen, where did you notice the Accept-Encoding: identity header? Can you share an example, since in my case, it doesn't seem to be present.

@kkeranen
Copy link
Author

kkeranen commented Jun 6, 2019

I got Accept-Encoding: identity with Fiddler, but I was using k6 v0.22.1 in that test. I am assuming that k6 v0.24 behaves similarly, because the behavior is quite similar in both cases. I have v0.24 in Ubuntu and v0.22.1 in Windows 10, and got Fiddler more easily working with Windows 10.

@na--
Copy link
Member

na-- commented Jun 6, 2019

The reason for the issue seems to be that Go's httputil.DumpRequestOut() just adds that header by default, probably to emulate the way http.DefaultTransport behaves. But k6 uses a custom http.Transport, which doesn't send the header, thus causing the mismatch.

This still seems like a Golang bug, but it's more a case of bad APIs, so I can't see a way for it to be fixed 😕... Potentially connected Go issue: golang/go#29607 (dead, unfortunately)

@na--
Copy link
Member

na-- commented Jun 6, 2019

And it's not even using the DefaultTransport, it's creating its own internal one 🤦‍♂️

Anyway, I'm not sure what the best way for this to be fixed on the k6 side is 😕 The obvious solution is that we ditch httputil.DumpRequestOut() and write something ourselves. This seems almost inevitable, since I don't think this could be fixed on the Go side, and we can also close #986 and #774.

Also, it seems to me that the current k6 architecture (where we dump the request before we send it) should also be changed. Besides the accept-encoding issue you found out, it won't allow us to determine if a request is going to be sent via HTTP/1.1 or HTTP2, any redirects that would happen, NTLM authentication, etc.

Instead, k6 should likely "inject" the HTTP request dumping in a manner closer to what you've done with Fiddler - by "listening" to the actual requests being sent and received. That could happen through something like the httputil.ReveseProxy or, more likely, through injecting another http.RoundTripper at the bottom of the stack that logs all HTTP requests and responses.

@kkeranen
Copy link
Author

kkeranen commented Jun 7, 2019

Well, I don't need to be fixed. It's enough that I know how it works. Therefore I suggest documenting this special feature so that others don't get confused and waste time investigating this. After all, k6 is performance testing tool and disabling/enabling compression is probably of great interest to k6 users.

Maybe this header problem was made worse by incorrect reception timing because it seems to affect the duration timing, too. I make a more detailed comment into that issue.

na-- added a commit that referenced this issue Aug 6, 2019
This makes the code slighly saner and fixes the bug where the first NTLM and digest authentication requests weren't displayed. But by no means does this fix all of the bugs in the http-debug code... For those, see:
 - #986
 - #1042
 - #774

 A discussion how some or all of them can be fixed can be found here: #1102 (comment)
na-- added a commit that referenced this issue Aug 6, 2019
…#1102)

Previously, there was an uncaught error (sometimes leading to panics) in the automatic response body decompression. This patch handles these decompression errors and even assigns a custom error code for them. As a bonus, k6 now also properly handles errors due to improper redirects, or too many redirects, and tags the resulting metric samples accordingly.

By necessity, I had to also move the digest authentication and http-debug handling to their own http.RoundTripper layers. This by no means solves any of the following issues:
 - #800
 - #986
 - #1042
 - #774

...but it hopefully slightly improves the situation. For the digest authentication, a proper authentication cache, and very likely a different library, still have to be used... And regarding `http-debug`, a discussion about how some or all the remaining issues can be fixed can be found here: #1102 (comment)
@jberryman
Copy link

k6 or Go runtime implicitly adds Accept-Encoding: identity, which is fine

aside: just making sure if I set ...identity;q=0 we override this? Otherwise not fine

@mstoykov
Copy link
Contributor

@jberryman the stdlib will not set Accept-Encoding, if it is already set. It also will be correctly reported by --http-debug.

Please though use the community forum for pure questins to not muddle the issues ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants