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

Expose /stats path only #376

Closed
stevesloka opened this issue May 10, 2018 · 4 comments · Fixed by #1092
Closed

Expose /stats path only #376

stevesloka opened this issue May 10, 2018 · 4 comments · Fixed by #1092
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Milestone

Comments

@stevesloka
Copy link
Member

stevesloka commented May 10, 2018

To gather metrics from Envoy the /stats needs to be exposed from the admin interface. If running under host networking all ports are bound to the host IP and network policies are not applicable.

I'd like to propose we create a static route inside Envoy to expose /stats over a configurable port (defaults to :8001). Given a standard deployment all L7 traffic would flow through a listener on :80 or :443 and stats metrics would flow through :8001.

The admin interface is still available via localhost so functionality is not lost.

@davecheney
Copy link
Contributor

#377 is a good starting point but I'd like to see these moved into the translator, or in code somehow for two reasons

  1. We have e2e tests that assume things like "when there are no ingress objects, there will be no listeners", this change breaks this assumption. It would be better to move this into the translator so we can cover it with unit and e2e tests.
  2. Make bootstrap mode optional #1 is still something I want to cover one day, and will become important if contour and envoy stop being co-located. When contour isn't used to create the bootstrap config we'll have to document all the settings that need to be provided, and the more we can move into the translator the less we need to document :)

There isn't a 0.7 milestone yet, but this sounds like a good candidate for that milestone.

@stevesloka
Copy link
Member Author

That makes a lot of sense. I only did the bootstrap route since it seemed like a static config vs dynamic. Sounds good to revisit and also possibly move the statsd pieces into that logic as well.

davecheney added a commit to davecheney/contour that referenced this issue May 11, 2018
Updates projectcontour#376

Omit the statsd listener and cluster unless its enabled.

Also:

- bind the stats listener to 127.0.0.1 by default, which matches the
statsd collector which is also listening on 127.0.0.1.
- add tests for enabled and disabled configurations

Signed-off-by: Dave Cheney <dave@cheney.net>
davecheney added a commit to davecheney/contour that referenced this issue May 11, 2018
Updates projectcontour#376

Omit the statsd listener and cluster unless its enabled.

Also:

- bind the stats listener to 127.0.0.1 by default, which matches the
statsd collector which is also listening on 127.0.0.1.
- add tests for enabled and disabled configurations

Signed-off-by: Dave Cheney <dave@cheney.net>
@davecheney davecheney added this to the 0.7.0 milestone May 14, 2018
@davecheney davecheney added kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels May 15, 2018
@davecheney davecheney modified the milestones: 0.7.0, 0.8.0 Oct 25, 2018
@davecheney davecheney modified the milestones: 0.8.0, 0.9.0 Nov 13, 2018
@davecheney davecheney modified the milestones: 0.9.0, 0.10.0 Jan 25, 2019
@davecheney davecheney modified the milestones: 0.10.0, 0.11.0 Feb 6, 2019
@davecheney davecheney modified the milestones: 0.11.0, 0.12.0 Apr 8, 2019
@timh timh removed this from the 0.12.0 milestone Apr 10, 2019
@davecheney
Copy link
Contributor

@stevesloka @rata if either of you have time to tackle this I'd appreciate it.

@stevesloka stevesloka self-assigned this Apr 29, 2019
@stevesloka stevesloka added this to the 0.13.0 milestone Apr 29, 2019
@stevesloka
Copy link
Member Author

@davecheney I'm trying to figure out a good integration pattern here to implement this. Everything we do is based upon the dag right now.

Do you think this approach would work? Or have a different idea?

davecheney added a commit to davecheney/contour that referenced this issue May 15, 2019
Updates projectcontour#876
Updates projectcontour#376

Convert ListenerCache static values from proto struct to typed config.
This could do with DRYing up with some more with helpers, which I'll
tackle in a followup PR.

Signed-off-by: Dave Cheney <dave@cheney.net>
sunjayBhatia pushed a commit that referenced this issue Jan 30, 2023
* multi-arch supported with `v0.8.0`
* `v0.8.0`, is now based on `gcr.io/distroless/static` and running nonroot

Signed-off-by: Christopher Banck <cbanck@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants