Skip to content
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

Improve server log startup message #358

Merged
merged 3 commits into from
Jan 19, 2022
Merged

Conversation

areveny
Copy link
Contributor

@areveny areveny commented Jan 19, 2022

Old output

dist % ./toxiproxy-server
INFO[0000] API HTTP server starting                      host=localhost port=8474 version=git

New output

dist % ./toxiproxy-server
INFO[0000] Toxiproxy HTTP server is starting             host=localhost port=8474 version=git
INFO[0000] Toxiproxy has started. Verify with `curl localhost:8474/version`

Closes #228

@miry miry self-assigned this Jan 19, 2022
@miry
Copy link
Contributor

miry commented Jan 19, 2022

Hi @areveny

Thank you for contribution. I have not seen in other application explanation how to use health endpoint. I would avoid write this in the logs.

The application should crash if there is some problems. Also I think the second message does not correctly says that server hast started listen for the port. But it was executed just write after the first message.

I would keep only one message:

INFO[0000] Starting HTTP server on endpoint localhost:8474   host=localhost port=8474 version=git

The basic verification that it is working:

$ toxiproxy-cli list

@areveny
Copy link
Contributor Author

areveny commented Jan 19, 2022

INFO[0000] Toxiproxy HTTP server starting on endpoint localhost:8474  host=localhost port=8474 version=git

Sounds good, here's the updated output. Do you think this closes the linked issue? Or should the documentation be updated.

api.go Outdated Show resolved Hide resolved
@miry
Copy link
Contributor

miry commented Jan 19, 2022

Sounds good, here's the updated output. Do you think this closes the linked issue? Or should the documentation be updated.

It is fine just to close the linked issue. The endpoint to check if toxiproxy ready is presented in README.

@areveny areveny force-pushed the update-start-message branch from 645a6c6 to c74b0be Compare January 19, 2022 17:16
@areveny
Copy link
Contributor Author

areveny commented Jan 19, 2022

dist % ./toxiproxy-server
INFO[0000] Starting HTTP server on endpoint localhost:8474  host=localhost port=8474 version=git

Thanks. It's now what you suggested. Really appreciate the feedback on a small change, I hope I can make more substantial contributions too.

@miry miry added this to the 2.4.0 milestone Jan 19, 2022
@miry miry merged commit 009a3d0 into Shopify:master Jan 19, 2022
@miry
Copy link
Contributor

miry commented Jan 19, 2022

@areveny Thank you for the contribution.

@miry miry added the Toxiproxy label Jan 20, 2022
@miry miry mentioned this pull request Jan 20, 2022
8 tasks
@neufeldtech
Copy link
Contributor

@areveny your changes have been released as part of https://github.com/Shopify/toxiproxy/releases/tag/v2.4.0. Thanks for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve server log message so people know Toxiproxy is working.
3 participants