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

Make ENVOY_BUG only log critical in official debug builds #14957

Closed
asraa opened this issue Feb 5, 2021 · 6 comments · Fixed by #15496
Closed

Make ENVOY_BUG only log critical in official debug builds #14957

asraa opened this issue Feb 5, 2021 · 6 comments · Fixed by #15496

Comments

@asraa
Copy link
Contributor

asraa commented Feb 5, 2021

Given that WASM has been seeing ENVOY_BUG crashes, we should consider adding a compile time flag that makes ENVOY_BUG only log critical/increment stat and NOT abort, even in debug mode.

We may want release branches to be non-crashing in official debug releases for the next few releases until extensions are safe and ENVOY_BUG conditions are fixed.

See proxy-wasm/proxy-wasm-cpp-host#95

cc @PiotrSikora: If we view ENVOY_BUGs as at least as strong as ASSERTs, why shouldn't they crash in debug mode? ASSERTs are also subject to being buggy.

@PiotrSikora
Copy link
Contributor

Note: it's not only Wasm but also ext_authz (and possibly ext_proc).

cc @PiotrSikora: If we view ENVOY_BUGs as at least as strong as ASSERTs, why shouldn't they crash in debug mode? ASSERTs are also subject to being buggy.

That's a very good point. It might be that we just only to fix this single buggy ENVOY_BUG.

@github-actions
Copy link

github-actions bot commented Mar 7, 2021

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Mar 7, 2021
@github-actions
Copy link

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

@bianpengyuan
Copy link
Contributor

Can we please reopen this? Thanks!

@mattklein123
Copy link
Member

I think per above we decided to just fix the buggy ENVOY_BUGs? Do we need to reopen this?

PiotrSikora added a commit to PiotrSikora/envoy that referenced this issue Mar 15, 2021
Fixes envoyproxy#14957.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
@PiotrSikora
Copy link
Contributor

@mattklein123 yeah, but that didn't happen yet, so let's reopen until the offending ENVOY_BUG is fixed (see: #15496).

@PiotrSikora PiotrSikora reopened this Mar 15, 2021
@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Mar 15, 2021
alyssawilk pushed a commit that referenced this issue Mar 24, 2021
#15496)

Fixes #14957.
Risk: medium (minor filter chain refactor)
Tests: new integration test

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants