Conversation
add08ee
to
269968a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are either limiting repetitive information in the README, or duplicating in each plugin how-to. It's your call whether the README has that core introduction or each folder has a complete tutorial. We need your recommendation on which approach you want to take.
|
||
In order to write a plugin for snap, it is necessary to define some methods to satisfy the appropriate interfaces. The interface is slightly different depending on what type (collector, processor, or publisher) of plugin is being written. The interfaces are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be best to give people the choice of writing a plugin right away. This additional context is excellent, though too much at this point. Some of it may go well under a ##a brief explanation of snap architecture
or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, added a separate sub-section
|
||
Communication between Snap and plugins uses the GRPC protocol. GRPC uses HTTP2, which improves performance versus HTTP1, and uses [protocol buffers](https://developers.google.com/protocol-buffers/) for simplicity, performance, and smaller memory size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/GRPC/gRPC/g. We can link to the docs on introduction too: http://www.grpc.io/docs/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added gRPC link
type Plugin interface { | ||
GetConfigPolicy() (ConfigPolicy, error) | ||
} | ||
Before starting writing Snap plugins, check out the Plugin Catalog to see if any suit your needs. If not, you need to reference the plugin packages that defines the type of structures and interfaces inside snap and then write plugin endpoints to implement the defined interfaces. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a key call to action for users. Point right to the plugin wishlist: https://github.com/intelsdi-x/snap/labels/plugin-wishlist and make this just before they decide to write their own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated to include plugin wishlist mention
CollectMetrics([]Metric) ([]Metric, error) | ||
} | ||
## Snap Plugin Go Library Examples | ||
You will find [example plugins](https://github.com/intelsdi-x/snap-plugin-lib-go/tree/master/examples) that cover the basics for writing collector, processor, and publisher plugins in teh examples folder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo - teh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
} | ||
>snap-plugin-[plugin-type]-[plugin-name] | ||
|
||
There are three plugin types supported by Snap: collector, processor, publisher |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this information right to have in the README or under each example? Mapping out the developer's path to this page makes me think that the README should be a brief intro, links to how-tos for each type, overview of what/why, links to where to get involved and that's it. Leaning toward limiting repetitive information in the README, though I'm concerned with duplicate in each plugin how-to as well. It's your call whether the README has that core introduction or each folder has a complete tutorial.
// Snap pipeline. | ||
type Processor interface { | ||
Plugin | ||
## Snap Plugin Go Library Implementaion Files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo - Implementaion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are either limiting repetitive information in the README, or duplicating in each plugin how-to. It's your call whether the README has that core introduction or each folder has a complete tutorial. We need your recommendation on which approach you want to take.
ce96a1d
to
0b7119f
Compare
|
||
This is a library for writing plugins for the [snap](https://github.com/intelsdi-x/snap) telemetry framework. The goal for this library is to make it super simple to write a plugin. | ||
A library for writing plugins in Go for the [Snap telemetry framework](https://github.com/intelsdi-x/snap) telemetry framework. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
repeated telemetry framework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
||
# Writing a plugin | ||
Snap has three different plugin types and for instructions on how to write a plugin check out the [collector](https://github.com/sarahjhh/snap-plugin-lib-go/blob/docs/examples/collector/README.md), processor, and publisher plugin docs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you ready to link to processor and publisher too or you holding off? I think it'd be good to capitalize them too since they're core concepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missed these links, added in now :)
###1. Get the plugin library repo: | ||
`go get github.com/intelsdi-x/snap-plugin-lib-go` will add the repo to your $GOPATH | ||
|
||
note: if you want to contribute to this repository you should fork the snap-plugin-lib-go repo and open a PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the intention of the note to let people know they can contribute or to warn them to fork before they open a PR?
|
||
## Starting a plugin | ||
|
||
After implementing a type that satisfies one of {collector, processor, publisher} interfaces, all that is left to do is a call the appropriate plugin.StartX() with your plugin specific options. That could be as simple as: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could be as easy as:
Are there cases when it isn't as simple as this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated text:
'For example with no meta options specified:' instead of
'That could be as simple as:'
## Testing | ||
For testing reference the [Snap Testing Guidelines](https://github.com/intelsdi-x/snap/blob/master/CONTRIBUTING.md#testing-guidelines). To test your plugin with Snap you will need to have [Snap](https://github.com/intelsdi-x/snap) installed, check out these docs for [Snap setup details](https://github.com/intelsdi-x/snap/blob/master/docs/BUILD_AND_TEST.md#getting-started). | ||
|
||
You should specify the test size (either small, medium, or large) with the specified build tag `// +build test-size`. The default testing will run all size tests if the build tag is not specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/You should/You will have to/
I link to the explanation of S/M/L would be nice there as well.
https://github.com/intelsdi-x/snap/blob/master/docs/BUILD_AND_TEST.md#test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
|
||
## Ready to Share | ||
|
||
When you've finished your plugin make sure to announce on [slack](https://intelsdi-x.herokuapp.com/) and get your plugin added to the [Plugin Catalog](https://github.com/intelsdi-x/snap/blob/master/docs/PLUGIN_CATALOG.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about it more, we should recommend people cut a release of their plugin when they're ready based on the plugin version they have as a const
in their file. Steps to do so - https://help.github.com/articles/creating-releases/
That will be consistent with how we cut releases on plugins and then fit nicely into our linking within the Plugin Catalog.
The last part stands: When you're ready to share a plugin and want to share it, post in Slack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
package rand | ||
``` | ||
|
||
## Ready to Share |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same edits to Ready to Share as above.
@@ -0,0 +1,142 @@ | |||
## Snap Plugin Go Library: Publisher Plugin Example | |||
Here you will find an example plugin that cover the basics for writing a publisher plugin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo - Example plugin that cover_s_
``` | ||
|
||
## Ready to Share | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above ^
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor improvements, particularly on what someone does when they finish writing the plugin itself.
It's okay that we don't cover everything to every detail - it has to be deep enough into the process without overwhelming the new user. I think you do a great job of balancing those.
@@ -0,0 +1,141 @@ | |||
## Snap Plugin Go Library: Processor Plugin Example | |||
Here you will find an example plugin that cover the basics for writing a processor plugin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo - example plugin that cover_s_
@@ -0,0 +1,144 @@ | |||
|
|||
## Snap Plugin Go Library: Collector Plugin Example | |||
Here you will find an example plugin that cover the basics for writing a collector plugin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo - example plugin that cover_s_
|
||
This is a library for writing plugins for the [snap](https://github.com/intelsdi-x/snap) telemetry framework. The goal for this library is to make it super simple to write a plugin. | ||
A library for writing plugins in Go for the [Snap telemetry framework](https://github.com/intelsdi-x/snap). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/A library/This is a library/
|
||
In order to write a plugin for snap, it is necessary to define some methods to satisfy the appropriate interfaces. The interface is slightly different depending on what type (collector, processor, or publisher) of plugin is being written. The interfaces are: | ||
Before writing any Snap plugins, check out the [Plugin Catalog](https://github.com/intelsdi-x/snap/blob/master/docs/PLUGIN_CATALOG.md) to see if any suit your needs. Also check the [plugin wishlist](https://github.com/intelsdi-x/snap/labels/plugin-wishlist) to see if anyone has already requested your desired plugin. If you do decide to write a plugin, open a new issue following the plugin wishlist guidelines and let us know you are working on one! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider restructuring with some bullets.
Before writing a snap plugin:
- Verify it doesn't already exist by reviewing the Plugin Catalog
- Verify it hasn't been started by reviewing the plugin wishlist
If you do decide to write a plugin, open a new issue following the plugin wishlist guidelines and let us know you are working on one!
// Collector is a plugin which is the source of new data in the Snap pipeline. | ||
type Collector interface { | ||
Plugin | ||
Snap itself runs as a master daemon with the core functionality that may load and unload plugin processes via either CLI or HTTP APIs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snap is an open and modular telemetry framework designed to simplify the collection, processing and publishing of data through a single HTTP based API. Plugins provide the functionality of collection, processing and publishing and can be loaded/unloaded, upgraded and swapped without requiring a restart of the Snap daemon.
// Snap pipeline. | ||
type Processor interface { | ||
Plugin | ||
Communication between Snap and plugins uses the gRPC protocol. [gRPC](http://www.grpc.io/docs/) uses HTTP2, which improves performance versus HTTP1, and uses [protocol buffers](https://developers.google.com/protocol-buffers/) for simplicity, performance, and smaller memory size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merging both of these paragraphs.
A snap plugin is a program that responds to a set of well defined grpc services with parameters and return types specified as protocol buffer messages (see plugin.proto). The Snap daemon handshakes with the plugin over stdout and then communicates over gRPC.
@@ -0,0 +1,135 @@ | |||
## Snap Plugin Go Library Examples | |||
Here you will find example plugins that cover the basics for writing collector, processor, and publisher plugins in the examples folder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the examples folder (since we started with "Here you will find")
Below are some sample commands to try: | ||
|
||
``` | ||
$ $GOPATH/snap/build/bin/snapctl plugin load example-collector |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$GOPATH is not correct here.
I would suggest removing the path to snapctl to all of these examples but add a note above stating that snapctl
and snapd
should be in their PATH.
|--main.go | ||
``` | ||
|
||
* The main file (for example `main.go`) allows each plugin to be a stand-alone binary executable. The main file will include: pluginName and pluginVersion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I woud scratch these two sentences.
## Testing | ||
For testing reference the [Snap Testing Guidelines](https://github.com/intelsdi-x/snap/blob/master/CONTRIBUTING.md#testing-guidelines). To test your plugin with Snap you will need to have [Snap](https://github.com/intelsdi-x/snap) installed, check out these docs for [Snap setup details](https://github.com/intelsdi-x/snap/blob/master/docs/BUILD_AND_TEST.md#getting-started). | ||
|
||
You will have to specify the [test size](https://github.com/intelsdi-x/snap/blob/master/docs/BUILD_AND_TEST.md#test) (either small, medium, or large) with the specified build tag `// +build test-size`. The default testing will run all size tests if the build tag is not specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each test file should specify the appropriate build tag such as "small", "medium", "large" (e.g. // +build small
).
The default testing will run all size tests if the build tag is not specified. If we leave this in it should include how go test
should also be run with the option -tags small
if the intent was to only run small tests.
``` | ||
|
||
## Ready to Share | ||
We recommend cutting a release your plugin based on the plugin version specified as a const in your code (in this example specified in main.go). Steps to [create a release](https://help.github.com/articles/creating-releases/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Steps to create a release. We recommend that your release version match the version in your plugin (example: here).
have updated and incorporated suggested changes. |
|
||
* Verify it doesn't already exist by reviewing the [Plugin Catalog](https://github.com/intelsdi-x/snap/blob/master/docs/PLUGIN_CATALOG.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor request, but thinking this could be less formal:
Before writing a Snap plugin:
- See if one already exists in the [Plugin Catalog]
- See if someone mentioned it in [the plugin wishlist]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
type Plugin interface { | ||
GetConfigPolicy() (ConfigPolicy, error) | ||
} | ||
If you do decide to write a plugin, open a new issue following the plugin wishlist guidelines and let us know you are working on one! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to wishlist guidelines - https://github.com/intelsdi-x/snap/blob/master/docs/PLUGIN_CATALOG.md#wish-list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
||
Process([]Metric, Config) ([]Metric, error) | ||
} | ||
A snap plugin is a program that responds to a set of well defined [grpc](http://www.grpc.io/) services with parameters and return types specified as protocol buffer messages (see [plugin.proto](https://github.com/intelsdi-x/snap/blob/master/control/plugin/rpc/plugin.proto)). The Snap daemon handshakes with the plugin over stdout and then communicates over gRPC. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/snap/Snap/g
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
||
## Build Plugins & Use with Snap | ||
|
||
To get these example collector, processor, and publisher plugins to build properly and work with Snap you will need to have [glide](https://glide.sh/) installed on your PATH. You should also add snapctl and snapd to your PATH. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in your $PATH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
To test these plugins with Snap, you will need to have [Snap](https://github.com/intelsdi-x/snap) installed, check out these docs for [Snap setup details](https://github.com/intelsdi-x/snap/blob/master/docs/BUILD_AND_TEST.md#getting-started). | ||
|
||
###1. Get the plugin library repo: | ||
`go get github.com/intelsdi-x/snap-plugin-lib-go` will add the repo to your $GOPATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running go get github.com/intelsdi-x/snap-plugin-lib-go
errors - is there syntax that won't show a build issue? I can't figure out why -d
isn't sufficient when I try. cc @IRCody @jcooklin @kindermoumoute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use either go get github.com/intelsdi-x/snap-plugin-lib-go/...
or go get github.com/intelsdi-x/snap-plugin-lib-go/v1/plugin
.
The go get command actually works with packages not repos(or at least that's my understanding). The error that shows up is because there is no top-level package. I think it could also be addressed by having an empty main.go
at the top level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this part be updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IRCody yes, sorry missed this earlier - will update now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, updated this
$ go build -o example-publisher examples/publisher/main.go | ||
``` | ||
|
||
###4. Run Snap, Load plugins, and Run Tasks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
capitalize Plugins
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k
An example of what using all of them would look like: | ||
|
||
```go | ||
plugin.StartCollector(mypackage.Mytype{}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are a
thourgh e
local variables or declared constants? Pointing to an example would be helpful if there's a good one around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
constants i think, more details in https://github.com/intelsdi-x/snap-plugin-lib-go/tree/master/v/1/plugin/meta.go linked in the beginning of section.
@IRCody any other examples we can link here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are just placeholders for whatever values a plugin author would want to put there. Maybe it would read more clearly as:
// An example using some arbitrary values:
plugin.StartCollector(
mypackage.Mytype{},
pluginName,
pluginVersion,
plugin.ConcurrencyCount(2),
plugin.Exclusive(true),
plugin.Unsecure(true),
plugin.RoutingStrategy(StickyRouter),
plugin.CacheTTL(time.Second))
Which both gives example of legit values and calls out that they are arbitrarily selected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @IRCody, updated to reflect your improved example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an incredibly good starting point for this repo. LGTM after the minor edits recommended in line.
``` | ||
## Snap Plugin Go Library Implementation Files | ||
You will find all the library packages in the [v1/plugin folder](v1/plugin). | ||
The [rpc folder](v1/plugin/rpc) contents are auto-generated and should not be modified by anyone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
originally I was trying to provide info on what's included in the repo, but with all other updates this is probably unnecessary, went ahead and took this section out.
An example of what using all of them would look like: | ||
|
||
```go | ||
plugin.StartCollector(mypackage.Mytype{}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are just placeholders for whatever values a plugin author would want to put there. Maybe it would read more clearly as:
// An example using some arbitrary values:
plugin.StartCollector(
mypackage.Mytype{},
pluginName,
pluginVersion,
plugin.ConcurrencyCount(2),
plugin.Exclusive(true),
plugin.Unsecure(true),
plugin.RoutingStrategy(StickyRouter),
plugin.CacheTTL(time.Second))
Which both gives example of legit values and calls out that they are arbitrarily selected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after responding to @IRCody's comments.
ebcfe74
to
9b7127d
Compare
You've made a plugin! Now it's time to share it. Create a release by following these [steps](https://help.github.com/articles/creating-releases/). We recommend that your release version match your plugin version, see example [here](https://github.com/intelsdi-x/snap-plugin-lib-go/blob/master/examples/collector/main.go#L29). | ||
|
||
Don't forget to announce your plugin release on [slack](https://intelsdi-x.herokuapp.com/) and get your plugin added to the [Plugin Catalog](https://github.com/intelsdi-x/snap/blob/master/docs/PLUGIN_CATALOG.md)! | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mjbrender-updated this last section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! @sarahjhh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
summary of changes: