feat(new sink): New `nats` sink #3605

erhlee-bird · 2020-08-27T20:58:43Z

Introduces a new NATS log sink.
Closes #1399.

erhlee-bird · 2020-08-27T21:05:00Z

To begin with, the code is basically just a straight copy of the console sink.
I've been able to compile and confirm with a basic setup that messages are being sent to NATS.

I copied some of the structure from various PRs I found in the repo, but definitely am missing a ton of things like documentation and metadata.

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

jamtur01 · 2020-10-14T17:59:35Z

Hi @erhlee-bird! Thanks for contributing. Anything we can help with here to get this out of draft?

erhlee-bird · 2020-10-14T18:05:37Z

Hey @jamtur01, I just started looking at this again today.

I've verified that I have a steel-thread running via hand-jammed integration tests.

I believe the next steps are

populate .meta/sinks/nats.toml.erb with requisite data
add in unit and integration tests
rebase to latest
run through the staging ci with bors?

Am I missing anything there?

update code to work with latest conventions on master
add cue-based documentation

jamtur01 · 2020-10-14T18:10:53Z

We just replaced the .meta stuff with Cue-based docs - have a look at the docs in docs/ for examples and we're not using Bors right now due to some bugs but otherwise, that sounds great!

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

erhlee-bird · 2020-10-14T21:14:11Z

@jamtur01, I think current is ready for review!

Thanks for the poke

jamtur01 · 2020-10-14T21:18:55Z

@erhlee-bird Looks like you just need to run cue fmt on the docs:

cue fmt docs/**/*.cue

Will work.

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

Cargo.toml

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

erhlee-bird · 2020-10-15T16:03:58Z

I'm not sure why the cue docs check is failing.
Any pointers there? I don't have any errors locally running make check-docs.

jszwedko

This is a great, thanks @erhlee-bird . I left some inline comments to address, but it is pretty close.

In addition to the in-line comments, I'd like to see us handle failed publishes differently by:

Using the acker() from the SinkContext (available in build) to only ack messages that were successfully published
Retry failed publishes indefinitely
Set reconnect_buffer_size on the nats client to 0 bytes so that the client doesn't buffer internally (to avoid message loss)

