Include scheduler logs in performance_report

[mrocklin]

We might want to add a new tab in the performance report that includes the logs from the scheduler

[mrocklin]

@quasiben in case anyone on your team wants to add this

[quasiben]

@charlesbluca do you have time to look at this issue? If you do, that'd be great especially given recent experience with performance_report updates

[charlesbluca]

Sure! I can look into this

[charlesbluca]

I have a pretty simple implementation of this working:

Happy to open a PR for this, although a thought that crossed my mind while working on this is that scheduler (and worker) logs might be easier to incorporate into the performance reports if we migrate some of the Tornado app information (where scheduler/worker logs currently reside) into the Bokeh dashboard's "Info" tab instead of just redirecting. Based on the nested nature of the Tornado app, I thought this might be a good place to try out something like the nested HTML reprs @jacobtomlinson has been working on.

[mrocklin]

Woot. Hooray for the fast turnaround.

I know that this is just an early/rough draft, but two small comments (you may already have these in mind)

1. We probably don't want that much space given to the log level. Currently half of our pixels are not-very-informative
2. We might want to get clever and bold or redden error/warning/critical logs

If we're being bound by Bokeh's table plot type then I would encourage us to switch to generating raw HTML. My experience has been that Bokeh's tables don't provide much, while they do restrict creativity a bit. Generating raw HTML for tables is pretty easy.

[charlesbluca]

Are we okay with including internal CSS on the dashboard? I've found that a combo of that with Bokeh's HTMLTemplateFormatter is pretty good for providing the conditional cell formatting we'd want for different log levels. That being said, autosizing the columns based on cell content seems limited, and might not give the results we want:

I'm generally fine with generating raw HTML, my biggest concern there would be if that can be dynamically generated in case we want to have a streamed version of this widget in the dashboard itself. Is there any examples within the dashboard of general raw HTML for something like this?

[mrocklin]

my biggest concern there would be if that can be dynamically generated in case we want to have a streamed version of this widget in the dashboard itself

Yeah, right now I think that it makes sense to optimize for static HTML, either for the performance report, or for the dashboard info pages (we serve static log reports there as well)

99% of the time I want logs it's to look at something that has already happened, rather than to look at ongoing activity. I think that if removing the option to stream logs can give us a better reader experience then we should go ahead with it.

[mrocklin]

You may want to check out http://localhost:8787/info/main/logs.html to see the current state of the dump of logs that we have. I suspect that we could improve on this in the future (although I think that getting logs into the performance report is higher priority)

[charlesbluca]

Yeah that makes sense; maybe in addition to adding logs to the perf reports, we could also update logs.html to roughly match the perf report output. Here's a rough recreation of the Bokeh table, with color coding for different log levels:

[mrocklin]

Here's a rough recreation of the Bokeh table, with color coding for different log levels

Cool! Also, I encourage you to think outside the Bokeh box here. It's a fine styling to mimic, but by no means special. We can break out of / extend that style if we find it helpful. For one thing, we probably don't need to restrict the space of the table in the same way that Bokeh does.

[mrocklin]

For example, if using the table forces us to specify a fixed size then let's drop the table entirely and use <p> tags or something else.

[charlesbluca]

The sizing here is less of a table problem and more of a Bokeh app quirk that might be going over my head - in general, to add the raw HTML to the app, I've been wrapping it in a Div() object which typically would have sizing applied through the same sizing_mode param that we use in other Bokeh components; that doesn't seem to work as expected here.

I'm okay with dropping the idea of a log table, though if this is to get around the sizing issue that might just require some Bokeh debugging.

[mrocklin]

You're probably already looking at it, but the code in Scheduler.performance_report that generates the first page of the performance report might be a good model. That manages raw HTML and seems to avoid the sizing issue that you're running into.

[charlesbluca]

Inspecting the HTML on that page, it looks like it has the same behavior of my table, in that regardless of the sizing policy specified, the containing Div() will size itself to fit the widest element in the raw HTML; that doesn't really matter for the summary page where the contents are on a white background, but is more apparent on a table with colored rows.

In that case, it might be best to go with your suggestion, and use simple <p> tags (with some additional styling for warning/critical/error) to present the logs, avoiding any styling that would change the background color.

[charlesbluca]

Here's what something like that would look like:

[mrocklin]

I'm curious Charles, what are your thoughts? You're as likely as anyone here to have to look at logs. What looks good to you here and what looks bad?

…

On Fri, Jun 11, 2021 at 10:01 AM Charles Blackmon-Luca < ***@***.***> wrote: Here's what something like that would look like: [image: image] <https://user-images.githubusercontent.com/20627856/121706311-82ed4580-caa3-11eb-9e99-446fda2b9eeb.png> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4781 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACKZTCTSX6X2MA5H7GCMKDTSIQLXANCNFSM44BSY7BA> .

[charlesbluca]

From an appearance perspective, I find this approach a little bland, though that might be expected since this is just some slightly formatted terminal output. I personally feel like in the long run, output like this might look better within the context of a larger page with collapsible elements for the scheduler/workers, where all of their logs could be viewed maybe with some additional info. But for now this should be good, I think I would just want to make the styling for warning/critical/error logs more visually distinct from info logs, and maybe do something visually appealing with the log level (an icon to the left of the log, for example).

From a UX perspective, I could see this being a chore to navigate in cases where there is an exceptional amount of logs, as it is essentially equivalent to scrolling through the logs on the terminal. That was part of the reason I initially gravitated towards a Bokeh table, as that offered sorting out of the box along with the potential for being filterable with some extra work. This is definitely something we could do with raw HTML, but it would require some server-side JS that I'm not really sure we would want.

Currently, I've only been testing these changes with trivial commands that generate minimal logs. I will try running this with more complicated scripts that generate logs closer to what the average user would have to look at.

[mrocklin]

I agree with you about the bland nature. I notice that you're using bold/italics for different levels. It might be worth adding a little bit of inline CSS to the <p> tags to add colors. I also think that we could maybe use some other tag in order to reduce spacing between the lines. I think that that might help things to feel more dense.

I agree that filtering would be good, but I also agree unfortunately that I don't know a good way to do this without JS.

cc'ing also @jacobtomlinson who might have suggestions around the right way to display these.

[charlesbluca]

Yeah, I think colors work well here, just need to pick some that have decent readability on a white background. Spacing is also easy enough to adjust:

[mrocklin]

That looks pretty good to me? Shall we try merging that in and then leaving improvement for a later PR?

[charlesbluca]

Sounds good to me! Opened in #4909