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

feature(monitoring): take screenshots with grafana image renderer #7503

Merged
merged 2 commits into from
May 31, 2024

Conversation

fruch
Copy link
Contributor

@fruch fruch commented May 28, 2024

  • switch to grafana image renderer
  • drop all of webdriver related implementation
  • remove grafana snapshots - it's not in use for quite some time

TODO:

  • - remove selenium from hydra image

Testing

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

@fruch fruch added test-provision-aws Run provision test on AWS test-provision-gce Run provision test on GCE test-provision-docker labels May 28, 2024
@fruch
Copy link
Contributor Author

fruch commented May 28, 2024

it's seems to be working, but one API keeps failing:

logger=context userId=0 orgId=1 uname= t=2024-05-28T20:36:19.657348907Z level=info msg="Request Completed" method=GET path=/api/user/orgs status=401 remote_addr=172.17.0.1 time_ms=1 duration=1.269817ms size=27 referer= handler=/api/user/orgs

it's adds one ugly banner at the top of the screenshot:
image

couldn't figure out why, @amnonh are you familiar with this ?

@fruch
Copy link
Contributor Author

fruch commented May 29, 2024

Provision tests didn't seem to generate the screenshot, needed to try manually to debug.

Prometheus snapshot on gce seems to be working with public address and failing (retrying for 30min)

@fruch fruch force-pushed the replace_wb_with_built_in_renderer branch 2 times, most recently from 89a7f2f to 6dc50f5 Compare May 29, 2024 13:47
@fruch
Copy link
Contributor Author

fruch commented May 29, 2024

V2:

  • added env variables to maximize the max height
  • remove the width height definition from dashboards

@fruch fruch force-pushed the replace_wb_with_built_in_renderer branch from 6dc50f5 to 943927d Compare May 29, 2024 13:49
@fruch
Copy link
Contributor Author

fruch commented May 29, 2024

a working example from provision test (it's not the full size of the dashboard, waiting for next run result that should be the full size):
https://cloudius-jenkins-test.s3.amazonaws.com/1883d5fb-24ee-4ced-a5ca-ffd85025de40/20240529_130043/grafana-screenshot-pr-7503-scylla-per-server-metrics-nemesis-20240529_130047-PR-provision-docker-PR-7503-monitor-node-1883d5fb-0.png

grafana-screenshot-pr-7503-scylla-per-server-metrics-nemesis-20240529_130047-PR-provision-docker-PR-7503-monitor-node-1883d5fb-0 (1)

@fruch fruch added the backport/none Backport is not required label May 29, 2024
@fruch fruch marked this pull request as ready for review May 29, 2024 13:52
Copy link
Contributor

@vponomaryov vponomaryov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if it works

@fruch fruch force-pushed the replace_wb_with_built_in_renderer branch from 943927d to 848aa58 Compare May 29, 2024 14:48
@fruch fruch force-pushed the replace_wb_with_built_in_renderer branch from 848aa58 to b291a3e Compare May 29, 2024 15:37
sdcm/cluster.py Outdated Show resolved Hide resolved
fruch added 2 commits May 29, 2024 23:18
- switch to grafana image renderer
- drop all of webdriver related implemetion
- remove grafana snapshots - it's not in use for quite some time
since we don't need it anymore for taking snapshots
we can remove it from the hydra image
@fruch fruch force-pushed the replace_wb_with_built_in_renderer branch from b291a3e to e2339bd Compare May 29, 2024 20:18
@fruch fruch added backport/6.0 and removed backport/none Backport is not required labels May 29, 2024
@fruch fruch requested review from vponomaryov and soyacz May 30, 2024 07:38
@fruch
Copy link
Contributor Author

fruch commented May 30, 2024

working on all backends, and also on ipv6

improves log collect times from 9-10m to ~2.5m

Copy link
Contributor

@vponomaryov vponomaryov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@soyacz soyacz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@soyacz
Copy link
Contributor

soyacz commented May 31, 2024

awesome change

@fruch
Copy link
Contributor Author

fruch commented May 31, 2024

awesome change

Throwing away code is satisfying

The sad part is it took me years to get to closing the loop on something I've asked years ago (thanks @amnonh)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/5.4-done Commit backported to 5.4 backport/5.4 Need backport to 5.4 backport/6.0-done Commit backported to 5.3 backport/6.0 backport/2023.1-done Commit backported to 2023.1 backport/2023.1 Need to backport to 2023.1 backport/2024.1-done Commit backported to 2024.1 backport/2024.1 Need backport to 2024.1 test-provision-aws Run provision test on AWS test-provision-docker test-provision-gce Run provision test on GCE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants