-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fill response flag for stats plugin #2527
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine. The only suggestion I have is that unrecognized bits should be supplied since this may need to work with future Envoy versions. E.g. drop LastFlag and just print out the flags as-is in case they overflow.
Can I get some review on this? Thanks! |
@kyessenov Does this look like a flake to you? It seems to be different from client timeout we saw before. There is no crashes and does not return any mismatches.. so likely the prometheus endpoint does not return metrics. Could this also happen because of stale listeners?
|
@bianpengyuan Yeah this is not a timeout. Either prometheus reports incorrect stats, or we failed to report one. Does this happen consistently? We can disable the test for now to get this in. |
@kyessenov haven't retried it. Let me retest it. |
/test proxy-presubmit-tsan |
In response to a cherrypick label: new pull request created: #2534 |
I noticed that it was polling stats endpoint for ~20s. I think it was just starved of CPU resources since TSAN builds are slow. |
This needs istio/envoy#116 to be in to work with 1.4 and envoyproxy/envoy-wasm to be updated to work with master.