prometheus / prometheus

The Prometheus monitoring system and time series database.
https://prometheus.io/
Apache License 2.0
55.25k stars 9.09k forks source link

Add visual emphasis to hovered-over lines in graphs #10133

Open alisakotliarova opened 2 years ago

alisakotliarova commented 2 years ago

This idea stems from the open (as of Jan 6 2022) issue about some colors not being visible in the React UI when it comes to graphs.

When a line in the graph is hovered over, there would be added emphasis such as

Why is this important?

This ensures that despite potential issues with generated colors being not visible enough against the background, colors being too similar in lines, or having a large amount of lines, a line can still become clearly visible if someone hovers over it.

This is very important for accessibility, as simply having a better color palette may not be enough for those with color blindness or other vision problems.

juliusv commented 2 years ago

Regarding https://github.com/prometheus/prometheus/issues/8256#issuecomment-1007518397: Correct, Flot (https://github.com/flot/flot, the charting library we use) uses <canvas> to draw things, so we basically have to go through Flot's plugin and event handling system to change things in there. You could register an event handler for the plothover event on a series like we do for the plotclick and plotselected events here: https://github.com/prometheus/prometheus/blob/931acc3ee8f0f4e0eac68b4a2c4d880df7d82944/web/ui/react-app/src/pages/graph/Graph.tsx#L109-L132. When hovering over a series, the third argument to the event handler will contain some information about the series (you'll have to check out with console.log(...) etc. what exactly is in there and how to correlate it with our series data).

I'd imagine you could then somehow modify the chart data based on the hover info somehow, but I'm not sure if we already make it easy to access a Flot series' line width. I see some mentions of width on http://www.flotcharts.org/flot/API/, but haven't looked closer yet, to be honest.

alisakotliarova commented 2 years ago

@juliusv Thank you so much! This helped me a lot.
So far I've successfully been able to add the event handler in the way you described which calls this function I added to prometheus/web/ui/react-app/src/pages/graph/GraphHelpers.ts.

Currently this is able to make the line that is being hovered over bolder than the others.

export const emphasizeLine = (chart: jquery.flot.plot, item: any): void => {
  const defaultLineWidth = chart.getOptions().series.points.lineWidth;
  const boldLineWidth = defaultLineWidth + 3;
  // Revert any lines that are currently emphasized
  chart.getData().forEach((line) => {
    if (line.lines) {
      line.lines.lineWidth = defaultLineWidth;
    }
  });
  // If a line is being hovered over, emphasize it
  if (item) {
    item.series.lines.lineWidth = boldLineWidth;
  }
  chart.draw();
};

The code design & organization is not ideal but it runs smoothly. For example, defaultLineWidth and boldLineWidth probably don't need to be defined every time this is run but I'm not 100% sure on where it should be moved to so I'm leaving it as is for now.

Next I'm going to work on figuring out how to add a shadow/highlight to the emphasized line.

alisakotliarova commented 2 years ago

I've come to a crossroads with this issue and I'm not sure what to do. The lines in the graphs are all part of a single HTML canvas element. It is possible to add a shadow to a line drawn in a canvas element, but I would have to add the shadow after the line has been started to be drawn and prior to the finalization of the line being drawn. In our case, both of these actions are done in the jquery.flot.js file. I'm not sure if I would be able to do this outside of the jquery.flot.js file unless I write a lot of redundant code.

I will continue to mull it over this weekend, perhaps I can draw a transparent line with a shadow over the "emphasized" line? There's a bit of math that's done in jquery.flot.js so again this may still add some redundancy. ¯_(ツ)_/¯

juliusv commented 2 years ago

@alisakotliarova Oh that's great that you already got the line width working! One thing to test is whether it still feels roughly as fast with that change as without, for a graph that shows ~100 series or so. We want to be careful about introducing anything that slows things down too much, but hopefully it'll be fine.

As for the extra emphasis, I agree that's more tricky. If we have to actually do custom canvas operations (not through Flot), I would tend to say that the extra code complexity would not be worth it. But emphasis through line width (and maybe highlighting the series color somewhat as well) might be enough?

alisakotliarova commented 2 years ago

@juliusv Thank you! I have a couple of updates/responses to your comment.

I noticed that there is actually a mechanism that emphasizes lines already. It's triggered when you hover over a series from the list that appears below the graph. It appears that this emphasis makes the line corresponding to the series be the topmost line and it dims the other lines in the graphs. As it is, this is certainly helpful for finding a line (especially when multiple lines are identical) but I still find myself struggling to find the highlighted line sometimes. So I think it would still be helpful to add bolding to the lines. My next goal is to try to track down this behavior (in the code) so I can add line-boldening to it and see if I can trigger it when the line is being hovered over as well.

As far as testing with ~100 series, do you have any pointers on how I can test that? I honestly have never used Prometheus before so I don’t have anything readily available to test with, I’ve just been using the setup I made with the tutorial which only shows a few lines.

juliusv commented 2 years ago

I noticed that there is actually a mechanism that emphasizes lines already. It's triggered when you hover over a series from the list that appears below the graph.

Right! Forgot about that :)

So I think it would still be helpful to add bolding to the lines. My next goal is to try to track down this behavior (in the code) so I can add line-boldening to it and see if I can trigger it when the line is being hovered over as well.

That behavior is at https://github.com/prometheus/prometheus/blob/2f4289a3bfaca725a14f41e30f1c91de92936242/web/ui/react-app/src/pages/graph/Graph.tsx#L187-L197 and https://github.com/prometheus/prometheus/blob/2f4289a3bfaca725a14f41e30f1c91de92936242/web/ui/react-app/src/pages/graph/GraphHelpers.ts#L70-L78

It just sets the color, so maybe it's doable to add setting the width in there as well.

As far as testing with ~100 series, do you have any pointers on how I can test that? I honestly have never used Prometheus before so I don’t have anything readily available to test with, I’ve just been using the setup I made with the tutorial which only shows a few lines.

Yes, you can set your Prometheus config to this, scraping some public demo service instances:

global:
  scrape_interval: 5s

scrape_configs:
- job_name: 'demo'
  static_configs:
    - targets:
      - 'demo.promlabs.com:10000'
      - 'demo.promlabs.com:10001'
      - 'demo.promlabs.com:10002'

...and then you could query for example for:

rate(demo_api_request_duration_seconds_bucket[5m])`

That gives you 702 series.

alisakotliarova commented 2 years ago

@juliusv Thank you for the help! I ran it with that and it's working just fine on my end.

Let me know if there's a better place to ask about this but how can I make branch off of main? I tried making one called alisakotliarova/bolden-graph-lines so I could create a PR and I'm getting a permissions denied error. Based on the Contributing doc I got the impression that I should be able to just make one so I'm not sure what I would be doing wrong. Thanks :)

juliusv commented 2 years ago

Thank you for the help! I ran it with that and it's working just fine on my end.

Great!

Let me know if there's a better place to ask about this but how can I make branch off of main?

If you're getting a permissions error, I guess you tried creating one in the main prometheus/prometheus repo directly on GitHub? That repo is only writeable by Prometheus team members, so the usual contribution flow (if you're not yet on team) is to fork the repo first into your own GitHub account and then create the branch in there. And then later, create a pull request back to prometheus/prometheus from there. Let me know if I can help with any of that!

alisakotliarova commented 2 years ago

@juliusv Thank you! I've submitted a PR with a question about unit testing.

beorn7 commented 2 months ago

Hello from the bug scrub!

@juliusv the attempt to address this stalled a long time ago. What's the state of this in the new UI? Does it still make sense? Is it already addressed?