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.

Sign up for GitHub

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

Jump to bottom

Support HTTP/2 METADATA frames #2394

Closed
birenroy opened this issue Jan 17, 2018 · 9 comments
Closed

Support HTTP/2 METADATA frames #2394

birenroy opened this issue Jan 17, 2018 · 9 comments
Assignees
@soya3129
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Milestone
1.14.0

Comments

@birenroy
Copy link
Contributor

birenroy commented Jan 17, 2018 •
edited
Loading

Description:
Add support for parsing, interpreting, and serializing HTTP/2 METADATA frames. METADATA is an extension to the HTTP/2 protocol that will be used to convey information about request and response streams out-of-line.

Google will bring the draft METADATA specification to the IETF for standardization. If the specification is adopted and any changes are made as part of the process, we should update Envoy to match.

Relevant Links:
Rough draft METADATA specification: https://docs.google.com/document/d/1DsPjl6qWxhcF28vzJey0xQJ2O2eRKHv7CBXt3EZyDjc/edit?usp=sharing

@alyssawilk @htuch @mattklein123
@mattklein123 mattklein123 added the enhancement Feature requests. Not bugs or questions. label Jan 17, 2018
@mattklein123
Copy link
Member

@birenroy can you make the doc public? Also, do you know offhand if implementing this can use existing extensibility mechanisms in nghttp2 or if it will require nghttp2 patches?

@alyssawilk
Copy link
Contributor

I believe we can implement it with nghttp2's existing extension support.
If we get something through the IETF as an H2 extension (which is plan A) we'd probably want native support for the long haul.

@birenroy
Copy link
Contributor Author

@mattklein123 I've updated the doc link; it should be public now. Sorry about that.

@mattklein123
Copy link
Member

@tatsuhiro-t ^^^ (FYI)

@tatsuhiro-t
Copy link

I think METADATA frame can be implemented using nghttp2's API because it does not change HTTP/2 protocol semantics (i.e., it can be freely ignored).

@tatsuhiro-t
Copy link

Re-reading a draft, requiring successive METADATA frame until END_METADATA might cause a trouble.
nghttp2 has no guarantee to notify all frames to application, so it cannot be sure that the above rule is obeyed.
I don't know how METADATA uses Never indexed stuff, but it would be better to allow interleaving other frames if possible.

@PiotrSikora PiotrSikora mentioned this issue Feb 28, 2018
Closed
@alyssawilk alyssawilk mentioned this issue Mar 5, 2018
Closed
@stale
Copy link

stale bot commented Jun 20, 2018

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 other activity occurs. Thank you for your contributions.

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jun 20, 2018
@alyssawilk alyssawilk added the help wanted Needs help! label Jun 20, 2018
@stale stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Jun 20, 2018
This was referenced Sep 24, 2018
Closed
Merged
@soya3129
Copy link
Contributor

Re-reading a draft, requiring successive METADATA frame until END_METADATA might cause a trouble.
nghttp2 has no guarantee to notify all frames to application, so it cannot be sure that the above rule is obeyed.
I don't know how METADATA uses Never indexed stuff, but it would be better to allow interleaving other frames if possible.

Hi @tatsuhiro-t , I am assuming you referred to the following sentence in the draft:

"If the END_METADATA bit is not set, this frame MUST be followed by another METADATA frame."

I can see it is a little misleading. But we don't require METADATA frames to be sent contiguously. I think the sentence tries to say there must be following METADATA frames if END_METADATA is not set. METADATA can interleave with other frame types.

It is recommended to send METADATA frames back to back. This way, the peer doesn't need to save the intermediate state for a long time. For some of the METADATA use cases, METADATA is required before any message can be sent or a session can be closed. In those cases, it helps if we submit METADATA as soon as they are available.

