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

Adding Benchmark section to our documentation page #3125

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

gligorisaev
Copy link
Contributor

@gligorisaev gligorisaev commented Sep 20, 2024

Proposed changes

This PR provides performance benchmarks for ThinEdge.io, including detailed CPU and memory usage for key processes. The metrics are measured on a device rackfslot1 running in the OSADL QA Farm.

  • Included are also the Munin plugin files which are located on the OSADL QA Farm at /etc/munin/plugins/

  • The tedgecpuprocent plugin is up and running, the tedgemonitorcpu is still in draft and under work because I am trying to trigger the needed condition to raise an alarm, commiting it here only because somebody can maybe see some problem, improvement, etc. to the code.

  • Prepared script for downloading the needed .svg files which are showing the graphs on the documents page.

  • Included is also the workflow file which should trigger automatic download of the files once per month on month beggining

TODO

  • Decision needed for the final location of the downloading script
  • adding the munin username and password to GIT Secrets
  • Decision needed on which repo the workflow will be located and automaticaly trigerred
  • Decision needed what should be the workload we want to measure and show in the benchmark

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s)
  • I ran cargo fmt as mentioned in CODING_GUIDELINES
  • I used cargo clippy as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@gligorisaev gligorisaev changed the title Adding Benchmard section to our documentation page Adding Benchmark section to our documentation page Sep 20, 2024
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Additional details and impacted files

📢 Thoughts on this report? Let us know!

Copy link
Contributor

Choose a reason for hiding this comment

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

This file has to be under ci/build_scripts and not under yet another top level directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The script was at that location, but if I understand right @reubenmiller, he proposed not to be there so thats why I have it in the To Do section as needed to be decided

Copy link
Contributor

@reubenmiller reubenmiller Sep 20, 2024

Choose a reason for hiding this comment

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

Though I'm pretty sure we can reduce some dependencies (e.g. requests which is not available by default in python) and just use a simple shell script which just depends on wget...

wget --user "$MUNIN_USERNAME" --password "$MUNIN_PASSWORD" https://munin.osadl.org/munin/osadl.org/rackfslot1.osadl.org/tedgecpuprocent-month.svg
wget --user "$MUNIN_USERNAME" --password "$MUNIN_PASSWORD" https://munin.osadl.org/munin/osadl.org/rackfslot1.osadl.org/memory-month.svg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@reubenmiller you are right, I changed the downloading script according you comment

sidebar_position: 10
---

# Benchmarks
Copy link
Contributor

Choose a reason for hiding this comment

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

A key point is missing: what is the workload used for these benchmarks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree fully, I put it in the ToDo section as decision needed what we want to be the workload


![Memory Usage](./memory-month.svg)

### Monitored Metrics:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see these metrics as useful for giving an idea of the memory footprint of thin-edge, since there are not directly related.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, there are other metrics which just look at the memory for each of the thin-edge.io related memory usage, e.g. https://munin.osadl.org/munin/osadl.org/rackfslot1.osadl.org/tedgemem.html

@gligorisaev gligorisaev marked this pull request as draft September 20, 2024 11:16

Below is a visual representation of the memory consumption on the device over the past month.

![Memory Usage](./memory-month.svg)
Copy link
Contributor

Choose a reason for hiding this comment

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

The memory script should only show the thin-edge.io applications and not the whole OS memory usage etc.

Signed-off-by: gligorisaev <[email protected]>
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.

3 participants