-
Notifications
You must be signed in to change notification settings - Fork 672
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 docs on how to get Envoy stats with prometheus #264
Add docs on how to get Envoy stats with prometheus #264
Conversation
7bb7d3f
to
9366c54
Compare
docs/prometheus.md
Outdated
@@ -0,0 +1,47 @@ | |||
# Prometheus | |||
|
|||
With Contour it is possible to get metrics from Envoy. In order for this to work properly, |
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.
s/it is possible to/you can
s/In order for this to work properly,/To do so,
docs/prometheus.md
Outdated
With Contour it is possible to get metrics from Envoy. In order for this to work properly, | ||
you must expose the Envoy admin socket and configure the Prometheus service discovery correctly. | ||
|
||
The admin socket can be made public by running |
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.
replace with:
"Make the admin socket public:"
(the command makes it clear; you don't need the extra words :-) )
docs/prometheus.md
Outdated
sed 's#"bootstrap", "/config/contour.yaml"#"bootstrap", "/config/contour.yaml", "--admin-address", "0.0.0.0"#g' <your-contour-deployment>.yaml | ||
``` | ||
|
||
Prometheus needs to have a configuration block that looks like this: |
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.
omit "to have"
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.
Nice PR -- just a few copyedits to tighten up the prose explanation. Thanks!
c0ab5a8
to
9723e4b
Compare
Signed-off-by: Alvaro Aleman <alv2412@googlemail.com>
Signed-off-by: Alvaro Aleman <alv2412@googlemail.com>
9723e4b
to
08d234e
Compare
Thanks for your feedback @Bradamant3, I've updated the PR |
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.
Thanks for the changes!
Thanks. This should probably get a call out on the release notes which are landing straight after this. |
Yeah a mention in the release notes would probably be useful. For me the stats are the single real selling point of Contour, because its the one thing Nginx really lacks. |
If you want to add something to the release notes you can send me a PR to release-notes.md before Thursday.
… On 13 Mar 2018, at 10:24, Jennifer Rondeau ***@***.***> wrote:
@Bradamant3 approved this pull request.
Thanks for the changes!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Signed-off-by: Daneyon Hansen <daneyonhansen@gmail.com>
This PR adds docs on how to get envoy stats with Prometheus. It also adds Prometheus annotations for all deployments/daemonsets.
What's not ideal about this approach is that
I saw metrics are on the roadmap, what do you think about resolving the two above mentioned issues by proxying the Envoy stats with Contour?