scylladb / argus

Apache License 2.0
4 stars 11 forks source link

improvement(graphs): Mark dependency changes #502

Closed soyacz closed 4 days ago

soyacz commented 1 week ago

In performance tests it's important to note changes of dependencies, like kernel, drivers, stress tools and SUT version.

This commit changes the way point is displayed: when there's package change (except SUT change) then the point gets a white background. Hovering over the point shows tooltip with all packages changes (including SUT).

Tooltip position was adjusted to be show above the point so mouse is not covering text.

closes: https://github.com/scylladb/argus/issues/490

image

fruch commented 1 week ago

@soyacz

can we add the the full scylla release as well ?

soyacz commented 1 week ago

@soyacz

can we add the the full scylla release as well ?

We can. See also that kernel changes almost every week, I wonder if we shouldn't mark only big changes like major version updates only (I mean still change will be shown, but point won't be marked).

fruch commented 1 week ago

@soyacz can we add the the full scylla release as well ?

We can. See also that kernel changes almost every week, I wonder if we shouldn't mark only big changes like major version updates only (I mean still change will be shown, but point won't be marked).

before I want to notice the change, we want to see them. then we can indicate there was a change on the inside the popup and last we can decide what to indicate on the graph

one more thing, not for this PR, we should be able to change the x axis to something else like driver version, or just plain data of the run, cause we don't have version change or date in the drivers CI (at least not yet, we didn't thought of it yet)

soyacz commented 1 week ago

before I want to notice the change, we want to see them. then we can indicate there was a change on the inside the popup and last we can decide what to indicate on the graph

Summarizing: Ok, any change in dependencies (except SUT) will be marked with different point color. All the changes, including SUT will be shown in tooltip. If this won't fit us well in the wild, we can adjust.

one more thing, not for this PR, we should be able to change the x axis to something else like driver version, or just plain data of the run, cause we don't have version change or date in the drivers CI (at least not yet, we didn't thought of it yet)

Generally, SUT is detected automatically based on which dependency changes the most often. If driver version/build date will be changing the most often then it becomes the SUT. This feature is used e.g. in SM tests. We can adjust it in followup issues if we need something else.

fruch commented 1 week ago

before I want to notice the change, we want to see them. then we can indicate there was a change on the inside the popup and last we can decide what to indicate on the graph

Summarizing: Ok, any change in dependencies (except SUT) will be marked with different point color. All the changes, including SUT will be shown in tooltip. If this won't fit us well in the wild, we can adjust.

one more thing, not for this PR, we should be able to change the x axis to something else like driver version, or just plain data of the run, cause we don't have version change or date in the drivers CI (at least not yet, we didn't thought of it yet)

Generally, SUT is detected automatically based on which dependency changes the most often. If driver version/build date will be changing the most often then it becomes the SUT. This feature is used e.g. in SM tests. We can adjust it in followup issues if we need something else.

not sure automatic would fit the best, consider a case we run CI with multiple release of scylla, let say OSS and enterprise ?

soyacz commented 1 week ago

run CI with multiple release of scylla, let say OSS and enterprise

package name for both is scylla-server so it works (just both anyway are separated for different releases on the graph)

fruch commented 1 week ago

run CI with multiple release of scylla, let say OSS and enterprise

package name for both is scylla-server so it works (just both anyway are separated for different releases on the graph)

in driver CI, we care to see the driver version change / commit, not necessarily scylla, in my example both driver, and scylla might be changing across time

soyacz commented 1 week ago

run CI with multiple release of scylla, let say OSS and enterprise

package name for both is scylla-server so it works (just both anyway are separated for different releases on the graph)

in driver CI, we care to see the driver version change / commit, not necessarily scylla, in my example both driver, and scylla might be changing across time

I doubt in testing performance while changing both dependencies at the same pace, anyway, we can adjust to whatever we need later.

soyacz commented 1 week ago

@fruch I adjusted the code, please see PR description to see the changes.

soyacz commented 1 week ago

@fruch I adjusted things we agreed yesterday: SUT version only, error threshold instead 'limit'. Also I did various fixes for tooltip: enlarge font, make it be above mouse cursor (so it does not cover text).

soyacz commented 1 week ago

@k0machi this one is unrelated to highlights (discussion) view widget and is relatively small. I propose to review it first so it can be merged and used.

soyacz commented 5 days ago

@k0machi ping