-
Notifications
You must be signed in to change notification settings - Fork 20
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
update tf charm to include metrics endpoint - Prometheus #133
Conversation
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.
You'll see that I also pushed a few changes in here to make the tests pass again, and add the missing charm libraries. There's one workflow still failing, but that one fails because you pushed this to your repo instead of the canonical one. Just make sure to push it to the canonical one next time
@@ -7,3 +7,7 @@ options: | |||
default: 10 | |||
description: Number of seconds to wait for keepalive connections | |||
type: int | |||
api_port: |
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.
Remove this section, we don't need api_port in the config, and it was actually recommended that we do not do this. Hardcoding things like this may seem unusual, but there's really no reason for it to change, and removing the option reduces the chances for config mismatches.
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.
No problem
charm/src/charm.py
Outdated
jobs=[ | ||
{ | ||
"static_configs": [ | ||
{"targets": [f"*:{self.config['api_port']}"]} |
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.
make sure to also remove the config item here
charm/src/charm.py
Outdated
@@ -80,7 +94,7 @@ def _require_nginx_route(self): | |||
charm=self, | |||
service_hostname=self.config["external_hostname"], | |||
service_name=self.app.name, | |||
service_port=5000, | |||
service_port=self.config["api_port"], |
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.
...and here
charm/src/charm.py
Outdated
@@ -162,7 +176,7 @@ def _pebble_layer(self): | |||
"--keep-alive", | |||
keepalive, | |||
"--bind", | |||
"0.0.0.0:5000", | |||
f"0.0.0.0:{self.config['api_port']}", |
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.
...and here
Description
Expose Prometheus metrics endpoint for COS lite
Resolved issues
https://warthogs.atlassian.net/browse/CERTTF-203
Tests
Tested via Juju / microK8s
Added "api_port" to config.yaml to make the multiple references to this API port dynamic, helpful with add'l references for the metrics endpoint.