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

Prepare for Monitoring with Prometheus #344

Merged
merged 6 commits into from
Oct 26, 2020
Merged

Prepare for Monitoring with Prometheus #344

merged 6 commits into from
Oct 26, 2020

Conversation

cimnine
Copy link
Collaborator

@cimnine cimnine commented Oct 18, 2020

Related Issue: n/a

New Behavior

Adds the foundations for monitoring with Prometheus, which is the built-in monitoring solution for Netbox.
The complete setup does not ship by default and has to be configuration by the user.
Such a system configuration is now documented in our wiki: https://github.com/netbox-community/netbox-docker/wiki/Monitoring.

Contrast to Current Behavior

Some files have to be overwritten (i.e. gunicorn_config.py and nginx.conf) in order to get proper monitoring ready.
This functionality is now backed-in and ready to be used.

Discussion: Benefits and Drawbacks

The main question is: Do we want to take on the additional maintenance work?

By moving most of the components to the wiki they are no longer part of the main distribution.
Therefore we don't have to maintain and test the monitoring as part of our release process.

Changes to the Wiki

A new page has been set-up: https://github.com/netbox-community/netbox-docker/wiki/Monitoring

Proposed Release Note Entry

Preparations for Monitoring with Prometheus #344

This project does now contain all changes that would be necessary to monitor Netbox Docker using Prometheus.
See https://github.com/netbox-community/netbox-docker/wiki/Monitoring for a sample setup.

Double Check

  • I have read the comments and followed the PR template.
  • I have explained my PR according to the information in the comments.
  • My PR targets the develop branch.

@cimnine cimnine added the enhancement The issue describes an enhancement that we would like to implement in the future. label Oct 18, 2020
@madnutter56
Copy link

I think that this somewhat reduces the value of this repository, in that it turns on prometheus/grafana by default. Environments that already have monitoring infrastructure set up and scaled will have issues with this setup, and there's no good way to override the extra stuff in compose.override.

I would suggest having override templates that include the additional containers to enable this, but not enabling it in the default compose.

@cimnine
Copy link
Collaborator Author

cimnine commented Oct 19, 2020

Thank you for your feedback @madnutter56. What do you think of adding these services in an additional docker-compose file, like docker-compose.monitoring.yml instead of the main docker-compose.yml?

One could then start the monitoring part separately, like docker-compose -f docker-compose.monitoring.yml up
Or if one wants to start everything together docker-compose -f docker-compose.yml -f docker-compose.monitoring.yml up would work.

@madnutter56
Copy link

Part of me wonders if it should be a separate repository entirely? Running the service and monitoring are distinctly different services after all. I'm not sure though, I think multiple compose files seems like a reasonable workaround.

@cimnine
Copy link
Collaborator Author

cimnine commented Oct 26, 2020

I think finally I'll just keep the changes to the nginx configuration in this PR and move everything else to a new wiki page.

@cimnine cimnine added this to the 0.25.0 milestone Oct 26, 2020
@cimnine cimnine requested a review from tobiasge October 26, 2020 14:29
@cimnine
Copy link
Collaborator Author

cimnine commented Oct 26, 2020

The scope of this PR was reduced. Most of the cleverness was moved to https://github.com/netbox-community/netbox-docker/wiki/Monitoring. The PR description was updated accordingly.

Copy link
Member

@tobiasge tobiasge left a comment

Choose a reason for hiding this comment

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

I would squash those commit, because I don't think the commit history is important in our develop / release branches.
Otherwise it good to merge.

@cimnine cimnine changed the title Adds Prometheus/Grafana monitoring infrastructure Prepare for Monitoring with Prometheus Oct 26, 2020
@cimnine cimnine merged commit e51f9cb into develop Oct 26, 2020
@cimnine cimnine deleted the Prometheus branch October 26, 2020 14:58
@cimnine cimnine modified the milestones: 0.25.0, 0.26.0 Oct 26, 2020
@cimnine cimnine added the hacktoberfest-accepted We think that such a PR is sufficiently good contribution, even if we don't merge it (yet). label Oct 26, 2020
@cimnine cimnine mentioned this pull request Oct 26, 2020
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue describes an enhancement that we would like to implement in the future. hacktoberfest-accepted We think that such a PR is sufficiently good contribution, even if we don't merge it (yet).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants