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

Migrate to async_nats NATS client #15534

Closed
makarchuk opened this issue Dec 11, 2022 · 1 comment · Fixed by #18165
Closed

Migrate to async_nats NATS client #15534

makarchuk opened this issue Dec 11, 2022 · 1 comment · Fixed by #18165
Labels
sink: nats Anything `nats` sink related source: nats Anything `nats` source related type: feature A value-adding code addition that introduce new functionality.

Comments

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

Vector currently uses nats::asynk client which is not a recommended way of using it, as newer official async api exists in a form of async_nats crate. nats crate is described as "Legacy synchronous client that supports" in it's own repo, which also mentions that This client will be deprecated soon.

Furthermore nats::asynk lacks async support for NATS JetStream (there's a sync one, but that doesn't fit for vector's purposes), and JetStream seems to be a pretty good candidate for becoming another Vector source (I'll create a separate feature request for this later and will try to contribute this client myself).

So it seems that migrating to a more modern and async native async_nats client would be benifitial for a project as a whole.

Wanted to mention that I'm sorry if I failed to follow a proper process here. I tried my best and please feel free to point out any mistakes I've made.

Attempted Solutions

No response

Proposal

I have prepared an MR that implements such a migration. Although there're a couple of things that concern me and I would like to hear an opinion on those:

  • async_nats doesn't support reconnect_buffer_size configuration option and judging by a comment in Vector code where this options is set Set reconnect_buffer_size on the nats client to 0 bytes so that the client doesn't buffer internally (to avoid message loss). it appears to be quite crucial. I can create a feature request in https://github.com/nats-io/nats.rs as it seems to me it's a matter of simple addition of a setter method.
  • Due to changes in error handling approach nats client no longer returns std::io::Error as it's error type, so I was not able to retain error_type field in internal_events/nats.rs

References

#15533

Version

No response

@makarchuk makarchuk added the type: feature A value-adding code addition that introduce new functionality. label Dec 11, 2022
@jszwedko jszwedko added sink: nats Anything `nats` sink related source: nats Anything `nats` source related labels Dec 12, 2022
@paolobarbolini
Copy link
Contributor

FYI I'm making a new attempt at migrating to it in #18165

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sink: nats Anything `nats` sink related source: nats Anything `nats` source 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.

3 participants