-
Notifications
You must be signed in to change notification settings - Fork 332
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
Add host
parameter to telemetry
config
#1033
Conversation
host
parameter to telemetry
config
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.
lgtm, the only suggestion is that if a more meaningful message could be added when exiting. The message "Address in use" does not have a context and for an operator is not helpful because it doesn't say the telemetry server failed to start because it's trying to use a port which is already allocated. If there's a way to at least say telemetry server
or something like that I think that would be helpful than just bubbling up the error from the web server
Error: Hermes failed to start, last error: Address already in use (os error 48)
It says right before exiting with this message:
which is not perfect but at least provides some more context. Is that enough or would you want it to also mention this in the final error? Maybe we could introduce an enum to capture the server error and then print it accordingly? |
Sorry, my bad. Indeed it says. I thought that ERROR line was only shown depending on the Thanks! |
* Fixing telemetry server listen host (informalsystems#1032) * Add `host` parameter for the telemetry server * Gracefully exit when telemetry service fails to start Co-authored-by: Romain Ruetschi <romain@informal.systems>
Closes: #1032
Description
Add
host
parameter totelemetry
config section, to allow access from a different machine in the network (eg. by using0.0.0.0
as the host).For contributor use:
docs/
) and code comments.Files changed
in the Github PR explorer.