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

Base Prometheus metrics handler #1830

Merged
merged 5 commits into from Oct 2, 2022
Merged

Conversation

AlekseyLobanov
Copy link
Contributor

@AlekseyLobanov AlekseyLobanov commented Jul 27, 2022

I want to add some standard metrics to navidrome. Prometheus metrics are useful for observability (including alerts).
I believe that many simple PRs are much better than 1 full.

What this PR has:

  1. Two options for enable/disable (default for compatibility) metrics and their path
  2. Standard prometheus handler with default golang metrics like process_start_time_seconds, go_threads and process_cpu_seconds_total
  3. Does not consume resources if disabled.

I believe that metrics path is best approach, compare to different port or some auth, including HTTP Basic Auth:

  1. If navidrome works behind reverse proxy, closing /metrics is the most simple way to protect them
  2. Otherwise, metrics path like /metrics_super_secret_path is simple to use and secure enough. The most serious problem in this case is plain HTTP connection over not secure connection which is not secure anyway.

This relates to #1548 , but not solves initial request for now

image

cmd/root.go Outdated Show resolved Hide resolved
@deluan
Copy link
Member

deluan commented Sep 27, 2022

Hey, sorry for the long delay! LGTM, just need to rebase with latest from master and I'll merge it right away. Thanks!

@AlekseyLobanov
Copy link
Contributor Author

Branch is now actual

@deluan deluan merged commit 552989a into navidrome:master Oct 2, 2022
@deluan
Copy link
Member

deluan commented Oct 3, 2022

Thanks!

@anarcat
Copy link

anarcat commented Oct 16, 2022

hi! are there any plans to expand this to (say) count the number of users, albums, artists, tracks, etc?

@djjudas21
Copy link

Thanks for the addition. How is this feature enabled with env vars? I need to add support for metrics to the helm chart but I'm not sure how the Go options correspond to env vars. Is prometheus.enabled equivalent to ND_PROMETHEUS_ENABLED?

@AlekseyLobanov
Copy link
Contributor Author

AlekseyLobanov commented Oct 16, 2022

How is this feature enabled with env vars? I need to add support for metrics to the helm chart but I'm not sure how the Go options correspond to env vars. Is prometheus.enabled equivalent to ND_PROMETHEUS_ENABLED?

I should add this to docs repository, definitely.

To enable metrics:

  1. Define variable ND_PROMETHEUS_ENABLED=true
  2. (optional) Use custom metrics path ND_PROMETHEUS_METRICSPATH=/metrics_secret_key

UPD: PR with documentation updates: navidrome/website#96

@AlekseyLobanov
Copy link
Contributor Author

hi! are there any plans to expand this to (say) count the number of users, albums, artists, tracks, etc?

Of course!

It was my first PR to this project (and first PR to golang project), I tried to create it as small as possible. It has only required dependencies and configuration layer required by Prometheus to run.

Next step is adding navidrome-specfic metrics. I have plans to do this in near future.

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants