Skip to content

docs: split consumed headers list based on request/response consumption#11200

Merged
mattklein123 merged 4 commits into
envoyproxy:masterfrom
snowp:split-consumed-header-docs
May 26, 2020
Merged

docs: split consumed headers list based on request/response consumption#11200
mattklein123 merged 4 commits into
envoyproxy:masterfrom
snowp:split-consumed-header-docs

Conversation

@snowp
Copy link
Copy Markdown
Contributor

@snowp snowp commented May 14, 2020

Signed-off-by: Snow Pettersen kpettersen@netflix.com

Commit Message: Splits the documentation for consumed headers on the router to differentiate between requests consumed from the downstream request versus the upstream response.

Signed-off-by: Snow Pettersen <kpettersen@netflix.com>
x-envoy-decorator-operation
^^^^^^^^^^^^^^^^^^^^^^^^^^^

Setting this header on egress requests, its value will override any locally defined
Copy link
Copy Markdown
Contributor Author

@snowp snowp May 14, 2020

Choose a reason for hiding this comment

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

i rephrased this to start the paragraph the same way as the other header, but this one is a bit different to the others because it describes two different behavior depending on whether its a request/response.

It might be nicer to have this doc speak about downstream/upstream vs egress/ingress since I don't think the router actually cares about the ingress/egress distinction (beyond some trace tags)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed to you want to just fix that?

@mattklein123 mattklein123 self-assigned this May 15, 2020
@mattklein123
Copy link
Copy Markdown
Member

Can you check the docs build?

/wait

Signed-off-by: Snow Pettersen <aickck@gmail.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

LGTM with small comment, thanks.

/wait

x-envoy-decorator-operation
^^^^^^^^^^^^^^^^^^^^^^^^^^^

Setting this header on egress requests, its value will override any locally defined
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed to you want to just fix that?

Signed-off-by: Snow Pettersen <aickck@gmail.com>
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Nice, just a few typos I caught.

/wait

^^^^^^^^^^^^^^^^

Setting this header on egress requests will cause Envoy to attempt to retry failed requests (number
Setting this header requests will cause Envoy to attempt to retry failed requests (number
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Setting this header requests will cause Envoy to attempt to retry failed requests (number
Setting this header will cause Envoy to attempt to retry failed requests (number

?

Setting this header on egress requests will cause Envoy to use a request
hedging strategy in the case of a per try timeout. This overrides the value set
in the :ref:`route configuration
Setting this header on cause Envoy to use a request hedging strategy in the case of a per try timeout.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
Setting this header on cause Envoy to use a request hedging strategy in the case of a per try timeout.
Setting this header will cause Envoy to use a request hedging strategy in the case of a per try timeout.

?

Signed-off-by: Snow Pettersen <kpettersen@netflix.com>
@mattklein123 mattklein123 merged commit b6f7235 into envoyproxy:master May 26, 2020
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.

2 participants