diff options
author | syasonik <syasonik@gitlab.com> | 2019-06-06 17:49:40 +0100 |
---|---|---|
committer | syasonik <syasonik@gitlab.com> | 2019-06-06 23:05:19 +0100 |
commit | ecdc50b198796275b5f0862c5cdb0dca5aac1213 (patch) | |
tree | fa7313c2256ef693252e403fed7949c7138dd77b | |
parent | cd94500a42dcdf381b3c67daa74e6e95b83c90b5 (diff) | |
download | gitlab-ce-ecdc50b198796275b5f0862c5cdb0dca5aac1213.tar.gz |
Switch errors to inherit from a base classprom-api-1
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.
5 files changed, 26 insertions, 25 deletions
diff --git a/lib/gitlab/metrics/dashboard/base_service.rb b/lib/gitlab/metrics/dashboard/base_service.rb index ce6ecf933a1..4664aee71f6 100644 --- a/lib/gitlab/metrics/dashboard/base_service.rb +++ b/lib/gitlab/metrics/dashboard/base_service.rb @@ -6,14 +6,13 @@ module Gitlab module Metrics module Dashboard class BaseService < ::BaseService - DASHBOARD_LAYOUT_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardLayoutError - MISSING_QUERY_ERROR = Gitlab::Metrics::Dashboard::Stages::EndpointInserter::MissingQueryError + PROCESSING_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardProcessingError def get_dashboard return error("#{dashboard_path} could not be found.", :not_found) unless path_available? success(dashboard: process_dashboard) - rescue DASHBOARD_LAYOUT_ERROR, MISSING_QUERY_ERROR => e + rescue PROCESSING_ERROR => e error(e.message, :unprocessable_entity) end diff --git a/lib/gitlab/metrics/dashboard/stages/base_stage.rb b/lib/gitlab/metrics/dashboard/stages/base_stage.rb index a6d1f974556..0db7b176e8d 100644 --- a/lib/gitlab/metrics/dashboard/stages/base_stage.rb +++ b/lib/gitlab/metrics/dashboard/stages/base_stage.rb @@ -5,7 +5,8 @@ module Gitlab module Dashboard module Stages class BaseStage - DashboardLayoutError = Class.new(StandardError) + DashboardProcessingError = Class.new(StandardError) + LayoutError = Class.new(DashboardProcessingError) DEFAULT_PANEL_TYPE = 'area-chart' @@ -25,15 +26,15 @@ module Gitlab protected 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 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 def missing_metrics! - raise DashboardLayoutError.new('Each "panel" must define an array :metrics') + raise LayoutError.new('Each "panel" must define an array :metrics') end def for_metrics diff --git a/lib/gitlab/metrics/dashboard/stages/endpoint_inserter.rb b/lib/gitlab/metrics/dashboard/stages/endpoint_inserter.rb index 565fcde5c2c..2a959854be0 100644 --- a/lib/gitlab/metrics/dashboard/stages/endpoint_inserter.rb +++ b/lib/gitlab/metrics/dashboard/stages/endpoint_inserter.rb @@ -5,7 +5,7 @@ module Gitlab module Dashboard module Stages class EndpointInserter < BaseStage - MissingQueryError = Class.new(StandardError) + MissingQueryError = Class.new(DashboardProcessingError) def transform! for_metrics do |metric| @@ -31,7 +31,7 @@ module Gitlab def query_for_metric(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 end diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index 0bfe6cb69d9..bdcb5914575 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -28,12 +28,8 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi end context 'when the dashboard contains a metric without a query' do - let(:project) do - project_with_dashboard( - dashboard_path, - { 'panel_groups' => [{ 'panels' => [{ 'metrics' => [{ 'id' => 'mock' }] }] }] }.to_yaml - ) - end + let(:dashboard) { { 'panel_groups' => [{ 'panels' => [{ 'metrics' => [{ 'id' => 'mock' }] }] }] } } + let(:project) { project_with_dashboard(dashboard_path, dashboard.to_yaml) } it_behaves_like 'misconfigured dashboard service response', :unprocessable_entity end diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index 8d0f237a78e..797d4daabe3 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -11,14 +11,11 @@ describe Gitlab::Metrics::Dashboard::Processor do let(:process_params) { [project, environment, dashboard_yml] } 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 - all_metrics.each do |metric| - expect(metric).to include(prometheus_endpoint_path: prometheus_path(metric[:query_range])) + expect(all_metrics).to satisfy_all do |metric| + metric[:prometheus_endpoint_path] == prometheus_path(metric[:query_range]) end end - # rubocop:enable RSpec/IteratedExpectation context 'when dashboard config corresponds to common metrics' do let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } @@ -70,7 +67,7 @@ describe Gitlab::Metrics::Dashboard::Processor do shared_examples_for 'errors with message' do |expected_message| 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) end @@ -93,6 +90,12 @@ describe Gitlab::Metrics::Dashboard::Processor do it_behaves_like 'errors with message', 'Each "panel" must define an array :metrics' 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 private @@ -114,9 +117,11 @@ describe Gitlab::Metrics::Dashboard::Processor do end def prometheus_path(query) - "/#{project.namespace.path}" \ - "/#{project.name}/environments/" \ - "#{environment.id}/prometheus/api/v1" \ - "/query_range?query=#{CGI.escape query}" + Gitlab::Routing.url_helpers.prometheus_api_project_environment_path( + project, + environment, + proxy_path: :query_range, + query: query + ) end end |