mirror of https://github.com/grafana/grafana
Docs: Edit of several Backend topics (part 5 of doc quality project) (#89073)
* Edit of several Backend topics (part 5 of doc quality project) * Proofread of files * Prettier fix * Update contribute/backend/upgrading-dependencies.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/README.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/README.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/recommended-practices.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/recommended-practices.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/recommended-practices.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/recommended-practices.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/recommended-practices.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Update contribute/backend/recommended-practices.md Co-authored-by: Jack Baldry <jack.baldry@grafana.com> * Rename doc filenames with gerunds --------- Co-authored-by: Jack Baldry <jack.baldry@grafana.com>pull/90417/head
parent
ff7c0edd64
commit
0ecff76600
@ -1,169 +1,151 @@ |
|||||||
# Currently recommended practices |
# Currently recommended practices |
||||||
|
|
||||||
We occasionally identify patterns that are either useful or harmful that |
Grafana Labs occasionally identifies patterns that may be useful or harmful so that we can introduce or remove from the codebase. |
||||||
we'll want to introduce or remove from the codebase. When the complexity |
When the complexity or importance of introducing or removing such idiomatic patterns is sufficiently high, we document it on this page to provide a reference. Because the relevance of these practices may vary over time, we call them _currently recommended practices_. |
||||||
or importance of introducing or removing such a pattern is sufficiently |
|
||||||
high, we'll document it here to provide an addressable local |
## Large-scale refactoring |
||||||
'currently recommended practice'. By collecting these practices in a |
|
||||||
single place, we're able to reference them and make it easier to have a |
|
||||||
shared understanding of how to write idiomatic code for the Grafana |
|
||||||
backend. |
|
||||||
|
|
||||||
Large-scale refactoring based on a new recommended practice is a |
Large-scale refactoring based on a new recommended practice is a |
||||||
delicate matter, and most of the time it's better to introduce the new |
delicate matter. It's usually better to introduce the new |
||||||
way incrementally over multiple releases and over time to balance the |
way incrementally over multiple releases and over time to balance the |
||||||
want to introduce new useful patterns and the need to keep Grafana |
desire to introduce new useful patterns with the need to keep Grafana |
||||||
stable. It's also easier to review and revert smaller chunks of changes, |
stable. It's also easier to review and revert smaller chunks of changes, |
||||||
reducing the risk of complications. |
reducing the risk of complications. |
||||||
|
|
||||||
| State | Description | |
## States of refactoring |
||||||
| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | |
|
||||||
| Proposed | The practice has been proposed and been positively received by the Grafana team. Following the proposal is a discretionary choice for developers. | |
Refer to the following table to identify important categories of refactoring. |
||||||
| Ongoing, active | The practice is actively being worked on. New code should adhere to the practice where at all possible. | |
|
||||||
| Ongoing, passive | There is no immediate active work on refactoring old code. New code should adhere to the practice where at all possible. | |
| State | Description | |
||||||
| Completed | The work has been done and there is no, or negligible, legacy code left that need refactoring. New code must adhere to the practice. | |
| ---------------- | ------------------------------------------------------------------------------------------------------------------------------------- | |
||||||
| Abandoned | The practice has no longer any active ongoing work and new code don't need to comply with the practice described. | |
| Proposed | This is an optional practice that has been proposed and received positively by the Grafana team. Follow this proposal as you choose. | |
||||||
|
| Ongoing, active | The practice is actively being worked on. New code should adhere to the practice whenever possible. | |
||||||
|
| Ongoing, passive | There's no immediate active work on refactoring old code. New code should adhere to the practice whenever possible. | |
||||||
|
| Completed | The work has been done and there is no, or negligible, legacy code left that needs refactoring. New code must adhere to the practice. | |
||||||
|
| Abandoned | The practice doesn't have any active ongoing work and new code doesn't need to comply with the practice described. | |
||||||
|
|
||||||
## 1 - Idiomatic Grafana code should be idiomatic Go code |
## 1 - Idiomatic Grafana code should be idiomatic Go code |
||||||
|
|
||||||
**Status:** Ongoing, passive. |
**Status:** Ongoing, passive. |
||||||
|
|
||||||
It'll be easier for contributors to start contributing to Grafana if our |
It's easier for contributors to start contributing to Grafana if our |
||||||
code is easily understandable. When there isn't a more specific Grafana |
code is easily understandable. When there isn't a more specific Grafana |
||||||
recommended practice, we recommend following the practices as put forth |
recommended practice, we recommend that you follow the practices as put forth |
||||||
by the Go project for development of Go code or the Go compiler itself |
by the Go project for development of Go code or the Go compiler itself |
||||||
when applicable. |
as appropriate. |
||||||
|
|
||||||
The first resource we recommend everyone developing Grafana's backend to |
Firstly, best practice is the online book [_Effective Go_](https://golang.org/doc/effective_go.html), which isn't updated to reflect more recent changes since Go was initially released, but which remains a good source for understanding the general differences between Go and other languages. |
||||||
skim is "[Effective Go](https://golang.org/doc/effective_go.html)", |
|
||||||
which isn't updated to reflect more recent changes since Go was |
|
||||||
initially released but remain a good source for understanding the |
|
||||||
general differences between Go and other languages. |
|
||||||
|
|
||||||
Secondly, the guidelines for [Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) |
Secondly, the guidelines for [Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) for the Go compiler can mostly be applied directly to the Grafana codebase. |
||||||
for the Go compiler can mostly be applied directly to the Grafana |
There are idiosyncrasies in Grafana, such as interfaces living closer to their declarations than to their users for services, and the documentation doesn't enforce public declarations. |
||||||
codebase. There are idiosyncrasies in Grafana such as interfaces living |
Instead, the documentation prioritizes high coverage aimed at end-users over documenting internals in the backend. |
||||||
closer to its declaration than to its users for services and don't |
|
||||||
enforce documentation of public declarations (prioritize high coverage |
|
||||||
of documentation aimed at end-users over documenting internals in the |
|
||||||
backend). |
|
||||||
|
|
||||||
- [Effective Go](https://golang.org/doc/effective_go.html) |
- [_Effective Go_](https://golang.org/doc/effective_go.html). |
||||||
- [Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments) |
- [Code Review Comments](https://github.com/golang/go/wiki/CodeReviewComments). |
||||||
|
|
||||||
## 100 - Global state |
## 100 - Global state |
||||||
|
|
||||||
**State:** Ongoing, passive. |
**State:** Ongoing, passive. |
||||||
|
|
||||||
Global state makes testing and debugging software harder, and it's something we want to avoid when possible. Unfortunately, there is quite a lot of global state in Grafana. |
Global state makes testing and debugging software harder, and it's something we want to avoid whenever possible. |
||||||
|
|
||||||
We want to migrate away from this by using |
Unfortunately, there's quite a lot of global state in Grafana. |
||||||
[Wire](https://github.com/google/wire) and dependency injection to pack |
We want to migrate away from this state by using |
||||||
|
[Wire](https://github.com/google/wire) and dependency injection to pack code. |
||||||
|
|
||||||
## 101 - Limit the use of the init() function |
## 101 - Limit use of the init() function |
||||||
|
|
||||||
**State:** Ongoing, passive. |
**State:** Ongoing, passive. |
||||||
|
|
||||||
Only use the init() function to register services/implementations. |
Don't use the `init()` function for any purpose other than registering services or implementations. |
||||||
|
|
||||||
## 102 - Settings refactoring |
## 102 - Refactor settings |
||||||
|
|
||||||
**State:** Ongoing, passive. |
**State:** Ongoing, passive. |
||||||
|
|
||||||
The plan is to move all settings to from package level vars in settings package to the [setting.Cfg](https://github.com/grafana/grafana/blob/df917663e6f358a076ed3daa9b199412e95c11f4/pkg/setting/setting.go#L210) struct. To access the settings, services and components can inject this setting.Cfg struct: |
We plan to move all settings from package-level vars in the settings package to the [`setting.Cfg`](https://github.com/grafana/grafana/blob/df917663e6f358a076ed3daa9b199412e95c11f4/pkg/setting/setting.go#L210) struct. To access the settings, services and components can inject `setting.Cfg`: |
||||||
|
|
||||||
- [Cfg struct](https://github.com/grafana/grafana/blob/df917663e6f358a076ed3daa9b199412e95c11f4/pkg/setting/setting.go#L210) |
- [`Cfg` struct](https://github.com/grafana/grafana/blob/df917663e6f358a076ed3daa9b199412e95c11f4/pkg/setting/setting.go#L210) |
||||||
- [Injection example](https://github.com/grafana/grafana/blob/c9773e55b234b7637ea97b671161cd856a1d3d69/pkg/services/cleanup/cleanup.go#L34) |
- [Injection](https://github.com/grafana/grafana/blob/c9773e55b234b7637ea97b671161cd856a1d3d69/pkg/services/cleanup/cleanup.go#L34) |
||||||
|
|
||||||
## 103 - Reduce the use of GoConvey |
## 103 - Reduce use of GoConvey |
||||||
|
|
||||||
**State:** Completed. |
**State:** Completed. |
||||||
|
|
||||||
We want to migrate away from using GoConvey. Instead, we want to use |
We encourage you to migrate away from using GoConvey. |
||||||
stdlib testing with [testify](https://github.com/stretchr/testify), |
Instead, we suggest the use of `stdlib` testing with [testify](https://github.com/stretchr/testify), because it's the most common approach in the Go community, and we think it will be easier for new contributors. |
||||||
because it's the most common approach in the Go community, and we think |
To learn more about how we want to write tests, refer to the [backend style guide](/contribute/backend/style-guide.md). |
||||||
it will be easier for new contributors. Read more about how we want to |
|
||||||
write tests in the [style guide](/contribute/backend/style-guide.md). |
|
||||||
|
|
||||||
## 104 - Refactor SqlStore |
## 104 - Refactor SqlStore |
||||||
|
|
||||||
**State:** Completed. |
**State:** Completed. |
||||||
|
|
||||||
The `sqlstore` handlers all use a global xorm engine variable. Refactor them to use the `SqlStore` instance. |
The `sqlstore` handlers all use a global `xorm` engine variable. Refactor them to use the `SqlStore` instance. |
||||||
|
|
||||||
## 105 - Avoid global HTTP handler functions |
## 105 - Avoid global HTTP handler functions |
||||||
|
|
||||||
**State:** Ongoing, passive. |
**State:** Ongoing, passive. |
||||||
|
|
||||||
Refactor HTTP handlers so that the handler methods are on the HttpServer instance or a more detailed handler struct. E.g (AuthHandler). This ensures they get access to HttpServer service dependencies (and Cfg object) and can avoid global state. |
Refactor HTTP handlers so that the handler methods are on the `HttpServer` instance or a more detailed handler struct. For example, `AuthHandler`. |
||||||
|
Doing so ensures that the handler methods get access to `HttpServer` service dependencies (and its `Cfg` object), so that global state may be avoided. |
||||||
|
|
||||||
## 106 - Date comparison |
## 106 - Compare dates |
||||||
|
|
||||||
**State:** Ongoing, passive. |
**State:** Ongoing, passive. |
||||||
|
|
||||||
Store newly introduced date columns in the database as epoch based |
Store newly introduced date columns in the database as epoch-based integers (that is, Unix timestamps) if they require date comparison. |
||||||
integers (i.e. unix timestamps) if they require date comparison. This |
This permits you to have a unified approach for comparing dates against all the supported databases instead of needing to handle dates differently for each database. |
||||||
permits a unified approach for comparing dates against all the supported |
Also, when you compare epoch-based integers you no longer need error-pruning transformations to and from other time zones. |
||||||
databases instead of handling dates differently for each database. Also, |
|
||||||
by comparing epoch based integers, we no longer need error pruning |
|
||||||
transformations to and from other time zones. |
|
||||||
|
|
||||||
## 107 - Avoid use of the simplejson package |
## 107 - Avoid the `simplejson` package |
||||||
|
|
||||||
**State:** Ongoing, passive |
**State:** Ongoing, passive |
||||||
|
|
||||||
Use of the `simplejson` package (`pkg/components/simplejson`) in place |
Don't use the `simplejson` package (`pkg/components/simplejson`) instead of types (that is, Go structs) because this results in code that is difficult to maintain. |
||||||
of types (Go structs) results in code that is difficult to maintain. |
|
||||||
Instead, create types for objects and use the Go standard library's |
Instead, create types for objects and use the Go standard library's |
||||||
[`encoding/json`](https://golang.org/pkg/encoding/json/) package. |
[`encoding/json`](https://golang.org/pkg/encoding/json/) package. |
||||||
|
|
||||||
## 108 - Provisionable\* |
## 108 - Enable provisioning |
||||||
|
|
||||||
**State:** Abandoned: Grafana's file based refactoring is limited to work natively only on on-premise installations of Grafana. We're looking at enhancing the use of the API to enable provisioning for all Grafana instances. |
**State:** Abandoned: The file-based refactoring of Grafana is limited to work natively only on on-premise installations of Grafana. |
||||||
|
We want to enhance the use of the API to enable provisioning for all Grafana instances in the future. |
||||||
|
|
||||||
All new features that require state should be possible to configure using config files. For example: |
All new features that require state should be able to configure Grafana using configuration files. |
||||||
|
For example: |
||||||
|
|
||||||
- [Data sources](https://github.com/grafana/grafana/tree/main/pkg/services/provisioning/datasources) |
- [Data sources](https://github.com/grafana/grafana/tree/main/pkg/services/provisioning/datasources) |
||||||
- [Alert notifiers](https://github.com/grafana/grafana/tree/main/pkg/services/provisioning/notifiers) |
- [Alert notifiers](https://github.com/grafana/grafana/tree/main/pkg/services/provisioning/notifiers) |
||||||
- [Dashboards](https://github.com/grafana/grafana/tree/main/pkg/services/provisioning/dashboards) |
- [Dashboards](https://github.com/grafana/grafana/tree/main/pkg/services/provisioning/dashboards) |
||||||
|
|
||||||
Today it's only possible to provision data sources and dashboards but this is something we want to support all over Grafana. |
Today it's only possible to provision data sources and dashboards, but we want to support it throughout Grafana in the future. |
||||||
|
|
||||||
### 109 - Use context.Context "everywhere" |
### 109 - Use `context.Context` |
||||||
|
|
||||||
**State:** Completed. |
**State:** Completed. |
||||||
|
|
||||||
The package [context](https://golang.org/pkg/context/) should be used |
You should use and propagate the package [`context`](https://golang.org/pkg/context/) through all the layers of your code. |
||||||
and propagated through all the layers of the code. For example the |
For example, the `context.Context` of an incoming API request should be propagated to any other layers being used, such as the bus, service layer, and database layer. |
||||||
`context.Context` of an incoming API request should be propagated to any |
Utility functions and methods normally don't need `context.Context`. |
||||||
other layers being used such as the bus, service and database layers. |
To follow Go best practices, any function or method that receives a |
||||||
Utility functions/methods normally doesn't need `context.Context`. |
`context.Context` argument should receive it as its [first parameter](https://github.com/golang/go/wiki/CodeReviewComments#contexts). |
||||||
To follow Go best practices, any function/method that receives a |
|
||||||
[`context.Context` argument should receive it as its first parameter](https://github.com/golang/go/wiki/CodeReviewComments#contexts). |
|
||||||
|
|
||||||
To be able to solve certain problems and/or implement and support |
|
||||||
certain features making sure that `context.Context` is passed down |
|
||||||
through all layers of the code is vital. Being able to provide |
|
||||||
contextual information for the full life-cycle of an API request allows |
|
||||||
us to use contextual logging, provide contextual information about the |
|
||||||
authenticated user, create multiple spans for a distributed trace of |
|
||||||
service calls and database queries etc. |
|
||||||
|
|
||||||
Code should use `context.TODO` when it's unclear which Context to use, |
We encourage you to make sure that `context.Context` is passed down through all layers of your code. |
||||||
or it is not yet available (because the surrounding function has not yet |
When you provide contextual information for the full life cycle of an API request, Grafana can use contextual logging. It also provides contextual information about the |
||||||
been extended to accept a `context.Context` argument). |
authenticated user, and it creates multiple spans for a distributed trace of service calls, database queries, and so on. |
||||||
|
|
||||||
More details in [Services](/contribute/backend/services.md), [Communication](/contribute/backend/communication.md) and [Database](/contribute/backend/database.md). |
Code should use `context.TODO` whenever it's unclear which `Context` to use, |
||||||
|
or if it isn't yet available because the surrounding function hasn't yet |
||||||
|
been extended to accept a `context.Context` argument. For more details, refer to the documentation: |
||||||
|
|
||||||
[Original design doc](https://docs.google.com/document/d/1ebUhUVXU8FlShezsN-C64T0dOoo-DaC9_r-c8gB2XEU/edit#). |
- [Services](/contribute/backend/services.md) |
||||||
|
- [Communication](/contribute/backend/communication.md) |
||||||
|
- [Database](/contribute/backend/database.md) |
||||||
|
|
||||||
## 110 - Move API error handling to service layer |
## 110 - Move API error handling to service layer |
||||||
|
|
||||||
**State:** Ongoing, passive. |
**State:** Ongoing, passive. |
||||||
|
|
||||||
All errors returned from Grafana's services should carry a status and |
All errors returned from services in Grafana should carry a status and |
||||||
the information necessary to provide a structured end-user facing |
the information necessary to provide a structured message that faces the end-user. Structured messages can be displayed on the frontend and may be [internationalized](../internationalization.md). |
||||||
message that the frontend can display and internationalize for |
|
||||||
end-users. |
|
||||||
|
|
||||||
More details in [Errors](/contribute/backend/errors.md). |
To learn more, refer to [Errors](/contribute/backend/errors.md). |
||||||
|
@ -0,0 +1,26 @@ |
|||||||
|
# Upgrade dependencies |
||||||
|
|
||||||
|
We recommend the practices in this documentation when upgrading the various backend dependencies of Grafana. |
||||||
|
|
||||||
|
## Protocol buffers (protobufs) |
||||||
|
|
||||||
|
Use the most recent stable version of the [protobuf library](http://github.com/golang/protobuf) in Grafana and the plugin SDK. |
||||||
|
|
||||||
|
Additionally, you typically want to upgrade your protobuf compiler toolchain and re-compile the protobuf files. |
||||||
|
|
||||||
|
> **Note:** You need Buf CLI installed and available in your path. For instructions, refer to the [Buf Docs documentation](https://buf.build/docs/installation). |
||||||
|
|
||||||
|
After you've installed Buf CLI, re-compile the protobuf files in Grafana and the plugin SDK. Use this code: |
||||||
|
|
||||||
|
```shell |
||||||
|
cd $GRAFANA |
||||||
|
make protobuf |
||||||
|
cd $GRAFANA_PLUGIN_SDK_GO |
||||||
|
mage protobuf |
||||||
|
``` |
||||||
|
|
||||||
|
After upgrading the protobuf dependency in Grafana and the plugin SDK, it is a best practice to test that your code still works before creating your PR. Specifically: |
||||||
|
|
||||||
|
- Test a plugin built with upgraded SDK on upgraded Grafana |
||||||
|
- Test a plugin built with non-upgraded SDK on upgraded Grafana |
||||||
|
- Test a plugin built with upgraded SDK on non-upgraded Grafana |
@ -0,0 +1,26 @@ |
|||||||
|
# Upgrade Go version |
||||||
|
|
||||||
|
We recommend the practices outlined in this documentation when you upgrade Go for use in Grafana development. |
||||||
|
|
||||||
|
## Example PR |
||||||
|
|
||||||
|
Refer to the following PR for an example of how to perform a Go upgrade: |
||||||
|
|
||||||
|
- [PR ##79329](https://github.com/grafana/grafana/pull/79329) |
||||||
|
|
||||||
|
## Main areas to update |
||||||
|
|
||||||
|
Change at least the following parts of Go and related files: |
||||||
|
|
||||||
|
- [`go.mod`](/go.mod#L3) |
||||||
|
- [`go.work`](/go.work#L1) |
||||||
|
- [`scripts/drone/variables.star`](/scripts/drone/variables.star#L6) |
||||||
|
- [`Makefile`](/Makefile#L12) |
||||||
|
- [`Dockerfile`](/Dockerfile#L6) |
||||||
|
|
||||||
|
Then, run `go mod tidy` and `go work sync`. Also, run `make drone` so changes reflect the updates to `.star` and `drone.yml` files. |
||||||
|
|
||||||
|
### Additional files to change |
||||||
|
|
||||||
|
- Look in the `.github/workflows` folder for what Go version is being used there in various workflows. |
||||||
|
- Make sure to create a PR with the corresponding changes in the `grafana/grafana-enterprise` repository. |
@ -1,26 +0,0 @@ |
|||||||
# Upgrading dependencies |
|
||||||
|
|
||||||
Notes on upgrading various backend dependencies. |
|
||||||
|
|
||||||
## Protobuf |
|
||||||
|
|
||||||
When upgrading the [protobuf](http://github.com/golang/protobuf) library in Grafana and the plugin SDK, |
|
||||||
you typically also want to upgrade your protobuf compiler toolchain and re-compile protobuf files. |
|
||||||
|
|
||||||
**Note:** You need Buf CLI installed and availabile in your path, see https://buf.build/docs/installation for instructions. |
|
||||||
|
|
||||||
Re-compile protobuf files in grafana and the plugin SDK: |
|
||||||
|
|
||||||
```shell |
|
||||||
cd $GRAFANA |
|
||||||
make protobuf |
|
||||||
cd $GRAFANA_PLUGIN_SDK_GO |
|
||||||
mage protobuf |
|
||||||
``` |
|
||||||
|
|
||||||
After upgrading the protobuf dependency in Grafana and the plugin SDK, it might be wise to test that things still work, |
|
||||||
before making corresponding PRs: |
|
||||||
|
|
||||||
- Test a plugin built with upgraded SDK on upgraded Grafana |
|
||||||
- Test a plugin built with non-upgraded SDK on upgraded Grafana |
|
||||||
- Test a plugin built with upgraded SDK on non-upgraded Grafana |
|
@ -1,20 +0,0 @@ |
|||||||
# Upgrading Go Version |
|
||||||
|
|
||||||
Notes on upgrading Go version. |
|
||||||
|
|
||||||
Example PR: https://github.com/grafana/grafana/pull/79329 |
|
||||||
|
|
||||||
## The main areas that need to change during the upgrade are: |
|
||||||
|
|
||||||
- [`go.mod`](/go.mod#L3) |
|
||||||
- [`go.work`](/go.work#L1) |
|
||||||
- [`scripts/drone/variables.star`](/scripts/drone/variables.star#L6) |
|
||||||
- [`Makefile`](/Makefile#L12) |
|
||||||
- [`Dockerfile`](/Dockerfile#L6) |
|
||||||
|
|
||||||
Then, run `go mod tidy` and `go work sync`. Also run `make drone` so changes to `.star` files are reflected and `drone.yml` is updated. |
|
||||||
|
|
||||||
### Additional files to change |
|
||||||
|
|
||||||
- Take a look in `.github/workflows` folder for what `go` version is being used there in various workflows. |
|
||||||
- Make sure to create a PR with the corresponding changes in `grafana/grafana-enterprise` repository. |
|
Loading…
Reference in new issue