diff options
12 files changed, 94 insertions, 18 deletions
diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb index f8ef23cd83e..9c6c6513a78 100644 --- a/app/controllers/projects/environments/prometheus_api_controller.rb +++ b/app/controllers/projects/environments/prometheus_api_controller.rb @@ -13,7 +13,7 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon ).execute if result.nil? - return render status: :accepted, json: { + return render status: :no_content, json: { status: _('processing'), message: _('Not ready yet. Try again later.') } diff --git a/config/routes/project.rb b/config/routes/project.rb index d44ff62bc2a..a1e769f6ca3 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -330,7 +330,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :metrics_dashboard get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', constraints: { format: nil } - get '/prometheus/api/v1/*proxy_path', to: 'environments/prometheus_api#proxy' + get '/prometheus/api/v1/*proxy_path', to: 'environments/prometheus_api#proxy', as: :prometheus_api end collection do diff --git a/lib/gitlab/metrics/dashboard/base_service.rb b/lib/gitlab/metrics/dashboard/base_service.rb index 94aabd0466c..4664aee71f6 100644 --- a/lib/gitlab/metrics/dashboard/base_service.rb +++ b/lib/gitlab/metrics/dashboard/base_service.rb @@ -6,13 +6,13 @@ module Gitlab module Metrics module Dashboard class BaseService < ::BaseService - DASHBOARD_LAYOUT_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardLayoutError + 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 => e + rescue PROCESSING_ERROR => e error(e.message, :unprocessable_entity) end diff --git a/lib/gitlab/metrics/dashboard/processor.rb b/lib/gitlab/metrics/dashboard/processor.rb index dd986020693..a33a010ad97 100644 --- a/lib/gitlab/metrics/dashboard/processor.rb +++ b/lib/gitlab/metrics/dashboard/processor.rb @@ -11,11 +11,13 @@ module Gitlab SYSTEM_SEQUENCE = [ Stages::CommonMetricsInserter, Stages::ProjectMetricsInserter, + Stages::EndpointInserter, Stages::Sorter ].freeze PROJECT_SEQUENCE = [ Stages::CommonMetricsInserter, + Stages::EndpointInserter, Stages::Sorter ].freeze 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 new file mode 100644 index 00000000000..2a959854be0 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/stages/endpoint_inserter.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module Gitlab + module Metrics + module Dashboard + module Stages + class EndpointInserter < BaseStage + MissingQueryError = Class.new(DashboardProcessingError) + + def transform! + for_metrics do |metric| + metric[:prometheus_endpoint_path] = endpoint_for_metric(metric) + end + end + + private + + def endpoint_for_metric(metric) + Gitlab::Routing.url_helpers.prometheus_api_project_environment_path( + project, + environment, + proxy_path: query_type(metric), + query: query_for_metric(metric) + ) + end + + def query_type(metric) + metric[:query] ? :query : :query_range + end + + def query_for_metric(metric) + query = metric[query_type(metric)] + + raise MissingQueryError.new('Each "metric" must define one of :query or :query_range') unless query + + query + end + end + end + end + end +end diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index d232408b775..fdef9bc5638 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -85,12 +85,12 @@ describe Projects::Environments::PrometheusApiController do context 'with nil result' do let(:service_result) { nil } - it 'returns 202 accepted' do + it 'returns 204 no_content' do get :proxy, params: environment_params expect(json_response['status']).to eq('processing') expect(json_response['message']).to eq('Not ready yet. Try again later.') - expect(response).to have_gitlab_http_status(:accepted) + expect(response).to have_gitlab_http_status(:no_content) end end diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json index 2d0af57ec2c..33393805464 100644 --- a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json @@ -2,7 +2,8 @@ "type": "object", "required": [ "unit", - "label" + "label", + "prometheus_endpoint_path" ], "oneOf": [ { "required": ["query"] }, @@ -14,7 +15,8 @@ "query": { "type": "string" }, "unit": { "type": "string" }, "label": { "type": "string" }, - "track": { "type": "string" } + "track": { "type": "string" }, + "prometheus_endpoint_path": { "type": "string" } }, "additionalProperties": false } diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index e88eb140b35..bdcb5914575 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi include MetricsDashboardHelpers set(:project) { build(:project) } - set(:environment) { build(:environment, project: project) } + set(:environment) { create(:environment, project: project) } let(:system_dashboard_path) { Gitlab::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH} describe '.find' do @@ -27,6 +27,13 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi it_behaves_like 'misconfigured dashboard service response', :unprocessable_entity end + context 'when the dashboard contains a metric without a query' do + 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 + context 'when the system dashboard is specified' do let(:dashboard_path) { system_dashboard_path } diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index be3c1095bd7..797d4daabe3 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -4,13 +4,19 @@ require 'spec_helper' describe Gitlab::Metrics::Dashboard::Processor do let(:project) { build(:project) } - let(:environment) { build(:environment, project: project) } + let(:environment) { create(:environment, project: project) } let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml') } describe 'process' do let(:process_params) { [project, environment, dashboard_yml] } let(:dashboard) { described_class.new(*process_params).process(insert_project_metrics: true) } + it 'includes a path for the prometheus endpoint with each metric' do + expect(all_metrics).to satisfy_all do |metric| + metric[:prometheus_endpoint_path] == prometheus_path(metric[:query_range]) + end + end + context 'when dashboard config corresponds to common metrics' do let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } @@ -61,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 @@ -84,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 @@ -99,7 +111,17 @@ describe Gitlab::Metrics::Dashboard::Processor do query_range: metric.query, unit: metric.unit, label: metric.legend, - metric_id: metric.id + metric_id: metric.id, + prometheus_endpoint_path: prometheus_path(metric.query) } end + + def prometheus_path(query) + Gitlab::Routing.url_helpers.prometheus_api_project_environment_path( + project, + environment, + proxy_path: :query_range, + query: query + ) + end end diff --git a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb index 162beb0268a..794ac5f109b 100644 --- a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_m set(:user) { build(:user) } set(:project) { build(:project) } - set(:environment) { build(:environment, project: project) } + set(:environment) { create(:environment, project: project) } before do project.add_maintainer(user) diff --git a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb index e71ce2481a3..2648fe990de 100644 --- a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Metrics::Dashboard::SystemDashboardService, :use_clean_rails_me include MetricsDashboardHelpers set(:project) { build(:project) } - set(:environment) { build(:environment, project: project) } + set(:environment) { create(:environment, project: project) } describe 'get_dashboard' do let(:dashboard_path) { described_class::SYSTEM_DASHBOARD_PATH } |