You can see the kafka sink for an example of the acking (open https://github.com/timberio/vector/blob/master/src/sinks/kafka.rs and search for "ack"). This will allow messages to sit in an intermediate buffer in vector until they can successfully be published to nats.

Let me know if that isn't clear.

Just noting that we should probably follow this work up with:

TLS support
Credentials support (user/pass, token, credentials file, nkey)

We can open separate issues for those once this is merged.

Thanks again!

src/sinks/nats.rs

jszwedko · 2020-10-16T18:46:57Z

docs/reference/components/sinks/nats.cue

+	}
+
+	configuration: {
+		url: {


I think we've switched to calling this sort of configuration endpoint per #1775, but that does feel weird in this context. I like url. I'm curious what @ktff thinks.

Yeah I saw the endpoint for other examples like Pulsar that looked similar.

I set it to url just to try and mimic the verbiage used by the nats.go library and the nats.io docs, but totally understand if it would make more sense to stay consistent within Vector context.

jszwedko · 2020-10-16T18:52:08Z

src/sinks/nats.rs

+impl NatsSink {
+    fn new(config: NatsSinkConfig) -> Self {
+        let options = nats::Options::new();
+        let nc = options.connect(&config.url).unwrap();


This method seems to initialize the outbound connection, which we generally try to avoid during component build time. Could we defer the connect to the run method? We should emit an error and propagate up an Err if it fails to connect (rather than panicing). I do note that it handles reconnects internally.

Also, could we set a user configurable name here? https://docs.rs/nats/0.8.0/nats/struct.Options.html#method.with_name . We could default to vector but let users override in the config.

Added a separate connect function to the SinkConfig.

Added the name config item.

src/sinks/nats.rs

jszwedko · 2020-10-16T20:02:25Z

I neglected that we aren't supporting encoding configuration here either. It would be nice to do that for consistency before this is merged. Again, the kafka sink (or the console sink) would be good references here.

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

erhlee-bird · 2020-10-19T20:45:37Z

@jszwedko I think I addressed all of your comments and am ready for another review.

jszwedko

A few last comments, but otherwise this looks good! Thanks for all of your work @erhlee-bird

And yes, @praveenperera , if you could sign-off the two commits, that would fix the DCO check.

src/sinks/nats.rs

docs/reference/components/sinks/nats.cue

jszwedko · 2020-11-12T18:06:49Z

I would like to get one more review on this just given it is a new component. ~~@JeanMertz do you mind taking a look?~~ Neglected that @lukesteensen was already tagged here.

praveenperera · 2020-11-12T18:08:28Z

A few last comments, but otherwise this looks good! Thanks for all of your work @erhlee-bird

And yes, @praveenperera , if you could sign-off the two commits, that would fix the DCO check.

Hey i’m not sure how to sign off on old commits. Since there have been new commits by @erhlee-bird after mine. I could rebase but don’t think I have permission to write to this branch.

jszwedko · 2020-11-12T19:16:40Z

@praveenperera 👍 it looks like you have access now at least? I think you can:

$ git rebase -i HEAD~20
# mark 2f8fcd0 and 9aab169 for edit
# when the rebase pauses, `git commit --amend --signoff && git rebase --continue`
$ git push -f

Signed-off-by: Praveen Perera <praveen@avencera.com>

Store `nats::Options` in owned struct

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

jszwedko

I pushed one more commit adding the integration tests to CI and an example of templating for the subject config.

Otherwise, this looks good to me! Thanks for pushing this over the line @erhlee-bird and @praveenperera !

And template example for subject config Signed-off-by: Jesse Szwedko <jesse@szwedko.me>

lukesteensen

Looks good to me!

jamtur01 · 2020-11-16T02:44:44Z

@erhlee-bird @praveenperera Thanks! Awesome contribution.

Co-authored-by: James Turnbull <james@lovedthanlost.net> Co-authored-by: Praveen Perera <ppraveen25@gmail.com> Co-authored-by: Jesse Szwedko <jesse@szwedko.me> Signed-off-by: Brian Menges <brian.menges@anaplan.com>

initial setup for a new sink addressing vectordotdev#1399

e5a4c06

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

erhlee-bird force-pushed the master branch from b2b5572 to e5a4c06 Compare August 31, 2020 12:45

jamtur01 requested a review from lukesteensen October 14, 2020 18:00

Merge branch 'master' into master

9a51e16

jamtur01 marked this pull request as ready for review October 14, 2020 18:06

jamtur01 requested a review from JeanMertz as a code owner October 14, 2020 18:06

jamtur01 added the domain: sinks Anything related to the Vector's sinks label Oct 14, 2020

erhlee-bird added 4 commits October 14, 2020 14:59

add nats-integration targets to Makefile

e56f531

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

update Cargo.lock for nats PR

c97788b

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

nats fix to latest conventions; add integration test

4a2f813

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

add nats cue documentation

f7b82b5

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

ran cue fmt and make fmt

a9a13ba

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

jamtur01 requested a review from jszwedko October 14, 2020 21:24

erhlee-bird added 3 commits October 14, 2020 17:30

remove encoding from nats docs

fd08bba

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

fix clippy errors for sinks/nats.rs

70fe73e

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

Merge branch 'master' of github.com:timberio/vector into master

ccf0562

praveenperera reviewed Oct 15, 2020

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

bump nats dependency to 0.8.0

fc5f7d9

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

jszwedko requested changes Oct 16, 2020

View reviewed changes

erhlee-bird added 2 commits October 19, 2020 14:31

enable codecs in docs and add 'name' config option

1ab5272

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

async nats; added acker, encoding, new events

ab5e132

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

jszwedko reviewed Nov 12, 2020

View reviewed changes

src/sinks/nats.rs Outdated Show resolved Hide resolved

src/sinks/nats.rs Outdated Show resolved Hide resolved

docs/reference/components/sinks/nats.cue Show resolved Hide resolved

erhlee-bird force-pushed the master branch from 47d2195 to 4a98c6c Compare November 12, 2020 18:21

praveenperera force-pushed the master branch 2 times, most recently from bbd7cf2 to 49d8c1b Compare November 12, 2020 19:00

praveenperera and others added 5 commits November 12, 2020 14:52

Store nats::Options in owned struct

46a4c24

Signed-off-by: Praveen Perera <praveen@avencera.com>

Remove useless convertion

c02abf0

Signed-off-by: Praveen Perera <praveen@avencera.com>

Merge pull request #1 from praveenperera/master

b8694b7

Store `nats::Options` in owned struct

Merge branch 'master' of github.com:timberio/vector into master

13228b5

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

update to nats v0.8.3

78d85a0

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

praveenperera force-pushed the master branch from 49d8c1b to 78d85a0 Compare November 12, 2020 19:54

erhlee-bird added 5 commits November 12, 2020 16:37

add nats sink missing keys event like the elasticsearch sink

f093537

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

change nats integration test to map over the subscription

8868142

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

reorder internal_events/mod.rs; implement generateconfig for nats sink

287416a

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

Merge remote-tracking branch 'upstream/master' into master

37f4e9e

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

update nats sink docs

943562d

Signed-off-by: Eric Lee <erhlee.bird@gmail.com>

jszwedko removed the request for review from JeanMertz November 13, 2020 14:57

jszwedko approved these changes Nov 13, 2020

View reviewed changes

jszwedko added 2 commits November 13, 2020 11:33

Merge branch 'master' into master

1b9dea6

Add test-integration-nats to CI

d61b207

And template example for subject config Signed-off-by: Jesse Szwedko <jesse@szwedko.me>

erhlee-bird requested a review from jamtur01 as a code owner November 13, 2020 16:50

jszwedko removed the request for review from jamtur01 November 13, 2020 16:51

lukesteensen approved these changes Nov 13, 2020

View reviewed changes

Merge branch 'master' into master

2631336

jamtur01 merged commit b8f83d7 into vectordotdev:master Nov 16, 2020

rugwirobaker mentioned this pull request May 29, 2021

Feature: nats logs source #7681

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(new sink): New `nats` sink #3605

feat(new sink): New `nats` sink #3605

erhlee-bird commented Aug 27, 2020

erhlee-bird commented Aug 27, 2020

jamtur01 commented Oct 14, 2020

erhlee-bird commented Oct 14, 2020 •

edited

Loading

jamtur01 commented Oct 14, 2020

erhlee-bird commented Oct 14, 2020

jamtur01 commented Oct 14, 2020

erhlee-bird commented Oct 15, 2020

jszwedko left a comment

jszwedko Oct 16, 2020

erhlee-bird Oct 16, 2020

jszwedko Oct 16, 2020

erhlee-bird Oct 19, 2020

jszwedko commented Oct 16, 2020

erhlee-bird commented Oct 19, 2020

jszwedko left a comment

jszwedko commented Nov 12, 2020 •

edited

Loading

praveenperera commented Nov 12, 2020

jszwedko commented Nov 12, 2020

jszwedko left a comment

lukesteensen left a comment

jamtur01 commented Nov 16, 2020 •

edited

Loading

feat(new sink): New nats sink #3605

feat(new sink): New nats sink #3605

Conversation

erhlee-bird commented Aug 27, 2020

erhlee-bird commented Aug 27, 2020

jamtur01 commented Oct 14, 2020

erhlee-bird commented Oct 14, 2020 • edited Loading

jamtur01 commented Oct 14, 2020

erhlee-bird commented Oct 14, 2020

jamtur01 commented Oct 14, 2020

erhlee-bird commented Oct 15, 2020

jszwedko left a comment

Choose a reason for hiding this comment

jszwedko Oct 16, 2020

Choose a reason for hiding this comment

erhlee-bird Oct 16, 2020

Choose a reason for hiding this comment

jszwedko Oct 16, 2020

Choose a reason for hiding this comment

erhlee-bird Oct 19, 2020

Choose a reason for hiding this comment

jszwedko commented Oct 16, 2020

erhlee-bird commented Oct 19, 2020

jszwedko left a comment

Choose a reason for hiding this comment

jszwedko commented Nov 12, 2020 • edited Loading

praveenperera commented Nov 12, 2020

jszwedko commented Nov 12, 2020

jszwedko left a comment

Choose a reason for hiding this comment

lukesteensen left a comment

Choose a reason for hiding this comment

jamtur01 commented Nov 16, 2020 • edited Loading

feat(new sink): New `nats` sink #3605

feat(new sink): New `nats` sink #3605

erhlee-bird commented Oct 14, 2020 •

edited

Loading

jszwedko commented Nov 12, 2020 •

edited

Loading

jamtur01 commented Nov 16, 2020 •

edited

Loading