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

improvement(graphs): Mark dependency changes #502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

soyacz
Copy link
Collaborator

@soyacz soyacz commented Nov 13, 2024

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: #490

image

@soyacz soyacz requested review from fruch and k0machi November 13, 2024 15:44
@soyacz soyacz self-assigned this Nov 13, 2024
@soyacz soyacz force-pushed the annotate-dependency-changes-in-graphs branch from 3f468b3 to 4f32a30 Compare November 13, 2024 15:53
@fruch
Copy link
Contributor

fruch commented Nov 13, 2024

@soyacz

can we add the the full scylla release as well ?

@soyacz
Copy link
Collaborator Author

soyacz commented Nov 14, 2024

@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
Copy link
Contributor

fruch commented Nov 14, 2024

@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
Copy link
Collaborator Author

soyacz commented Nov 14, 2024

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.

@soyacz soyacz marked this pull request as draft November 14, 2024 10:43
@fruch
Copy link
Contributor

fruch commented Nov 14, 2024

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
Copy link
Collaborator Author

soyacz commented Nov 14, 2024

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
Copy link
Contributor

fruch commented Nov 14, 2024

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
Copy link
Collaborator Author

soyacz commented Nov 14, 2024

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.

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: scylladb#490
@soyacz soyacz force-pushed the annotate-dependency-changes-in-graphs branch from 4f32a30 to 5fe45ee Compare November 14, 2024 13:35
@soyacz soyacz marked this pull request as ready for review November 14, 2024 13:35
@soyacz
Copy link
Collaborator Author

soyacz commented Nov 14, 2024

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

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.

Automatically annotate performance graphs with dependency changes
2 participants