Skip to content

API: Add Overload Manager ResetReason and ResponseFlag.#17044

Merged
antoniovicente merged 3 commits intoenvoyproxy:mainfrom
KBaichoo:flags
Jun 21, 2021
Merged

API: Add Overload Manager ResetReason and ResponseFlag.#17044
antoniovicente merged 3 commits intoenvoyproxy:mainfrom
KBaichoo:flags

Conversation

@KBaichoo
Copy link
Copy Markdown
Contributor

@KBaichoo KBaichoo commented Jun 18, 2021

Signed-off-by: Kevin Baichoo kbaichoo@google.com

Commit Message: Add Overload Manager ResetReason and ResponseFlag.
Additional Description: For the changes needed in Issue #15791 we will have overload manager reset streams. Given that, we'd like to have a response flag for overload manager resets. This change defines that response flag.
Risk Level: Low
Testing:Unit Tests
Docs Changes: Included
Release Notes: Included
Platform Specific Features: N/A
Related Issue #15791

@KBaichoo
Add Overload Manager ResetReason and ResponseFlag.
b0336fe 
Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Jun 18, 2021

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #17044 was opened by KBaichoo.

see: more, trace.

@KBaichoo
Copy link
Copy Markdown
Contributor Author

KBaichoo commented Jun 18, 2021

/assign @antoniovicente

fuqianggao
fuqianggao reviewed Jun 18, 2021
View reviewed changes
docs/root/version_history/current.rst Outdated
New Features
------------

* access_log: added the new response flag `OM` for Overload Manager termination. The error flag will be set when the http stream is terminated by overload manager.
Copy link
Copy Markdown
Contributor

@fuqianggao fuqianggao Jun 18, 2021

Choose a reason for hiding this comment

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

maybe add ref: to the proto, similar to the one below?

Copy link
Copy Markdown
Contributor Author

@KBaichoo KBaichoo Jun 18, 2021

Choose a reason for hiding this comment

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

Done.

@antoniovicente
Copy link
Copy Markdown
Contributor

antoniovicente commented Jun 18, 2021

You have test failures which seem related to this PR.

/wait

2021-06-18T18:30:14.7762567Z ==================== Test output for //test/extensions/access_loggers/grpc:grpc_access_log_utils_test:
2021-06-18T18:30:14.7763192Z [==========] Running 1 test from 1 test suite.
2021-06-18T18:30:14.7763881Z [----------] Global test environment set-up.
2021-06-18T18:30:14.7764642Z [----------] 1 test from UtilityResponseFlagsToAccessLogResponseFlagsTest
2021-06-18T18:30:14.7765238Z [ RUN      ] UtilityResponseFlagsToAccessLogResponseFlagsTest.All
2021-06-18T18:30:14.7766559Z TestRandomGenerator running with seed -1346820440
2021-06-18T18:30:14.7767412Z test/extensions/access_loggers/grpc/grpc_access_log_utils_test.cc:51: Failure
2021-06-18T18:30:14.7768045Z Expected equality of these values:
2021-06-18T18:30:14.7768573Z   common_access_log_expected.DebugString()
2021-06-18T18:30:14.7770982Z     Which is: "response_flags {\n  failed_local_healthcheck: true\n  no_healthy_upstream: true\n  upstream_request_timeout: true\n  local_reset: true\n  upstream_remote_reset: true\n  upstream_connection_failure: true\n  upstream_connection_termination: true\n  upstream_overflow: true\n  no_route_found: true\n  delay_injected: true\n  fault_injected: true\n  rate_limited: true\n  unauthorized_details {\n    reason: EXTERNAL_SERVICE\n  }\n  rate_limit_service_error: true\n  downstream_connection_termination: true\n  upstream_retry_limit_exceeded: true\n  stream_idle_timeout: true\n  invalid_envoy_request_headers: true\n  downstream_protocol_error: true\n  upstream_max_stream_duration_reached: true\n  response_from_cache_filter: true\n  no_filter_config_found: true\n  duration_timeout: true\n  upstream_protocol_error: true\n  no_cluster_found: true\n}\n"
2021-06-18T18:30:14.7773421Z   common_access_log.DebugString()
2021-06-18T18:30:14.7775880Z     Which is: "response_flags {\n  failed_local_healthcheck: true\n  no_healthy_upstream: true\n  upstream_request_timeout: true\n  local_reset: true\n  upstream_remote_reset: true\n  upstream_connection_failure: true\n  upstream_connection_termination: true\n  upstream_overflow: true\n  no_route_found: true\n  delay_injected: true\n  fault_injected: true\n  rate_limited: true\n  unauthorized_details {\n    reason: EXTERNAL_SERVICE\n  }\n  rate_limit_service_error: true\n  downstream_connection_termination: true\n  upstream_retry_limit_exceeded: true\n  stream_idle_timeout: true\n  invalid_envoy_request_headers: true\n  downstream_protocol_error: true\n  upstream_max_stream_duration_reached: true\n  response_from_cache_filter: true\n  no_filter_config_found: true\n  duration_timeout: true\n  upstream_protocol_error: true\n  no_cluster_found: true\n  overload_manager: true\n}\n"
2021-06-18T18:30:14.7778378Z With diff:
2021-06-18T18:30:14.7778774Z @@ +27,4 @@
2021-06-18T18:30:14.7779207Z    upstream_protocol_error: true
2021-06-18T18:30:14.7779671Z    no_cluster_found: true
2021-06-18T18:30:14.7780123Z +  overload_manager: true
2021-06-18T18:30:14.7780509Z  }\n

@KBaichoo
Formatting, test fix.
9c01981 
Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
@KBaichoo
Docs.
4e2eaf1 
Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
@KBaichoo
Copy link
Copy Markdown
Contributor Author

KBaichoo commented Jun 19, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

repokitteh-read-only bot commented Jun 19, 2021

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17044 (comment) was created by @KBaichoo.

see: more, trace.

htuch
htuch reviewed Jun 21, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@htuch htuch 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

antoniovicente
antoniovicente approved these changes Jun 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente left a comment

Choose a reason for hiding this comment

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

Thanks.

@antoniovicente antoniovicente changed the title API: Add Overload Manager ResponseFlag. API: Add Overload Manager ResetReason and ResponseFlag. Jun 21, 2021
@antoniovicente antoniovicente merged commit 1aa65ac into envoyproxy:main Jun 21, 2021
@wjuan-AFK wjuan-AFK mentioned this pull request Jun 25, 2021
Merged
mum4k pushed a commit to envoyproxy/nighthawk that referenced this pull request Jun 25, 2021
@wjuan-AFK
Update Envoy to 0af62845667 (Jun 21th 2021) (#709)
b686f0d 
- Adding new enum to source/client/stream_decoder.cc to reflect changes in envoyproxy/envoy#17044
- Adding a binding for external dependency on gtest introduced in envoyproxy/envoy#16687
- Sync to latest envoy included changes to `.bazelrc` and `ci/run_envoy_docker.sh` copied directly.

Signed-off-by: William Juan <66322422+wjuan-AFK@users.noreply.github.com>
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
@KBaichoo
API: Add Overload Manager ResetReason and ResponseFlag. (envoyproxy#1…
7a35cda 
…7044)

Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@htuch htuch htuch left review comments

@fuqianggao fuqianggao fuqianggao left review comments

@antoniovicente antoniovicente antoniovicente approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@htuch htuch

@antoniovicente antoniovicente

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@KBaichoo @antoniovicente @htuch @fuqianggao