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

[ZK filter] Add per-opcode decoder error metrics #31138

Merged

Conversation

Winbobob
Copy link
Member

@Winbobob Winbobob commented Dec 1, 2023

Commit Message: [ZK filter] Add per-opcode decoder error metrics
Additional Description: Currently, the ZK proxy filter only emits one decoder error metric, which include all decoder errors. This PR tries to emit per-opcode decoder error metrics, which is helpful for debugging. These metrics can be turned on or off via a proto field enable_per_opcode_decoder_error_metrics.
Risk Level: Low
Testing: Unit test
Docs Changes: docs/root/configuration/listeners/network_filters/zookeeper_proxy_filter.rst
Release Notes: changelogs/current.yaml

Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #31138 was opened by Winbobob.

see: more, trace.

Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #31138 was opened by Winbobob.

see: more, trace.

Signed-off-by: Zhewei Hu <zhu@pinterest.com>
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
@Winbobob Winbobob force-pushed the add-per-opcode-decoder-error-metrics branch from a1646ee to d99739f Compare December 1, 2023 20:50
Signed-off-by: Zhewei Hu <zhu@pinterest.com>
@Winbobob Winbobob marked this pull request as ready for review December 1, 2023 21:23
@Winbobob
Copy link
Member Author

Winbobob commented Dec 1, 2023

Hi @JuniorHsu, please review this PR whenever you have a chance, thank you!

Copy link
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

/lgtm api

@repokitteh-read-only repokitteh-read-only bot removed the api label Dec 2, 2023
Copy link
Contributor

@JuniorHsu JuniorHsu left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@mattklein123 mattklein123 merged commit 695752b into envoyproxy:main Dec 3, 2023
106 checks passed
Winbobob added a commit to Winbobob/envoy that referenced this pull request Dec 8, 2023
…31138)"

This reverts commit 695752b.

Signed-off-by: Zhewei Hu <zhu@pinterest.com>
lizan pushed a commit that referenced this pull request Dec 12, 2023
…31245)

This is the prerequisite of reverting #30438 in order to fix the ZK proxy filter "Uncaught Exception" issue.

Risk Level: Low
Testing: Unit test
Docs Changes: Revert doc changes in #31138
Release Notes: Revert release notes in #31138
Platform Specific Features: N/A

Signed-off-by: Zhewei Hu <zhu@pinterest.com>
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.

4 participants