Repository navigation
Conversation
9669136 to
b7c12b7
Compare
sbidoul
left a comment
There was a problem hiding this comment.
Thanks! A few thoughts after a first read.
| """Register gauges for a specific database in this channel.""" | ||
| self._unregister_metrics() | ||
| self._metrics_labels = { | ||
| "db": self.db_name, |
There was a problem hiding this comment.
Note to self: I think self.db_name can be None if this is the global ChannelManager. Test that this works.
There was a problem hiding this comment.
It renders db="None". Not ideal. So maybe __all__?
|
|
||
| def _capacity_gauge(self) -> float: | ||
| if self.capacity is None: | ||
| return math.inf |
There was a problem hiding this comment.
Note to self: test that prometheus client renders math.inf correctly
There was a problem hiding this comment.
It renders +Inf. Now need to see what prometheus and grafana do with that.
3ddb459 to
672ba4d
Compare
672ba4d to
09e5a53
Compare
|
Can you also add |
09e5a53 to
d77f62e
Compare
d77f62e to
35ce9a2
Compare
Late to the party here and not familiar with prometheus, but is it appropriate to hardcode a metrics collection agent and additional python dependencies into the main |
It looks like Prometheus metrics collected by the CloudWatch agent, so that removes some decent portion of my argument against. |
35ce9a2 to
4a6a3c7
Compare
|
@amh-mw on our wish list we have the extraction of the metrics in a separate module and the possibility to plug other observability tools such as opentelemetry. The state of this PR is still in draft. Your input are welcome nevertheless, and I agree with you that we might not want to enforce the usage of Prometheus. |
fbf36a8 to
0ce8156
Compare
abf7848 to
761804b
Compare
b11e72d to
e4c8ec6
Compare
e4c8ec6 to
d094cf6
Compare
d094cf6 to
a00f891
Compare
This adds a 'db' label to channel gauges to track which databases have jobs in each channel. This allows for better monitoring when multiple databases share the same channel structure.
Not sure that's useful. Let's wait for the actual use case.
8ea969e to
db858ba
Compare
This is based on #976