IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Glue: Correlations minor APIs behavior improvements (#56078)

Browse Source 
* add correlation config type, CorrelationConfig validator & default values

* make config required when creating correlations

* make targetUID optional, add validation for createCommand & configType

* fix tests

* update remaining tests

* fix lint error

* Update pkg/services/correlations/models.go

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>

* update docs

Co-authored-by: Piotr Jamróz <pm.jamroz@gmail.com>
This commit is contained in:
Giordano Ricci 2022-10-04 09:39:55 +01:00 committed by GitHub
parent 3381629d3d
commit 489b302c03
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 313 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
devenv/datasources.yaml
View File

@ -242,6 +242,7 @@ datasources:
label: "Logs to metrics"
description: "Related metrics stored in Prometheus"
config:
type: query
target:
expr: "{ job=\"test\" }"
field: "labels"

54
docs/sources/developers/http_api/correlations.md
View File

@ -34,6 +34,11 @@ Authorization: Bearer eyJrIjoiT0tTcG1pUlY2RnVKZTFVaDFsNFZXdE9ZWmNrMkZYbk
"targetUID": "PDDA8E780A17E7EF1",
"label": "My Label",
"description": "Logs to Traces",
"config": {
"type": "query",
"field": "message",
"target": {},
}
}
```
@ -55,7 +60,12 @@ Content-Type: application/json
"label": "My Label",
"sourceUID": "uyBf2637k",
"targetUID": "PDDA8E780A17E7EF1",
"uid": "50xhMlg9k"
"uid": "50xhMlg9k",
"config": {
"type": "query",
"field": "message",
"target": {},
}
}
}
```
@ -138,7 +148,12 @@ Content-Type: application/json
"label": "My Label",
"sourceUID": "uyBf2637k",
"targetUID": "PDDA8E780A17E7EF1",
"uid": "J6gn7d31L"
"uid": "J6gn7d31L",
"config": {
"type": "query",
"field": "message",
"target": {}
}
}
}
```
@ -175,7 +190,12 @@ Content-Type: application/json
"label": "My Label",
"sourceUID": "uyBf2637k",
"targetUID": "PDDA8E780A17E7EF1",
"uid": "J6gn7d31L"
"uid": "J6gn7d31L",
"config": {
"type": "query",
"field": "message",
"target": {},
}
}
```
@ -211,14 +231,24 @@ Content-Type: application/json
"label": "My Label",
"sourceUID": "uyBf2637k",
"targetUID": "PDDA8E780A17E7EF1",
"uid": "J6gn7d31L"
"uid": "J6gn7d31L",
"config": {
"type": "query",
"field": "message",
"target": {},
}
},
{
"description": "Logs to Metrics",
"label": "Another Label",
"sourceUID": "uyBf2637k",
"targetUID": "P15396BDD62B2BE29",
"uid": "uWCpURgVk"
"uid": "uWCpURgVk",
"config": {
"type": "query",
"field": "message",
"target": {},
}
}
]
```
@ -255,14 +285,24 @@ Content-Type: application/json
"label": "My Label",
"sourceUID": "uyBf2637k",
"targetUID": "PDDA8E780A17E7EF1",
"uid": "J6gn7d31L"
"uid": "J6gn7d31L",
"config": {
"type": "query",
"field": "message",
"target": {},
}
},
{
"description": "Loki to Tempo",
"label": "Another Label",
"sourceUID": "PDDA8E780A17E7EF1",
"targetUID": "P15396BDD62B2BE29",
"uid": "uWCpURgVk"
"uid": "uWCpURgVk",
"config": {
"type": "query",
"field": "message",
"target": {},
}
}
]
```

14
pkg/services/correlations/database.go
View File

