Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

Add datasources list method #26

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add datasources list method #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ae4dc9
Add datasources list method
Sep 27, 2019
9817e1b
Change JSONData and SecureJsonData into map[string]interface
gpolek Oct 11, 2019
41af9d1
Set proper fields for test struct literal
gpolek Oct 11, 2019
c3297a9
Merge pull request #1 from gpolek/master
danielllek Oct 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,10 @@ type DataSource struct {
}

// JSONData is a representation of the datasource `jsonData` property
type JSONData struct {
AssumeRoleArn string `json:"assumeRoleArn,omitempty"`
AuthType string `json:"authType,omitempty"`
CustomMetricsNamespaces string `json:"customMetricsNamespaces,omitempty"`
DefaultRegion string `json:"defaultRegion,omitempty"`
TlsSkipVerify bool `json:"tlsSkipVerify,omitempty"`
}
type JSONData map[string]interface{}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious to hear your reasoning for changing the types here. Do we have to worry about any potential consumers of Datasource method since they'll have to change how they access the JSONData and SecureJSONData properties?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple DataSource types available for Grafana and struct here allows only to use go-grafana-api only with one DataSource (I assume it was something with AWS).

If we loosen typing here to map[string]interface{} we are able to pass different jsonData and secureJsonData needed for other datasource types.

We know breaking changes are not welcomed into the interfaces and we are more than happy to make this possible without breaking changes but I don't see a way how to achieve that. Maybe with reflect or changing type to some Inteface but I am not a fan of either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaking changes aside, I'm not sure it's necessary to remove the schema from jsonData and secureJsonData entirely to support additional data source types. There tends to be some overlap in values between the different data sources; how we've handled different schemas for similar objects is to merge the maps of all the possible keys of the different data source types and tack on the omitempty tag to most of them so that they won't be sent unless explicitly used. Does that sound like an alright approach to you?


// SecureJSONData is a representation of the datasource `secureJsonData` property
type SecureJSONData struct {
AccessKey string `json:"accessKey,omitempty"`
SecretKey string `json:"secretKey,omitempty"`
}
type SecureJSONData map[string]interface{}

func (c *Client) NewDataSource(s *DataSource) (int64, error) {
data, err := json.Marshal(s)
Expand Down Expand Up @@ -97,6 +88,33 @@ func (c *Client) UpdateDataSource(s *DataSource) error {
return nil
}

func (c *Client) DataSources() ([]DataSource, error) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a test for this method, and even nicer that it'd have an example of the payload the api returns.

Copy link

@gpolek gpolek Oct 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will prepare some additional tests.

Regarding payload, for example we use Azure Monitor and Zabbix, both having different jsonData and secureJsopData

Azure Monitor:

{
    "id": 101,
    "orgId": 1,
    "name": "SOME_DATASOURCE_NAME",
    "type": "grafana-azure-monitor-datasource",
    "typeLogoUrl": "",
    "access": "proxy",
    "url": "/api/datasources/proxy/101",
    "password": "",
    "user": "",
    "database": "",
    "basicAuth": false,
    "basicAuthUser": "",
    "basicAuthPassword": "",
    "withCredentials": false,
    "isDefault": false,
    "jsonData": {
        "appInsightsAppId": "00000000-0000-0000-0000-000000000000",
        "azureLogAnalyticsSameAs": false,
        "cloudName": "azuremonitor"
    },
    "secureJsonData": {
        "appInsightsApiKey":"bUtGHDSq7XtxsKeBUrfwYfk6QhGWZRP2
    },
    "version": 5,
    "readOnly": false
}

Zabbix:

{
    "id": 67,
    "orgId": 1,
    "name": "Zabbix",
    "type": "alexanderzobnin-zabbix-datasource",
    "typeLogoUrl": "",
    "access": "proxy",
    "url": "http://zabbi/api_jsonrpc.php",
    "password": "",
    "user": "",
    "database": "",
    "basicAuth": false,
    "basicAuthUser": "",
    "basicAuthPassword": "",
    "withCredentials": false,
    "isDefault": false,
    "jsonData": {
        "addThresholds": true,
        "alerting": true,
        "alertingMinSeverity": 3,
        "dbConnectionDatasourceId": null,
        "dbConnectionEnable": false,
        "disableReadOnlyUsersAck": false,
        "keepCookies": [],
        "password": "KdrVK8U0x3hgBppY",
        "trends": false,
        "username": "grafana",
        "zabbixVersion": 4
    },
    "secureJsonData": {},
    "version": 8,
    "readOnly": false
}

datasources := make([]DataSource, 0)

path := fmt.Sprintf("/api/datasources")
req, err := c.newRequest("GET", path, nil, nil)
if err != nil {
return nil, err
}

resp, err := c.Do(req)
if err != nil {
return datasources, err
}
if resp.StatusCode != 200 {
return datasources, errors.New(resp.Status)
}

data, err := ioutil.ReadAll(resp.Body)
if err != nil {
return datasources, err
}

err = json.Unmarshal(data, &datasources)

return datasources, err
}

func (c *Client) DataSource(id int64) (*DataSource, error) {
path := fmt.Sprintf("/api/datasources/%d", id)
req, err := c.newRequest("GET", path, nil, nil)
Expand Down
14 changes: 7 additions & 7 deletions datasource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ func TestNewDataSource(t *testing.T) {
Access: "access",
IsDefault: true,
JSONData: JSONData{
AssumeRoleArn: "arn:aws:iam::123:role/some-role",
AuthType: "keys",
CustomMetricsNamespaces: "SomeNamespace",
DefaultRegion: "us-east-1",
TlsSkipVerify: true,
"assumeRoleArn": "arn:aws:iam::123:role/some-role",
"authType": "keys",
"customMetricsNamespaces": "SomeNamespace",
"defaultRegion": "us-east-1",
"tlsSkipVerify": true,
},
SecureJSONData: SecureJSONData{
AccessKey: "123",
SecretKey: "456",
"accessKey": "123",
"secretKey": "456",
},
}

Expand Down