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

Samples with response_time None crashes stats.py #1087

Closed
cyberw opened this issue Sep 22, 2019 · 5 comments · Fixed by #1088
Closed

Samples with response_time None crashes stats.py #1087

cyberw opened this issue Sep 22, 2019 · 5 comments · Fixed by #1088
Labels
hacktoberfest See https://hacktoberfest.digitalocean.com for more info

Comments

@cyberw
Copy link
Collaborator

cyberw commented Sep 22, 2019

Certain async protocols/clients (like websockets, MQ etc) dont really have a response time. They should be able to report a None response time. Using zero is not good enough, because a test may contain a mix of synchronous requests (like HTTP) and async, and we dont want to "taint" the average response time with zeroes.

from locust import HttpLocust, task, TaskSet
from locust.events import request_success

class UserBehavior(TaskSet):
    @task
    def my_task(self):
        request_success.fire(request_type="foo", name="bar", response_time=None, response_length=0)


class MyHttpLocust(HttpLocust):
    task_set = UserBehavior

Result:

[2019-09-22 09:57:13,877] lafp-mac-JG5J.int.svenskaspel.se/ERROR/stderr: Traceback (most recent call last):
  File "/Users/lafp/git/locust/locust/core.py", line 361, in run
    self.execute_next_task()
  File "/Users/lafp/git/locust/locust/core.py", line 387, in execute_next_task
    self.execute_task(task["callable"], *task["args"], **task["kwargs"])
  File "/Users/lafp/git/locust/locust/core.py", line 399, in execute_task
    task(self, *args, **kwargs)
  File "/Users/lafp/git/locust-plugins/examples/foo.py", line 10, in my_task
    request_success.fire(request_type="foo", name="bar", response_time=None, response_length=0)
  File "/Users/lafp/git/locust/locust/events.py", line 34, in fire
    handler(**kwargs)
  File "/Users/lafp/git/locust/locust/stats.py", line 559, in on_request_success
    global_stats.log_request(request_type, name, response_time, response_length)
  File "/Users/lafp/git/locust/locust/stats.py", line 93, in log_request
    self.total.log(response_time, content_length)
  File "/Users/lafp/git/locust/locust/stats.py", line 239, in log
    self._log_response_time(response_time)
  File "/Users/lafp/git/locust/locust/stats.py", line 250, in _log_response_time
    self.total_response_time += response_time
TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType'
@cyberw cyberw changed the title Samples with response time None crashes stats.py Samples with response_time None crashes stats.py Sep 22, 2019
@heyman
Copy link
Member

heyman commented Oct 18, 2019

I agree that it makes sense to be able to report requests without response times so that they wouldn't taint the stats for ordinary requests.

The #1088 PR should have tests. I'm a little bit concerned that the changes could have unintended effects since the code up until now have always assumed that each request has a response time. That might be an unfounded concern though, but in any case, I think these changes should come with multiple tests where non-response-time requests are made both by themselves, as well as together with "normal" requests that have a response time.

@cyberw
Copy link
Collaborator Author

cyberw commented Oct 18, 2019

I'll try to add add tests for None response times mixed with integer response times

@heyman
Copy link
Member

heyman commented Oct 18, 2019

Cool! Hopefully that should be able to catch any unintended consequences if there are any (I've been looking a bit more at the code, and couldn't see any immediate problems with the changes).

@cyberw
Copy link
Collaborator Author

cyberw commented Oct 18, 2019

When I added the tests I immediately found some issues :) (related to average and median calculation)

I am trying to fix them...

@cyberw
Copy link
Collaborator Author

cyberw commented Oct 21, 2019

I've fixed the issues with calculating averages and median.

@cyberw cyberw added the hacktoberfest See https://hacktoberfest.digitalocean.com for more info label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest See https://hacktoberfest.digitalocean.com for more info
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants