diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7522e243d..b6b131e06 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -63,21 +63,47 @@ When reporting an issue, details are key. Include the following: ## Notes on GitHub Usage It's worth noting that we don't use all the native GitHub features for issue management. For instance, it's uncommon for us to assign issues to the developer who will address it. Here are notes on what we do use. -### Issue Labels -Snap maintainers have a set of labels we'll use to keep up with issues that are organized: - -GitHub Tagging Strategy +### TL;DR Labels +We use a number of labels for context in the main framework of Snap. Plugin repository labels will keep it simple. If you want to contribute to Snap, here are the most helpful ones for you: -* **bug** - the classic definition of missing or misbehaving code from existing functionality (this includes malfunctioning tests) -* **feature request** - any new functionality or improvements/enhancements to existing functionality. Note that we use a single term for this (instead of both feature & enhancement labels) since it's prioritized in identical ways during sprint planning -* **question** - discussions related to snap, its administration or other details that do not outline how to address the request -* **RFC** - short for [request for comment](https://en.wikipedia.org/wiki/Request_for_Comments). These are discussions of snap features requests that include detailed opinions of implementation that are up for discussion +1. **help-wanted** ([link](https://github.com/intelsdi-x/snap/labels/help-wanted)) - some specific issues maintainers would like help addressing +2. **type/rfc** ([link](https://github.com/intelsdi-x/snap/labels/type%2Frfc)) - we need active feedback on *how best* to solve these issues +3. **plugin-wishlist** ([link](https://github.com/intelsdi-x/snap/labels/plugin-wishlist)) - these are a great opportunity to write a plugin -We also add contextual notes we'll use to provide more information regarding an issue: - - * **in progress** - we're taking action (right now). It's best not to develop your own solution to an issue in this state. Comments are welcome - * **help wanted** - A useful flag to show this issue would benefit from community support. Please comment or, if it's not in progress, say you'd like to take on the request - * **on hold** - an idea that gained momentum but has not yet been put into a maintainer's queue to complete. Used to inform any trackers of this status - * **tracked** - this issue is in the JIRA backlog for the team working on snap - * **duplicate** - used to tag issues which are identical to other issues _OR_ which are resolved by the same fix of another issue (either case) - * **wontfix** - the universal sign that we won't fix this issue. This tag is important to use as we separate out the nice-to-have features from our strategic direction +### Issue Labels +Snap maintainers have a set of labels we use to keep up with issues. They are separated into namespaces: + +* **type/** - the category of issue. All issues will have one or more +* **reviewed/** - indicator a maintainer reviewed the issue. All issues should have one or more +* **breaking-change/** - added to an Issue to note its merge would result in a change to existing behavior throughout the framework +* **component/** - issues related to a particular package in the framework +* **area/** - issues related to an overall theme and does not map to a single package +* **effort/** - amount of work to do related to resolving or merging this code change + +Other indicators: +* **reviewed/on-hold** - an idea that gained momentum but has not yet been put into a maintainer's queue to complete. Used to inform any trackers of this status +* **tracked** - this issue is in the JIRA backlog for the team working on Snap +* **reviewed/duplicate** - used to tag issues which are identical to other issues _OR_ which are resolved by the same fix of another issue (either case) +* **reviewed/wont-fix** - the universal sign that we won't fix this issue. This tag is important to use as we separate out the nice-to-have features from the maintainer's agreed upon strategic direction +* **wip-do-not-merge** - was made to clarify that a PR was just beginning to be worked, specifically for a PR to indicate it is not ready for review yet + +The difference between bugs, features and enhancements can be confusing. To be extra clear, we reduced it down to two options. Here are their definitions: +* **type/bug** - the classic definition of missing or misbehaving code from existing functionality (this includes malfunctioning tests) +* **type/feature-or-enhancement** - any new functionality or improvements/enhancements to existing functionality. We use one label because it's prioritized in identical ways during sprint planning + +For the sake of clarity, here are a few scenarios you might see play out. + +As a maintainer: +* An issue is opened stating that Snap is not working. Upon review, the maintainer finds it is an issue with a plugin. She will label the issue with **reviewed/wrong-repo** and open a new issue under the plugin where she tags the original issue reporter, links the original issue and labels it **bug** (which is available in plugins repositories). +* An issue is opened stating that Snap is not working. It turns out to be related to Snap's functionality. The maintainer will label it **type/bug**. She has time to write the fix to this issue immediately, so she labels the issue as **reviewed/in-progress**. She finds it maps to the Scheduler package and adds additional context with **component/scheduler**. As she begins to write the fix, she opens a PR that says "Fixes #" for the previous issue and labels it **wip-do-not-merge**. When she wants another maintainer to review her PR, she will remove the **wip-do-not-merge** label. +* As PR is opened that will change Snap functionality (examples at [#977](https://github.com/intelsdi-x/snap/pull/977) & [#803](https://github.com/intelsdi-x/snap/pull/803)). A maintainer labels it **reviewed/needs-2nd-review** and proceeds with the normal code review. If the initial maintainer labels LGTM, another maintainer must review it again. A discussion must take place with a technical lead before merging. +* A PR is opened which changes the metadata structure for a plugin. A maintainer labels it **reviewed/needs-2nd-review** and adds whatever **breaking-change/** labels are appropriate. If the initial maintainer labels LGTM, another maintainer must review it again. A discussion must take place with a technical lead before merging. This corresponding issue is added to a milestone that corresponds with its targeted release (real example at [#871](https://github.com/intelsdi-x/snap/issues/871)). +* A PR is opened that edits a small amount of markdown or string output text. A maintainer labels it **effort/small**, gives it a quick review to ensure it renders, writes LGTM and merges it themselves (example: [#1139](https://github.com/intelsdi-x/snap/issues/1139)). +* An issue is opened that a maintainer believes could be solved quickly and with no impact outside of its package. She labels it **effort/small** and **help-wanted** to let external contributors know they can pick this up. + +And as a contributor: +* A contributor has an idea to improve Snap. He opens an issue with guidelines on how to fix it. A maintainer likes the idea, label it **type/feature-or-enhancement**. Once a maintainer or contributor begins working on the issue, it's labeled **reviewed/in-progress**. A PR is opened early in the development of the feature and labeled **wip-do-not-merge**. The label is removed once it's time for a maintainer to review the PR. +* A contributor has an idea to improve Snap. He opens an issue with guidelines on how to fix it and the maintainer labels it **type/feature-or-enhancement**. A maintainer believes the approach requires more user input and labels it **type/rfc** to indicate it's an open discussion on implementation. Once a maintainer or contributor begins working on the issue, it's labeled **reviewed/in-progress**. A PR is opened early in the development of the feature and labeled **wip-do-not-merge**. The label is removed once it's time for a maintainer to review the PR. Whoever authors the PR should check back on the original issues thread for further feedback until code is merged. +* A contributor wants to understand more about Snap and opens an issue. A maintainer sees its resolution will be an answer to the contributor, so she labels it **type/question**. The question is closed once an answer is given. If good ideas of how to improve Snap come up during that thread, she may open other issues and label them **type/** based on whether they are missing docs, improvements or bugs. + +If you read through all of this, you're awesome, well-informed and ready to dive in! diff --git a/README.md b/README.md index 996035b7c..45dad02b5 100644 --- a/README.md +++ b/README.md @@ -292,28 +292,7 @@ Amongst the many awesome contributors, there are the maintainers. These maintain * Committing to reviewing pull requests, issues, and addressing comments/questions as quickly as possible * Acting as a point of contact for questions -Just tag **@intelsdi-x/snap-maintainers** if you need to get some attention on an issue. If at any time, you don't get a quick enough response, reach out to any of the following team members directly: - - - - - - - - - - - - - - - - - - -
@andrzej-k@andrzej-k@candysmurf@candysmurf@ConnorDoyle@ConnorDoyle@danielscottt@danielscottt@geauxvirtual@geauxvirtual@jcooklin@jcooklin
@lynxbat@lynxbat@marcin-krolik@marcin-krolik@mjbrender@mjbrender@nqn@nqn@tiffanyfj@tiffanyfj@IzabellaRaulin@IzabellaRaulin
- -We're also looking to have maintainers from the community. Please let us know if you would like to become one by opening an Issue titled "interested in becoming a maintainer." We are currently working on a more official process. +Just tag **@intelsdi-x/snap-maintainers** if you need to get some attention on an issue. If at any time, you don't get a quick enough response, reach out to us [on Slack](http://slack.snap-telemetry.io) ## Thank You And **thank you!** Your contribution, through code and participation, is incredibly important to us. diff --git a/docs/METRICS.md b/docs/METRICS.md index 03b6fb68d..e51d2235a 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -5,12 +5,12 @@ A metric in snap has the following fields. * Namespace `[]core.NamespaceElement` * Uniquely identifies the metric -* LastAdertisedTime `time.Time` +* LastAdvertisedTime `time.Time` * Describes when the metric was added to the metric catalog * Version `int` * Is bound to the version of the plugin * Multiple versions of the same metric can be added to the catalog - * Unless specified in the task manifest the latest available metric will collected + * Unless specified in the Task Manifest, the latest available metric will collected * Config `*cdata.ConfigDataNode` * Contains data needed to collect a metric * Examples include 'uri', 'username', 'password', 'paths' @@ -18,10 +18,10 @@ A metric in snap has the following fields. * The collected data * Tags `map[string]string` * Are key value pairs that provide additional meta data about the metric - * May be added by the framework or other plugins (processors) + * May be added by the framework or other plugins (processors) * The framework currently adds the following standard tag to all metrics * `plugin_running_on` describing on which host the plugin is running - * May be added by a task manifests as described [here](https://github.com/intelsdi-x/snap/pull/941) + * May be added by a task manifests as described [here](https://github.com/intelsdi-x/snap/pull/941) * May be added by the snapd config as described [here](https://github.com/intelsdi-x/snap/issues/827) * Unit `string` * Describes the magnititude being measured @@ -69,8 +69,8 @@ A `NamespaceElement` has the following fields. ### Static Metric Namespace Example -Given a static metric identified by the namespace `/intel/psutil/load/load1` the `NamespaceElement`s would -have values of 'intel', 'psutil', 'load' and "load1" respectively. The `Name` and `Description` fields would have +Given a static metric identified by the namespace `/intel/psutil/load/load1` the `NamespaceElement`s would +have values of 'intel', 'psutil', 'load' and "load1" respectively. The `Name` and `Description` fields would have empty values. The metric's namespace could be created using the following constructor function. @@ -81,15 +81,15 @@ namespace := core.NewNamespace("intel", "psutil", "load", "load1") ### Dynamic Metric Namespace Example -Dyanmic namespaces enable collector plugins to embed runtime data in the namespace with just enough meta data to enable -downstrean plugins (processors and publishers) the ability to extract the data and transform the namespace into its +Dynamic namespaces enable collector plugins to embed runtime data in the namespace with just enough metadata to enable +downstrean plugins (processors and publishers) the ability to extract the data and transform the namespace into its canonical form often required by some backends. Given a dynamic metric identified by the namespace `/intel/libvirt/*/disk/*/wrreq` the `NamespaceElement`s would have values of 'intel', 'libvirt', '*', 'disk', '*' and 'wrreq' respectively. The `Name` and `Description` fields of the 2nd and 4th elements would also contain non empty values. -The metric's namespace could be created using the following constructor function. +The metric's namespace could be created using the following constructor function. ``` ns := core.NewNamespace("intel", "libvirt") @@ -99,8 +99,8 @@ ns := core.NewNamespace("intel", "libvirt") .AddStaticElement("wrreq") ``` -It is *important* to note that the `NamespaceElement` fields `Name` and `Description` should *only* have non emtpy string -values when the element they are describing is dynamic in which case the `Value` field would contain the string vale "*". +It is *important* to note that the `NamespaceElement` fields `Name` and `Description` should *only* have non-empty string +values when the element they are describing is dynamic in which case the `Value` field would contain the string value "*". You can find an example of the influxdb publisher creating tags out of the dynamic elements of the namespace and publishing to a time series [here](https://github.com/intelsdi-x/snap-plugin-publisher-influxdb/blob/b253302ddfc94e3b444780328d0f503a6d73e3e0/influx/influx.go#L164-L176). diff --git a/docs/PLUGIN_AUTHORING.md b/docs/PLUGIN_AUTHORING.md index c236a7df4..6b820d9de 100644 --- a/docs/PLUGIN_AUTHORING.md +++ b/docs/PLUGIN_AUTHORING.md @@ -88,7 +88,7 @@ Example: Snap validates the metrics exposed by plugin and, if validation failed, return an error and not load the plugin. ##### c) static and dynamic metrics -Snap supports both static and dynamic metrics. You can find more detail about static and dynamic metrics [here](./METRICS.md). +Snap supports both static and dynamic metrics. You can find more detail about static and dynamic metrics [here](./METRICS.md). ### Mandatory packages There are three mandatory packages that every plugin must use. Other than those three packages, you can use other packages as necessary. There is no danger of colliding dependencies as plugins are separated processes. The mandatory packages are: @@ -148,7 +148,7 @@ func Meta() *plugin.PluginMeta { Snap uses [logrus](http://github.com/Sirupsen/logrus) to log. Your plugins can use it, or any standard Go log package. Each plugin has its log file. If no logging directory is specified, logs are in the /tmp directory of the running machine. INFO is the logging level for the release version of plugins. Loggers are excellent resources for debugging. You can also use Go GDB or [delve](https://github.com/derekparker/delve) to debug. ## Building and running the tests -While developing a plugin, unit and integration tests need to be performed. Snap uses [goconvery](http://github.com/smartystreets/goconvey/convey) for unit tests. You are welcome to use it or any other unit test framework. For the integration tests, you have to set up $SNAP_PATH and some necessary direct, or indirect dependencies. Using Docker container for integration tests is an effective testing strategy. Integration tests may define an input workflow. Refer to a sample [integration test input](https://github.com/intelsdi-x/snap/blob/master/examples/configs/snap-config-sample.json). +While developing a plugin, unit and integration tests need to be performed. Snap uses [goconvey](http://github.com/smartystreets/goconvey/convey) for unit tests. You are welcome to use it or any other unit test framework. For the integration tests, you have to set up $SNAP_PATH and some necessary direct, or indirect dependencies. Using Docker container for integration tests is an effective testing strategy. Integration tests may define an input workflow. Refer to a sample [integration test input](https://github.com/intelsdi-x/snap/blob/master/examples/configs/snap-config-sample.json). For example, to run a plugin integration test ```