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

Improve UX of HAR converter's output scripts #117

Open
na-- opened this issue Sep 12, 2018 · 6 comments
Open

Improve UX of HAR converter's output scripts #117

na-- opened this issue Sep 12, 2018 · 6 comments

Comments

@na--
Copy link
Member

na-- commented Sep 12, 2018

With some refactoring and a few new features we can make the k6 HAR converter produce much more readable and user-friendly scripts without loosing anything in correctness or functionality. Some of the things I think can be changed:

  • move the long and ugly user-agent header to the global options object and remove it from each request (unless for some reason it's different than the global, though that should basically never happen with HAR files)
  • remove the unnecessary host header by default (i.e. for example by adding a new --remove-headers option in the HAR converter that by default has host in it)
  • remove the connection header when its value is keep-alive, since that's what the default HTTP 1.1 value is is that's what k6 does by default
  • remove the accept-encoding headers (arguable if we should remove it, but I think since it's super dependent on the HTTP client, we actually should clear it by default, because it's totally possible for a web-browser to put encodings there that we don't support in k6, thereby causing a bug)
  • unless users actually use the result (i.e. they use --correlate or something like that), don't save it in res
  • make any single-request http.batch() call to actually just use http.request() or even http.get()/http.post()/etc. calls
  • maybe remove the accept http header by default, since I think it doesn't usually do all that much and is almost universally ignored by servers, while adding messiness to the code
  • after this is implemented, we'd have a new k6 option that allows us to specify global headers (i.e. headers that we inject in every request). We currently do something like this, but it's restricted only to the userAgent, if we had the general option we can also use it for other headers that are basically the same for every request in a HAR file, like the Accept-Language (and Accept-Encoding, if we decide it's not worth removing it).
  • figure out repeating request options (like a single Referer header) and factor them out

Generally I'd like if generated scripts go from looking like this:

import { group, sleep } from 'k6';
import http from 'k6/http';

// Version: 1.3
// Creator: Load Impact URL test analyzer

export let options = {
  stages: [
    {
      "duration": "5m0s",
      "target": 25
    }
  ],
  maxRedirects: 0,
  discardResponseBodies: true,
};

export default function() {

  group("page_1 - http://test.loadimpact.com", function() {
    let req, res;
    req = [{
      "method": "get",
      "url": "http://test.loadimpact.com/",
      "params": {
        "headers": {
          "Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8",
          "Connection": "keep-alive",
          "Accept-Encoding": "gzip, deflate",
          "Host": "test.loadimpact.com",
          "Accept-Language": "en-US",
          "Upgrade-Insecure-Requests": "1",
          "User-Agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/62.0.3183.0 Safari/537.36"
        }
      }
    }];
    res = http.batch(req);
    sleep(0.65);
    req = [{
      "method": "get",
      "url": "http://test.loadimpact.com/style.css",
      "params": {
        "headers": {
          "Accept": "text/css,*/*;q=0.1",
          "Connection": "keep-alive",
          "Accept-Encoding": "gzip, deflate",
          "Referer": "http://test.loadimpact.com/",
          "Host": "test.loadimpact.com",
          "Accept-Language": "en-US",
          "User-Agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/62.0.3183.0 Safari/537.36"
        }
      }
    },{
      "method": "get",
      "url": "http://test.loadimpact.com/images/logo.png",
      "params": {
        "headers": {
          "Accept": "image/webp,image/apng,image/*,*/*;q=0.8",
          "Connection": "keep-alive",
          "Accept-Encoding": "gzip, deflate",
          "Referer": "http://test.loadimpact.com/style.css",
          "Host": "test.loadimpact.com",
          "Accept-Language": "en-US",
          "User-Agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/62.0.3183.0 Safari/537.36"
        }
      }
    }];
    res = http.batch(req);
    // Random sleep between 5s and 10s
    sleep(Math.floor(Math.random()*5+5));
  });
}

to something more like this:

import { group, sleep } from 'k6';
import http from 'k6/http';

// Version: 1.3
// Creator: Load Impact URL test analyzer

export let options = {
  stages: [
    {
      "duration": "5m0s",
      "target": 25
    }
  ],
  maxRedirects: 0,
  discardResponseBodies: true,
  headers: {
    "Accept-Language": "en-US",
    "User-Agent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/62.0.3183.0 Safari/537.36",
  },
};

export default function () {
  group("page_1 - http://test.loadimpact.com", function () {
    http.get("http://test.loadimpact.com/", {
      "headers": {
        "Upgrade-Insecure-Requests": "1",
      }
    });
    sleep(0.65);

    http.batch([{
      "method": "get",
      "url": "http://test.loadimpact.com/style.css",
      "params": {
        "headers": {
          "Referer": "http://test.loadimpact.com/",
        }
      }
    }, {
      "method": "get",
      "url": "http://test.loadimpact.com/images/logo.png",
      "params": {
        "headers": {
          "Referer": "http://test.loadimpact.com/style.css",
        }
      }
    }]);

    // Random sleep between 5s and 10s
    sleep(Math.floor(Math.random() * 5 + 5));
  });
}
@na--
Copy link
Member Author

na-- commented Sep 20, 2018

Another thing we should improve is the handling of special/non-printable characters in the generated scripts. For example, instead of having an invisible null-byte in the middle of a random string, we'd be better of if the string contained \u0000 instead - more readable, understandable,copy-paste-able, and much less error-prone.

@na--
Copy link
Member Author

na-- commented Sep 21, 2018

Regarding request bodies, we can check the content-type in the HAR file and write any binary bodies as a base64-encoded string in the generated script. And as mentioned in the previous comment, any special characters in text bodies would still have to be escaped.

@na--
Copy link
Member Author

na-- commented Oct 15, 2018

As seen in this issue, another reason for massively refactoring (i.e. rewriting) the HAR converter is the somewhat poor request dependency detection in the current implementation

@ppcano
Copy link

ppcano commented Nov 22, 2018

@na-- I ❤️ this idea; this is a great improvement of the k6/http API, and everyone will benefit from it, not only users using the HAR converter.

Because the issue is quite big and most of the above changes are "independent" improvements of the current API, I think the issue could be splitted into several issues which could be handled separately; this structure could make easier to start, delegate the work and see the progress on the goal.

This approach is used in other OSS projects, for example: emberjs/ember.js#16927 and ember-cli/ember-cli#7530.

@na--
Copy link
Member Author

na-- commented Jul 30, 2019

We shouldn't add the Content-Length header to the parameters of requests in the converted scripts. There are a couple of reasons for this:

  • Before Add ability to compress request body k6#989, k6 would set the Content-Length header value to the correct one, even if the user had manually specified something else. But after that PR, any manually set Content-Length header won't be touched, so if users edit scripts converted from HAR files, they may inadvertently specify wrong requests.
  • If we don't set a Content-Length header, k6 (or rather, the Go stdlib) will calculate and set the correct one for us.

@imiric
Copy link

imiric commented Mar 28, 2023

I moved this from the k6 repo, since this was originally meant for the built-in k6 convert command, but that has been deprecated for a long time, and these changes likely won't be done in k6. I think there are some valuable points worth addressing here, so I didn't want to close it.

We're currently working on a new HTTP API (initial design document), so it's best if this work is done after the new API is implemented.

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

No branches or pull requests

3 participants