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

Add Log Viewer to Modern UI #2440

Merged
merged 5 commits into from Nov 1, 2023
Merged

Conversation

andrewbaldwin44
Copy link
Collaborator

@andrewbaldwin44 andrewbaldwin44 commented Oct 30, 2023

Proposal

  • Add custom log handler LogReader to capture all logs
  • Add a route to web.py to return the logs in JSON format
  • Add tests to ensure correct behaviour from /logs
  • Add a new tab in the modern UI to fetch from /logs and display the out
image
  • Add an exception handler to catch and log uncaught exceptions
image

Fixes #2437

@cyberw
Copy link
Collaborator

cyberw commented Oct 30, 2023

Something off about python 3.8? Havent tried it out just yet, but it looks very promising!

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/2437 branch 3 times, most recently from 41a1c10 to 2c421ca Compare October 30, 2023 20:32
@andrewbaldwin44
Copy link
Collaborator Author

Strange, I'm not sure why Python 3.8 specifically was failing with the last build but seems everything is passing now

@cyberw
Copy link
Collaborator

cyberw commented Oct 31, 2023

This is already a great improvement, but can we also show unhandled exceptions?

For example, if I use an invalid host url (asdfasdf) this is logged on command line, but nothing shown in the UI.

[2023-10-31 14:53:34,306] svenskaspel.se/INFO/locust.runners: All users spawned: {"QuickstartUser": 1} (1 total users)
Traceback (most recent call last):
File "src/gevent/greenlet.py", line 908, in gevent._gevent_cgreenlet.Greenlet.run
File "/Users/lafp/git/locust/locust/user/users.py", line 177, in run_user
user.run()
File "/Users/lafp/git/locust/locust/user/users.py", line 143, in run
self.on_start()
File "/Users/lafp/git/locust/examples/locustfile.py", line 20, in on_start
self.client.post("/login", json={"username": "foo", "password": "bar"})
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/sessions.py", line 635, in post
return self.request("POST", url, data=data, json=json, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/git/locust/locust/clients.py", line 135, in request
response = self._send_request_safe_mode(method, url, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/git/locust/locust/clients.py", line 180, in _send_request_safe_mode
return super().request(method, url, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/sessions.py", line 573, in request
prep = self.prepare_request(req)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/sessions.py", line 484, in prepare_request
p.prepare(
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/models.py", line 368, in prepare
self.prepare_url(url, params)
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/models.py", line 439, in prepare_url
raise MissingSchema(
requests.exceptions.MissingSchema: Invalid URL 'asdfasdf/login': No scheme supplied. Perhaps you meant https://asdfasdf/login?
2023-10-31T13:53:34Z <Greenlet at 0x10374b060: run_user(<locustfile.QuickstartUser object at 0x1052c5350>)> failed with MissingSchema

@andrewbaldwin44
Copy link
Collaborator Author

@cyberw How were you able to reproduce this exactly? When I ran this the error would be caught by the greenlet_exception_logger and logged using the logger with the level set as CRITICAL

locust --modern-ui -f locust.py -H asdfasdf --autostart
image

If we want errors to be captured in a more general sense I believe we would have to do so using app.errorhandler

@cyberw
Copy link
Collaborator

cyberw commented Oct 31, 2023

I set the url in the ui, maybe that is the difference?

Or maybe it is because it happened in the on_start method? in that case that is another bug :)

Could you have a go at doing it (using app.errorhandler or whatever)? Showing almost all errors in the UI is kinda confusing :)

@cyberw
Copy link
Collaborator

cyberw commented Oct 31, 2023

app.errorhandler doesnt seem to catch this. Even with that change, I dont see it in the UI.

I'm running locust -H https://foo.com --modern-ui -f examples/locustfile.py (and then setting host to a bad url in the UI)

Traceback (most recent call last):
File "src/gevent/greenlet.py", line 908, in gevent._gevent_cgreenlet.Greenlet.run
File "/Users/lafp/git/locust/locust/user/users.py", line 177, in run_user
user.run()
File "/Users/lafp/git/locust/locust/user/users.py", line 143, in run
self.on_start()
File "/Users/lafp/git/locust/examples/locustfile.py", line 20, in on_start
self.client.post("/login", json={"username": "foo", "password": "bar"})
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/sessions.py", line 635, in post
return self.request("POST", url, data=data, json=json, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/git/locust/locust/clients.py", line 135, in request
response = self._send_request_safe_mode(method, url, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/git/locust/locust/clients.py", line 180, in _send_request_safe_mode
return super().request(method, url, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/sessions.py", line 573, in request
prep = self.prepare_request(req)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/sessions.py", line 484, in prepare_request
p.prepare(
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/models.py", line 368, in prepare
self.prepare_url(url, params)
File "/Users/lafp/.pyenv/versions/3.11.1/lib/python3.11/site-packages/requests/models.py", line 439, in prepare_url
raise MissingSchema(
requests.exceptions.MissingSchema: Invalid URL 'asdf/login': No scheme supplied. Perhaps you meant https://asdf/login?

@cyberw
Copy link
Collaborator

cyberw commented Oct 31, 2023

If you want, I can investigate it "from the other end" (correctly handling unhandled exceptions in on_start).

@cyberw cyberw changed the title [Feature/2437] Add Log Viewer to Modern UI Add Log Viewer to Modern UI Oct 31, 2023
@andrewbaldwin44
Copy link
Collaborator Author

andrewbaldwin44 commented Oct 31, 2023

Ah I see the issue, the on_start is being called outside the context of the Flask application, so naturally the Flask app will not catch the error. And I was previously not able to reproduce the issue because my locustfile did not have an on_start

@andrewbaldwin44
Copy link
Collaborator Author

@cyberw Adding logs to _send_request_safe_mode would allow us to capture any application errors that can happen while requests are being made. Any other relevant exceptions should be caught by the Flask error handler (given that they are within the context of the Flask app) or the greenlet_exception_logger

image

@cyberw
Copy link
Collaborator

cyberw commented Nov 1, 2023

That is an improvement, but not perfect. It will duplicate the error in the console output (which isnt terrible) but more importantly it doesnt handle other types of exceptions happening in on_start.

Anyways this isnt really your fault. I can investigate it later on, unless you want to dig deeper. Ok to merge?

@andrewbaldwin44
Copy link
Collaborator Author

Yeah do you prefer me to remove the last commit before merging? I'd be happy to dig into this more but I think fixing the error handling for the on_start is a ticket in and of itself. I'm already not clear as to why errors from the on_start are not being captured by the greenlet_exception_logger and it will take me some time to understand the full flow here as I don't have the same level of context in the application here as you :)

@cyberw
Copy link
Collaborator

cyberw commented Nov 1, 2023

Yea, lets remove the commit. It could theoretically cause log spam for someone (if they are catching the exception themselves)

@cyberw cyberw merged commit 0427175 into locustio:master Nov 1, 2023
14 checks passed
@cyberw
Copy link
Collaborator

cyberw commented Nov 1, 2023

I went ahead and fixed it right away :) #2442

@andrewbaldwin44
Copy link
Collaborator Author

Nice! Thanks for taking care of that :)

apereocas-bot pushed a commit to apereo/cas that referenced this pull request Nov 1, 2023
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [locust](https://togithub.com/locustio/locust) | `==2.18.0` -> `==2.18.1` | [![age](https://developer.mend.io/api/mc/badges/age/pypi/locust/2.18.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/locust/2.18.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/locust/2.18.0/2.18.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/locust/2.18.0/2.18.1?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>locustio/locust (locust)</summary>

### [`v2.18.1`](https://togithub.com/locustio/locust/releases/tag/2.18.1)

[Compare Source](https://togithub.com/locustio/locust/compare/2.18.0...2.18.1)

#### What's Changed

-   Modern UI shows wrong (old) hostname when setting hostname in start dialog by [@&#8203;andrewbaldwin44](https://togithub.com/andrewbaldwin44) in [locustio/locust#2436
-   Fix for UserClass picker not loading all available Shape Classes by [@&#8203;mikenester](https://togithub.com/mikenester) in [locustio/locust#2441
-   Add Log Viewer to Modern UI by [@&#8203;andrewbaldwin44](https://togithub.com/andrewbaldwin44) in [locustio/locust#2440
-   Log unhandled exceptions thrown in User.on_start by [@&#8203;cyberw](https://togithub.com/cyberw) in [locustio/locust#2442

**Full Changelog**: locustio/locust@2.18.0...2.18.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 10pm every weekday,before 6am every weekday" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/apereo/cas).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMS41IiwidXBkYXRlZEluVmVyIjoiMzcuMzEuNSIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9-->
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

Successfully merging this pull request may close these issues.

Add Log Viewer to Modern UI
2 participants