docs: Added Thanos Go style guide and some development tips. (thanos-…

…io#2359) * docs: Added Thanos Go style guide and some development tips. Signed-off-by: Bartlomiej Plotka <[email protected]> * Addressed comments; added TOC and image. Signed-off-by: Bartlomiej Plotka <[email protected]> * Added more rules. Signed-off-by: Bartlomiej Plotka <[email protected]> * Grammarly fixes! Signed-off-by: Bartlomiej Plotka <[email protected]>
bill3tt · Apr 12, 2020 · c4fe34f · c4fe34f
1 parent 1de4552
commit c4fe34f
Show file tree

Hide file tree

Showing 5 changed files with 1,189 additions and 20 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -4,6 +4,23 @@ This document explain the process of contributing to the Thanos project.
 
 First of all please follow the [code of conduct](CODE_OF_CONDUCT.md) in all your interactions with the project.
 
+## TOC
+
+- [Contributing](#contributing)
+  * [TOC](#toc)
+  * [Thanos Philosophy](#thanos-philosophy)
+  * [Feedback / Issues](#feedback---issues)
+  * [Adding New Features / Components](#adding-new-features---components)
+  * [Components Naming](#components-naming)
+  * [Development](#development)
+    + [Prerequisites](#prerequisites)
+    + [First steps](#first-steps)
+    + [Pull Request Process](#pull-request-process)
+    + [Dependency management](#dependency-management)
+    + [Advanced testing](#advanced-testing)
+
+<small><i>Table of contents generated with <a href='http://ecotrust-canada.github.io/markdown-toc/'>markdown-toc</a></i></small>
+
 ## Thanos Philosophy
 
 The philosophy of Thanos and our community is borrowing much from UNIX philosophy and the golang programming language.
@@ -36,10 +53,6 @@ method with the owners of this repository before making a change.
 
 Adding a large new feature or/and component to Thanos should be done by first creating a [proposal](docs/proposals) document outlining the design decisions of the change, motivations for the change, and any alternatives that might have been considered.
 
-## Prerequisites
-
-* It is strongly recommended that you use OSX or popular Linux distributions systems e.g. Ubuntu, Redhat, or OpenSUSE for development.
-
 ## Components Naming
 
 Thanos is a distributed system composed with several services and CLI tools as listed [here](cmd/thanos).
@@ -61,12 +74,28 @@ However, when speaking about those or explaining we use `actor` noun form: `stor
 * Documentation
 * Code comments
 
-## Pull Request Process
+## Development
+
+The following section explains various suggestions and procedures to note during development of Thanos.
+
+### Prerequisites
+
+* It is strongly recommended that you use Linux distributions systems or OSX for development.
+* Go 1.13.9 or newer installed.
+
+### First Steps
+
+It's key to get familiarized with style guide and mechanics of Thanos, especially if your contribution touches more than one
+component of the Thanos distributed system. We recommend:
+
+* Reading the [getting started docs](docs/getting-started.md) and working through them, or alternatively working through the [Thanos tutorial](https://katacoda.com/thanos).
+* Familiarizing yourself with our [coding style guidelines.](docs/contributing/coding-style-guide.md).
+* Familiarizing yourself with the [Makefile](Makefile) commands, for example `format`, `build`, `proto`, `docker` and `test`.
+`make help` will print most of available commands with details.
+
+### Pull Request Process
 
-1. Read [getting started docs](docs/getting-started.md) and prepare Thanos.
-2. Familiarize yourself with Go standard style guides Thanos follows: [this](https://golang.org/doc/effective_go.html) and [this](https://github.com/golang/go/wiki/CodeReviewComments).
-3. Familiarize yourself with [Makefile](Makefile) commands like `format`, `build`, `proto` and `test`.
-4. Fork thanos-io/thanos.git and start development from your own fork. Here are sample steps to setup your development environment:
+1. Fork thanos-io/thanos.git and start development from your own fork. Here are sample steps to setup your development environment:
 
 ```console
 $ mkdir -p $GOPATH/src/github.com/thanos-io
@@ -80,7 +109,8 @@ $ make build
 $ ./thanos -h
 ```
 
-5. Keep PRs as small as possible. For each of your PR, you create one branch based on the latest master. Chain them if needed (base PR on other PRs). Here are sample steps you can follow. You can get more details about the workflow from [here](https://gist.github.com/Chaser324/ce0505fbed06b947d962).
+1. Keep PRs as small as possible. For each of your PRs, you create a new branch based on the latest master.
+Chain them if needed (base one PR on other PRs). You can read more details about the workflow from [here](https://gist.github.com/Chaser324/ce0505fbed06b947d962).
 
 ```console
 $ git checkout master
@@ -92,19 +122,21 @@ $ <Iterate your development>
 $ git push origin <your_branch_for_new_pr>
 ```
 
-6. Add unit tests for new functionalities. Add e2e tests if functionality is major.
-7. If you don't have a live object store ready add this envvar to skip tests for these:
-- THANOS_TEST_OBJSTORE_SKIP=GCS,S3,AZURE,SWIFT,COS,ALIYUNOSS
+1. Add unit tests for new functionality. Add e2e tests for major changes to functionality.
+1. If you don't have a live object store ready, you can use the `make test-local` command.
 
-If you skip all of these, the store specific tests will be run against memory object storage only.
-CI runs GCS and inmem tests only for now. Not having these variables will produce auth errors against GCS, AWS, Azure or COS tests.
+NOTE: this command skips tests against live object storage systems by specifying environment variables; this causes the
+store-specific tests to be run against memory and filesystem object storages only. The CI tests run `make test-ci` instead
+(uses GCS, AWS).
 
-8. If your change affects users (adds or removes feature) consider adding the item to [CHANGELOG](CHANGELOG.md)
-9. You may merge the Pull Request in once you have the sign-off of at least one developers with write access, or if you
+Not specifying these variables will result in auth errors against GCS, AWS, Azure, COS etc.
+
+1. If your change affects users (adds or removes feature) consider adding the item to the [CHANGELOG](CHANGELOG.md).
+1. You may merge the Pull Request once you have the sign-off of at least one developer with write access, or if you
    do not have permission to do that, you may request the second reviewer to merge it for you.
-10. If you feel like your PR waits too long for a review, feel free to ping [`#thanos-prs`](https://slack.cncf.io/) channel on our slack for review!
+1. If you feel like your PR is waiting too long for a review, feel free to ping the [`#thanos-prs`](https://slack.cncf.io/) channel on our slack for a review!
 
-## Dependency management
+### Dependency management
 
 The Thanos project uses [Go modules](https://golang.org/cmd/go/#hdr-Modules__module_versions__and_more) to manage dependencies on external packages. This requires a working Go environment with version 1.11 or greater and git installed.
 
@@ -127,3 +159,26 @@ git commit
 ```
 
 You have to commit the changes to `go.mod` and `go.sum` before submitting the pull request.
+
+### Advanced testing
+
+At some point during development it is useful, in addition to running unit or e2e tests, to run and play with Thanos components manually. While you
+can run any component manually by crafting specific flags for a test setup, there are already some nice tools and scripts available.
+Consider the following methods:
+
+* [quickstart.sh](https://github.com/thanos-io/thanos/blob/b08c0ea62abfe4dcf1400da0e37598f0cd8fa8cf/scripts/quickstart.sh): this script spins
+up a simple example setup. Do `make build` before running the script to build the `thanos` binary first.
+* `make test-e2e`: the e2e tests cover most of the setups and functionality Thanos offers. It's extremely easy to add `time.Sleep(10* time.Minutes)`
+at certain points in the tests (e.g for compactor [here](https://github.com/thanos-io/thanos/blob/8f492a9f073f819019dd9f044e346a1e1fa730bc/test/e2e/compact_test.go#L379)).
+This way when you run `make test-e2e`, the test will sleep for some time, allowing you to connect to the setup manually using the port printed in the logs. For example:
+
+```bash
+querier-1: level=info name=querier-1 ts=2020-04-01T12:53:56.101029491Z caller=http.go:56 service=http/server component=query msg="listening for requests and metrics" address=:80
+querier-1: level=info name=querier-1 ts=2020-04-01T12:53:56.101106805Z caller=intrumentation.go:48 msg="changing probe status" status=ready
+querier-1: level=info name=querier-1 ts=2020-04-01T12:53:56.101290229Z caller=grpc.go:106 service=gRPC/server component=query msg="listening for StoreAPI gRPC" address=:9091
+Ports for container: e2e_test_store_gateway-querier-1 Mapping: map[80:32825 9091:32824]
+```
+
+This output indicates that the HTTP (`80`) endpoint will be available on `http://localhost:32825`. You can quickly craft your own test case with our framework as well!
+
+NOTE: `make docker` has to work in order for `make test-e2e` to run. This currently might not work properly on macOS.
diff --git a/Makefile b/Makefile
@@ -125,7 +125,7 @@ define require_clean_work_tree
 endef
 
 help: ## Displays help.
-	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n\nTargets:\n"} /^[a-zA-Z_-]+:.*?##/ { printf "  \033[36m%-10s\033[0m %s\n", $$1, $$2 }' $(MAKEFILE_LIST)
+	@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n  make \033[36m<target>\033[0m\n\nTargets:\n"} /^[a-z0-9A-Z_-]+:.*?##/ { printf "  \033[36m%-10s\033[0m %s\n", $$1, $$2 }' $(MAKEFILE_LIST)
 
 .PHONY: all
 all: format build