Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Analyse grafana: Use minimal schema #247

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 36 additions & 0 deletions pkg/analyse/dashboard.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package analyse
Copy link
Contributor

Choose a reason for hiding this comment

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

I would write a comment in this mentioning the purpose of these (same thing you wrote on the PR description) to avoid someone coming and completing it with all fields.


type (
Board struct {
ID uint `json:"id,omitempty"`
UID string `json:"uid,omitempty"`
Slug string `json:"slug"`
Title string `json:"title"`
Panels []*Panel `json:"panels"`
Rows []*Row `json:"rows"`
Templating Templating `json:"templating"`
}
Templating struct {
List []TemplateVar `json:"list"`
}
TemplateVar struct {
Name string `json:"name"`
Query interface{} `json:"query"`
Type string `json:"type"`
}
Panel struct {
Targets []Target `json:"targets,omitempty"`
Title string `json:"title"`
Panels []Panel `json:"panels"` // row panel type
Type string `json:"type"`
}
Target struct {
RefID string `json:"refId"`
Datasource string `json:"datasource,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

This will not parse modern dashboards, where this is an object. So we still need either #239 or my change from grafana-tools/sdk#201

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I don't think this field actually gets used anywhere at the moment, so i could remove it.

However if we fork, I agree that it makes more sense to just use the fork instead of the above approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have a fork at https://github.com/colega/grafana-tools-sdk which you can use here as go mod replacement if you need, I've been merging all the fixes into master there.

// For Prometheus
Expr string `json:"expr,omitempty"`
}
Row struct {
Panels []Panel `json:"panels"`
}
)
24 changes: 12 additions & 12 deletions pkg/analyse/grafana.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

log "github.com/sirupsen/logrus"

"github.com/grafana-tools/sdk"
"github.com/pkg/errors"
"github.com/prometheus/prometheus/promql/parser"
)
Expand All @@ -27,15 +26,16 @@ type DashboardMetrics struct {
ParseErrors []string `json:"parse_errors"`
}

func ParseMetricsInBoard(mig *MetricsInGrafana, board sdk.Board) {
func ParseMetricsInBoard(mig *MetricsInGrafana, board Board) {
var parseErrors []error
metrics := make(map[string]struct{})

// Iterate through all the panels and collect metrics
for _, panel := range board.Panels {
parseErrors = append(parseErrors, metricsFromPanel(*panel, metrics)...)
if panel.RowPanel != nil {
for _, subPanel := range panel.RowPanel.Panels {
// row panel
if panel.Panels != nil {
for _, subPanel := range panel.Panels {
parseErrors = append(parseErrors, metricsFromPanel(subPanel, metrics)...)
}
}
Expand Down Expand Up @@ -77,7 +77,7 @@ func ParseMetricsInBoard(mig *MetricsInGrafana, board sdk.Board) {

}

func metricsFromTemplating(templating sdk.Templating, metrics map[string]struct{}) []error {
func metricsFromTemplating(templating Templating, metrics map[string]struct{}) []error {
parseErrors := []error{}
for _, templateVar := range templating.List {
if templateVar.Type != "query" {
Expand All @@ -100,8 +100,8 @@ func metricsFromTemplating(templating sdk.Templating, metrics map[string]struct{
}
err := parseQuery(query, metrics)
if err != nil {
parseErrors = append(parseErrors, errors.Wrapf(err, "query=%v", query))
log.Debugln("msg", "promql parse error", "err", err, "query", query)
parseErrors = append(parseErrors, errors.Wrapf(err, "error parsing templating query: %v", query))
log.Debugln("msg", "promql parse error: templating query", "err", err, "query", query)
continue
}
} else {
Expand All @@ -114,16 +114,16 @@ func metricsFromTemplating(templating sdk.Templating, metrics map[string]struct{
return parseErrors
}

func metricsFromPanel(panel sdk.Panel, metrics map[string]struct{}) []error {
// for now, only look at panels with a prometheus datasource/targets
func metricsFromPanel(panel Panel, metrics map[string]struct{}) []error {
var parseErrors []error

targets := panel.GetTargets()
if targets == nil {
parseErrors = append(parseErrors, fmt.Errorf("unsupported panel type: %q", panel.CommonPanel.Type))
if panel.Targets == nil {
parseErrors = append(parseErrors, fmt.Errorf("unsupported panel type (no targets in panel): %q", panel.Type))
return parseErrors
}

for _, target := range *targets {
for _, target := range panel.Targets {
// Prometheus has this set.
if target.Expr == "" {
continue
Expand Down
3 changes: 1 addition & 2 deletions pkg/commands/analyse_dashboards.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"

"github.com/grafana-tools/sdk"
"gopkg.in/alecthomas/kingpin.v2"

"github.com/grafana/cortex-tools/pkg/analyse"
Expand All @@ -21,7 +20,7 @@ func (cmd *DashboardAnalyseCommand) run(k *kingpin.ParseContext) error {
output.OverallMetrics = make(map[string]struct{})

for _, file := range cmd.DashFilesList {
var board sdk.Board
var board analyse.Board
buf, err := loadFile(file)
if err != nil {
return err
Expand Down
19 changes: 15 additions & 4 deletions pkg/commands/analyse_grafana.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ type GrafanaAnalyseCommand struct {
}

func (cmd *GrafanaAnalyseCommand) run(k *kingpin.ParseContext) error {
output := &analyse.MetricsInGrafana{}
var (
boardLinks []sdk.FoundBoard
rawBoard []byte
board analyse.Board
err error
output *analyse.MetricsInGrafana
)
output.OverallMetrics = make(map[string]struct{})

ctx, cancel := context.WithTimeout(context.Background(), cmd.readTimeout)
Expand All @@ -35,15 +41,20 @@ func (cmd *GrafanaAnalyseCommand) run(k *kingpin.ParseContext) error {
return err
}

boardLinks, err := c.SearchDashboards(ctx, "", false)
boardLinks, err = c.SearchDashboards(ctx, "", false)
if err != nil {
return err
}

for _, link := range boardLinks {
board, _, err := c.GetDashboardByUID(ctx, link.UID)
rawBoard, _, err = c.GetRawDashboardBySlug(ctx, link.URI)
Copy link
Contributor

@colega colega Mar 24, 2022

Choose a reason for hiding this comment

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

Nit: I'd keep := here.

if err != nil {
fmt.Fprintf(os.Stderr, "%s for %s %s\n", err, link.UID, link.Title)
fmt.Fprintf(os.Stderr, "%s for %s\n", err, link.URI)
continue
}

if err = json.Unmarshal(rawBoard, &board); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
if err = json.Unmarshal(rawBoard, &board); err != nil {
if err := json.Unmarshal(rawBoard, &board); err != nil {

Even if the outcome is the same, we don't really want to modify the outer err variable.

fmt.Fprintf(os.Stderr, "%s for %s\n", err, link.URI)
continue
}
analyse.ParseMetricsInBoard(output, board)
Expand Down
5 changes: 2 additions & 3 deletions pkg/commands/analyse_grafana_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/json"
"testing"

"github.com/grafana-tools/sdk"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

Expand All @@ -25,7 +24,7 @@ var dashboardMetrics = []string{
}

func TestParseMetricsInBoard(t *testing.T) {
var board sdk.Board
var board analyse.Board
output := &analyse.MetricsInGrafana{}
output.OverallMetrics = make(map[string]struct{})

Expand All @@ -40,7 +39,7 @@ func TestParseMetricsInBoard(t *testing.T) {
}

func TestParseMetricsInBoardWithTimeseriesPanel(t *testing.T) {
var board sdk.Board
var board analyse.Board
output := &analyse.MetricsInGrafana{}
output.OverallMetrics = make(map[string]struct{})

Expand Down