Backend style guide

Grafana's backend has been developed for a long time with a mix of code styles. This guide explains how we want to write Go code in the future.

Unless stated otherwise, use the guidelines listed in the following articles:

Linting and formatting

To ensure consistency across the Go codebase, we require all code to pass a number of linter checks.

We use the standard following linters:

In addition to the standard linters, we also use:

To run all linters, use the lint-go Makefile target:

make lint-go

Testing

We value clean and readable code, that is loosely coupled and covered by unit tests. This makes it easier to collaborate and maintain the code.

Tests must use the standard library, testing. For assertions, prefer using testify.

The majority of our tests uses GoConvey but that's something we want to avoid going forward.

In the sqlstore package we do database operations in tests and while some might say that's not suited for unit tests. We think they are fast enough and provide a lot of value.

Assertions

Use respectively assert.* functions to make assertions that should not halt the test ("soft checks") and require.* functions to make assertions that should halt the test ("hard checks"). Typically you want to use the latter type of check to assert that errors have or have not happened, since continuing the test after such an assertion fails is chaotic (the system under test will be in an undefined state) and you'll often have segfaults in practice.

Sub-tests

Use t.Run to group sub-test cases, since it allows common setup and teardown code, plus lets you run each test case in isolation when debugging. Don't use t.Run to e.g. group assertions.

Cleanup

Use t.Cleanup to clean up resources in tests. It's a less fragile choice than defer, since it's independent of which function you call it in. It will always execute after the test is over in reverse call order (last t.Cleanup first, same as defer).

Globals

As a general rule of thumb, avoid using global variables, since they make the code difficult to maintain and reason about, and to write tests for. The Grafana codebase currently does use a lot of global variables, especially when it comes to configuration, but that is a problem we're trying to solve.

Pointers

In general, use value types and only reach for pointers when there's a real need. The reason being that pointers increase the risk of bugs, since a pointer can be nil and dereferencing a nil pointer leads to a panic (AKA segfault). Valid reasons to use a pointer include (but not necessarily limited to):

You might need to pass a modifiable argument to a function
Copying an object might incur a performance hit (benchmark to check your assumptions, copying is often faster than allocating heap memory)
You might need nil to tell if a variable isn't set, although usually it's better to use the type's zero value to tell instead

Database

In database related code, we follow certain patterns.

Foreign keys

While they can be useful, we don't generally use foreign key constraints in Grafana, for historical and technical reasons. See this comment by Torkel for context.

Unique columns

If a column, or column combination, should be unique, add a corresponding uniqueness constraint through a migration.

JSON

The simplejson package is used a lot throughout the backend codebase, but it's legacy, so if at all possible avoid using it in new code. Use json-iterator instead, which is a more performant drop-in alternative to the standard encoding/json package. While encoding/json is a fine choice, profiling shows that json-iterator may be 3-4 times more efficient for encoding. We haven't profiled its parsing performance yet, but according to json-iterator's own benchmarks, it appears even more superior in this department.

4.8 KiB Raw Blame History