diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5ff6b2977560..2ada24a762335 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -74,6 +74,17 @@ make test-all Use `make docker-kill` to stop the containers. +### For more developer resources +- [Code Style][codestyle] +- [Deprecation][deprecation] +- [Logging][logging] +- [Metric Format Changes][metricformat] +- [Packaging][packaging] +- [Logging][logging] +- [Packaging][packaging] +- [Profiling][profiling] +- [Reviews][reviews] +- [Sample Config][sample config] [cla]: https://www.influxdata.com/legal/cla/ [new issue]: https://github.com/influxdata/telegraf/issues/new/choose @@ -82,3 +93,11 @@ Use `make docker-kill` to stop the containers. [processors]: /docs/PROCESSORS.md [aggregators]: /docs/AGGREGATORS.md [outputs]: /docs/OUTPUTS.md +[codestyle]: /docs/developers/CODE_STYLE.md +[deprecation]: /docs/developers/DEPRECATION.md +[logging]: /docs/developers/LOGGING.md +[metricformat]: /docs/developers/METRIC_FORMAT_CHANGES.md +[packaging]: /docs/developers/PACKAGING.md +[profiling]: /docs/developers/PROFILING.md +[reviews]: /docs/developers/REVIEWS.md +[sample config]: /docs/developers/SAMPLE_CONFIG.md diff --git a/docs/developers/CODE_STYLE.md b/docs/developers/CODE_STYLE.md new file mode 100644 index 0000000000000..1bbb2b14d84c4 --- /dev/null +++ b/docs/developers/CODE_STYLE.md @@ -0,0 +1,7 @@ +# Code Style +Code is required to be formatted using `gofmt`, this covers most code style +requirements. It is also highly recommended to use `goimports` to +automatically order imports. + +Please try to keep lines length under 80 characters, the exact number of +characters is not strict but it generally helps with readability. diff --git a/docs/developers/DEPRECATION.md b/docs/developers/DEPRECATION.md new file mode 100644 index 0000000000000..a3da79a5ac8e8 --- /dev/null +++ b/docs/developers/DEPRECATION.md @@ -0,0 +1,88 @@ +# Deprecation +Deprecation is the primary tool for making changes in Telegraf. A deprecation +indicates that the community should move away from using a feature, and +documents that the feature will be removed in the next major update (2.0). + +Key to deprecation is that the feature remains in Telegraf and the behavior is +not changed. + +We do not have a strict definition of a breaking change. All code changes +change behavior, the decision to deprecate or make the change immediately is +decided based on the impact. + +## Deprecate plugins + +Add a comment to the plugin's sample config, include the deprecation version +and any replacement. + +```toml +[[inputs.logparser]] + ## DEPRECATED: The 'logparser' plugin is deprecated in 1.10. Please use the + ## 'tail' plugin with the grok data_format as a replacement. +``` + +Add the deprecation warning to the plugin's README: + +```markdown +# Logparser Input Plugin + +### **Deprecated in 1.10**: Please use the [tail][] plugin along with the +`grok` [data format][]. + +[tail]: /plugins/inputs/tail/README.md +[data formats]: /docs/DATA_FORMATS_INPUT.md +``` + +Log a warning message if the plugin is used. If the plugin is a +ServiceInput, place this in the `Start()` function, for regular Input's log it only the first +time the `Gather` function is called. +```go +log.Println("W! [inputs.logparser] The logparser plugin is deprecated in 1.10. " + + "Please use the tail plugin with the grok data_format as a replacement.") +``` +## Deprecate options + +Mark the option as deprecated in the sample config, include the deprecation +version and any replacement. +```toml + ## Broker URL + ## deprecated in 1.7; use the brokers option + # url = "amqp://localhost:5672/influxdb" +``` + +In the plugins configuration struct, mention that the option is deprecated: + +```go +type AMQPConsumer struct { + URL string `toml:"url"` // deprecated in 1.7; use brokers +} +``` + +Finally, use the plugin's `Init() error` method to display a log message at warn level. The message should include the offending configuration option and any suggested replacement: +```go +func (a *AMQPConsumer) Init() error { + if p.URL != "" { + p.Log.Warnf("Use of deprecated configuration: 'url'; please use the 'brokers' option") + } + return nil +} +``` + +## Deprecate metrics + +In the README document the metric as deprecated. If there is a replacement field, +tag, or measurement then mention it. + +```markdown +- system + - fields: + - uptime_format (string, deprecated in 1.10: use `uptime` field) +``` + +Add filtering to the sample config, leave it commented out. + +```toml +[[inputs.system]] + ## Uncomment to remove deprecated metrics. + # fielddrop = ["uptime_format"] +``` diff --git a/docs/developers/LOGGING.md b/docs/developers/LOGGING.md new file mode 100644 index 0000000000000..60de15699a6e8 --- /dev/null +++ b/docs/developers/LOGGING.md @@ -0,0 +1,75 @@ +# Logging + +## Plugin Logging + +You can access the Logger for a plugin by defining a field named `Log`. This +`Logger` is configured internally with the plugin name and alias so they do not +need to be specified for each log call. + +```go +type MyPlugin struct { + Log telegraf.Logger `toml:"-"` +} +``` + +You can then use this Logger in the plugin. Use the method corresponding to +the log level of the message. +```go +p.Log.Errorf("Unable to write to file: %v", err) +``` + +## Agent Logging + +In other sections of the code it is required to add the log level and module +manually: +```go +log.Printf("E! [agent] Error writing to %s: %v", output.LogName(), err) +``` + +## When to Log + +Log a message if an error occurs but the plugin can continue working. For +example if the plugin handles several servers and only one of them has a fatal +error, it can be logged as an error. + +Use logging judiciously for debug purposes. Since Telegraf does not currently +support setting the log level on a per module basis, it is especially important +to not over do it with debug logging. + +If the plugin is listening on a socket, log a message with the address of the socket: +```go +p.log.InfoF("Listening on %s://%s", protocol, l.Addr()) +``` + +## When not to Log + +Don't use logging to emit performance data or other meta data about the plugin, +instead use the `internal` plugin and the `selfstats` package. + +Don't log fatal errors in the plugin that require the plugin to return, instead +return them from the function and Telegraf will handle the logging. + +Don't log for static configuration errors, check for them in a plugin `Init()` +function and return an error there. + +Don't log a warning every time a plugin is called for situations that are +normal on some systems. + +## Log Level + +The log level is indicated by a single character at the start of the log +message. Adding this prefix is not required when using the Plugin Logger. +- `D!` Debug +- `I!` Info +- `W!` Warning +- `E!` Error + +## Style + +Log messages should be capitalized and be a single line. + +If it includes data received from another system or process, such as the text +of an error message, the text should be quoted with `%q`. + +Use the `%v` format for the Go error type instead of `%s` to ensure a nil error +is printed. diff --git a/docs/developers/METRIC_FORMAT_CHANGES.md b/docs/developers/METRIC_FORMAT_CHANGES.md new file mode 100644 index 0000000000000..32bfe0a2db5a7 --- /dev/null +++ b/docs/developers/METRIC_FORMAT_CHANGES.md @@ -0,0 +1,42 @@ +# Metric Format Changes + +When making changes to an existing input plugin, care must be taken not to change the metric format in ways that will cause trouble for existing users. This document helps developers understand how to make metric format changes safely. + +## Changes can cause incompatibilities +If the metric format changes, data collected in the new format can be incompatible with data in the old format. Database queries designed around the old format may not work with the new format. This can cause application failures. + +Some metric format changes don't cause incompatibilities. Also, some unsafe changes are necessary. How do you know what changes are safe and what to do if your change isn't safe? + +## Guidelines +The main guideline is just to keep compatibility in mind when making changes. Often developers are focused on making a change that fixes their particular problem and they forget that many people use the existing code and will upgrade. When you're coding, keep existing users and applications in mind. + +### Renaming, removing, reusing +Database queries refer to the metric and its tags and fields by name. Any Telegraf code change that changes those names has the potential to break an existing query. Similarly, removing tags or fields can break queries. + +Changing the meaning of an existing tag value or field value or reusing an existing one in a new way isn't safe. Although queries that use these tags/field may not break, they will not work as they did before the change. + +Adding a field doesn't break existing queries. Queries that select all fields and/or tags (like "select * from") will return an extra series but this is often useful. + +### Performance and storage +Time series databases can store large amounts of data but many of them don't perform well on high cardinality data. If a metric format change includes a new tag that holds high cardinality data, database performance could be reduced enough to cause existing applications not to work as they previously did. Metric format changes that dramatically increase the number of tags or fields of a metric can increase database storage requirements unexpectedly. Both of these types of changes are unsafe. + +### Make unsafe changes opt-in +If your change has the potential to seriously affect existing users, the change must be opt-in. To do this, add a plugin configuration setting that lets the user select the metric format. Make the setting's default value select the old metric format. When new users add the plugin they can choose the new format and get its benefits. When existing users upgrade, their config files won't have the new setting so the default will ensure that there is no change. + +When adding a setting, avoid using a boolean and consider instead a string or int for future flexibility. A boolean can only handle two formats but a string can handle many. For example, compare use_new_format=true and features=["enable_foo_fields"]; the latter is much easier to extend and still very descriptive. + +If you want to encourage existing users to use the new format you can log a warning once on startup when the old format is selected. The warning should tell users in a gentle way that they can upgrade to a better metric format. If it doesn't make sense to maintain multiple metric formats forever, you can change the default on a major release or even remove the old format completely. See [[Deprecation]] for details. + +### Utility +Changes should be useful to many or most users. A change that is only useful for a small number of users may not accepted, even if it's off by default. + +## Summary table + +| | delete | rename | add | +| ------- | ------ | ------ | --- | +| metric | unsafe | unsafe | safe | +| tag | unsafe | unsafe | be careful with cardinality | +| field | unsafe | unsafe | ok as long as it's useful for existing users and is worth the added space | + +## References +InfluxDB Documentation: "Schema and data layout" diff --git a/docs/developers/PACKAGING.md b/docs/developers/PACKAGING.md new file mode 100644 index 0000000000000..836fd01973d3f --- /dev/null +++ b/docs/developers/PACKAGING.md @@ -0,0 +1,44 @@ +# Packaging + +## Package using Docker + +This packaging method uses the CI images, and is very similar to how the +official packages are created on release. This is the recommended method for +building the rpm/deb as it is less system dependent. + +Pull the CI images from quay, the version corresponds to the version of Go +that is used to build the binary: +``` +docker pull quay.io/influxdb/telegraf-ci:1.9.7 +``` + +Start a shell in the container: +``` +docker run -ti quay.io/influxdb/telegraf-ci:1.9.7 /bin/bash +``` + +From within the container: +``` +go get -d github.com/influxdata/telegraf +cd /go/src/github.com/influxdata/telegraf + +# Use tag of Telegraf version you would like to build +git checkout release-1.10 +git reset --hard 1.10.2 +make deps + +# This builds _all_ platforms and architectures; will take a long time +./scripts/build.py --release --package +``` + +If you would like to only build a subset of the packages run this: + +``` +# Use the platform and arch arguments to skip unwanted packages: +./scripts/build.py --release --package --platform=linux --arch=amd64 +``` + +From the host system, copy the build artifacts out of the container: +``` +docker cp romantic_ptolemy:/go/src/github.com/influxdata/telegraf/build/telegraf-1.10.2-1.x86_64.rpm . +``` diff --git a/docs/developers/PROFILING.md b/docs/developers/PROFILING.md new file mode 100644 index 0000000000000..81cdf1980304d --- /dev/null +++ b/docs/developers/PROFILING.md @@ -0,0 +1,55 @@ +# Profiling +This article describes how to collect performance traces and memory profiles +from Telegraf. If you are submitting this for an issue, please include the +version.txt generated below. + +Use the `--pprof-addr` option to enable the profiler, the easiest way to do +this may be to add this line to `/etc/default/telegraf`: +``` +TELEGRAF_OPTS="--pprof-addr localhost:6060" +``` + +Restart Telegraf to activate the profile address. + +#### Trace Profile +Collect a trace during the time where the performance issue is occurring. This +example collects a 10 second trace and runs for 10 seconds: +``` +curl 'http://localhost:6060/debug/pprof/trace?seconds=10' > trace.bin +telegraf --version > version.txt +go env GOOS GOARCH >> version.txt +``` + +The `trace.bin` and `version.txt` files can be sent in for analysis or, if desired, you can +analyze the trace with: +``` +go tool trace trace.bin +``` + +#### Memory Profile +Collect a heap memory profile: +``` +curl 'http://localhost:6060/debug/pprof/heap' > mem.prof +telegraf --version > version.txt +go env GOOS GOARCH >> version.txt +``` + +Analyze: +``` +$ go tool pprof mem.prof +(pprof) top5 +``` + +#### CPU Profile +Collect a 30s CPU profile: +``` +curl 'http://localhost:6060/debug/pprof/profile' > cpu.prof +telegraf --version > version.txt +go env GOOS GOARCH >> version.txt +``` + +Analyze: +``` +go tool pprof cpu.prof +(pprof) top5 +``` diff --git a/docs/developers/REVIEWS.md b/docs/developers/REVIEWS.md new file mode 100644 index 0000000000000..d97fe16cdc772 --- /dev/null +++ b/docs/developers/REVIEWS.md @@ -0,0 +1,147 @@ +# Reviews + +Expect several rounds of back and forth on reviews, non-trivial changes are +rarely accepted on the first pass. + +While review cannot be exhaustively documented, there are several things that +should always be double checked. + +All pull requests should follow the style and best practices in the +[CONTRIBUTING.md](https://github.com/influxdata/telegraf/blob/master/CONTRIBUTING.md) +document. + +## Reviewing Plugin Code + +- Avoid variables scoped to the package. Everything should be scoped to the plugin struct, since multiple instances of the same plugin are allowed and package-level variables will cause race conditions. +- SampleConfig must match the readme, but not include the plugin name. +- structs should include toml tags for fields that are expected to be editable from the config. eg `toml:"command"` (snake_case) +- plugins that want to log should declare the Telegraf logger, not use the log package. eg: +```Go + Log telegraf.Logger `toml:"-"` +``` +(in tests, you can do `myPlugin.Log = testutil.Logger{}`) +- Initialization and config checking should be done on the `Init() error` function, not in the Connect, Gather, or Start functions. +- plugins should avoid synchronization code if they are not starting goroutines. Plugin functions are never called in parallel. +- avoid goroutines when you don't need them and removing them would simplify the code +- errors should almost always be checked. +- avoid boolean fields when a string or enumerated type would be better for future extension. Lots of boolean fields also make the code difficult to maintain. +- use config.Duration instead of internal.Duration +- compose tls.ClientConfig as opposed to specifying all the TLS fields manually +- http.Client should be declared once on `Init() error` and reused, (or better yet, on the package if there's no client-specific configuration). http.Client has built-in concurrency protection and reuses connections transparently when possible. +- avoid doing network calls in loops where possible, as this has a large performance cost. This isn't always possible to avoid. +- when processing batches of records with multiple network requests (some outputs that need to partition writes do this), return an error when you want the whole batch to be retried, log the error when you want the batch to continue without the record +- consider using the StreamingProcessor interface instead of the (legacy) Processor interface +- avoid network calls in processors when at all possible. If it's necessary, it's possible, but complicated (see processor.reversedns). +- avoid dependencies when: + - they require cgo + - they pull in massive projects instead of small libraries + - they could be replaced by a simple http call + - they seem unnecessary, superfluous, or gratuitous +- consider adding build tags if plugins have OS-specific considerations +- use the right logger log levels so that Telegraf is normally quiet eg `plugin.Log.Debugf()` only shows up when running Telegraf with `--debug` +- consistent field types: dynamically setting the type of a field should be strongly avoided as it causes problems that are difficult to solve later, made worse by having to worry about backwards compatibility in future changes. For example, if an numeric value comes from a string field and it is not clear if the field can sometimes be a float, the author should pick either a float or an int, and parse that field consistently every time. Better to sometimes truncate a float, or to always store ints as floats, rather than changing the field type, which causes downstream problems with output databases. +- backwards compatibility: We work hard not to break existing configurations during new changes. Upgrading Telegraf should be a seamless transition. Possible tools to make this transition smooth are: + - enumerable type fields that allow you to customize behavior (avoid boolean feature flags) + - version fields that can be used to opt in to newer changed behavior without breaking old (see inputs.mysql for example) + - a new version of the plugin if it has changed significantly (eg outputs.influxdb and outputs.influxdb_v2) + - Logger and README deprecation warnings + - changing the default value of a field can be okay, but will affect users who have not specified the field and should be approached cautiously. + - The general rule here is "don't surprise me": users should not be caught off-guard by unexpected or breaking changes. + + +## Testing + +Sufficient unit tests must be created. New plugins must always contain +some unit tests. Bug fixes and enhancements should include new tests, but +they can be allowed if the reviewer thinks it would not be worth the effort. + +[Table Driven Tests](https://github.com/golang/go/wiki/TableDrivenTests) are +encouraged to reduce boiler plate in unit tests. + +The [stretchr/testify](https://github.com/stretchr/testify) library should be +used for assertions within the tests when possible, with preference towards +github.com/stretchr/testify/require. + +Primarily use the require package to avoid cascading errors: +```go +assert.Equal(t, lhs, rhs) # avoid +require.Equal(t, lhs, rhs) # good +``` + +## Configuration + +The config file is the primary interface and should be carefully scrutinized. + +Ensure the [[SampleConfig]] and +[README](https://github.com/influxdata/telegraf/blob/master/plugins/inputs/EXAMPLE_README.md) +match with the current standards. + +READMEs should: +- be spaces, not tabs +- be indented consistently, matching other READMEs +- have two `#` for comments +- have one `#` for defaults, which should always match the default value of the plugin +- include all appropriate types as a list for enumerable field types +- include a useful example, avoiding "example", "test", etc. +- include tips for any common problems +- include example output from the plugin, if input/processor/aggregator/parser/serializer + +## Metric Schema + +Telegraf metrics are heavily based on InfluxDB points, but have some +extensions to support other outputs and metadata. + +New metrics must follow the recommended +[schema design](https://docs.influxdata.com/influxdb/latest/concepts/schema_and_data_layout/). +Each metric should be evaluated for _series cardinality_, proper use of tags vs +fields, and should use existing patterns for encoding metrics. + +Metrics use `snake_case` naming style. + +### Enumerations + +Generally enumeration data should be encoded as a tag. In some cases it may +be desirable to also include the data as an integer field: +``` +net_response,result=success result_code=0i +``` + +### Histograms + +Use tags for each range with the `le` tag, and `+Inf` for the values out of +range. This format is inspired by the Prometheus project: +``` +cpu,le=0.0 usage_idle_bucket=0i 1486998330000000000 +cpu,le=50.0 usage_idle_bucket=2i 1486998330000000000 +cpu,le=100.0 usage_idle_bucket=2i 1486998330000000000 +cpu,le=+Inf usage_idle_bucket=2i 1486998330000000000 +``` + +### Lists + +Lists are tricky, but the general technique is to encode using a tag, creating +one series be item in the list. + +### Counters + +Counters retrieved from other projects often are in one of two styles, +monotonically increasing without reset and reset on each interval. No attempt +should be made to switch between these two styles but if given the option it +is preferred to use the non-reseting variant. This style is more resilient in +the face of downtime and does not contain a fixed time element. + +## Go Best Practices + +In general code should follow best practice describe in [Code Review +Comments](https://github.com/golang/go/wiki/CodeReviewComments). + +### Networking + +All network operations should have appropriate timeouts. The ability to +cancel the option, preferably using a context, is desirable but not always +worth the implementation complexity. + +### Channels + +Channels should be used in judiciously as they often complicate the design and +can easily be used improperly. Only use them when they are needed. diff --git a/docs/developers/SAMPLE_CONFIG.md b/docs/developers/SAMPLE_CONFIG.md new file mode 100644 index 0000000000000..f6202145d27b6 --- /dev/null +++ b/docs/developers/SAMPLE_CONFIG.md @@ -0,0 +1,76 @@ +# Sample Configuration + +The sample config file is generated from a results of the `SampleConfig()` and +`Description()` functions of the plugins. + +You can generate a full sample +config: +``` +telegraf config +``` + +You can also generate the config for a particular plugin using the `-usage` +option: +``` +telegraf --usage influxdb +``` + +## Style + +In the config file we use 2-space indention. Since the config is +[TOML](https://github.com/toml-lang/toml) the indention has no meaning. + +Documentation is double commented, full sentences, and ends with a period. +```toml + ## This text describes what an the exchange_type option does. + # exchange_type = "topic" +``` + +Try to give every parameter a default value whenever possible. If an +parameter does not have a default or must frequently be changed then have it +uncommented. +```toml + ## Brokers are the AMQP brokers to connect to. + brokers = ["amqp://localhost:5672"] +``` + + +Options where the default value is usually sufficient are normally commented +out. The commented out value is the default. +```toml + ## What an exchange type is. + # exchange_type = "topic" +``` + +If you want to show an example of a possible setting filled out that is +different from the default, show both: +```toml + ## Static routing key. Used when no routing_tag is set or as a fallback + ## when the tag specified in routing tag is not found. + ## example: routing_key = "telegraf" + # routing_key = "" +``` + +Unless parameters are closely related, add a space between them. Usually +parameters is closely related have a single description. +```toml + ## If true, queue will be declared as an exclusive queue. + # queue_exclusive = false + + ## If true, queue will be declared as an auto deleted queue. + # queue_auto_delete = false + + ## Authentication credentials for the PLAIN auth_method. + # username = "" + # password = "" +``` + +An parameters should usually be describable in a few sentences. If it takes +much more than this, try to provide a shorter explanation and provide a more +complex description in the Configuration section of the plugins +[README](https://github.com/influxdata/telegraf/blob/master/plugins/inputs/EXAMPLE_README.md) + +Boolean parameters should be used judiciously. You should try to think of +something better since they don't scale well, things are often not truly +boolean, and frequently end up with implicit dependencies: this option does +something if this and this are also set. diff --git a/docs/maintainers/CHANGELOG.md b/docs/maintainers/CHANGELOG.md new file mode 100644 index 0000000000000..8935ad70ca74e --- /dev/null +++ b/docs/maintainers/CHANGELOG.md @@ -0,0 +1,43 @@ +# Changelog + +The changelog contains the list of changes by version in addition to release +notes. The file is updated immediately after adding a change that impacts +users. Changes that don't effect the functionality of Telegraf, such as +refactoring code, are not included. + +The changelog entries are added by a maintainer after merging a pull request. +We experimented with requiring the pull request contributor to add the entry, +which had a nice side-effect of reducing the number of changelog only commits +in the log history, however this had several drawbacks: + +- The entry often needed reworded. +- Entries frequently caused merge conflicts. +- Required contributor to know which version a change was accepted into. +- Merge conflicts made it more time consuming to backport changes. + +Changes are added only to the first version a change is added in. For +example, a change backported to 1.7.2 would only appear under 1.7.2 and not in +1.8.0. This may become confusing if we begin supporting more than one +previous version but works well for now. + +## Updating + +If the change resulted in deprecation, mention the deprecation in the Release +Notes section of the version. In general all changes that require or +recommend the user to perform an action when upgrading should be mentioned in +the release notes. + +If a new plugin has been added, include it in a section based on the type of +the plugin. + +All user facing changes, including those already mentioned in the release +notes or new plugin sections, should be added to either the Features or +Bugfixes section. + +Features should generally link to the pull request since this describes the +actual implementation. Bug fixes should link to the issue instead of the pull +request since this describes the problem, if a bug has been fixed but does not +have an issue then it is okay to link to the pull request. + +It is usually okay to just use the shortlog commit message, but if needed +it can differ or be further clarified in the changelog. diff --git a/docs/maintainers/LABELS.md b/docs/maintainers/LABELS.md new file mode 100644 index 0000000000000..72840394a94bb --- /dev/null +++ b/docs/maintainers/LABELS.md @@ -0,0 +1,34 @@ +# Labels + +This page describes the meaning of the various +[labels](https://github.com/influxdata/telegraf/labels) we use on the Github +issue tracker. + +## Categories + +New issues are usually labeled one of `feature request`, `bug`, or `question`. +If you are unsure what label to apply you can use the `need more info` label +and if there is another issue you can add the duplicate label and close the +new issue. + +New pull requests are usually labeled one of `enhancement`, `bugfix` or `new +plugin`. + +## Additional Labels + +Apply any of the `area/*` labels that match. If an area doesn't exist, new +ones can be added but it is not a goal to have an area for all issues. + +If the issue only applies to one platform, you can use a `platform/*` label. +These are only applied to single platform issues which are not on Linux. + +The `breaking change` label can be added to issues and pull requests that +would result in a breaking change. + +Apply `performance` to issues and pull requests that address performance +issues. + +For bugs you may want to add `panic`, `regression`, or `upstream` to provide +further detail. + +Labels starting with `pm` or `vert` are not applied by maintainers. diff --git a/docs/maintainers/PULL_REQUESTS.md b/docs/maintainers/PULL_REQUESTS.md new file mode 100644 index 0000000000000..c41e4dd138788 --- /dev/null +++ b/docs/maintainers/PULL_REQUESTS.md @@ -0,0 +1,67 @@ +# Pull Requests + +## Before Review + +Ensure that the CLA is signed. The only exemption would be non-copyrightable +changes such as fixing a typo. + +Check that all tests are passing. Due to intermittent errors in the CI tests +it may be required to check the cause of test failures and restart failed +tests and/or create new issues to fix intermittent test failures. + +Ensure that PR is opened against the master branch as all changes are merged +to master initially. It is possible to change the branch a pull request is +opened against but it often results in many conflicts, change it before +reviewing and then if needed ask the contributor to rebase. + +Ensure there are no merge conflicts. If there are conflicts, ask the +contributor to merge or rebase. + +## Review + +[Review the pull request](Review). + +## Merge + +Determine what release the change will be applied to. New features should +be added only to master, and will be released in the next minor version (1.x). +Bug fixes can be backported to the current release branch to go out with the +next patch release (1.7.x) unless the bug is too risky to backport or there is +an easy workaround. Set the correct milestone on the pull request and any +associated issue. + +All pull requests are merged using the "Squash and Merge" strategy on Github. +This method is used because many pull requests do not have a clean change +history and this method allows us to normalize commit messages as well as +simplifies backporting. + +After selecting "Squash and Merge" you may need to rewrite the commit message. +Usually the body of the commit messages should be cleared as well, unless it +is well written and applies to the entire changeset. Use imperative present +tense for the first line of the message: instead of "I added tests for" or +"Adding tests for," use "Add tests for.". The default merge commit messages +include the PR number at the end of the commit message, keep this in the final +message. If applicable mention the plugin in the message. + +**Example Enhancement:** + +> Add user tag to procstat input (#4386) + +**Example Bug Fix:** + +> Fix output format of printer processor (#4417) + +## After Merge + +[Update the Changelog](Changelog). + +If required, backport the patch and the changelog update to the current +release branch. Usually this can be done by cherry picking the commits: +``` +git cherry-pick -x aaaaaaaa bbbbbbbb +``` + +Backporting changes to the changelog often pulls in unwanted changes. After +cherry picking commits, double check that the only the expected lines are +modified and if needed clean up the changelog and amend the change. Push the +new master and release branch to Github. diff --git a/docs/maintainers/RELEASES.md b/docs/maintainers/RELEASES.md new file mode 100644 index 0000000000000..3c05cdf968715 --- /dev/null +++ b/docs/maintainers/RELEASES.md @@ -0,0 +1,97 @@ +# Releases + +## Release Branch + +On master, update `etc/telegraf.conf` and commit: +```sh +./telegraf config > etc/telegraf.conf +``` + +Create the new release branch: +```sh +git checkout -b release-1.15 +``` + +Push the changes: +```sh +git push origin release-1.15 master +``` + +Update next version strings on master: +```sh +git checkout master +echo 1.16.0 > build_version.txt +``` + +## Release Candidate + +Release candidates are created only for new minor releases (ex: 1.15.0). Tags +are created but some of the other tasks, such as adding a changelog entry are +skipped. Packages are added to the github release page and posted to +community but are not posted to package repos or docker hub. +```sh +git checkout release-1.15 +git commit --allow-empty -m "Telegraf 1.15.0-rc1" +git tag -s v1.15.0-rc1 -m "Telegraf 1.15.0-rc1" +git push origin release-1.15 v1.15.0-rc1 +``` + +## Release + +On master, set the release date in the changelog and cherry-pick the change +back: +```sh +git checkout master +vi CHANGELOG.md +git commit -m "Set 1.8.0 release date" +git checkout release-1.8 +git cherry-pick -x +``` + +Double check that the changelog was applied as desired, or fix it up and +amend the change before pushing. + +Tag the release: +```sh +git checkout release-1.8 +# This just improves the `git show 1.8.0` output +git commit --allow-empty -m "Telegraf 1.8.0" +git tag -s v1.8.0 -m "Telegraf 1.8.0" +``` + +Check that the version was set correctly, the tag can always be altered if a +mistake is made but only before you push it to Github: +```sh +make +./telegraf --version +Telegraf v1.8.0 (git: release-1.8 aaaaaaaa) +``` + +When you push a branch with a tag to Github, CircleCI will be triggered to +build the packages. +```sh +git push origin master release-1.8 v1.8.0 +``` + +Set the release notes on Github. + +Update webpage download links. + +Update apt and yum repositories hosted at repos.influxdata.com. + +Update the package signatures on S3, these are used primarily by the docker images. + +Update docker image [influxdata/influxdata-docker](https://github.com/influxdata/influxdata-docker): +```sh +cd influxdata-docker +git co master +git pull +git co -b telegraf-1.8.0 +telegraf/1.8/Dockerfile +telegraf/1.8/alpine/Dockerfile +git commit -am "telegraf 1.8.0" +``` + +Official company post to RSS/community. + +Update documentation on docs.influxdata.com