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

Warn on v2 xDS API use #10815

Closed
htuch opened this issue Apr 16, 2020 · 1 comment · Fixed by #10964 or #13238
Closed

Warn on v2 xDS API use #10815

htuch opened this issue Apr 16, 2020 · 1 comment · Fixed by #10964 or #13238
Assignees
Labels
api/v3 Major version release @ end of Q3 2019 priority/high
Milestone

Comments

@htuch
Copy link
Member

htuch commented Apr 16, 2020

Now that v2 xDS is deprecated, we should warn (log/stats), just as we do for individual deprecated fields on the use of the API (with runtime override). This is ideally done ASAP, since we want to give folks a long runway to migrate to v3.

CC @alyssawilk @mattklein123 @envoyproxy/api-shepherds

@htuch
Copy link
Member Author

htuch commented Sep 22, 2020

Reopening this as we want to increase verbosity when upgrading v2 filter configs inside a v3 envelope resource.

@htuch htuch reopened this Sep 22, 2020
@htuch htuch self-assigned this Sep 22, 2020
@htuch htuch removed the help wanted Needs help! label Sep 22, 2020
htuch added a commit to htuch/envoy that referenced this issue Sep 22, 2020
Inspired by a log statement needed for envoyproxy#10815, this patch:

* Adds _MISC variants of sparse macros.

* Sends _MISC log messages to fancy logger (these were previously
  ignored). This is a bit weird, as "misc" is also a logger and
  we use file-specific loggers in fancy logger, but in reality, most
  folks treat this macro as the generic Envoy log statement.

* Avoid evaluating expressions in ENVOY_LOG* with fancy logger when
  beneath the configured log level.

Risk level: Low
Testing: Mostly covered by log_macros_test already, added some _MISC
  coverage in SparseLogMacrosTest.

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit to htuch/envoy that referenced this issue Sep 23, 2020
The earlier PR envoyproxy#10964 added in validation that we're not using v2 xDS
transport, but we didn't have any warning on v2 bootstraps, v2
extension configs inside a v3 config and also for v2 resource types.

This PR adds support for these validations by warning and incrementing
the deprecated feature use stat. The following notifications are added:
* Rate limited warning log
* Trace-level log
* The runtime.deprecated_features is bumped, but only after server
  initialization, so this doesn't apply to bootstrap. This is consistent
  with how other deprecated features are treated today.

Risk level: Medium (it's possible that this will cause monitoring to
alert, the boosting logic is also quite complex).
Testing: Unit tests and server bootstrap tests added.

Fixes envoyproxy#10815

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit that referenced this issue Sep 25, 2020
Inspired by a log statement needed for #10815, this patch:

* Adds _MISC variants of sparse macros.

* Sends _MISC log messages to fancy logger (these were previously
  ignored). This is a bit weird, as "misc" is also a logger and
  we use file-specific loggers in fancy logger, but in reality, most
  folks treat this macro as the generic Envoy log statement.

* Avoid evaluating expressions in ENVOY_LOG* with fancy logger when
  beneath the configured log level.

Risk level: Low
Testing: Mostly covered by log_macros_test already, added some _MISC
  coverage in SparseLogMacrosTest.

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit that referenced this issue Sep 30, 2020
The earlier PR #10964 added in validation that we're not using v2 xDS
transport, but we didn't have any warning on v2 bootstraps, v2
extension configs inside a v3 config and also for v2 resource types.

This PR adds support for these validations by warning and incrementing
the deprecated feature use stat. The following notifications are added:

- Rate limited warning log
- Trace-level log
- The runtime.deprecated_features is bumped, but only after server
  initialization, so this doesn't apply to bootstrap. This is consistent
  with how other deprecated features are treated today.
Risk level: Medium (it's possible that this will cause monitoring to alert, the boosting logic is also quite complex).
Testing: Unit tests and server bootstrap tests added.

Fixes #10815

Signed-off-by: Harvey Tuch <htuch@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api/v3 Major version release @ end of Q3 2019 priority/high
Projects
None yet
2 participants