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

Test with client and server having different frame size limits #60

Closed
tigrannajaryan opened this issue Jan 28, 2022 · 6 comments
Closed

Test with client and server having different frame size limits #60

tigrannajaryan opened this issue Jan 28, 2022 · 6 comments
Assignees
@andykellr
Labels
required-for-stable Required to be resolved before 1.0

Comments

@tigrannajaryan
Copy link
Member

I was advised that different websocket implementations may have different frame size limits (typically 32K or64K bytes) which may result in interoperability problems. This needs testing and confirming that either it is not an issue or it needs a solution/workaround.
@tigrannajaryan tigrannajaryan mentioned this issue Feb 3, 2022
Open
@tigrannajaryan tigrannajaryan added the required-for-stable Required to be resolved before 1.0 label May 31, 2022
@andykellr andykellr self-assigned this Aug 23, 2022
@andykellr
Copy link
Contributor

While investigating this issue, many of the references I found regarding frame size limits refer to the 32K limit in AWS API Gateway.

Because of the WebSocket frame-size quota of 32 KB, a message larger than 32 KB must be split into multiple frames, each 32 KB or smaller. If a larger message (or larger frame size) is received, the connection is closed with code 1009.

https://docs.aws.amazon.com/apigateway/latest/developerguide/limits.html

This particular problem would present itself if an OpAMP server were hosted in AWS behind API Gateway and Agents were connecting to the server using an OpAMP client implementation with a WebSocket library that sent frames larger than 32K.

@tigrannajaryan
Copy link
Member Author

tigrannajaryan commented Sep 20, 2022 •
edited
Loading

Thanks for the research.

What do you think if we add a (SHOULD) recommendation that Client implementations must limit outgoing frame size to 32KB?

I wonder what frame size our current opamp-go implementation uses.

@tigrannajaryan
Copy link
Member Author

Apparently it is not me who thinks 32KB frame/128KB message is unreasonably low limit: https://repost.aws/questions/QUpz6uWU0SSnunjzPrbmLtRg/api-gateway-websockets-size-limit-feedback

@tigrannajaryan
Copy link
Member Author

I don't think we want to do message fragmentation and reassembly ourselves. That would dramatically complicate OpAMP just to accommodate one particular implementation that has unusually low limits.

I think we can probably do is this:

  • Add a warning in the spec that some implementations have low frame/message size limits that may cause delivery failures and recommend to use plain HTTP transport if that happens.
  • In our opamp-go implementation try to fragment large messages into small frames. Perhaps the frame size can be a client/server config option. For Gorilla Websockets using NextWriter should result in slicing into frames of at most WriteBufferSize.
  • We should probably also file a feature request to Gorilla Websockets to add frame size limit as a proper option so that we don't rely on an undocumented behavior.

Any other ideas?

@tigrannajaryan
Copy link
Member Author

tigrannajaryan commented Sep 20, 2022 •
edited
Loading

Filed a feature request to gorilla websocket for now: gorilla/websocket#814

@tigrannajaryan tigrannajaryan mentioned this issue Oct 3, 2022
Closed
tigrannajaryan added a commit to tigrannajaryan/opamp-spec that referenced this issue Nov 9, 2022
@tigrannajaryan
Adding a header for WebSocket messages to allow future extensibility
ac44546 
Resolves open-telemetry#133

Thi is ane extensibility mechanism - a header for WebSocket messages. Every WebSocket message is composed of 2 parts: varint-encoded unsigned 64 bit integer header, followed by the serialized Protobuf AgentToServer/ServerToAgent messages.

The current value of the header is equal to 0. In the future when non-zero values of headers are introduced their use should be negotiated during the connection phase (via HTTP headers or via Capabilities fields). This is necessary to ensure interoperability between different OpAMP versions. Non-zero header values will not be a breaking change, they will only used after successful negotiation between the Client and Server.

Note: we don't need the header for plain HTTP transport. Any extensibility for HTTP can be done via HTTP headers (not suitable for WebSocket since it cannot work per message - HTTP headers are sent once per connection).

## Example Usage

The header can be used in the future for example in the following ways.

### Message Fragmentation