@soya3129 soya3129 mentioned this issue Oct 19, 2018
Merged
@mattklein123 mattklein123 added this to the 1.9.0 milestone Oct 31, 2018
alyssawilk pushed a commit that referenced this issue Nov 8, 2018
@soya3129 @alyssawilk
Add Metadata to codec (#4796)
7b96648 
Description: Add MetadataEncoder and MetadataDecoder to codec. I also corrected some of the design decisions in MetadataEnocder and MetadataDecoder so that they are more suitable for codec. Those corrections includes: 1. let StreamImpl owns the decoded metadata, not the decoder. 2. Remove stream_id field in decoder and encoder since stream id is stored in StreamImpl. 3. Remove set/getMaxMetadataSize(), because nghttp2 uses default max frame size.

This is the second step towards supporting metadata in envoy. The next step would be add filters to add/delete/modify metadata.

Risk Level: Low. Not used.
Testing: Unit test.
Docs Changes:
Release Notes:
#2394

Signed-off-by: Yang Song <[email protected]>
@soya3129 soya3129 mentioned this issue Nov 13, 2018
Merged
alyssawilk pushed a commit that referenced this issue Nov 26, 2018
@soya3129 @alyssawilk
Proxy metadata in responses (#5021)
b6ff832 
Description: Allow envoy to proxy metadata in responses.

Risk Level: Low. Not used.
Testing: Integration test. (Unit tests will be added with filters)
Docs Changes: inline
Release Notes: n/a
Part of #2394

Signed-off-by: Yang Song <[email protected]>
This was referenced Nov 27, 2018
Closed
Merged
@soya3129 soya3129 mentioned this issue Dec 4, 2018
Merged
@alyssawilk alyssawilk modified the milestones: 1.9.0, 1.10.0 Dec 13, 2018
alyssawilk pushed a commit that referenced this issue Jan 15, 2019
@soya3129 @alyssawilk
Consume and insert metadata in responses (#5165)
180b079 
Description: Add filter interfaces to consume and insert metadata in responses (from upstream to downstream).
Risk Level: Low. Not used in production.
Testing: Integration tests.
Docs Changes: inline
Release Notes: 
#2394

Signed-off-by: Yang Song <[email protected]>
@soya3129 soya3129 mentioned this issue Jan 18, 2019
Merged
fredlas pushed a commit to fredlas/envoy that referenced this issue Mar 5, 2019
@soya3129 @fredlas
Add Metadata to codec (envoyproxy#4796)
b46128a 
Description: Add MetadataEncoder and MetadataDecoder to codec. I also corrected some of the design decisions in MetadataEnocder and MetadataDecoder so that they are more suitable for codec. Those corrections includes: 1. let StreamImpl owns the decoded metadata, not the decoder. 2. Remove stream_id field in decoder and encoder since stream id is stored in StreamImpl. 3. Remove set/getMaxMetadataSize(), because nghttp2 uses default max frame size.

This is the second step towards supporting metadata in envoy. The next step would be add filters to add/delete/modify metadata.

Risk Level: Low. Not used.
Testing: Unit test.
Docs Changes:
Release Notes:
envoyproxy#2394

Signed-off-by: Yang Song <[email protected]>
Signed-off-by: Fred Douglas <[email protected]>
fredlas pushed a commit to fredlas/envoy that referenced this issue Mar 5, 2019
@soya3129 @fredlas
Proxy metadata in responses (envoyproxy#5021)
eee081d 
Description: Allow envoy to proxy metadata in responses.

Risk Level: Low. Not used.
Testing: Integration test. (Unit tests will be added with filters)
Docs Changes: inline
Release Notes: n/a
Part of envoyproxy#2394

Signed-off-by: Yang Song <[email protected]>
Signed-off-by: Fred Douglas <[email protected]>
fredlas pushed a commit to fredlas/envoy that referenced this issue Mar 5, 2019
@soya3129 @fredlas
Consume and insert metadata in responses (envoyproxy#5165)
5b3eb54 
Description: Add filter interfaces to consume and insert metadata in responses (from upstream to downstream).
Risk Level: Low. Not used in production.
Testing: Integration tests.
Docs Changes: inline
Release Notes: 
envoyproxy#2394

Signed-off-by: Yang Song <[email protected]>
Signed-off-by: Fred Douglas <[email protected]>
@mattklein123 mattklein123 modified the milestones: 1.10.0, 1.11.0 Mar 20, 2019
@gargnupur gargnupur mentioned this issue Jun 25, 2019
Closed
@alyssawilk alyssawilk modified the milestones: 1.11.0, 1.12.0 Jul 8, 2019
@lizan lizan mentioned this issue Aug 1, 2019
Merged
@mattklein123 mattklein123 modified the milestones: 1.12.0, 1.13.0 Oct 10, 2019
@PiotrSikora PiotrSikora mentioned this issue Nov 21, 2019
Closed
@mattklein123 mattklein123 modified the milestones: 1.13.0, 1.14.0 Dec 5, 2019
@mattklein123
Copy link
Member

I'm going to close this as this is implemented (to some degree). We can open new issues as needed.

@lambdai lambdai mentioned this issue Mar 22, 2021
Open
copybara-service bot pushed a commit to google/quiche that referenced this issue Jun 24, 2022
@bnc-google @copybara-github
Add HttpEncoder::SerializeMetadataFrameHeader().
cb3863a 
Add HTTP/3 SETTING_ENABLE_METADATA.
Add HTTP/3 METADATA frame type.
Add HttpEncoder::SerializeMetadataFrameHeader() method to serialize METADATA frame header.

Setting identifier and frame type match HTTP/2 ones, see
quiche/spdy/core/metadata_extension.cc.

See envoyproxy/envoy#2394 and design document linked
therein for HTTP/2 METADATA.  HTTP/3 implementation will be very similar.

PiperOrigin-RevId: 457048873
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@soya3129 soya3129

Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Milestone
1.14.0
Development

No branches or pull requests
5 participants
@tatsuhiro-t @mattklein123 @birenroy @alyssawilk @soya3129