contrib: skeleton app structure & Dogstatsd nsqd addon

[dm03514]

The goal of this is to add a contrib pacakge which allows for extensions/addons for NSQD. The idea was to make the contrib as separate from NSQD as possible. To do this the contrib nsqd addons are passed a reference to NSQD and can only interact with it through its exported methods.

It was also the hope that contrib addons would be completely segregated from core so that updating implementation or command line flags shouldn't require touching core at all.

DatadogStatsd was chosen to deliver the contrib package.

• [x] Get Datadogstatsd actually talking to local statsd :(
• [x] Cleanup flags - (a single multi-value (array) flag)
• [x] Dogstatsd test coverage
• [x] Have default ddstatsd options be defined in contrib/dogstatsd.go instead of contrib/nsq.go
• [x] AddModuleGoroutine() && Exit channel passed to Loop
• [x] README on how to add contrib modules
• [x] contrib/nsqd.go make sure that options are provided for each module initialized
• [x] update contrib/nsq.go to filter out valid opts and only pass those to the addon initializer

refs #904

Verified Datadogstatsd can communicate with local udp server

Started with:

$ ./apps/nsqd/nsqd --mod-opt=-dogstatsd-address=127.0.0.1:8125

Verified in local UDP echo server:

waiting to receive message
received 72 bytes from ('127.0.0.1', 55315)
nsq.channel.in_flight_count:0|g|#topic_name:name,channel_name:super_cool
sent 72 bytes back to ('127.0.0.1', 55315)

waiting to receive message
received 71 bytes from ('127.0.0.1', 55315)
nsq.channel.deferred_count:0|g|#topic_name:name,channel_name:super_cool
sent 71 bytes back to ('127.0.0.1', 55315)

waiting to receive message
received 70 bytes from ('127.0.0.1', 55315)
nsq.channel.requeue_count:0|c|#topic_name:name,channel_name:super_cool
sent 70 bytes back to ('127.0.0.1', 55315)

waiting to receive message
received 70 bytes from ('127.0.0.1', 55315)
nsq.channel.timeout_count:0|c|#topic_name:name,channel_name:super_cool
sent 70 bytes back to ('127.0.0.1', 55315)

[ploxiln]

I was thinking these things would be called something like "optional modules". Though "contrib" isn't too bad.

My idea for command-line flags, and the config file, is to pass through a single multi-value (array) flag, which would be called something like --mod or --mod-opt. It would be used like:

... --mod=ddstatsd=address=127.0.0.1:2345 --mod=ddstatsd=interval=30

The main "module" or "contrib" system would only initialize a "module" if there are any options for that module. It passes the module just that module's options as an array like ["address=127.0.0.1:2345", "interval=30"].

This way, everything about a module's options is only in the module, and nsqd flag and config file handling is unaffected except for a single --mod that collects an array of strings and just passes them along.

Just my humble ideas :)

[dm03514]

@ploxiln with the mod-opts flag and pushing the opts parsing to the optional modules, do you know of any way to show the optional modules CLI options on the top level nsqd --help ? Is that a requirement?

[ploxiln]

Yeah, it won't be convenient to show the module options in nsqd --help, that's a downside. But you could add a --mod-opts-help flag to call into each module to print its options ... just an idea.

[dm03514]

@ploxiln awesome! ty

[ploxiln]

@mreiferson - have any opinions on this, the structure it is developing?

[mreiferson]

Sorry I'm late following up on this — I feel like we should investigate how we could structure this using https://golang.org/pkg/plugin/. It would be linux-only, but it would allow us to define some sort of API in nsqd and then support runtime-loaded plugins that could be maintained separately, e.g. this datadog stats plugin.

[ploxiln]

Eh, I think it's a bit early to use Plugin

[ploxiln]

I just re-added a comment about this approach. But you should probably not actually work on it if @mreiferson is against this.

I am sorry if I've led you on a fruitless path, but in my defense, I've tried to be clear that I was only giving my opinion 😬

[mreiferson]

Hah!

I guess my position is that, unless it's truly going to be an opt-in plugin model, then why jump through all the hoops with internal code to structure it like that? I guess the argument would be that adopting a flag API like what we've got here would allow us to migrate to a plugin model in the future?

[dm03514]

sorry obviously very new to nsq and am new to go too:

• would the plugin required end user to compile with plugin flag for each plugin they wanted enabled?
• could you imagine nsq providing a binary with all plugins enabled?
• I just read about plugins for the first time last night, would it be possible to view it as an implementation detail (Was this your last point?) and change once a a clear pattern for how plugins are likely to work?
• is the main concern still supporting paid/proprietary services/protocols in nsq core?

Thank you Danny

On Sunday, July 23, 2017, Matt Reiferson [email protected] wrote:

Hah!

I guess my position is that, unless it's truly going to be an opt-in plugin model, then why jump through all the hoops with internal code to structure it like that? I guess the argument would be that adopting a flag API like what we've got here would allow us to migrate to a plugin model in the future?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nsqio/nsq/pull/909#issuecomment-317281208, or mute the thread https://github.com/notifications/unsubscribe-auth/AATpqxjGm3dQaMLaXMfME1shoutyBDy3ks5sQ7JmgaJpZM4N-afW .

[ploxiln]

I can see a path forward to plugins, from this model. nsqd would need to look in a directory (probably from a flag option) for plugin files named e.g. name.so. It would get a reference to the "New()" function and the "Start()" function, and call them similarly as this does.

The point of plugins is so that they can be compiled separately from nsqd, from code in separate repositories, and then used with the released build of nsqd (I think). This repo might itself include some nsqd plugins, built along with all the apps by make. This would be good as an example and ongoing validation of the plugin interface, and for this kind of "contrib-addon-but-popular" functionality. But the main point of the plugins would be to enable separate-repo separate-build nsqd addons.

I don't think "paid/priority" is a thing we're thinking about. Just about trying to enable people to do what they want, without "muddying up" the core code, or needed to get agreement from all core contributors. It's really just another way to organize code to optimize for maintenance and flexibility.

[dm03514]

@ploxiln Could you possibly take another look? I added the initializer table to only initialize addons if one of their options are passed.

[ploxiln]

I have a couple minor stylistic quibbles, but I like the structure. @mreiferson are you somewhat convinced yet ;)

[dm03514]

@ploxiln updated to filter valid options (and only pass those to addon initializer), and user more accurate option matching :)

[zouyee]

nice work！

[ploxiln]

I'll take another look real soon, was just a bit busy at work ...

[dm03514]

Thank you!

On Friday, August 18, 2017, Pierce Lopez [email protected] wrote:

I'll take another look real soon, was just a bit busy at work ...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nsqio/nsq/pull/909#issuecomment-323493838, or mute the thread https://github.com/notifications/unsubscribe-auth/AATpq4l1KmPfsQT8PgPlccmxBMt9ABHGks5sZkVbgaJpZM4N-afW .

[ploxiln]

@mreiferson I think I can clean this up a bit, in terms of API, before the next release, and I think I can work on transitioning this to use go plugins (though they are still linux-only in go-1.9 I think). Thoughts?

@dm03514 can you squash these commits into one?

[ploxiln]

actually, everyone just wait for a few days, I'll open another PR (which includes and adds to this one)

[mreiferson]

WAITING 😳

[dm03514]

Is there any work I can help with?

[ploxiln]

It's very reasonable to wonder what is going on with this PR. I happen to be on a vacation overseas, so you won't see any update from me before the end of this month. (For what it's worth, I was busy trying to wrap things up at work before the vacation.) So, I apologize for holding this up. I have not forgotten about it :)

[dm03514]

Thank you @ploxiln !