Ability to exclude response time of failed requests from time based metrics #1363

[nmondal]

Is your feature request related to a problem? Please describe.

Currently Locust stats for time based metrics (ART, median, percentiles) include response times from all requests, including failed. From my experience, In most cases it much affects metrics for successful requests and can make it completely incorrect.

Examples:

100 successful requests with ART 1000 ms + 20 failed requests with 503 error (Service Temporary Unavailable) and fast response time 5 ms In this case ART in report will be "improved" by failed requests (~830 ms vs actual 1000), and the more failed requests in test, the better result will be in results. 100 successful requests with ART 1000 ms, and 5 failed requests with 504 Server error in 60 seconds (because of proxy gateway issues) In this case, successful results will be much worsened by failures - (3.8 sec vs actual 1 sec)

Describe the solution you'd like

In order to keep old behavior by default and not break compatibility, there might be additional command line argument, like --exclude-failed-response-time or --exclude-failures With using this parameter, response times from failed requests must be excluded for calculation of time based metrics. The number of failures and failures rate shouldn't be affected.

StatsEntry class should be extended with response_times_success & response_times_failures fields, that should be used with calculation functions in case of using exclude failures parameter.

Functions/properties to be refactored (at least): calculate_response_time_percentile avg_response_time median_response_time log_response_time

Describe alternatives you've considered

Any additions and suggestions are welcome!.

Additional context

https://github.com/locustio/locust/issues/1363 The original issue was reported in 2021. I am doing perf testing and tuning from 2010+, and I concur. It was closed because of lack of volunteers. We would fix it. As a community we must support this, and I am personally trying to fix it, with a PR to be updated soon.

@vstepanov-lohika-tix

[nmondal]

Added the PR: https://github.com/locustio/locust/pull/2382

[nmondal]

Hello @cyberw can you please help us with this?

[vs-odessa]

@nmondal, there was an attempt at the implementation: https://github.com/locustio/locust/pull/2353/files After discussions, it was rejected due to low demand for the feature.

[oumsofiane1]

+1

[cyberw]

https://github.com/locustio/locust/pull/2382 is still open but needs work.

[amard33p]

@cyberw Are there any caveats to using solution/workaround suggested by @vs-odessa in the older PR? If there are none, then this is a perfectly acceptable solution for us. https://github.com/locustio/locust/pull/2353#issuecomment-1588817460

with self.client.request(method, url, name=name, catch_response=False) as response:
            if 'should-fail' in URL:
                response.request_meta['response_time'] = None

[cyberw]

I dont think there are any caveats for that (and it could even be implemented as an event listener to make it apply for all requests)

[github-actions[bot]]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 10 days.

[aronaks]

It would be nice if such option is available

[github-actions[bot]]

This issue was closed because it has been stalled for 10 days with no activity. This does not necessarily mean that the issue is bad, but it most likely means that nobody is willing to take the time to fix it. If you have found Locust useful, then consider contributing a fix yourself!

[danielhjz]

Hi. is this feature supported? @cyberw
Or if we could take a PR for constribution?

[cyberw]

https://github.com/locustio/locust/pull/2382 was opened but never finished. Havent had a look at it now, but I think it can probably be finished.

I almost prefer the workaround though:

with self.client.request(method, url, name=name, catch_response=False) as response:
        if 'should-fail' in URL:
            response.request_meta['response_time'] = None

(and as I mentioned, this could be implemented as a request event handler)

[danielhjz]

with self.client.request(method, url, name=name, catch_response=False) as response:
        if 'should-fail' in URL:
            response.request_meta['response_time'] = None

(and as I mentioned, this could be implemented as a request event handler)

Thanks for reply, i will take a try for this.

[Alex-TG001]

Hello, I'm trying to make our final metrics based on what we get from filtering out failed requests. If I changed the logic of the on_requests function like this, would it works? These codes are in /locust/runners.py @cyberw

[cyberw]

sounds like that can work, but you are on your own :)

personally I'd prefer overwriting request_meta['response_time'] rather than changing the actual code.

[Alex-TG001]

Thanks for your kindly reply, I will try it.