fix: outlier detection disabled by default

[lsjostro]

What type of PR is this?

• "fix(translator): outlier detection need to be explicit enabled by a BTP"

What this PR does / why we need it:
disable passive health check if not explicitly enabled by a BTP. It's misleading for users that passive health check is enabled with default values without user knowing.

Use case where you don't need outlier detection:
If you users have tiers of proxies (envoy have clusters backends which is also proxies), outlier detection shouldn't be enabled.

Which issue(s) this PR fixes:
Fixes #4854

Release Notes: No

[arkodg]

hey @lsjostro can you sign your commit/ commit amend and force push

[arkodg]

can you also add a note in https://github.com/envoyproxy/gateway/blob/main/release-notes/current.yaml under the breaking changes section highlighting that this now disabled by default and If needed, needs to be explicitly set in https://gateway.envoyproxy.io/docs/api/extension_types/#backendtrafficpolicy

[lsjostro]

hey @lsjostro can you sign your commit/ commit amend and force push

trying out my workflow with jj so let me look into that.. so bear with me 😄
excellent challenge though! (git commit -s)😄

[lsjostro]

for ref 😄 martinvonz/jj#1399

[zhaohuabing]

While I agree that passive health checks does not make sense when EG is used as the first tier of a proxy chain, I believe they can be beneficial in most scenarios where EG sits directly in front of services. Considering this, I see two potential approaches:

1. Enable passive health checks by default and allow users to explicitly disable them using a BTP.

2. Disable passive health checks by default and include a best practices section in the documentation to guide users on enabling them when needed.

Personally, I lean slightly toward the first option, as it provides a more resilient out-of-the-box experience.

My above assumptions may not be correct, I’d like to hear more practical opinions from users who are actively using EG in their environments.

[arkodg]

If 1. is picked, a user setting active health will need to explicitly disable passive health check

[zhaohuabing]

If 1. is picked, a user setting active health will need to explicitly disable passive health check

Yeah, that's a good point. An implicit health check could be hard to be noticed.

We can go ahead with option 2 and add a best practices section in the documentation to ask users to set it if needed.

[lsjostro]

agree option 2 is better otherwise we'll need to document why we choose to enable passive health by default and how that will benefit the user. I think it's better to let the user decide and don't have something enabled saying we know better than the user.

[arkodg]

hey @lsjostro can you run make testdata as well ? and commit those changes

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.30%. Comparing base (b9f9a9f) to head (80c4548).
Report is 5 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4856   +/-   ##
=======================================
  Coverage   66.29%   66.30%           
=======================================
  Files         209      209           
  Lines       31912    31955   +43     
=======================================
+ Hits        21157    21188   +31     
- Misses       9507     9519   +12     
  Partials     1248     1248           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arkodg]

LGTM thanks !
the scan failure seems unrelated to this PR