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

Added prometheus integration #350

Merged
merged 5 commits into from
Apr 15, 2024
Merged

Added prometheus integration #350

merged 5 commits into from
Apr 15, 2024

Conversation

Forest-Troll
Copy link

@Forest-Troll Forest-Troll commented Apr 9, 2024

Added prometheus client
Added scheduler for compute current spool metrics and info

Default endpoint host:port/metrics/

Dmitry Belousov added 2 commits April 9, 2024 15:25
Added scheduler for compute current spool metrics and info
@Donkie
Copy link
Owner

Donkie commented Apr 13, 2024

Good idea!

  1. You've updated all dependencies for the project, which could have unforeseen consequences, and is outside the scope of this PR. Please only add the prometheus dependencies and don't touch the others.
  2. I think this should be an opt-in thing behind an environment variable, since not many people will use it and it would just sit every minute consuming resources for no reason.
  3. It seems it only works if the path is /metrics/ but not for /metrics, think you could fix that?
  4. Please install Black to format the files correctly.

@jangrewe
Copy link

Just chiming in that i'd highly appreciate this, as i use Prom/Grafana heavily to monitor my printers - and being able to integrate the filament usage would be awesome!

Dmitry Belousov added 3 commits April 14, 2024 22:59
Rollback pyproject.toml
added env variable SPOOLMAN_METRICS_ENABLED for enabled database collector

set metrics path as /metrics

checked Null values at vendors, init weight and price
removed some logs
@Forest-Troll
Copy link
Author

Remove this 2nd logger line, not useful.

Done


This will now only get run if automatic backups are enabled and the database is a sqlite one, which is irrelevant of metrics collection.

Run with SPOOLMAN_METRICS_ENABLED=TRUE env variable only


These two logger lines serve better as "debug" level and not info level.

Set debug level


Change this to async for session in get_db_session(): and you don't need to create your own get_session function.

Done


Must handle price potentially being None
Must handle row.vendor.name and row.weight potentially being None

Handles added, may be need moved to models, but i made it at metrics.py


Good idea!

  1. You've updated all dependencies for the project, which could have unforeseen consequences, and is outside the scope of this PR. Please only add the prometheus dependencies and don't touch the others.
  2. I think this should be an opt-in thing behind an environment variable, since not many people will use it and it would just sit every minute consuming resources for no reason.
  3. It seems it only works if the path is /metrics/ but not for /metrics, think you could fix that?
  4. Please install Black to format the files correctly.
  1. I am used last pdm, and it change format of lockfile. Indicated pdm version at README.
  2. Added SPOOLMAN_METRICS_ENABLED, default FALSE. But you can add some application metrics by functions, if needed.
  3. Change url, with some WA.
  4. Installed and formated.

PPS: Unfortunately i'm not true developer (python and other languages). Thanks for understanding.

@Donkie
Copy link
Owner

Donkie commented Apr 15, 2024

Great updates! All items resolved. Thanks for the contribution :)

@Donkie Donkie merged commit 92a5868 into Donkie:master Apr 15, 2024
12 checks passed
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.

None yet

3 participants