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

Send logs from workers to master and improve log viewer tab in the Web UI #2750

Merged
merged 9 commits into from
Jun 7, 2024

Conversation

andrewbaldwin44
Copy link
Collaborator

Proposal

  1. Re-style and colorize the logs received in the UI
  2. Receive and display logs from the workers

image
image

@andrewbaldwin44
Copy link
Collaborator Author

andrewbaldwin44 commented Jun 3, 2024

@cyberw Opening as a draft just to check that worker logs are something we want to add and then I will add tests for this

Let me know if you're also good with how the logs are displayed now

@cyberw
Copy link
Collaborator

cyberw commented Jun 3, 2024

This looks great. Can we maybe limit the number of log messages sent from workers to something like 10 per log interval? (I dont want workers spamming the master)

And also, if there are no logs, skip sending the message entirely (typically the workers should be very quiet)

And it needs some basic tests too.

@cyberw
Copy link
Collaborator

cyberw commented Jun 3, 2024

Side note: if we switch from mypy to pylance/pyright it might become fast enough to add it to the precommit hook.

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/log-improvements branch from cd3d8cf to 08a9585 Compare June 4, 2024 12:53
@andrewbaldwin44
Copy link
Collaborator Author

I updated now so that we only push worker logs when we see new logs. I think this should be sufficiently quiet since the workers rarely log more than one or two logs unless there are issues

@andrewbaldwin44 andrewbaldwin44 marked this pull request as ready for review June 4, 2024 13:49
@cyberw
Copy link
Collaborator

cyberw commented Jun 4, 2024

I updated now so that we only push worker logs when we see new logs. I think this should be sufficiently quiet since the workers rarely log more than one or two logs unless there are issues

The thing I'm worried about is if a worker does have an issue - in that case we dont want it bringing the master down with it by spamming lots of messages :) (if all workers have issues, then maybe it doesnt matter but if only one breaks down it would be nice to prevent this)

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/log-improvements branch 2 times, most recently from 344f8b8 to 7047966 Compare June 5, 2024 13:01
@mquinnfd
Copy link
Contributor

mquinnfd commented Jun 5, 2024

Not suggesting adding another flag but would "worker logs being sent to master" be configurable in some way?

Not sure if we're alone but not uncommon to run with hundreds -> thousands of worker processes and 100k+ users attached to a single master

In larger scenarios it might get absolutely obliterated

@cyberw
Copy link
Collaborator

cyberw commented Jun 5, 2024

Not suggesting adding another flag but would "worker logs being sent to master" be configurable in some way?

Not sure if we're alone but not uncommon to run with hundreds -> thousands of worker processes and 100k+ users attached to a single master

In larger scenarios it might get absolutely obliterated

Good point. We might want to think a little more on this. But in the normal case, workers should be kinda quiet though?

@mquinnfd
Copy link
Contributor

mquinnfd commented Jun 5, 2024

Not suggesting adding another flag but would "worker logs being sent to master" be configurable in some way?
Not sure if we're alone but not uncommon to run with hundreds -> thousands of worker processes and 100k+ users attached to a single master
In larger scenarios it might get absolutely obliterated

Good point. We might want to think a little more on this. But in the normal case, workers should be kinda quiet though?

Normally you'd expect so yeah, don't think anyone's going to be logging out info during task executions or anything, more likely that a cascading failure kills the master if, say an external SUT goes down and suddenly all the workers start logging large error logs with exception traces at the same time

(btw the log beautification is immense, well played @andrewbaldwin44 👏)

@andrewbaldwin44
Copy link
Collaborator Author

andrewbaldwin44 commented Jun 5, 2024

@mquinnfd Thanks for the comments! There is some logic for sending the logs from the workers. For example, if there are no new logs the workers won't send a message, plus it will quit sending messages all together if the worker tries to send > 10 logs at one time

    def logs_reporter(self) -> None:
        while True:
            current_logs = get_logs()

            if (len(current_logs) - len(self.logs)) > 10:
                current_logs = [*self.logs, "The worker experienced an issue and will stop outputting logs"]
                self.send_message("logs", {"worker_id": self.client_id, "logs": current_logs})
                self._send_logs(current_logs)
                break
            if len(current_logs) > len(self.logs):
                self._send_logs(current_logs)

            self.logs = current_logs
            gevent.sleep(WORKER_LOG_REPORT_INTERVAL)

@cyberw cyberw changed the title Improve the Log Viewer in the Web UI Send logs from workers to master and improve log viewer tab in the Web UI Jun 5, 2024
@andrewbaldwin44
Copy link
Collaborator Author

@mquinnfd WORKER_LOG_REPORT_INTERVAL may now be overridden and if set to a negative value, it will be disabled completely

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/log-improvements branch 7 times, most recently from 2939120 to 64a5662 Compare June 5, 2024 19:22
@mquinnfd
Copy link
Contributor

mquinnfd commented Jun 6, 2024

@mquinnfd WORKER_LOG_REPORT_INTERVAL may now be overridden and if set to a negative value, it will be disabled completely

Hero!
I use this as a library as well so I have a few options, but this is a grand compromise

@mquinnfd
Copy link
Contributor

mquinnfd commented Jun 6, 2024

Also recommending something like Colourblindly here for the log colours as the text itself is fairly narrow - don't wanna inadvertently make some entries harder to read

There a good write-up on this here (but you probably already know your way around this as a FE specialist 🙇)

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/log-improvements branch from 64a5662 to 62a7193 Compare June 6, 2024 11:23
@andrewbaldwin44
Copy link
Collaborator Author

andrewbaldwin44 commented Jun 6, 2024

Yeah Lighthouse works really well for testing contrast between colors, rather than just guessing by eye
image
I haven't been paying much attention to accessibility in this project but it's never a bad idea, I'll make the colors a little darker 👍

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/log-improvements branch from 62a7193 to e912793 Compare June 6, 2024 11:50
locust/runners.py Outdated Show resolved Hide resolved
@cyberw
Copy link
Collaborator

cyberw commented Jun 7, 2024

You know what would make this even more awesome? If you could make the headings for each worker the same color as the highest log level from that worker.

@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/log-improvements branch 2 times, most recently from a767486 to 0a83954 Compare June 7, 2024 12:48
@andrewbaldwin44 andrewbaldwin44 force-pushed the feature/log-improvements branch from 0a83954 to baff575 Compare June 7, 2024 13:15
@cyberw
Copy link
Collaborator

cyberw commented Jun 7, 2024

Send it?
image

@andrewbaldwin44
Copy link
Collaborator Author

Yup it's ready! 🚀

@cyberw cyberw merged commit 9095779 into locustio:master Jun 7, 2024
14 checks passed
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.

3 participants