search

canonical / prometheus-scrape-config-k8s-operator

https://charmhub.io/prometheus-scrape-config-k8s
Apache License 2.0
1 stars 1 forks source link

Comment about relation_joined out of date? #14

Closed ca-scribner closed 2 years ago

ca-scribner commented 2 years ago

This comment mentions

We register to both the relation being created and joined

but only relation_created is observed. Maybe the comment is out of date?

simskij commented 2 years ago

Whether it's a missing listen or an outdated comment remains to be seen. Thanks for reporting it, Andrew!

sed-i commented 2 years ago

It was introduced by #4 but removed by #6.

Looking at _update_metrics_consumer_relation it seems that all should work if we only observe relation-joined (instead of created). @rbarry82 ?

Actually it should work just as fine with relation-created, because it should fire for every newly related app, and we have a leader guard in place anyway, so there shouldn't be a difference between observing joined and created. @balbirthomas ?

simskij commented 2 years ago

Actually it should work just as fine with relation-created, because it should fire for every newly related app, and we have a leader guard in place anyway, so there shouldn't be a difference between observing joined and created.

As mentioned in https://github.com/canonical/prometheus-scrape-config-k8s-operator/pull/4, both are needed for different circumstances.

balbirthomas commented 2 years ago

@dstathis Would you have the bandwidth to pick this up ? Looks like the comments in question were added by Michele quite some time ago and may indeed be obsolete . Alternatively if you find they are still relevant it may just be a question of replacing the event binding with relation joined or even addition an additional event binding for relation joined as a robust measure.