We found that some WebSocket implementations (e.g. [AWS API Gateway](open-telemetry#60 (comment))) limit the message size to 128KB. The only way to overcome this limitation is to perform message fragmentation and reassembly in OpAMP itself. This can be done by using one bit in the header to indicate whether the message is the last fragment.

### Support Other Compression Algorithms

WebSocket has built-in compression. What compression is used depends on the WebSocket implementation and realistically only "deflate" compression implementations are available. There are more modern compression algorithms (e.g. zstd), but it is impossible to use in OpAMP since most known WebSocket implementations simply don't support them.

The header can easily allow supporting these other compression algorithms. We can use one or more of the header bits to indicate the compression algorithm used.

Note: this sort of extension can be also done via HTTP header negotiation. However, not all WebSocket implementations allow custom HTTP headers (browsers don't).
@tigrannajaryan tigrannajaryan mentioned this issue Nov 9, 2022
Merged
tigrannajaryan added a commit to tigrannajaryan/opamp-spec that referenced this issue Nov 9, 2022
@tigrannajaryan
Adding a header for WebSocket messages to allow future extensibility
32dd947 
Resolves open-telemetry#133

Thi is ane extensibility mechanism - a header for WebSocket messages. Every WebSocket message is composed of 2 parts: varint-encoded unsigned 64 bit integer header, followed by the serialized Protobuf AgentToServer/ServerToAgent messages.

The current value of the header is equal to 0. In the future when non-zero values of headers are introduced their use should be negotiated during the connection phase (via HTTP headers or via Capabilities fields). This is necessary to ensure interoperability between different OpAMP versions. Non-zero header values will not be a breaking change, they will only used after successful negotiation between the Client and Server.

Note: we don't need the header for plain HTTP transport. Any extensibility for HTTP can be done via HTTP headers (not suitable for WebSocket since it cannot work per message - HTTP headers are sent once per connection).

## Example Usage

The header can be used in the future for example in the following ways.

### Message Fragmentation

We found that some WebSocket implementations (e.g. [AWS API Gateway](open-telemetry#60 (comment))) limit the message size to 128KB. The only way to overcome this limitation is to perform message fragmentation and reassembly in OpAMP itself. This can be done by using one bit in the header to indicate whether the message is the last fragment.

### Support Other Compression Algorithms

WebSocket has built-in compression. What compression is used depends on the WebSocket implementation and realistically only "deflate" compression implementations are available. There are more modern compression algorithms (e.g. zstd), but it is impossible to use in OpAMP since most known WebSocket implementations simply don't support them.

The header can easily allow supporting these other compression algorithms. We can use one or more of the header bits to indicate the compression algorithm used.

Note: this sort of extension can be also done via HTTP header negotiation. However, not all WebSocket implementations allow custom HTTP headers (browsers don't).
tigrannajaryan added a commit that referenced this issue Nov 9, 2022
@tigrannajaryan
Adding a header for WebSocket messages to allow future extensibility (#…
57de221 
…138)

Resolves #133

Thi is ane extensibility mechanism - a header for WebSocket messages. Every WebSocket message is composed of 2 parts: varint-encoded unsigned 64 bit integer header, followed by the serialized Protobuf AgentToServer/ServerToAgent messages.

The current value of the header is equal to 0. In the future when non-zero values of headers are introduced their use should be negotiated during the connection phase (via HTTP headers or via Capabilities fields). This is necessary to ensure interoperability between different OpAMP versions. Non-zero header values will not be a breaking change, they will only used after successful negotiation between the Client and Server.

Note: we don't need the header for plain HTTP transport. Any extensibility for HTTP can be done via HTTP headers (not suitable for WebSocket since it cannot work per message - HTTP headers are sent once per connection).

## Example Usage

The header can be used in the future for example in the following ways.

### Message Fragmentation

We found that some WebSocket implementations (e.g. [AWS API Gateway](#60 (comment))) limit the message size to 128KB. The only way to overcome this limitation is to perform message fragmentation and reassembly in OpAMP itself. This can be done by using one bit in the header to indicate whether the message is the last fragment.

### Support Other Compression Algorithms

WebSocket has built-in compression. What compression is used depends on the WebSocket implementation and realistically only "deflate" compression implementations are available. There are more modern compression algorithms (e.g. zstd), but it is impossible to use in OpAMP since most known WebSocket implementations simply don't support them.

The header can easily allow supporting these other compression algorithms. We can use one or more of the header bits to indicate the compression algorithm used.

Note: this sort of extension can be also done via HTTP header negotiation. However, not all WebSocket implementations allow custom HTTP headers (browsers don't).
@tigrannajaryan
Copy link
Member Author

We have added an ability to introduce message fragmentation in the future, which is how we will probably solve this problem.

I think there is nothing else to do at the moment on this issue. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andykellr andykellr

Labels
required-for-stable Required to be resolved before 1.0
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@andykellr @tigrannajaryan