@ -34,11 +34,13 @@ func (s CorrelationsService) createCorrelation(ctx context.Context, cmd CreateCo
return ErrSourceDataSourceReadOnly
}
if err = s.DataSourceService.GetDataSource(ctx, &datasources.GetDataSourceQuery{
OrgId: cmd.OrgId,
Uid: cmd.TargetUID,
}); err != nil {
return ErrTargetDataSourceDoesNotExists
if cmd.TargetUID != nil {
if err = s.DataSourceService.GetDataSource(ctx, &datasources.GetDataSourceQuery{
OrgId: cmd.OrgId,
Uid: *cmd.TargetUID,
}); err != nil {
return ErrTargetDataSourceDoesNotExists
}
}
_, err = session.Insert(correlation)
@ -206,7 +208,7 @@ func (s CorrelationsService) deleteCorrelationsBySourceUID(ctx context.Context,
func (s CorrelationsService) deleteCorrelationsByTargetUID(ctx context.Context, cmd DeleteCorrelationsByTargetUIDCommand) error {
return s.SQLStore.WithDbSession(ctx, func(session *sqlstore.DBSession) error {
_, err := session.Delete(&Correlation{TargetUID: cmd.TargetUID})
_, err := session.Delete(&Correlation{TargetUID: &cmd.TargetUID})
return err
})
}

57
pkg/services/correlations/models.go
View File

@ -1,7 +1,9 @@
package correlations
import (
"encoding/json"
"errors"
"fmt"
)
var (
@ -11,20 +13,49 @@ var (
ErrCorrelationFailedGenerateUniqueUid = errors.New("failed to generate unique correlation UID")
ErrCorrelationNotFound = errors.New("correlation not found")
ErrUpdateCorrelationEmptyParams = errors.New("not enough parameters to edit correlation")
ErrInvalidConfigType = errors.New("invalid correlation config type")
)
// CorrelationConfigTarget is the target data query specific to target data source (Correlation.TargetUID)
// swagger:model
type CorrelationConfigTarget interface{}
type CorrelationConfigType string
const (
ConfigTypeQuery CorrelationConfigType = "query"
)
func (t CorrelationConfigType) Validate() error {
if t != ConfigTypeQuery {
return fmt.Errorf("%s: \"%s\"", ErrInvalidConfigType, t)
}
return nil
}
// swagger:model
type CorrelationConfig struct {
// Field used to attach the correlation link
// required:true
Field string `json:"field"`
Field string `json:"field" binding:"Required"`
// Target type
// required:true
Type CorrelationConfigType `json:"type" binding:"Required"`
// Target data query
// required:true
Target CorrelationConfigTarget `json:"target"`
Target map[string]interface{} `json:"target" binding:"Required"`
}
func (c CorrelationConfig) MarshalJSON() ([]byte, error) {
target := c.Target
if target == nil {
target = map[string]interface{}{}
}
return json.Marshal(struct {
Type CorrelationConfigType `json:"type"`
Field string `json:"field"`
Target map[string]interface{} `json:"target"`
}{
Type: ConfigTypeQuery,
Field: c.Field,
Target: target,
})
}
// Correlation is the model for correlations definitions
@ -38,7 +69,7 @@ type Correlation struct {
SourceUID string `json:"sourceUID" xorm:"pk 'source_uid'"`
// UID of the data source the correlation points to
// example:PE1C5CBDA0504A6A3
TargetUID string `json:"targetUID" xorm:"target_uid"`
TargetUID *string `json:"targetUID" xorm:"target_uid"`
// Label identifying the correlation
// example: My Label
Label string `json:"label" xorm:"label"`
@ -67,7 +98,7 @@ type CreateCorrelationCommand struct {
SkipReadOnlyCheck bool `json:"-"`
// Target data source UID to which the correlation is created
// example:PE1C5CBDA0504A6A3
TargetUID string `json:"targetUID" binding:"Required"`
TargetUID *string `json:"targetUID"`
// Optional label identifying the correlation
// example: My label
Label string `json:"label"`
@ -76,7 +107,17 @@ type CreateCorrelationCommand struct {
Description string `json:"description"`
// Arbitrary configuration object handled in frontend
// example: { field: "job", target: { query: "job=app" } }
Config CorrelationConfig `json:"config"`
Config CorrelationConfig `json:"config" binding:"Required"`
}
func (c CreateCorrelationCommand) Validate() error {
if err := c.Config.Type.Validate(); err != nil {
return err
}
if c.TargetUID == nil && c.Config.Type == ConfigTypeQuery {
return fmt.Errorf("correlations of type \"%s\" must have a targetUID", ConfigTypeQuery)
}
return nil
}
// swagger:model

91
pkg/services/correlations/models_test.go Normal file
View File

@ -0,0 +1,91 @@
package correlations
import (
"encoding/json"
"testing"
"github.com/stretchr/testify/require"
)
func TestCorrelationModels(t *testing.T) {
t.Run("CreateCorrelationCommand Validate", func(t *testing.T) {
t.Run("Successfully validates a correct create command", func(t *testing.T) {
targetUid := "targetUid"
config := &CorrelationConfig{
Field: "field",
Target: map[string]interface{}{},
Type: ConfigTypeQuery,
}
cmd := &CreateCorrelationCommand{
SourceUID: "some-uid",
OrgId: 1,
TargetUID: &targetUid,
Config: *config,
}
require.NoError(t, cmd.Validate())
})
t.Run("Fails if target UID is not set and config type = query", func(t *testing.T) {
config := &CorrelationConfig{
Field: "field",
Target: map[string]interface{}{},
Type: ConfigTypeQuery,
}
cmd := &CreateCorrelationCommand{
SourceUID: "some-uid",
OrgId: 1,
Config: *config,
}
require.Error(t, cmd.Validate())
})
t.Run("Fails if config type is unknown", func(t *testing.T) {
config := &CorrelationConfig{
Field: "field",
Target: map[string]interface{}{},
Type: "unknown config type",
}
cmd := &CreateCorrelationCommand{
SourceUID: "some-uid",
OrgId: 1,
Config: *config,
}
require.Error(t, cmd.Validate())
})
})
t.Run("CorrelationConfigType Validate", func(t *testing.T) {
t.Run("Successfully validates a correct type", func(t *testing.T) {
type test struct {
input CorrelationConfigType
assertion require.ErrorAssertionFunc
}
tests := []test{
{input: "query", assertion: require.NoError},
{input: "link", assertion: require.Error},
}
for _, tc := range tests {
tc.assertion(t, tc.input.Validate())
}
})
})
t.Run("CorrelationConfig JSON Marshaling", func(t *testing.T) {
t.Run("Applies a default empty object if target is not defined", func(t *testing.T) {
config := CorrelationConfig{
Field: "field",
Type: ConfigTypeQuery,
}
data, err := json.Marshal(config)
require.NoError(t, err)
require.Equal(t, `{"type":"query","field":"field","target":{}}`, string(data))
})
})
}

3
pkg/services/provisioning/datasources/config_reader_test.go
View File

@ -271,7 +271,8 @@ func TestDatasourceAsConfig(t *testing.T) {
t.Run("Deleting datasource deletes existing correlations", func(t *testing.T) {
store := &spyStore{items: []*datasources.DataSource{{Name: "old-data-source", OrgId: 1, Id: 1, Uid: "some-uid"}}}
orgStore := &mockOrgStore{}
correlationsStore := &mockCorrelationsStore{items: []correlations.Correlation{{UID: "some-uid", SourceUID: "some-uid", TargetUID: "target-uid"}}}
targetUid := "target-uid"
correlationsStore := &mockCorrelationsStore{items: []correlations.Correlation{{UID: "some-uid", SourceUID: "some-uid", TargetUID: &targetUid}}}
dc := newDatasourceProvisioner(logger, store, correlationsStore, orgStore)
err := dc.applyChanges(context.Background(), deleteOneDatasource)
if err != nil {

19
pkg/services/provisioning/datasources/datasources.go
View File

@ -138,20 +138,19 @@ func (dc *DatasourceProvisioner) applyChanges(ctx context.Context, configPath st
func makeCreateCorrelationCommand(correlation map[string]interface{}, SourceUID string, OrgId int64) (correlations.CreateCorrelationCommand, error) {
var json = jsoniter.ConfigCompatibleWithStandardLibrary
targetUID, ok := correlation["targetUID"].(string)
if !ok {
return correlations.CreateCorrelationCommand{}, fmt.Errorf("correlation missing targetUID")
}
createCommand := correlations.CreateCorrelationCommand{
SourceUID: SourceUID,
TargetUID: targetUID,
Label: correlation["label"].(string),
Description: correlation["description"].(string),
OrgId: OrgId,
SkipReadOnlyCheck: true,
}
targetUID, ok := correlation["targetUID"].(string)
if ok {
createCommand.TargetUID = &targetUID
}
if correlation["config"] != nil {
jsonbody, err := json.Marshal(correlation["config"])
if err != nil {
@ -164,6 +163,14 @@ func makeCreateCorrelationCommand(correlation map[string]interface{}, SourceUID
}
createCommand.Config = config
} else {
// when provisioning correlations without config we default to type="query"
createCommand.Config = correlations.CorrelationConfig{
Type: correlations.ConfigTypeQuery,
}
}
if err := createCommand.Validate(); err != nil {
return correlations.CreateCorrelationCommand{}, err
}
return createCommand, nil

132
pkg/tests/api/correlations/correlations_create_test.go
View File

@ -120,7 +120,12 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", "nonexistent-ds-uid"),
body: fmt.Sprintf(`{
"targetUID": "%s"
"targetUID": "%s",
"config": {
"type": "query",
"field": "message",
"target": {}
}
}`, writableDs),
user: adminUser,
})
@ -139,11 +144,16 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
require.NoError(t, res.Body.Close())
})
t.Run("inexistent target data source should result in a 404", func(t *testing.T) {
t.Run("inexistent target data source should result in a 404 if config.type=query", func(t *testing.T) {
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", writableDs),
body: `{
"targetUID": "nonexistent-uid-uid"
"targetUID": "nonexistent-uid-uid",
"config": {
"type": "query",
"field": "message",
"target": {}
}
}`,
user: adminUser,
})
@ -166,7 +176,12 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", readOnlyDS),
body: fmt.Sprintf(`{
"targetUID": "%s"
"targetUID": "%s",
"config": {
"type": "query",
"field": "message",
"target": {}
}
}`, readOnlyDS),
user: adminUser,
})
@ -189,7 +204,12 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", writableDs),
body: fmt.Sprintf(`{
"targetUID": "%s"
"targetUID": "%s",
"config": {
"type": "query",
"field": "message",
"target": {}
}
}`, readOnlyDS),
user: adminUser,
})
@ -204,46 +224,18 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
require.Equal(t, "Correlation created", response.Message)
require.Equal(t, writableDs, response.Result.SourceUID)
require.Equal(t, readOnlyDS, response.Result.TargetUID)
require.Equal(t, readOnlyDS, *response.Result.TargetUID)
require.Equal(t, "", response.Result.Description)
require.Equal(t, "", response.Result.Label)
require.NoError(t, res.Body.Close())
})
t.Run("Should correctly create a correlation without a config", func(t *testing.T) {
description := "a description"
label := "a label"
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", writableDs),
body: fmt.Sprintf(`{
"targetUID": "%s",
"description": "%s",
"label": "%s"
}`, writableDs, description, label),
user: adminUser,
})
require.Equal(t, http.StatusOK, res.StatusCode)
responseBody, err := io.ReadAll(res.Body)
require.NoError(t, err)
var response correlations.CreateCorrelationResponseBody
err = json.Unmarshal(responseBody, &response)
require.NoError(t, err)
require.Equal(t, "Correlation created", response.Message)
require.Equal(t, writableDs, response.Result.SourceUID)
require.Equal(t, writableDs, response.Result.TargetUID)
require.Equal(t, description, response.Result.Description)
require.Equal(t, label, response.Result.Label)
require.NoError(t, res.Body.Close())
})
t.Run("Should correctly create a correlation with a correct config", func(t *testing.T) {
description := "a description"
label := "a label"
fieldName := "fieldName"
configType := correlations.ConfigTypeQuery
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", writableDs),
body: fmt.Sprintf(`{
@ -251,10 +243,11 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
"description": "%s",
"label": "%s",
"config": {
"field": "fieldName",
"type": "%s",
"field": "%s",
"target": { "expr": "foo" }
}
}`, writableDs, description, label),
}`, writableDs, description, label, configType, fieldName),
user: adminUser,
})
require.Equal(t, http.StatusOK, res.StatusCode)
@ -268,10 +261,11 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
require.Equal(t, "Correlation created", response.Message)
require.Equal(t, writableDs, response.Result.SourceUID)
require.Equal(t, writableDs, response.Result.TargetUID)
require.Equal(t, writableDs, *response.Result.TargetUID)
require.Equal(t, description, response.Result.Description)
require.Equal(t, label, response.Result.Label)
require.Equal(t, "fieldName", response.Result.Config.Field)
require.Equal(t, configType, response.Result.Config.Type)
require.Equal(t, fieldName, response.Result.Config.Field)
require.Equal(t, map[string]interface{}{"expr": "foo"}, response.Result.Config.Target)
require.NoError(t, res.Body.Close())
@ -305,4 +299,62 @@ func TestIntegrationCreateCorrelation(t *testing.T) {
require.NoError(t, res.Body.Close())
})
t.Run("Should not create a correlation without a config", func(t *testing.T) {
description := "a description"
label := "a label"
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", writableDs),
body: fmt.Sprintf(`{
"targetUID": "%s",
"description": "%s",
"label": "%s"
}`, writableDs, description, label),
user: adminUser,
})
require.Equal(t, http.StatusBadRequest, res.StatusCode)
responseBody, err := io.ReadAll(res.Body)
require.NoError(t, err)
var response errorResponseBody
err = json.Unmarshal(responseBody, &response)
require.NoError(t, err)
require.Contains(t, response.Message, "bad request data")
require.NoError(t, res.Body.Close())
})
t.Run("Should not create a correlation with an invalid config type", func(t *testing.T) {
description := "a description"
label := "a label"
configType := "nonexistent-config-type"
res := ctx.Post(PostParams{
url: fmt.Sprintf("/api/datasources/uid/%s/correlations", writableDs),
body: fmt.Sprintf(`{
"targetUID": "%s",
"description": "%s",
"label": "%s",
"config": {
"type": "%s"
}
}`, writableDs, description, label, configType),
user: adminUser,
})
require.Equal(t, http.StatusBadRequest, res.StatusCode)
responseBody, err := io.ReadAll(res.Body)
require.NoError(t, err)
var response errorResponseBody
err = json.Unmarshal(responseBody, &response)
require.NoError(t, err)
require.Contains(t, response.Message, "bad request data")
require.Contains(t, response.Error, correlations.ErrInvalidConfigType.Error())
require.Contains(t, response.Error, configType)
require.NoError(t, res.Body.Close())
})
}

4
pkg/tests/api/correlations/correlations_delete_test.go
View File

@ -159,7 +159,7 @@ func TestIntegrationDeleteCorrelation(t *testing.T) {
t.Run("deleting a correlation pointing to a read-only data source should work", func(t *testing.T) {
correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
SourceUID: writableDs,
TargetUID: writableDs,
TargetUID: &writableDs,
OrgId: writableDsOrgId,
})
@ -191,7 +191,7 @@ func TestIntegrationDeleteCorrelation(t *testing.T) {
t.Run("should correctly delete a correlation", func(t *testing.T) {
correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
SourceUID: writableDs,
TargetUID: readOnlyDS,
TargetUID: &readOnlyDS,
OrgId: writableDsOrgId,
})

8
pkg/tests/api/correlations/correlations_read_test.go
View File

@ -73,9 +73,10 @@ func TestIntegrationReadCorrelation(t *testing.T) {
dsWithCorrelations := createDsCommand.Result
correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
SourceUID: dsWithCorrelations.Uid,
TargetUID: dsWithCorrelations.Uid,
TargetUID: &dsWithCorrelations.Uid,
OrgId: dsWithCorrelations.OrgId,
Config: correlations.CorrelationConfig{
Type: correlations.ConfigTypeQuery,
Field: "foo",
Target: map[string]interface{}{},
},
@ -92,17 +93,18 @@ func TestIntegrationReadCorrelation(t *testing.T) {
// This creates 2 records in the correlation table that should never be returned by the API.
// Given all tests in this file work on the assumption that only a single correlation exists,
// this covers the case where bad data exists in the database.
nonExistingDsUID := "THIS-DOES-NOT_EXIST"
err := ctx.env.SQLStore.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
created, err := sess.InsertMulti(&[]correlations.Correlation{
{
UID: "uid-1",
SourceUID: dsWithoutCorrelations.Uid,
TargetUID: "THIS-DOES-NOT_EXIST",
TargetUID: &nonExistingDsUID,
},
{
UID: "uid-2",
SourceUID: "THIS-DOES-NOT_EXIST",
TargetUID: dsWithoutCorrelations.Uid,
TargetUID: &dsWithoutCorrelations.Uid,
},
})
require.Equal(t, int64(2), created)

6
pkg/tests/api/correlations/correlations_update_test.go
View File

@ -165,7 +165,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
t.Run("updating a without data should result in a 400", func(t *testing.T) {
correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
SourceUID: writableDs,
TargetUID: writableDs,
TargetUID: &writableDs,
OrgId: writableDsOrgId,
})
@ -231,7 +231,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
t.Run("updating a correlation pointing to a read-only data source should work", func(t *testing.T) {
correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
SourceUID: writableDs,
TargetUID: writableDs,
TargetUID: &writableDs,
OrgId: writableDsOrgId,
Label: "a label",
})
@ -260,7 +260,7 @@ func TestIntegrationUpdateCorrelation(t *testing.T) {
t.Run("should correctly update correlations", func(t *testing.T) {
correlation := ctx.createCorrelation(correlations.CreateCorrelationCommand{
SourceUID: writableDs,
TargetUID: writableDs,
TargetUID: &writableDs,
OrgId: writableDsOrgId,
Label: "0",
Description: "0",