Skip to content

Add custom metrics exporter#1444

Merged
theCalcaholic merged 5 commits intodevelfrom
feature/custom-prometheus-exporter
Apr 8, 2022
Merged

Add custom metrics exporter#1444
theCalcaholic merged 5 commits intodevelfrom
feature/custom-prometheus-exporter

Conversation

@theCalcaholic
Copy link
Copy Markdown
Collaborator

@theCalcaholic theCalcaholic commented Mar 24, 2022

Include custom Prometheus metrics exporter from https://github.com/theCalcaholic/ncp-metrics-exporter. Currently supports only backup monitoring.

TODO:

  • metrics.sh: Install and manage ncp-metrics-exporter
  • Add template ncp-metrics.conf.sh
  • nextcloud.conf.sh: Add endpoint /metrics/ncp
  • Docker support (including existing metrics)
    • Prevent installation on debian 10 + docker
  • [ ] Integrate local prometheus and grafana services as an option (feasibility evaluation pending)

Testing

I'm testing the app by installing it into an ncp instance (either via ncp-update feature/custom-prometheus-exporter or by directly using the curl installer with this branch) and then activating it via ncp-config. After a backup has been created (e.g. via nc-backup), it should appear in https:///metrics/ncp and https:///metrics/system should be populated.

TODO:

  • docker
    • x86_64
      • debian 10
      • debian 11
    • i386
    • arm
    • arm64
  • VM / bare metal
    • x86_64
      • debian 10
      • debian 11
    • i386
    • arm64
      • debian 10
      • debian 11
    • arm
      • debian 10
      • debian 11

@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch from 284b370 to 5cb2c82 Compare March 24, 2022 17:12
@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch 3 times, most recently from 21368c1 to 5be2a79 Compare March 28, 2022 23:00
@theCalcaholic theCalcaholic marked this pull request as ready for review March 28, 2022 23:00
@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch 3 times, most recently from fcd96ff to c9009eb Compare March 28, 2022 23:36
@theCalcaholic
Copy link
Copy Markdown
Collaborator Author

I have now squashed all intermediate commits so the remaining ones are meaningful. As far as I'm concerned, the PR is now ready to be reviewed/merged (I'll probably move the local prometheus/grafana service feature to its own PR, as that's again a lot of work).

Copy link
Copy Markdown
Member

@nachoparker nachoparker left a comment

Choose a reason for hiding this comment

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

why are we touching backups and nc-datadir here? how is it affected by metrics?

Comment thread bin/ncp/BACKUPS/nc-backup-auto.sh Outdated
@theCalcaholic
Copy link
Copy Markdown
Collaborator Author

theCalcaholic commented Mar 30, 2022

why are we touching backups and nc-datadir here? how is it affected by metrics?

The new metrics exporter adds backup monitoring. For that to work I need the paths to the backup locations which are stored in a config file at /usr/local/etc/ncp-metrics.cfg. I added a hook to all of the scripts that change backup paths which triggers the regeneration of the config file and a restart of the custom metrics exporter.

@nachoparker
Copy link
Copy Markdown
Member

understood, LGTM then. Feel free to merge to devel if it has been tested in both VM and docker. I normally use the tag_and_push script fyi

@theCalcaholic
Copy link
Copy Markdown
Collaborator Author

theCalcaholic commented Mar 31, 2022

Good to know. I didn't notice that script yet :)

I already tested it in docker and VM, but I want to change it to use 64bit binaries on 64bit architectures. That makes the install script less complicated (no dependencies on different apt packages on different archs) and thus makes ncp instances more consistent and I think that's preferable on the long run.

Also I like the approach you took with the notify_push binaries: to download all binaries and just use the correct one. I'll adjust it to do the same with ncp-metrics-exporter :)

@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch 3 times, most recently from bb53e99 to ef3e60b Compare April 8, 2022 07:34
Signed-off-by: Tobias K <6317548+theCalcaholic@users.noreply.github.com>
Signed-off-by: Tobias K <6317548+theCalcaholic@users.noreply.github.com>
Signed-off-by: Tobias K <6317548+theCalcaholic@users.noreply.github.com>
@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch 7 times, most recently from d4d7397 to 959f833 Compare April 8, 2022 10:05
Signed-off-by: Tobias K <6317548+theCalcaholic@users.noreply.github.com>
@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch from 959f833 to d21c417 Compare April 8, 2022 12:55
@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch from d21c417 to 1ce2135 Compare April 8, 2022 13:04
- Upgrade ncp-metrics-exporter to v1.1.0
- Install prometheus-node-exporter-collectors when dist-upgrading from buster

Signed-off-by: Tobias K <6317548+theCalcaholic@users.noreply.github.com>
@theCalcaholic theCalcaholic force-pushed the feature/custom-prometheus-exporter branch from 1ce2135 to d42a0c8 Compare April 8, 2022 13:07
@theCalcaholic theCalcaholic merged commit 2fbb82d into devel Apr 8, 2022
@delete-merged-branch delete-merged-branch Bot deleted the feature/custom-prometheus-exporter branch April 8, 2022 13:09
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.

2 participants