Commit 72b3d5ff authored by Mikolaj Wawrzyniak's avatar Mikolaj Wawrzyniak

Add metrics dashboard validation to grapQL

In order to enable retrieval of information about metrics
dashboard YAML schema validation errors, we need to exted metrics
dashboard graphQL resource.
parent b839e74f
...@@ -10,6 +10,9 @@ module Types ...@@ -10,6 +10,9 @@ module Types
field :path, GraphQL::STRING_TYPE, null: true, field :path, GraphQL::STRING_TYPE, null: true,
description: 'Path to a file with the dashboard definition' description: 'Path to a file with the dashboard definition'
field :schema_validation_warnings, [GraphQL::STRING_TYPE], null: true,
description: 'Dashboard schema validation warnings'
field :annotations, Types::Metrics::Dashboards::AnnotationType.connection_type, null: true, field :annotations, Types::Metrics::Dashboards::AnnotationType.connection_type, null: true,
description: 'Annotations added to the dashboard', description: 'Annotations added to the dashboard',
resolver: Resolvers::Metrics::Dashboards::AnnotationResolver resolver: Resolvers::Metrics::Dashboards::AnnotationResolver
......
...@@ -20,15 +20,17 @@ module PerformanceMonitoring ...@@ -20,15 +20,17 @@ module PerformanceMonitoring
end end
def find_for(project:, user:, path:, options: {}) def find_for(project:, user:, path:, options: {})
dashboard_response = Gitlab::Metrics::Dashboard::Finder.find(project, user, options.merge(dashboard_path: path)) template = { path: path, environment: options[:environment] }
return unless dashboard_response[:status] == :success rsp = Gitlab::Metrics::Dashboard::Finder.find(project, user, options.merge(dashboard_path: path))
new( case rsp[:http_status] || rsp[:status]
{ when :success
path: path, new(template.merge(rsp[:dashboard] || {})) # when there is empty dashboard file returned rsp is still a success
environment: options[:environment] when :unprocessable_entity
}.merge(dashboard_response[:dashboard]) new(template) # validation error
) else
nil # any other error
end
end end
end end
...@@ -36,6 +38,15 @@ module PerformanceMonitoring ...@@ -36,6 +38,15 @@ module PerformanceMonitoring
self.as_json(only: yaml_valid_attributes).to_yaml self.as_json(only: yaml_valid_attributes).to_yaml
end end
# This method is planned to be refactored as a part of https://gitlab.com/gitlab-org/gitlab/-/issues/219398
# implementation. For new existing logic was reused to faster deliver MVC
def schema_validation_warnings
self.class.from_json(self.as_json)
nil
rescue ActiveModel::ValidationError => exception
exception.model.errors.map { |attr, error| "#{attr}: #{error}" }
end
private private
def yaml_valid_attributes def yaml_valid_attributes
......
---
title: Add dashboard schema validation warnings as metrics dashboard GraphQL field
merge_request: 33592
author:
type: added
...@@ -7040,6 +7040,11 @@ type MetricsDashboard { ...@@ -7040,6 +7040,11 @@ type MetricsDashboard {
Path to a file with the dashboard definition Path to a file with the dashboard definition
""" """
path: String path: String
"""
Dashboard schema validation warnings
"""
schemaValidationWarnings: [String!]
} }
type MetricsDashboardAnnotation { type MetricsDashboardAnnotation {
......
...@@ -19616,6 +19616,28 @@ ...@@ -19616,6 +19616,28 @@
}, },
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
},
{
"name": "schemaValidationWarnings",
"description": "Dashboard schema validation warnings",
"args": [
],
"type": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
}
},
"isDeprecated": false,
"deprecationReason": null
} }
], ],
"inputFields": null, "inputFields": null,
...@@ -1060,6 +1060,7 @@ Autogenerated return type of MergeRequestSetWip ...@@ -1060,6 +1060,7 @@ Autogenerated return type of MergeRequestSetWip
| Name | Type | Description | | Name | Type | Description |
| --- | ---- | ---------- | | --- | ---- | ---------- |
| `path` | String | Path to a file with the dashboard definition | | `path` | String | Path to a file with the dashboard definition |
| `schemaValidationWarnings` | String! => Array | Dashboard schema validation warnings |
## MetricsDashboardAnnotation ## MetricsDashboardAnnotation
......
...@@ -362,6 +362,8 @@ When **Metrics Dashboard YAML definition is invalid** at least one of the follow ...@@ -362,6 +362,8 @@ When **Metrics Dashboard YAML definition is invalid** at least one of the follow
1. `query_range: 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) 1. `unit: can't be blank` [learn more](#metrics-metrics-properties)
Metrics Dashboard YAML definition validation information is also available as a [GraphQL API field](../../../api/graphql/reference/index.md#metricsdashboard)
#### Dashboard YAML properties #### Dashboard YAML properties
Dashboards have several components: Dashboards have several components:
......
...@@ -7,7 +7,7 @@ describe GitlabSchema.types['MetricsDashboard'] do ...@@ -7,7 +7,7 @@ describe GitlabSchema.types['MetricsDashboard'] do
it 'has the expected fields' do it 'has the expected fields' do
expected_fields = %w[ expected_fields = %w[
path annotations path annotations schema_validation_warnings
] ]
expect(described_class).to have_graphql_fields(*expected_fields) expect(described_class).to have_graphql_fields(*expected_fields)
......
...@@ -42,7 +42,7 @@ describe PerformanceMonitoring::PrometheusDashboard do ...@@ -42,7 +42,7 @@ describe PerformanceMonitoring::PrometheusDashboard do
it 'raises error with corresponding messages', :aggregate_failures do it 'raises error with corresponding messages', :aggregate_failures do
expect { subject }.to raise_error do |error| expect { subject }.to raise_error do |error|
expect(error).to be_kind_of(ActiveModel::ValidationError) expect(error).to be_kind_of(ActiveModel::ValidationError)
expect(error.model.errors.messages).to eql(errors_messages) expect(error.model.errors.messages).to eq(errors_messages)
end end
end end
end end
...@@ -172,20 +172,51 @@ describe PerformanceMonitoring::PrometheusDashboard do ...@@ -172,20 +172,51 @@ describe PerformanceMonitoring::PrometheusDashboard do
dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment }) dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment })
expect(dashboard_instance).to be_instance_of described_class expect(dashboard_instance).to be_instance_of described_class
expect(dashboard_instance.environment).to be environment expect(dashboard_instance.environment).to eq environment
expect(dashboard_instance.path).to be path expect(dashboard_instance.path).to eq path
end end
end end
context 'dashboard has NOT been found' do context 'dashboard has NOT been found' do
it 'returns nil' do it 'returns nil' do
allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(status: :error) allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(http_status: :not_found)
dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment }) dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment })
expect(dashboard_instance).to be_nil expect(dashboard_instance).to be_nil
end end
end end
context 'dashboard has invalid schema', :aggregate_failures do
it 'still returns dashboard object' do
expect(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(http_status: :unprocessable_entity)
dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment })
expect(dashboard_instance).to be_instance_of described_class
expect(dashboard_instance.environment).to eq environment
expect(dashboard_instance.path).to eq path
end
end
end
describe '#schema_validation_warnings' do
context 'when schema is valid' do
it 'returns nil' do
expect(described_class).to receive(:from_json)
expect(described_class.new.schema_validation_warnings).to be_nil
end
end
context 'when schema is invalid' do
it 'returns array with errors messages' do
instance = described_class.new
instance.errors.add(:test, 'test error')
expect(described_class).to receive(:from_json).and_raise(ActiveModel::ValidationError.new(instance))
expect(described_class.new.schema_validation_warnings).to eq ['test: test error']
end
end
end end
describe '#to_yaml' do describe '#to_yaml' do
......
...@@ -9,25 +9,19 @@ describe 'Getting Metrics Dashboard' do ...@@ -9,25 +9,19 @@ describe 'Getting Metrics Dashboard' do
let(:project) { create(:project) } let(:project) { create(:project) }
let!(:environment) { create(:environment, project: project) } let!(:environment) { create(:environment, project: project) }
let(:fields) do
<<~QUERY
#{all_graphql_fields_for('MetricsDashboard'.classify)}
QUERY
end
let(:query) do let(:query) do
%( graphql_query_for(
query { 'project', { 'fullPath' => project.full_path },
project(fullPath:"#{project.full_path}") { query_graphql_field(
environments(name: "#{environment.name}") { :environments, { 'name' => environment.name },
nodes { query_graphql_field(
metricsDashboard(path: "#{path}"){ :nodes, nil,
#{fields} query_graphql_field(
} :metricsDashboard, { 'path' => path },
} all_graphql_fields_for('MetricsDashboard'.classify)
} )
} )
} )
) )
end end
...@@ -63,7 +57,29 @@ describe 'Getting Metrics Dashboard' do ...@@ -63,7 +57,29 @@ describe 'Getting Metrics Dashboard' do
it 'returns metrics dashboard' do it 'returns metrics dashboard' do
dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard']
expect(dashboard).to eql("path" => path) expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil)
end
context 'invalid dashboard' do
let(:path) { '.gitlab/dashboards/metrics.yml' }
let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndasboard: ''" }) }
it 'returns metrics dashboard' do
dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard')
expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: can't be blank"])
end
end
context 'empty dashboard' do
let(:path) { '.gitlab/dashboards/metrics.yml' }
let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) }
it 'returns metrics dashboard' do
dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard')
expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: can't be blank"])
end
end end
end end
...@@ -72,7 +88,7 @@ describe 'Getting Metrics Dashboard' do ...@@ -72,7 +88,7 @@ describe 'Getting Metrics Dashboard' do
it_behaves_like 'a working graphql query' it_behaves_like 'a working graphql query'
it 'return snil' do it 'returns nil' do
dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard'] dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard']
expect(dashboard).to be_nil expect(dashboard).to be_nil
......
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