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

Reject non-numeric values for the Rate metric #908

Closed
sniku opened this issue Jan 28, 2019 · 3 comments
Closed

Reject non-numeric values for the Rate metric #908

sniku opened this issue Jan 28, 2019 · 3 comments
Labels

Comments

@sniku
Copy link
Collaborator

sniku commented Jan 28, 2019

Example script that demonstrates the problem:

import { check, sleep } from "k6";
import http from "k6/http";
import { Rate } from "k6/metrics";


let sqlCount = new Rate("sql_count");

export let options = {
  vus: 1,
  duration: "1m",
};

export default function() {
  let res = http.get("http://test.loadimpact.com");
  
  // take custom header and convert it to int
  // if header is missing the value will be NaN
  let req_sql_count = parseInt(res.headers.sql_count, 10);
  
  // NaN is happily accepted by k6, but not accepted by Load Impact.
  sqlCount.add(req_sql_count);
}

My expectation was for the sqlCount metric to ignore non-numeric values. Instead it accepts anything, but LoadImpact drops the entire data packages, so even data for the initial request is missing in the cloud.

@sniku sniku added the bug label Jan 28, 2019
@na--
Copy link
Member

na-- commented Jan 28, 2019

I'm not sure we should ignore invalid values, we probably have to throw an error in such situations, to minimize any user confusion.

@sniku
Copy link
Collaborator Author

sniku commented Feb 3, 2019

I don't have strong opinion in either direction, as far as I can see there are 2 options:

  1. Throw JS exception and interrupt iteration
  2. Log a warning, ignore value and continue the iteration.

There are annoying drawbacks with each option.

  1. Kills iterations; requires extra code to do error checking; user gets confused why iterations didn't finish
  2. is PHP; user gets confused why data is silently missing

Option 2 would make it easy for the user to record correct data even when the SUT is overloaded and returning HTTP 400-500.

In my case, server was returning HTTP-200, with correct headers until it got overloaded, and started returning 400 (without expected header).

For option 1. the correct script code would have to be something like this:

let res = http.get("http://test.loadimpact.com");
if(typeof res.headers.sql_count !== "undefined" ){
    let req_sql_count = parseInt(res.headers.sql_count, 10);
    sqlCount.add(req_sql_count);
}

When writing scripts like this, it's tricky at first to predict that the if check is necessary.

@na-- na-- modified the milestones: v1.0.0, v0.26.0 Aug 27, 2019
@na-- na-- modified the milestones: v0.26.0, v0.27.0 Oct 10, 2019
@na-- na-- modified the milestones: v0.27.0, v0.28.0 May 21, 2020
@na--
Copy link
Member

na-- commented May 21, 2020

Closing this in favor of the more comprehensive #1435.

@na-- na-- closed this as completed May 21, 2020
@na-- na-- removed this from the v0.28.0 milestone May 21, 2020
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

2 participants