ppcano
commented
3 years ago I think the metricEmissionHooks option is unnecessary.
import http from 'k6/http'
export let options = {
metricEmissionHooks: {
http: {
successHook: 'myHttpSuccessHook',
}
}
};
export function myHttpSuccessHook(response){
return response.status >= 200 && response.status <= 204
}
export default function() {
http.get("https://httpbin.test.k6.io/status/200");
}Instead, we can define a name convention for the hook method. For example,didSuccessRequest.
import http from 'k6/http'
export function didSuccessRequest(response){
return response.status >= 200 && response.status <= 204
}
export default function() {
http.get("https://httpbin.test.k6.io/status/200");
}
simskij
mstoykov
imiric
na--
legander
sniku
allansson
Doesn't look that terrible, but I would argue that makes #1319 more relevant ;), so I am not for just adding the above code and leaving it.
tom-miseur
I'm opening a new issue instead of building on top of https://github.com/loadimpact/k6/issues/1311 to start the discussion with a clean slate. I believe this proposal addresses most if not all issues brought up in the original proposal.
Before discussing the implementation details and effort required to build this feature, let's discuss why we want this feature at all.
Basic requirements
This basic script must:
Discussion about the metric type for
http_reqs_failureThere are two possible metric types for the new
http_reqs_failure. Rate or Counter. Both types have their advantages, and it's not entirely clear which one is better for this use case.Advantages of Rate:
http_reqs_failure: rate<0.1Advantages of Counter:
http_reqsmetric.The end-of-test summary for this test should look similar to this:
Output when using Counter metric
Output when using Rate metric
Neither
RatenorCountercovers all possible use-cases. I thinkRateis preferable overCounter.If we added
countto theRatemetric, the output could possibly look similar to thisNote, I'm not really advocating for this, just pointing out that neither Rate nor Counter cover all use cases.
Why do we have failures and successes as separate metrics?
The obvious critique of the above suggestion is to say that
http_reqs_successis unnecessary because it's opposite ofhttp_reqs_failure. This is true, but some outputs don't allow to define logic, and therefore it's not possible to showhttp_reqs_successunless k6 itself produces it.Once the metric filtering feature is developed, I would suggest we exclude
http_reqs_successby default.http_req_duration and other httpreq* metrics.
The core requirement of this feature is to be able to see http_req_duration for successful requests only.
There are two possibilities here:
http_req_durationfor failureshttp_req_durationwithfailed:true|falsetag and display filtered values.Let's discuss both approaches
Don't emit http Trend metrics for failed requests
In this approach,
http_req_durationand other http metrics won't include failed requests towards the metric's internal state.Users who want to track error timings can define custom metrics like this:
Tag
http_req_durationwithfailed:true|falsetag and display filtered values.With this approach, we would emit the
http_req_durationand friends as we used to, but we will tag values withfailed:true|falseThe default-end-of-test summary would display only successful requests like this:
The most problematic issue with this approach is that some outputs don't ingest tags and won't be able to display http_req_duration for successful requests only.
Examples of
http_req_durationandhttp_reqs_failurek6 should produce with this approach.Cloud support
There are additional considerations for the
k6 cloudsupport.Performance insights
The "performance insights" feature and web app currently assume that successful requests have status 200-399.
Both approaches listed above solve these problems, although in different ways.
In approach 1, we would only get timings for successful requests and therefore we won't show timings for failed requests. We will still get tagged
We would probably redesign this UI to separate failures from successes in a better way.
http_reqs_failuremetrics and therefore will be able to show errors without timings.In approach 2, we would get a new standard tag called
failedto allhttp_req_*metrics, includinghttp_req_li_all. Timings would still be shown for errors (although probably not useful), but the background of the row would be determined by thefailedtag.(alternative) Why don't we skip the new metrics and purely rely on
failedtag?It's possible to extend the existing
http_reqscounter metric by tagging requests withfailedand changing the metric type toRate. If that's done, the following script would be possible,Possible end-of-test summary:
Possible problems with this approach are:
I'm (currently) against this alternative.
Defining failure
To determine if a request has failed or succeeded, a JavaScript hook function is invoked after the request, but before the metrics emission. This proposal builds on https://github.com/loadimpact/k6/issues/1716
per-request handling
Sometimes users need to handle special cases.
Alternative 1 - handle inside the hook
Alternative 2 - override the hook per request
What about the redirect chains?
This is up for discussion.
Should the hook fire on every request in the chain or only on the last one?
Concerns
http_req_li_alland otherhttp_req_*metrics.