Commit 62538f0f authored by Mikolaj Wawrzyniak's avatar Mikolaj Wawrzyniak

Add blob viewer for metrics dashboard yaml files

To enable validation of metrics dashboard definitions in yaml files
we need to enable to display blobs containing that file to be displayed
in auxiliary mode.
parent 04d6297d
...@@ -50,6 +50,7 @@ class Blob < SimpleDelegator ...@@ -50,6 +50,7 @@ class Blob < SimpleDelegator
BlobViewer::License, BlobViewer::License,
BlobViewer::Contributing, BlobViewer::Contributing,
BlobViewer::Changelog, BlobViewer::Changelog,
BlobViewer::MetricsDashboardYml,
BlobViewer::CargoToml, BlobViewer::CargoToml,
BlobViewer::Cartfile, BlobViewer::Cartfile,
......
# frozen_string_literal: true
module BlobViewer
class MetricsDashboardYml < Base
include ServerSide
include Gitlab::Utils::StrongMemoize
include Auxiliary
self.partial_name = 'metrics_dashboard_yml'
self.loading_partial_name = 'metrics_dashboard_yml_loading'
self.file_types = %i(metrics_dashboard)
self.binary = false
def valid?
errors.blank?
end
def errors
strong_memoize(:errors) do
prepare!
parse_blob_data
end
end
private
def parse_blob_data
::PerformanceMonitoring::PrometheusDashboard.from_json(YAML.safe_load(blob.data))
nil
rescue Psych::SyntaxError => error
wrap_yml_syntax_error(error)
rescue ActiveModel::ValidationError => invalid
invalid.model.errors
end
def wrap_yml_syntax_error(error)
::PerformanceMonitoring::PrometheusDashboard.new.errors.tap do |errors|
errors.add(:'YAML syntax', error.message)
end
end
end
end
...@@ -13,7 +13,7 @@ module PerformanceMonitoring ...@@ -13,7 +13,7 @@ module PerformanceMonitoring
def from_json(json_content) def from_json(json_content)
dashboard = new( dashboard = new(
dashboard: json_content['dashboard'], dashboard: json_content['dashboard'],
panel_groups: json_content['panel_groups'].map { |group| PrometheusPanelGroup.from_json(group) } panel_groups: json_content['panel_groups']&.map { |group| PrometheusPanelGroup.from_json(group) }
) )
dashboard.tap(&:validate!) dashboard.tap(&:validate!)
......
...@@ -15,7 +15,7 @@ module PerformanceMonitoring ...@@ -15,7 +15,7 @@ module PerformanceMonitoring
title: json_content['title'], title: json_content['title'],
y_label: json_content['y_label'], y_label: json_content['y_label'],
weight: json_content['weight'], weight: json_content['weight'],
metrics: json_content['metrics'].map { |metric| PrometheusMetric.from_json(metric) } metrics: json_content['metrics']&.map { |metric| PrometheusMetric.from_json(metric) }
) )
panel.tap(&:validate!) panel.tap(&:validate!)
......
...@@ -13,7 +13,7 @@ module PerformanceMonitoring ...@@ -13,7 +13,7 @@ module PerformanceMonitoring
panel_group = new( panel_group = new(
group: json_content['group'], group: json_content['group'],
priority: json_content['priority'], priority: json_content['priority'],
panels: json_content['panels'].map { |panel| PrometheusPanel.from_json(panel) } panels: json_content['panels']&.map { |panel| PrometheusPanel.from_json(panel) }
) )
panel_group.tap(&:validate!) panel_group.tap(&:validate!)
......
- if viewer.valid?
= icon('check fw')
= _('Metrics Dashboard YAML definition is valid.')
- else
= icon('warning fw')
= _('Metrics Dashboard YAML definition is invalid:')
%ul
- viewer.errors.messages.each do |error|
%li= error.join(': ')
= link_to _('Learn more'), help_page_path('user/project/integrations/prometheus.md', anchor: 'defining-custom-dashboards-per-project')
= icon('spinner spin fw')
= _('Metrics Dashboard YAML definition') + '…'
= link_to _('Learn more'), help_page_path('user/project/integrations/prometheus.md')
---
title: Added validation for YAML files with metrics dashboard definitions.
merge_request: 33202
author:
type: added
...@@ -326,6 +326,33 @@ new branch. ...@@ -326,6 +326,33 @@ new branch.
If you select your **default** branch, the new dashboard becomes immediately available. If you select your **default** branch, the new dashboard becomes immediately available.
If you select another branch, this branch should be merged to your **default** branch first. If you select another branch, this branch should be merged to your **default** branch first.
#### Dashboard YAML syntax validation
> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33202) in GitLab 13.1.
To confirm your dashboard definition contains valid YAML syntax:
1. Navigate to **{doc-text}** **Repository > Files**.
1. Navigate to your dashboard file in your repository.
1. Review the information pane about the file, displayed above the file contents.
Files with valid syntax display **Metrics Dashboard YAML definition is valid**,
and files with invalid syntax display **Metrics Dashboard YAML definition is invalid**.
![Metrics Dashboard_YAML_syntax_validation](img/prometheus_dashboard_yaml_validation_v13_1.png)
When **Metrics Dashboard YAML definition is invalid** at least one of the following messages is displayed:
1. `dashboard: can't be blank` [learn more](#dashboard-top-level-properties)
1. `panel_groups: can't be blank` [learn more](#dashboard-top-level-properties)
1. `group: can't be blank` [learn more](#panel-group-panel_groups-properties)
1. `panels: can't be blank` [learn more](#panel-group-panel_groups-properties)
1. `metrics: can't be blank` [learn more](#panel-panels-properties)
1. `title: can't be blank` [learn more](#panel-panels-properties)
1. `query: can't be blank` [learn more](#metrics-metrics-properties)
1. `query_range: can't be blank` [learn more](#metrics-metrics-properties)
1. `unit: can't be blank` [learn more](#metrics-metrics-properties)
#### Dashboard YAML properties #### Dashboard YAML properties
Dashboards have several components: Dashboards have several components:
......
...@@ -13753,6 +13753,15 @@ msgstr "" ...@@ -13753,6 +13753,15 @@ msgstr ""
msgid "Metrics Dashboard" msgid "Metrics Dashboard"
msgstr "" msgstr ""
msgid "Metrics Dashboard YAML definition"
msgstr ""
msgid "Metrics Dashboard YAML definition is invalid:"
msgstr ""
msgid "Metrics Dashboard YAML definition is valid."
msgstr ""
msgid "Metrics and profiling" msgid "Metrics and profiling"
msgstr "" msgstr ""
......
...@@ -555,6 +555,53 @@ describe 'File blob', :js do ...@@ -555,6 +555,53 @@ describe 'File blob', :js do
end end
end end
describe '.gitlab/dashboards/custom-dashboard.yml' do
before do
project.add_maintainer(project.creator)
Files::CreateService.new(
project,
project.creator,
start_branch: 'master',
branch_name: 'master',
commit_message: "Add .gitlab/dashboards/custom-dashboard.yml",
file_path: '.gitlab/dashboards/custom-dashboard.yml',
file_content: file_content
).execute
visit_blob('.gitlab/dashboards/custom-dashboard.yml')
end
context 'valid dashboard file' do
let(:file_content) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) }
it 'displays an auxiliary viewer' do
aggregate_failures do
# shows that dashboard yaml is valid
expect(page).to have_content('Metrics Dashboard YAML definition is valid.')
# shows a learn more link
expect(page).to have_link('Learn more')
end
end
end
context 'invalid dashboard file' do
let(:file_content) { "dashboard: 'invalid'" }
it 'displays an auxiliary viewer' do
aggregate_failures do
# shows that dashboard yaml is invalid
expect(page).to have_content('Metrics Dashboard YAML definition is invalid:')
expect(page).to have_content("panel_groups: can't be blank")
# shows a learn more link
expect(page).to have_link('Learn more')
end
end
end
end
context 'LICENSE' do context 'LICENSE' do
before do before do
visit_blob('LICENSE') visit_blob('LICENSE')
......
# frozen_string_literal: true
require 'spec_helper'
describe BlobViewer::MetricsDashboardYml do
include FakeBlobHelpers
include RepoHelpers
let_it_be(:project) { create(:project, :repository) }
let(:blob) { fake_blob(path: '.gitlab/dashboards/custom-dashboard.yml', data: data) }
let(:sha) { sample_commit.id }
subject(:viewer) { described_class.new(blob) }
context 'when the definition is valid' do
let(:data) { File.read(Rails.root.join('config/prometheus/common_metrics.yml')) }
describe '#valid?' do
it 'calls prepare! on the viewer' do
allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json)
expect(viewer).to receive(:prepare!)
viewer.valid?
end
it 'returns true' do
expect(PerformanceMonitoring::PrometheusDashboard)
.to receive(:from_json).with(YAML.safe_load(data))
expect(viewer.valid?).to be_truthy
end
end
describe '#errors' do
it 'returns nil' do
allow(PerformanceMonitoring::PrometheusDashboard).to receive(:from_json)
expect(viewer.errors).to be nil
end
end
end
context 'when definition is invalid' do
let(:error) { ActiveModel::ValidationError.new(PerformanceMonitoring::PrometheusDashboard.new.tap(&:validate)) }
let(:data) do
<<~YAML
dashboard:
YAML
end
describe '#valid?' do
it 'returns false' do
expect(PerformanceMonitoring::PrometheusDashboard)
.to receive(:from_json).and_raise(error)
expect(viewer.valid?).to be_falsey
end
end
describe '#errors' do
it 'returns validation errors' do
allow(PerformanceMonitoring::PrometheusDashboard)
.to receive(:from_json).and_raise(error)
expect(viewer.errors).to be error.model.errors
end
end
end
context 'when YAML syntax is invalid' do
let(:data) do
<<~YAML
dashboard: 'empty metrics'
panel_groups:
- group: 'Group Title'
YAML
end
describe '#valid?' do
it 'returns false' do
expect(PerformanceMonitoring::PrometheusDashboard).not_to receive(:from_json)
expect(viewer.valid?).to be_falsey
end
end
describe '#errors' do
it 'returns validation errors' do
yaml_wrapped_errors = { 'YAML syntax': ["(<unknown>): did not find expected key while parsing a block mapping at line 1 column 1"] }
expect(viewer.errors).to be_kind_of ActiveModel::Errors
expect(viewer.errors.messages).to eql(yaml_wrapped_errors)
end
end
end
end
...@@ -38,24 +38,123 @@ describe PerformanceMonitoring::PrometheusDashboard do ...@@ -38,24 +38,123 @@ describe PerformanceMonitoring::PrometheusDashboard do
end end
describe 'validations' do describe 'validations' do
context 'when dashboard is missing' do shared_examples 'validation failed' do |errors_messages|
before do it 'raises error with corresponding messages', :aggregate_failures do
json_content['dashboard'] = nil expect { subject }.to raise_error do |error|
expect(error).to be_kind_of(ActiveModel::ValidationError)
expect(error.model.errors.messages).to eql(errors_messages)
end
end
end end
subject { described_class.from_json(json_content) } context 'dashboard definition is missing panels_groups and dashboard keys' do
let(:json_content) do
{
"dashboard" => nil
}
end
it { expect { subject }.to raise_error(ActiveModel::ValidationError) } it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"]
end
context 'group definition is missing panels and group keys' do
let(:json_content) do
{
"dashboard" => "Dashboard Title",
"templating" => {
"variables" => {
"variable1" => %w(value1 value2 value3)
}
},
"panel_groups" => [{ "group" => nil }]
}
end end
context 'when panel groups are missing' do it_behaves_like 'validation failed', panels: ["can't be blank"], group: ["can't be blank"]
before do
json_content['panel_groups'] = []
end end
subject { described_class.from_json(json_content) } context 'panel definition is missing metrics and title keys' do
let(:json_content) do
{
"dashboard" => "Dashboard Title",
"templating" => {
"variables" => {
"variable1" => %w(value1 value2 value3)
}
},
"panel_groups" => [{
"group" => "Group Title",
"panels" => [{
"type" => "area-chart",
"y_label" => "Y-Axis"
}]
}]
}
end
it_behaves_like 'validation failed', metrics: ["can't be blank"], title: ["can't be blank"]
end
context 'metrics definition is missing unit, query and query_range keys' do
let(:json_content) do
{
"dashboard" => "Dashboard Title",
"templating" => {
"variables" => {
"variable1" => %w(value1 value2 value3)
}
},
"panel_groups" => [{
"group" => "Group Title",
"panels" => [{
"type" => "area-chart",
"title" => "Chart Title",
"y_label" => "Y-Axis",
"metrics" => [{
"id" => "metric_of_ages",
"label" => "Metric of Ages",
"query_range" => nil
}]
}]
}]
}
end
it_behaves_like 'validation failed', unit: ["can't be blank"], query_range: ["can't be blank"], query: ["can't be blank"]
end
# for each parent entry validation first is done to its children,
# whole execution is stopped on first encountered error
# which is the one that is reported
context 'multiple offences on different levels' do
let(:json_content) do
{
"dashboard" => nil,
"panel_groups" => [{
"group" => nil,
"panels" => [{
"type" => "area-chart",
"title" => nil,
"y_label" => "Y-Axis",
"metrics" => [{
"id" => "metric_of_ages",
"label" => "Metric of Ages",
"query_range" => 'query'
}, {
"id" => "metric_of_ages",
"unit" => "count",
"label" => "Metric of Ages",
"query_range" => nil
}]
}]
}, {
"group" => 'group',
"panels" => nil
}]
}
end
it { expect { subject }.to raise_error(ActiveModel::ValidationError) } it_behaves_like 'validation failed', unit: ["can't be blank"]
end end
end end
end end
......
...@@ -32,7 +32,7 @@ describe PerformanceMonitoring::PrometheusPanelGroup do ...@@ -32,7 +32,7 @@ describe PerformanceMonitoring::PrometheusPanelGroup do
describe 'validations' do describe 'validations' do
context 'when group is missing' do context 'when group is missing' do
before do before do
json_content['group'] = nil json_content.delete('group')
end end
subject { described_class.from_json(json_content) } subject { described_class.from_json(json_content) }
......
...@@ -54,7 +54,7 @@ describe PerformanceMonitoring::PrometheusPanel do ...@@ -54,7 +54,7 @@ describe PerformanceMonitoring::PrometheusPanel do
context 'when metrics are missing' do context 'when metrics are missing' do
before do before do
json_content['metrics'] = [] json_content.delete('metrics')
end end
subject { described_class.from_json(json_content) } subject { described_class.from_json(json_content) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment