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

server: provide server health status when stats disabled #8482

Merged
merged 4 commits into from
Oct 23, 2019

Conversation

venilnoronha
Copy link
Member

Description: This fixes a bug where the server health check returns an incorrect response when server stats are disabled i.e. due to relying on the server.live gauge.
Risk Level: Low
Testing: Added a new test
Docs Changes: N/A
Release Notes: N/A
Fixes #8473

@venilnoronha venilnoronha changed the title Provide server health status when stats disabled server: provide server health status when stats disabled Oct 7, 2019
@jmarantz jmarantz self-assigned this Oct 9, 2019
jmarantz
jmarantz previously approved these changes Oct 21, 2019
@jmarantz
Copy link
Contributor

@envoyproxy/senior-maintainers

@mattklein123 mattklein123 self-assigned this Oct 21, 2019
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM modulo @jmarantz comments.

/wait

Signed-off-by: Venil Noronha <veniln@vmware.com>
Signed-off-by: Venil Noronha <veniln@vmware.com>
* Create a `request` utility function
* Import `testing` functions via `using`

Signed-off-by: Venil Noronha <veniln@vmware.com>
Signed-off-by: Venil Noronha <veniln@vmware.com>
@mattklein123
Copy link
Member

@venilnoronha please do not force push, thank you!

@mattklein123
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks. Nice cleanup.

@mattklein123 mattklein123 merged commit e7c7ca4 into envoyproxy:master Oct 23, 2019
@venilnoronha
Copy link
Member Author

@mattklein123 I had to force push coz I rebased this branch with master. Will merge next time on.

spenceral added a commit to spenceral/envoy that referenced this pull request Oct 23, 2019
* master: (54 commits)
  Update dependencies - Go, Bazel toos, xxHash, nanopb, rules_go, curl, protobuf, Jinja, yaml-cpp, re2 (envoyproxy#8728)
  test: increase coverage of listener_manager_impl.cc (envoyproxy#8737)
  test: modify some macros to reduce number of uncovered lines reported (envoyproxy#8725)
  build: add a script to setup clang (envoyproxy#8682)
  http: fix ssl_redirect on external (envoyproxy#8664)
  docs: update fedora build requirements (envoyproxy#8641)
  fix draining listener removal logs (envoyproxy#8733)
  dubbo: fix router doc (envoyproxy#8734)
  server: provide server health status when stats disabled (envoyproxy#8482)
  router: adding a knob to configure a cap on buffering for shadowing/retries (envoyproxy#8574)
  tcp proxy: add default 1 hour idle timeout (envoyproxy#8705)
  thrift: fix filter names in docs (envoyproxy#8726)
  Quiche changes to avoid gcc flags on Windows (envoyproxy#8514)
  test: increase test coverage in Router::HeaderParser (envoyproxy#8721)
  admin: add drain listeners endpoint  (envoyproxy#8415)
  buffer filter: add content-length when ending stream with trailers (envoyproxy#8609)
  clarify draining option docs (envoyproxy#8712)
  build: ignore go-control-plane mirror git commit error code (envoyproxy#8703)
  api: remove API type database from checked in artifacts. (envoyproxy#8716)
  admin: correct help strings (envoyproxy#8710)
  ...

Signed-off-by: Spencer Lewis <slewis@squareup.com>
derekargueta pushed a commit to derekargueta/envoy that referenced this pull request Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Filtering server live stats breaks health checking filter
3 participants