-
-
Notifications
You must be signed in to change notification settings - Fork 73
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
metrics: add option metrics.addr
for ipv6 support
#168
base: master
Are you sure you want to change the base?
Conversation
Counterpart to mautrix/python#168
`prometheus_client` supports IPv6 addresses for a while now[1]. This isn't used here however because the default 0.0.0.0 is used. Added an option `metrics.addr` that makes this configurable. [1] prometheus/client_python@39d2360
cc @tulir :) |
@@ -188,6 +188,7 @@ def start_prometheus(self) -> None: | |||
try: | |||
enabled = self.config["metrics.enabled"] | |||
listen_port = self.config["metrics.listen_port"] | |||
addr = self.config.get("metrics.addr", "0.0.0.0") |
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.
Since we are using these values here, I think we need to have corresponding copy
s in mautrix/bridge/config.py
. See https://github.com/mautrix/twitter/blob/master/mautrix_twitter/config.py#L48-L49 for an example of what it should look like.
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.
oh right, didn't notice because I think I don't use that part.
Will update accordingly later:)
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.
@sumnerevans OK, I confused a few things when I read your comment, apologies!
I already did that for the bridge I've been using, namely mautrix/telegram#955. In fact, I'm already using that productively, so this code is not untested :)
Do you want me to file fixup PRs for the other python bridges or is it OK for you to update when bumping mautrix/python there? If so, are all bridges to update in this org?
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.
ping @sumnerevans
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.
Not sure what should be done here, @tulir ?
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.
ping @tulir
prometheus_client
supports IPv6 addresses for a while now[1]. This isn't used here however because the default 0.0.0.0 is used. Added an optionmetrics.addr
that makes this configurable.Tested with mautrix-telegram.
[1] prometheus/client_python@39d2360