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.

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

Support binary payload for the websocket sink #18037

Closed
zhongchen opened this issue Jul 20, 2023 · 5 comments · Fixed by #18060
Closed

Support binary payload for the websocket sink #18037

zhongchen opened this issue Jul 20, 2023 · 5 comments · Fixed by #18060
Labels
sink: websocket Anything `websocket` sink related type: feature A value-adding code addition that introduce new functionality.

Comments

@zhongchen
Copy link
Contributor

A note for the community

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Use Cases

I need to transfer proto-encoded data from a source to a WebSocket sink. Currently, the WebSocket sink converts the encoded payload into text before sending it over the wire using from_utf8_lossy. As a result, it corrupts my proto payload since proto payloads are not utf-8 strings.

                            let message = Message::text(String::from_utf8_lossy(&bytes));
                            let message_len = message.len();

                            ws_sink.send(message).await.map(|_| {
                                events_sent.emit(CountByteSize(1, event_byte_size));
                                bytes_sent.emit(ByteSize(message_len));
                            })

Attempted Solutions

No response

Proposal

Provide one additional option for the Websocket sink to choose between Text and Binary payload for transferring the data.

The underlying Websocket lib used by Vector supports binary payload.

https://docs.rs/websocket/latest/websocket/message/struct.Message.html#method.binary

References

No response

Version

No response

@zhongchen zhongchen added the type: feature A value-adding code addition that introduce new functionality. label Jul 20, 2023
@zamazan4ik
Copy link
Contributor

I think you could be interested in this PR: #18019

@zhongchen
Copy link
Contributor Author

@zamazan4ik For my specific use case, I only want to use vector to pass through the data as is. I will try to open a PR for the issue since I don't think it is hard to do, just need to add one additional option to configure websocket to send binary frames instead of always sending text frames.

@jszwedko jszwedko added the sink: websocket Anything `websocket` sink related label Jul 21, 2023
@jszwedko
Copy link
Member

Thanks @zhongchen ! I agree that it seems like we should avoid encoding as UTF-8 always.

@zhongchen
Copy link
Contributor Author

@jszwedko If that's the proposed fix, then it is much easier. #18060

@neuronull
Copy link
Contributor

👋 Hi @zhongchen , thanks for opening a PR to address this.

I think when Jesse said

I agree that it seems like we should avoid encoding as UTF-8 always.

He didn't mean that we should never encode as utf-8 , but that we should not always assume the user wants to encode as utf-8.

Will follow-up on the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sink: websocket Anything `websocket` sink related type: feature A value-adding code addition that introduce new functionality.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants