diff --git a/contribute/architecture/backend/communication.md b/contribute/architecture/backend/communication.md index f59f598cdf5..151da5978fc 100644 --- a/contribute/architecture/backend/communication.md +++ b/contribute/architecture/backend/communication.md @@ -2,134 +2,131 @@ Grafana uses a _bus_ to pass messages between different parts of the application. All communication over the bus happens synchronously. -> **Deprecated:** The bus has officially been deprecated, however, we're still using the command/query objects paradigms. +## Commands and queries -There are three types of messages: _events_, _commands_, and _queries_. +Grafana structures arguments to [services](services.md) using a command/query +separation where commands are instructions for a mutation and queries retrieve +records from a service. -## Events +Services should define their methods as `func[T, U any](ctx context.Context, args T) (U, error)`. -An event is something that happened in the past. Since an event has already happened, you can't change it. Instead, you can react to events by triggering additional application logic to be run, whenever they occur. +Each function should take two arguments. First, a `context.Context` that +carries information about the tracing span, cancellation, and similar +runtime information that might be relevant to the call. Secondly, `T` is +a `struct` defined in the service's root package (see the instructions +for [package hierarchy](package-hierarchy.md)) that contains zero or +more arguments that can be passed to the method. -> Because they happened in the past, event names are written in past tense, such as `UserCreated`, and `OrgUpdated`. - -### Subscribe to an event - -In order to react to an event, you first need to _subscribe_ to it. - -To subscribe to an event, register an _event listener_ in the service's `Init` method: +The return values is more flexible, and may consist of none, one, or two +values. If there are two values returned, the second value should be +either an `bool` or `error` indicating the success or failure of the +call. The first value `U` carries a value of any exported type that +makes sense for the service. -```go -func (s *MyService) Init() error { - s.bus.AddEventListener(s.UserCreated) - return nil -} +Following is an example of an interface providing method signatures for +some calls adhering to these guidelines: -func (s *MyService) UserCreated(event *events.UserCreated) error { - // ... +``` +type Alphabetical interface { + // GetLetter returns either an error or letter. + GetLetter(context.Context, GetLetterQuery) (Letter, error) + // ListCachedLetters cannot fail, and doesn't return an error. + ListCachedLetters(context.Context, ListCachedLettersQuery) Letters + // DeleteLetter doesn't have any return values other than errors, so it + // returns only an error. + DeleteLetter(context.Contxt, DeleteLetterCommand) error } ``` -**Tip:** Browse the available events in the `events` package. +> Because we request an operation to be performed, command are written in imperative mood, such as `CreateFolderCommand`, `GetDashboardQuery` and `DeletePlaylistCommand`. -### Publish an event +The use of complex types for arguments in Go means a few different +things for us, it provides us with the equivalent of named parameters +from other languages, and it reduces the headache of figuring out which +argument is which that often occurs with three or more arguments. -If you want to let other parts of the application react to changes in a service, you can publish your own events: +On the flip-side, it means that all input parameters are optional and +that it is up to the programmer to make sure that the zero value is +useful or at least safe for all fields and that while it's easy to add +another field, if that field must be set for the correct function of the +service that is not detectable at compile time. -```go -event := &events.StickersSentEvent { - UserID: "taylor", - Count: 1, -} -if err := s.bus.Publish(event); err != nil { - return err -} -``` +### Queries with Result fields -## Commands +Some queries have a Result field that is mutated and populated by the +method being called. This is a remainder from when the _bus_ was used +for sending commands and queries as well as for events. -A command is a request for an action to be taken. Unlike an event's fire-and-forget approach, a command can fail as it is handled. The handler will then return an error. +All bus commands and queries had to implement the Go type +`func(ctx context.Context, msg interface{}) error` +and mutation of the `msg` variable or returning structured information in +`error` were the two most convenient ways to communicate with the caller. -> Because we request an operation to be performed, command are written in imperative mood, such as `CreateFolderCommand`, and `DeletePlaylistCommand`. +All `Result` fields should be refactored so that they are returned from +the query method: -### Dispatch a command - -To dispatch a command, pass the `context.Context` and object to the `DispatchCtx` method: +``` +type GetQuery struct { + Something int -```go -// context.Context from caller -ctx := req.Request.Context() -cmd := &models.SendStickersCommand { - UserID: "taylor", - Count: 1, + Result ResultType } -if err := s.bus.DispatchCtx(ctx, cmd); err != nil { - if err == bus.ErrHandlerNotFound { - return nil - } - return err + +func (s *Service) Get(ctx context.Context, cmd *GetQuery) error { + // ...do something + cmd.Result = result + return nil } ``` -> **Note:** `DispatchCtx` will return an error if no handler is registered for that command. +should become -> **Note:** `Dispatch` currently exists and requires no `context.Context` to be provided, but it's strongly suggested to not use this since there's an ongoing refactoring to remove usage of non-context-aware functions/methods and use context.Context everywhere. - -**Tip:** Browse the available commands in the `models` package. - -### Handle commands - -Let other parts of the application dispatch commands to a service, by registering a _command handler_: - -To handle a command, register a command handler in the `Init` function. - -```go -func (s *MyService) Init() error { - s.bus.AddHandlerCtx(s.SendStickers) - return nil +``` +type GetQuery struct { + Something int } -func (s *MyService) SendStickers(ctx context.Context, cmd *models.SendStickersCommand) error { - // ... +func (s *Service) Get(ctx context.Context, cmd GetQuery) (ResultType, error) { + // ...do something + return result, nil } ``` -> **Note:** The handler method may return an error if unable to complete the command. +## Events -> **Note:** `AddHandler` currently exists and requires no `context.Context` to be provided, but it's strongly suggested to not use this since there's an ongoing refactoring to remove usage of non-context-aware functions/methods and use context.Context everywhere. +An event is something that happened in the past. Since an event has already happened, you can't change it. Instead, you can react to events by triggering additional application logic to be run, whenever they occur. -## Queries +> Because they happened in the past, event names are written in past tense, such as `UserCreated`, and `OrgUpdated`. -A command handler can optionally populate the command sent to it. This pattern is commonly used to implement _queries_. +### Subscribe to an event -### Making a query +In order to react to an event, you first need to _subscribe_ to it. -To make a query, dispatch the query instance just like you would a command. When the `DispatchCtx` method returns, the `Results` field contains the result of the query. +To subscribe to an event, register an _event listener_ in the service's `Init` method: ```go -// context.Context from caller -ctx := req.Request.Context() -query := &models.FindDashboardQuery{ - ID: "foo", -} -if err := bus.Dispatch(ctx, query); err != nil { - return err +func (s *MyService) Init() error { + s.bus.AddEventListener(s.UserCreated) + return nil } -// The query now contains a result. -for _, item := range query.Results { + +func (s *MyService) UserCreated(event *events.UserCreated) error { // ... } ``` -> **Note:** `Dispatch` currently exists and requires no `context.Context` to be provided, but it's strongly suggested to not use this since there's an ongoing refactoring to remove usage of non-context-aware functions/methods and use context.Context everywhere. +**Tip:** Browse the available events in the `events` package. -### Return query results +### Publish an event -To return results for a query, set any of the fields on the query argument before returning: +If you want to let other parts of the application react to changes in a service, you can publish your own events: ```go -func (s *MyService) FindDashboard(ctx context.Context, query *models.FindDashboardQuery) error { - // ... - query.Result = dashboard - return nil +event := &events.StickersSentEvent { + UserID: "taylor", + Count: 1, +} +if err := s.bus.Publish(event); err != nil { + return err } ``` diff --git a/contribute/architecture/backend/errors.md b/contribute/architecture/backend/errors.md new file mode 100644 index 00000000000..fa02090b6d8 --- /dev/null +++ b/contribute/architecture/backend/errors.md @@ -0,0 +1,81 @@ +# Errors + +Grafana introduced its own error type `github.com/grafana/grafana/pkg/util/errutil.Error` +in June 2022. It's built on top of the Go `error` interface extended to +contain all the information necessary by Grafana to handle errors in an +informative and safe way. + +Previously, Grafana has passed around regular Go errors and have had to +rely on bespoke solutions in API handlers to communicate informative +messages to the end-user. With the new `errutil.Error`, the API handlers +can be slimmed as information about public messaging, structured data +related to the error, localization metadata, log level, HTTP status +code, and so forth are carried by the error. + +## Basic use + +### Declaring errors + +For a service, declare the different categories of errors that may occur +from your service (this corresponds to what you might want to have +specific public error messages or their templates for) by globally +constructing variables using the `errutil.NewBase(status, messageID, opts...)` +function. + +The status code loosely corresponds to HTTP status codes and provides a +default log level for errors to ensure that the request logging is +properly informing administrators about various errors occurring in +Grafana (e.g. `StatusBadRequest` is generally speaking not as relevant +as `StatusInternal`). All available status codes live in the `errutil` +package and have names starting with `Status`. + +The messageID is constructed as `.` where +the `` corresponds to the root service directory per +[the package hierarchy](package-hierarchy.md) and `` +is a short identifier using dashes for word separation that identifies +the specific category of errors within the service. + +To set a static message sent to the client when the error occurs, the +`errutil.WithPublicMessage(message string)` option may be appended to +the NewBase function call. For dynamic messages or more options, refer +to the `errutil` package's GoDocs. + +Errors are then constructed using the `Base.Errorf` method, which +functions like the [fmt.Errorf](https://pkg.go.dev/fmt#Errorf) method +except that it creates an `errutil.Error`. + +```go +package main + +import ( + "errors" + "github.com/grafana/grafana/pkg/util/errutil" + "example.org/thing" +) + +var ErrBaseNotFound = errutil.NewBase(errutil.StatusNotFound, "main.not-found", errutil.WithPublicMessage("Thing not found")) + +func Look(id int) (*Thing, error) { + t, err := thing.GetByID(id) + if errors.Is(err, thing.ErrNotFound) { + return nil, ErrBaseNotFound.Errorf("did not find thing with ID %d: %w", id, err) + } + + return t, nil +} +``` + +Check out [errutil's GoDocs](https://pkg.go.dev/github.com/grafana/grafana@v0.0.0-20220621133844-0f4fc1290421/pkg/util/errutil) +for details on how to construct and use Grafana style errors. + +### Handling errors in the API + +API handlers use the `github.com/grafana/grafana/pkg/api/response.Err` +function to create responses based on `errutil.Error`s. + +> **Note:** (@sakjur 2022-06) `response.Err` requires all errors to be +> `errutil.Error` or it'll be considered an internal server error. +> This is something that should be fixed in the near future to allow +> fallback behavior to make it possible to correctly handle Grafana +> style errors if they're present but allow fallback to a reasonable +> default otherwise. diff --git a/contribute/architecture/backend/package-hierarchy.md b/contribute/architecture/backend/package-hierarchy.md index d940910cdd0..7f52da3682e 100644 --- a/contribute/architecture/backend/package-hierarchy.md +++ b/contribute/architecture/backend/package-hierarchy.md @@ -1,16 +1,217 @@ # Package hierarchy -The Go package hierarchy in Grafana should be organized logically (Ben Johnson's -[article](https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1) served as inspiration), according to the -following principles: +The Go packages in Grafana should be packaged by feature, keeping +packages as small as reasonable while retaining a clear sole ownership +of a single domain. -- Domain types and interfaces should be in "root" packages (not necessarily at the very top, of the hierarchy, but - logical roots) -- Sub-packages should depend on roots - sub-packages here typically contain implementations, for example of services +[Ben Johnson's standard package layout](https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1) serves as +inspiration for the way we organize packages. + +## Principles of how to structure a service in Grafana + +[](services.md) + +### Domain types and interfaces should be in local "root" packages + +Let's say you're creating a _tea pot_ service, place everything another +service needs to interact with the tea pot service in +_pkg/services/teapot_, choosing a name according to +[Go's package naming conventions](https://go.dev/blog/package-names). + +Typically, you'd have one or more interfaces that your service provides +in the root package along with any types, errors, and other constants +that makes sense for another service interacting with this service to +use. + +Avoid depending on other services when structuring the root package to +reduce the risk of running into circular dependencies. + +### Sub-packages should depend on roots, not the other way around + +Small-to-medium sized packages should be able to have only a single +sub-package containing the implementation of the service. By moving the +implementation into a separate package we reduce the risk of triggering +circular dependencies (in Go, circular dependencies are evaluated per +package and this structure logically moves it to be per type or function +declaration). + +Large packages may need utilize multiple sub-packages at the discretion +of the implementor. Keep interfaces and domain types to the root +package. + +### Try to name sub-packages for project wide uniqueness + +Prefix sub-packages with the service name or an abbreviation of the +service name (whichever is more appropriate) to provide an ideally +unique package name. This allows `teaimpl` to be distinguished from +`coffeeimpl` without the need for package aliases, and encourages the +use of the same name to reference your package throughout the codebase. + +### A well-behaving service provides test doubles for itself + +Other services may depend on your service, and it's good practice to +provide means for those services to set up a test instance of the +dependency as needed. Refer to +[Google Testing's Testing on the Toilet: Know Your Test Doubles](https://testing.googleblog.com/2013/07/testing-on-toilet-know-your-test-doubles.html) for a brief +explanation of how we semantically aim to differentiate fakes, mocks, +and stubs within our codebase. + +Place test doubles in a sub-package to your root package named +`test` or `test`, such that the `teapot` service may have the +`teapottest` or `teatest` + +A stub or mock may be sufficient if the service is not a dependency of a +lot of services or if it's called primarily for side effects so that a +no-op default behavior makes sense. + +Services which serve many other services and where it's feasible should +provide an in-memory backed test fake that can be used like the +regular service without the need of complicated setup. + +### Separate store and logic + +When building a new service, data validation, manipulation, scheduled +events and so forth should be collected in a service implementation that +is built to be agnostic about its store. + +The storage should be an interface that is not directly called from +outside the service and should be kept to a minimum complexity to +provide the functionality necessary for the service. + +A litmus test to reduce the complexity of the storage interface is +whether an in-memory implementation is a feasible test double to build +to test the service. + +### Outside the service root + +Some parts of the service definition remains outside the +service directory and reflects the legacy package hierarchy. +As of June 2022, the parts that remain outside the service are: + +#### Migrations + +`pkg/services/sqlstore/migrations` contains all migrations for SQL +databases, for all services (not including Grafana Enterprise). +Migrations are written per the [database.md](database.md#migrations) document. + +#### API endpoints + +`pkg/api/api.go` contains the endpoint definitions for the most of +Grafana HTTP API (not including Grafana Enterprise). ## Practical example -The `pkg/plugins` package contains plugin domain types, for example `DataPlugin`, and also interfaces -such as `RequestHandler`. Then you have the `pkg/plugins/managers` subpackage, which contains concrete implementations -such as the service `PluginManager`. The subpackage `pkg/plugins/backendplugin/coreplugin` contains `plugins.DataPlugin` -implementations. +The following is a simplified example of the package structure for a +service that doesn't do anything in particular. + +None of the methods or functions are populated and in practice most +packages will consist of multiple files. There isn't a Grafana-wide +convention for which files should exist and contain what. + +`pkg/services/alphabetical` + +``` +package alphabetical + +type Alphabetical interface { + // GetLetter returns either an error or letter. + GetLetter(context.Context, GetLetterQuery) (Letter, error) + // ListCachedLetters cannot fail, and doesn't return an error. + ListCachedLetters(context.Context, ListCachedLettersQuery) Letters + // DeleteLetter doesn't have any return values other than errors, so it + // returns only an error. + DeleteLetter(context.Contxt, DeltaCommand) error +} + +type Letter byte + +type Letters []Letter + +type GetLetterQuery struct { + ID int +} + +// Create queries/commands for methods even if they are empty. +type ListCachedLettersQuery struct {} + +type DeleteLetterCommand struct { + ID int +} + +``` + +`pkg/services/alphabetical/alphabeticalimpl` + +``` +package alphabeticalimpl + +// this name can be whatever, it's not supposed to be used from outside +// the service except for in Wire. +type Svc struct { … } + +func ProviceSvc(numbers numerical.Numerical, db db.DB) Svc { … } + +func (s *Svc) GetLetter(ctx context.Context, q root.GetLetterQuery) (root.Letter, error) { … } +func (s *Svc) ListCachedLetters(ctx context.Context, q root.ListCachedLettersQuery) root.Letters { … } +func (s *Svc) DeleteLetter(ctx context.Context, q root.DeleteLetterCommand) error { … } + +type letterStore interface { + Get(ctx.Context, id int) (root.Letter, error) + Delete(ctx.Context, root.DeleteLetterCommand) error +} + +type sqlLetterStore struct { + db.DB +} + +func (s *sqlStore) Get(ctx.Context, id int) (root.Letter, error) { … } +func (s *sqlStore) Delete(ctx.Context, root.DeleteLetterCommand) error { … } +``` + +## Legacy package hierarchy + +> **Note:** A lot of services still adhere to the legacy model as outlined below. While it is ok to +> extend existing services based on the legacy model, you are _strongly_ encouraged to structure any +> new services or major refactorings using the new package layout. + +Grafana has long used a package-by-layer layout where domain types +are placed in **pkg/models**, all SQL logic in **pkg/services/sqlstore**, +and so forth. + +This is an example of how the _tea pot_ service could be structured +throughout the codebase in the legacy model. + +- _pkg/_ + - _api/_ + - _api.go_ contains the endpoints for the + - _tea_pot.go_ contains methods on the _pkg/api.HTTPServer_ type + that interacts with the service based on queries coming in via the HTTP + API. + - _dtos/tea_pot.go_ extends the _pkg/models_ file with types + that are meant for translation to and from the API. It's not as commonly + present as _pkg/models_. + - _models/tea_pot.go_ contains the models for the service, this + includes the _command_ and _query_ structs that are used when calling + the service or SQL store methods related to the service and also any + models representing an abstraction provided by the service. + - _services/_ + - _sqlstore_ + - _tea_pot.go_ contains SQL queries for + interacting with stored objects related to the tea pot service. + - _migrations/tea_pot.go_ contains the migrations necessary to + build the + - _teapot/\*_ contains functions or a service for doing + logical operations beyond those done in _pkg/api_ or _pkg/services/sqlstore_ + for the service. + +The implementation of legacy services varies widely from service to +service, some or more of these files may be missing and there may be +more files related to a service than those listed here. + +Some legacy services providing infrastructure will also take care of the +integration with several domains. The cleanup service both +provides the infrastructure to occasionally run cleanup scripts and +defines the cleanup scripts. Ideally, this would be migrated +to only handle the scheduling and synchronization of clean up jobs. +The logic for the individual jobs would be placed with a service that is +related to whatever is being cleaned up.