Commit ecdc50b1 authored by syasonik's avatar syasonik

Switch errors to inherit from a base class

Error classes associated with individual stages of
dashboard processing tend to have very long names.
As dashboard post-processing includes more steps,
we will likely need to handle more error cases.
Refactoring to have all errors inherit from a specific
base class will help accommodate this and keep the code
more readable.
parent cd94500a
...@@ -6,14 +6,13 @@ module Gitlab ...@@ -6,14 +6,13 @@ module Gitlab
module Metrics module Metrics
module Dashboard module Dashboard
class BaseService < ::BaseService class BaseService < ::BaseService
DASHBOARD_LAYOUT_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardLayoutError PROCESSING_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardProcessingError
MISSING_QUERY_ERROR = Gitlab::Metrics::Dashboard::Stages::EndpointInserter::MissingQueryError
def get_dashboard def get_dashboard
return error("#{dashboard_path} could not be found.", :not_found) unless path_available? return error("#{dashboard_path} could not be found.", :not_found) unless path_available?
success(dashboard: process_dashboard) success(dashboard: process_dashboard)
rescue DASHBOARD_LAYOUT_ERROR, MISSING_QUERY_ERROR => e rescue PROCESSING_ERROR => e
error(e.message, :unprocessable_entity) error(e.message, :unprocessable_entity)
end end
......
...@@ -5,7 +5,8 @@ module Gitlab ...@@ -5,7 +5,8 @@ module Gitlab
module Dashboard module Dashboard
module Stages module Stages
class BaseStage class BaseStage
DashboardLayoutError = Class.new(StandardError) DashboardProcessingError = Class.new(StandardError)
LayoutError = Class.new(DashboardProcessingError)
DEFAULT_PANEL_TYPE = 'area-chart' DEFAULT_PANEL_TYPE = 'area-chart'
...@@ -25,15 +26,15 @@ module Gitlab ...@@ -25,15 +26,15 @@ module Gitlab
protected protected
def missing_panel_groups! def missing_panel_groups!
raise DashboardLayoutError.new('Top-level key :panel_groups must be an array') raise LayoutError.new('Top-level key :panel_groups must be an array')
end end
def missing_panels! def missing_panels!
raise DashboardLayoutError.new('Each "panel_group" must define an array :panels') raise LayoutError.new('Each "panel_group" must define an array :panels')
end end
def missing_metrics! def missing_metrics!
raise DashboardLayoutError.new('Each "panel" must define an array :metrics') raise LayoutError.new('Each "panel" must define an array :metrics')
end end
def for_metrics def for_metrics
......
...@@ -5,7 +5,7 @@ module Gitlab ...@@ -5,7 +5,7 @@ module Gitlab
module Dashboard module Dashboard
module Stages module Stages
class EndpointInserter < BaseStage class EndpointInserter < BaseStage
MissingQueryError = Class.new(StandardError) MissingQueryError = Class.new(DashboardProcessingError)
def transform! def transform!
for_metrics do |metric| for_metrics do |metric|
...@@ -31,7 +31,7 @@ module Gitlab ...@@ -31,7 +31,7 @@ module Gitlab
def query_for_metric(metric) def query_for_metric(metric)
query = metric[query_type(metric)] query = metric[query_type(metric)]
raise MissingQueryError.new('Missing required metric key: one of :query or :query_range') unless query raise MissingQueryError.new('Each "metric" must define one of :query or :query_range') unless query
query query
end end
......
...@@ -28,12 +28,8 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi ...@@ -28,12 +28,8 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi
end end
context 'when the dashboard contains a metric without a query' do context 'when the dashboard contains a metric without a query' do
let(:project) do let(:dashboard) { { 'panel_groups' => [{ 'panels' => [{ 'metrics' => [{ 'id' => 'mock' }] }] }] } }
project_with_dashboard( let(:project) { project_with_dashboard(dashboard_path, dashboard.to_yaml) }
dashboard_path,
{ 'panel_groups' => [{ 'panels' => [{ 'metrics' => [{ 'id' => 'mock' }] }] }] }.to_yaml
)
end
it_behaves_like 'misconfigured dashboard service response', :unprocessable_entity it_behaves_like 'misconfigured dashboard service response', :unprocessable_entity
end end
......
...@@ -11,14 +11,11 @@ describe Gitlab::Metrics::Dashboard::Processor do ...@@ -11,14 +11,11 @@ describe Gitlab::Metrics::Dashboard::Processor do
let(:process_params) { [project, environment, dashboard_yml] } let(:process_params) { [project, environment, dashboard_yml] }
let(:dashboard) { described_class.new(*process_params).process(insert_project_metrics: true) } let(:dashboard) { described_class.new(*process_params).process(insert_project_metrics: true) }
# rubocop:disable RSpec/IteratedExpectation
# Cop disabled "all" matcher doesn't offer access to the element
it 'includes a path for the prometheus endpoint with each metric' do it 'includes a path for the prometheus endpoint with each metric' do
all_metrics.each do |metric| expect(all_metrics).to satisfy_all do |metric|
expect(metric).to include(prometheus_endpoint_path: prometheus_path(metric[:query_range])) metric[:prometheus_endpoint_path] == prometheus_path(metric[:query_range])
end end
end end
# rubocop:enable RSpec/IteratedExpectation
context 'when dashboard config corresponds to common metrics' do context 'when dashboard config corresponds to common metrics' do
let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') }
...@@ -70,7 +67,7 @@ describe Gitlab::Metrics::Dashboard::Processor do ...@@ -70,7 +67,7 @@ describe Gitlab::Metrics::Dashboard::Processor do
shared_examples_for 'errors with message' do |expected_message| shared_examples_for 'errors with message' do |expected_message|
it 'raises a DashboardLayoutError' do it 'raises a DashboardLayoutError' do
error_class = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardLayoutError error_class = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardProcessingError
expect { dashboard }.to raise_error(error_class, expected_message) expect { dashboard }.to raise_error(error_class, expected_message)
end end
...@@ -93,6 +90,12 @@ describe Gitlab::Metrics::Dashboard::Processor do ...@@ -93,6 +90,12 @@ describe Gitlab::Metrics::Dashboard::Processor do
it_behaves_like 'errors with message', 'Each "panel" must define an array :metrics' it_behaves_like 'errors with message', 'Each "panel" must define an array :metrics'
end end
context 'when the dashboard contains a metric which is missing a query' do
let(:dashboard_yml) { { panel_groups: [{ panels: [{ metrics: [{}] }] }] } }
it_behaves_like 'errors with message', 'Each "metric" must define one of :query or :query_range'
end
end end
private private
...@@ -114,9 +117,11 @@ describe Gitlab::Metrics::Dashboard::Processor do ...@@ -114,9 +117,11 @@ describe Gitlab::Metrics::Dashboard::Processor do
end end
def prometheus_path(query) def prometheus_path(query)
"/#{project.namespace.path}" \ Gitlab::Routing.url_helpers.prometheus_api_project_environment_path(
"/#{project.name}/environments/" \ project,
"#{environment.id}/prometheus/api/v1" \ environment,
"/query_range?query=#{CGI.escape query}" proxy_path: :query_range,
query: query
)
end end
end end
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