Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Commit

Permalink
Update to repository labeling strategy
Browse files Browse the repository at this point in the history 
Fixes #1066

Labeling needs to expand to match our expanding user base and maintainers. We now have:

* Namespaced labeling based on the type of work
* Indicators that will allow new users contribute to Snap
* Indicators to help maintainers avoid merging incomplete code
* Example walkthroughs of how committers and maintainers may use all this new label goodness
* Comment on difference between Snap framework and plugin labels
* Refactoring of maintainer space of README
* Some further minor typos
  • Loading branch information
@mbbroberg
mbbroberg committed Aug 22, 2016
1 parent 4e6b19d commit df51bec
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 51 deletions.
58 changes: 42 additions & 16 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:

<img src="http://i.imgur.com/epDE8RO.jpg" alt="GitHub Tagging Strategy" width="500">
### 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!
23 changes: 1 addition & 22 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:

<table border="0" cellspacing="0" cellpadding="0">
<tr>
<td width="125"><a href="https://github.com/andrzej-k"><sub>@andrzej-k</sub><img src="https://avatars.githubusercontent.com/u/13486250" alt="@andrzej-k"></a></td>
<td width="125"><a href="https://github.com/candysmurf"><sub>@candysmurf</sub><img src="https://avatars.githubusercontent.com/u/13841563" alt="@candysmurf"></a></td>
<td width="125"><a href="https://github.com/ConnorDoyle"><sub>@ConnorDoyle</sub><img src="https://avatars.githubusercontent.com/u/379372" alt="@ConnorDoyle"></a></td>
<td width="125"><a href="https://github.com/danielscottt"><sub>@danielscottt</sub><img src="https://avatars.githubusercontent.com/u/1194436" alt="@danielscottt"></a></td>
<td width="125"><a href="https://github.com/geauxvirtual"><sub>@geauxvirtual</sub><img src="https://avatars.githubusercontent.com/u/1395030" alt="@geauxvirtual"></a></td>
<td width="125"><a href="http://github.com/jcooklin"><sub>@jcooklin</sub><img src="https://avatars.githubusercontent.com/u/862968" alt="@jcooklin"></a></td>
</tr>
<tr>
<td width="125"><a href="https://github.com/lynxbat"><sub>@lynxbat</sub><img src="https://avatars.githubusercontent.com/u/1278669" width="100" alt="@lynxbat"></a></td>
<td width="125"><a href="https://github.com/marcin-krolik"><sub>@marcin-krolik</sub><img src="https://avatars.githubusercontent.com/u/14905131" width="100" alt="@marcin-krolik"></a></td>
<td width="125"><a href="https://github.com/mjbrender"><sub>@mjbrender</sub><img src="https://avatars.githubusercontent.com/u/1744971" width="100" alt="@mjbrender"></a></td>
<td width="125"><a href="https://github.com/nqn"><sub>@nqn</sub><img src="https://avatars.githubusercontent.com/u/897374" width="100" alt="@nqn"></a></td>
<td width="125"><a href="https://github.com/tiffanyfj"><sub>@tiffanyfj</sub><img src="https://avatars.githubusercontent.com/u/12282848" width="100" alt="@tiffanyfj"></a></td>
<td width="125"><a href="https://github.com/IzabellaRaulin"><sub>@IzabellaRaulin</sub><img src="https://avatars0.githubusercontent.com/u/11335874" width="100" alt="@IzabellaRaulin"></a></td>
</tr>
</table>

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.
22 changes: 11 additions & 11 deletions docs/METRICS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,23 @@ 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'
* Data `interface{}`
* 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
Expand Down Expand Up @@ -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.
Expand All @@ -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")
Expand All @@ -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).
Expand Down
4 changes: 2 additions & 2 deletions docs/PLUGIN_AUTHORING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
```
Expand Down

0 comments on commit df51bec

Please sign in to comment.