-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(prometheus): Prometheus Scraper service #1838
Conversation
Co-authored-by: Aaron van Meerten <aaron.van.meerten@gmail.com>
Changed according to the PR review - added the PROSODY_METRICS_ALLOWED_CIDR variable - arranged the variable in alphabetic order
Added 2 environment variable for prosody metrics collection - PROSODY_ENABLE_METRICS - PROSODY_METRICS_ALLOWED_CIDR
prometheus/prometheus.yml
Outdated
static_configs: | ||
- targets: ["jvb:8080"] | ||
|
||
- job_name: "prometheus-jicofo" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to have separate jobs for each target?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes separation of concern and better individual service configuration. Suppose, we want to add authorization/authentication for one of the components or change the scrape_interval
then it would be easier to maintain.
We can have a single job named prometheus
and have all the targets listed in there. Then we are required to create a common config with all the features listed for each and every service.
Whatever way we choose, it should be best for future use and should have ease of modification.
prometheus.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow setting restart policy from outside like using an env variable - see etherpad.yml - I would go with on-failure:2 by default https://docs.docker.com/config/containers/start-containers-automatically/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated the restart policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot see it in code
prometheus/prometheus.yml
Outdated
@@ -0,0 +1,9 @@ | |||
scrape_configs: | |||
# - job_name: "prometheus" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the code commented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I pushed the code that I was testing. Would change it and push it again.
Prometheus service restart policy
* 29a4523 feat(jvb) add JVB_CC_TRUST_BWE environment variable * 0f7be54 feat(prometheus): Prometheus container and basic scraping configuration (#1838) * 9c87bba feat(jitsi-meet): added grafana loki and otel integration for log analysis (#1844) * fcb90ba fix(web): whiteboard config.js syntax error (#1851) * 811518b misc: working on unstable
Added Prometheus Container for Metrics Collection
Summary:
prometheus.yml
file.Details:
docker-compose -f docker-compose.yml -f prometheus.yml up
.prometheus/prometheus.yml
, which includes scrape configurations defining targets and the service name.Next Steps:
Testing:
http://localhost:9090
.Contributor - @24kushang