From 6f7974dccd819fb6198f12b5d2d210e7d0f571d2 Mon Sep 17 00:00:00 2001 From: syasonik Date: Mon, 22 Jul 2019 17:02:54 +0300 Subject: Support dashboard params for metrics dashboard https://gitlab.com/gitlab-org/gitlab-ce/issues/62971 Adds support to EnvironmentsController#metrics_dashboard for the following params: group, title, y_label These params are used to uniquely identify a panel on the metrics dashboard. Metrics are stored in several places, so this adds utilities to find a specific panel from the database or filesystem depending on the metric specified. Also moves some shared utilities into separate classes, notably default values and errors. --- .../projects/environments_controller.rb | 7 +- app/models/prometheus_metric_enums.rb | 20 ++++-- .../metrics/dashboard/base_embed_service.rb | 35 +++++++++ app/services/metrics/dashboard/base_service.rb | 9 +-- .../dashboard/custom_metric_embed_service.rb | 80 +++++++++++++++++++++ .../metrics/dashboard/default_embed_service.rb | 16 ++--- .../metrics/dashboard/dynamic_embed_service.rb | 62 ++++++++++++++++ lib/gitlab/metrics/dashboard/defaults.rb | 14 ++++ lib/gitlab/metrics/dashboard/errors.rb | 34 +++++++++ lib/gitlab/metrics/dashboard/finder.rb | 45 ++++++------ lib/gitlab/metrics/dashboard/service_selector.rb | 76 ++++++++++++++++++++ lib/gitlab/metrics/dashboard/stages/base_stage.rb | 4 +- .../projects/environments_controller_spec.rb | 46 ++++++++++-- .../gitlab/metrics/dashboard/schemas/metrics.json | 3 +- spec/lib/gitlab/metrics/dashboard/defaults_spec.rb | 8 +++ spec/lib/gitlab/metrics/dashboard/finder_spec.rb | 76 +++++++++++++++++++- .../metrics/dashboard/service_selector_spec.rb | 67 +++++++++++++++++ .../dashboard/custom_metric_embed_service_spec.rb | 84 ++++++++++++++++++++++ .../dashboard/dynamic_embed_service_spec.rb | 74 +++++++++++++++++++ spec/support/helpers/metrics_dashboard_helpers.rb | 8 +++ 20 files changed, 712 insertions(+), 56 deletions(-) create mode 100644 app/services/metrics/dashboard/base_embed_service.rb create mode 100644 app/services/metrics/dashboard/custom_metric_embed_service.rb create mode 100644 app/services/metrics/dashboard/dynamic_embed_service.rb create mode 100644 lib/gitlab/metrics/dashboard/defaults.rb create mode 100644 lib/gitlab/metrics/dashboard/errors.rb create mode 100644 lib/gitlab/metrics/dashboard/service_selector.rb create mode 100644 spec/lib/gitlab/metrics/dashboard/defaults_spec.rb create mode 100644 spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb create mode 100644 spec/services/metrics/dashboard/custom_metric_embed_service_spec.rb create mode 100644 spec/services/metrics/dashboard/dynamic_embed_service_spec.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index ccd54b369fa..07eb689d031 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -165,7 +165,8 @@ class Projects::EnvironmentsController < Projects::ApplicationController project, current_user, environment, - embedded: params[:embedded] + dashboard_path: params[:dashboard], + **dashboard_params.to_h.symbolize_keys ) elsif Feature.enabled?(:environment_metrics_show_multiple_dashboards, project) result = dashboard_finder.find( @@ -233,6 +234,10 @@ class Projects::EnvironmentsController < Projects::ApplicationController params.require([:start, :end]) end + def dashboard_params + params.permit(:embedded, :group, :title, :y_label) + end + def dashboard_finder Gitlab::Metrics::Dashboard::Finder end diff --git a/app/models/prometheus_metric_enums.rb b/app/models/prometheus_metric_enums.rb index 6cb22cc69cd..d58f825f222 100644 --- a/app/models/prometheus_metric_enums.rb +++ b/app/models/prometheus_metric_enums.rb @@ -9,13 +9,17 @@ module PrometheusMetricEnums aws_elb: -3, nginx: -4, kubernetes: -5, - nginx_ingress: -6, + nginx_ingress: -6 + }.merge(custom_groups).freeze + end - # custom/user groups + # custom/user groups + def self.custom_groups + { business: 0, response: 1, system: 2 - } + }.freeze end def self.group_details @@ -50,16 +54,20 @@ module PrometheusMetricEnums group_title: _('System metrics (Kubernetes)'), required_metrics: %w(container_memory_usage_bytes container_cpu_usage_seconds_total), priority: 5 - }.freeze, + }.freeze + }.merge(custom_group_details).freeze + end - # custom/user groups + # custom/user groups + def self.custom_group_details + { business: { group_title: _('Business metrics (Custom)'), priority: 0 }.freeze, response: { group_title: _('Response metrics (Custom)'), - priority: -5 + priority: -5 }.freeze, system: { group_title: _('System metrics (Custom)'), diff --git a/app/services/metrics/dashboard/base_embed_service.rb b/app/services/metrics/dashboard/base_embed_service.rb new file mode 100644 index 00000000000..7a686b740e1 --- /dev/null +++ b/app/services/metrics/dashboard/base_embed_service.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +# Base class for embed services. Contains a few basic helper +# methods that the embed services share. +module Metrics + module Dashboard + class BaseEmbedService < ::Metrics::Dashboard::BaseService + def cache_key + "dynamic_metrics_dashboard_#{identifiers}" + end + + protected + + def dashboard_path + params[:dashboard_path] + end + + def group + params[:group] + end + + def title + params[:title] + end + + def y_label + params[:y_label] + end + + def identifiers + [dashboard_path, group, title, y_label].join('|') + end + end + end +end diff --git a/app/services/metrics/dashboard/base_service.rb b/app/services/metrics/dashboard/base_service.rb index b331bf51874..8a42675c66d 100644 --- a/app/services/metrics/dashboard/base_service.rb +++ b/app/services/metrics/dashboard/base_service.rb @@ -5,17 +5,14 @@ module Metrics module Dashboard class BaseService < ::BaseService - PROCESSING_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardProcessingError - NOT_FOUND_ERROR = Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError + include Gitlab::Metrics::Dashboard::Errors def get_dashboard return error('Insufficient permissions.', :unauthorized) unless allowed? success(dashboard: process_dashboard) - rescue NOT_FOUND_ERROR - error("#{dashboard_path} could not be found.", :not_found) - rescue PROCESSING_ERROR => e - error(e.message, :unprocessable_entity) + rescue StandardError => e + handle_errors(e) end # Summary of all known dashboards for the service. diff --git a/app/services/metrics/dashboard/custom_metric_embed_service.rb b/app/services/metrics/dashboard/custom_metric_embed_service.rb new file mode 100644 index 00000000000..094a8091409 --- /dev/null +++ b/app/services/metrics/dashboard/custom_metric_embed_service.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +# Responsible for returning a dashboard containing a specified +# custom metric. Creates panels based on the matching metric +# stored in the database. +# +# Use Gitlab::Metrics::Dashboard::Finder to retrive dashboards. +module Metrics + module Dashboard + class CustomMetricEmbedService < ::Metrics::Dashboard::BaseEmbedService + include Gitlab::Metrics::Dashboard::Defaults + + # Returns a new dashboard with only the matching + # metrics from the system dashboard, stripped of + # group info. + # + # Note: This overrides the method #raw_dashboard, + # which means the result will not be cached. This + # is because we are inserting DB info into the + # dashboard before post-processing. This ensures + # we aren't acting on deleted or out-of-date metrics. + # + # @return [Hash] + def raw_dashboard + panels_not_found!(identifiers) if panels.empty? + + { 'panel_groups' => [{ 'panels' => panels }] } + end + + private + + # Generated dashboard panels for each metric which + # matches the provided input. + # @return [Array] + def panels + @panels ||= metrics.map { |metric| panel_for_metric(metric) } + end + + # Metrics which match the provided inputs. + # @return [ActiveRecordRelation] + def metrics + project.prometheus_metrics.select do |metric| + metric.group == group_key && + metric.title == title && + metric.y_label == y_label + end + end + + # Returns a symbol representing the group that + # the dashboard's group title belongs to. + # It will be one of the keys found under + # PrometheusMetricEnums.custom_groups. + # + # @return [String] + def group_key + PrometheusMetricEnums + .group_details + .find { |_, details| details[:group_title] == group } + &.first + &.to_s + end + + # Returns a representation of a PromtheusMetric + # as a dashboard panel. As the panel is generated + # on the fly, we're using the default values for + # info we can't get from the DB. + # + # @return [Hash] + def panel_for_metric(metric) + { + type: DEFAULT_PANEL_TYPE, + weight: DEFAULT_PANEL_WEIGHT, + title: metric.title, + y_label: metric.y_label, + metrics: [metric.queries.first.merge(metric_id: metric.id)] + } + end + end + end +end diff --git a/app/services/metrics/dashboard/default_embed_service.rb b/app/services/metrics/dashboard/default_embed_service.rb index 8b01b44fc98..147037fa284 100644 --- a/app/services/metrics/dashboard/default_embed_service.rb +++ b/app/services/metrics/dashboard/default_embed_service.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true # Responsible for returning a filtered system dashboard -# containing only the default embedded metrics. In future, -# this class may be updated to support filtering to -# alternate metrics/panels. +# containing only the default embedded metrics. This class +# operates by selecting metrics directly from the system +# dashboard. # # Why isn't this filtering in a processing stage? By filtering # here, we ensure the dynamically-determined dashboard is cached. @@ -11,7 +11,7 @@ # Use Gitlab::Metrics::Dashboard::Finder to retrive dashboards. module Metrics module Dashboard - class DefaultEmbedService < ::Metrics::Dashboard::BaseService + class DefaultEmbedService < ::Metrics::Dashboard::BaseEmbedService # For the default filtering for embedded metrics, # uses the 'id' key in dashboard-yml definition for # identification. @@ -33,10 +33,6 @@ module Metrics { 'panel_groups' => [{ 'panels' => panels }] } end - def cache_key - "dynamic_metrics_dashboard_#{metric_identifiers.join('_')}" - end - private # Returns an array of the panels groups on the @@ -51,11 +47,11 @@ module Metrics # the panel is in the list of identifiers to collect. def matching_panel?(panel) panel['metrics'].any? do |metric| - metric_identifiers.include?(metric['id']) + identifiers.include?(metric['id']) end end - def metric_identifiers + def identifiers DEFAULT_EMBEDDED_METRICS_IDENTIFIERS end end diff --git a/app/services/metrics/dashboard/dynamic_embed_service.rb b/app/services/metrics/dashboard/dynamic_embed_service.rb new file mode 100644 index 00000000000..927efe37a80 --- /dev/null +++ b/app/services/metrics/dashboard/dynamic_embed_service.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +# Responsible for returning a filtered project dashboard +# containing only the request-provided metrics. The result +# is then cached for future requests. Metrics are identified +# based on a combination of identifiers for now, but the ideal +# would be similar to the approach in DefaultEmbedService, but +# a single unique identifier is not currently available across +# all metric types (custom, project-defined, cluster, or system). +# +# Use Gitlab::Metrics::Dashboard::Finder to retrive dashboards. +module Metrics + module Dashboard + class DynamicEmbedService < ::Metrics::Dashboard::BaseEmbedService + include Gitlab::Utils::StrongMemoize + + # Returns a new dashboard with only the matching + # metrics from the system dashboard, stripped of groups. + # @return [Hash] + def get_raw_dashboard + not_found! if panels.empty? + + { 'panel_groups' => [{ 'panels' => panels }] } + end + + private + + def panels + strong_memoize(:panels) do + not_found! unless base_dashboard + not_found! unless groups = base_dashboard['panel_groups'] + not_found! unless matching_group = find_group(groups) + not_found! unless all_panels = matching_group['panels'] + + find_panels(all_panels) + end + end + + def base_dashboard + strong_memoize(:base_dashboard) do + Gitlab::Metrics::Dashboard::Finder.find_raw(project, dashboard_path: dashboard_path) + end + end + + def find_group(groups) + groups.find do |candidate_group| + candidate_group['group'] == group + end + end + + def find_panels(all_panels) + all_panels.select do |panel| + panel['title'] == title && panel['y_label'] == y_label + end + end + + def not_found! + panels_not_found!(identifiers) + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/defaults.rb b/lib/gitlab/metrics/dashboard/defaults.rb new file mode 100644 index 00000000000..3c39a7c6911 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/defaults.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Central point for managing default attributes from within +# the metrics dashboard module. +module Gitlab + module Metrics + module Dashboard + module Defaults + DEFAULT_PANEL_TYPE = 'area-chart' + DEFAULT_PANEL_WEIGHT = 0 + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/errors.rb b/lib/gitlab/metrics/dashboard/errors.rb new file mode 100644 index 00000000000..1739a4e6738 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/errors.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Central point for managing errors from within the metrics +# dashboard module. Handles errors from dashboard retrieval +# and processing steps, as well as defines shared error classes. +module Gitlab + module Metrics + module Dashboard + module Errors + PanelNotFoundError = Class.new(StandardError) + + PROCESSING_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardProcessingError + NOT_FOUND_ERROR = Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError + + def handle_errors(error) + case error + when PROCESSING_ERROR + error(error.message, :unprocessable_entity) + when NOT_FOUND_ERROR + error("#{dashboard_path} could not be found.", :not_found) + when PanelNotFoundError + error(error.message, :not_found) + else + raise error + end + end + + def panels_not_found!(opts) + raise PanelNotFoundError.new("No panels matching properties #{opts}") + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/finder.rb b/lib/gitlab/metrics/dashboard/finder.rb index 1373830844b..66c4d662a6c 100644 --- a/lib/gitlab/metrics/dashboard/finder.rb +++ b/lib/gitlab/metrics/dashboard/finder.rb @@ -12,21 +12,37 @@ module Gitlab # @param project [Project] # @param user [User] # @param environment [Environment] - # @param opts - dashboard_path [String] Path at which the - # dashboard can be found. Nil values will - # default to the system dashboard. - # @param opts - embedded [Boolean] Determines whether the + # @param options - embedded [Boolean] Determines whether the # dashboard is to be rendered as part of an # issue or location other than the primary # metrics dashboard UI. Returns only the # Memory/CPU charts of the system dash. + # @param options - dashboard_path [String] Path at which the + # dashboard can be found. Nil values will + # default to the system dashboard. + # @param options - group [String] Title of the group + # to which a panel might belong. Used by + # embedded dashboards. + # @param options - title [String] Title of the panel. + # Used by embedded dashboards. + # @param options - y_label [String] Y-Axis label of + # a panel. Used by embedded dashboards. # @return [Hash] - def find(project, user, environment, dashboard_path: nil, embedded: false) - service_for_path(dashboard_path, embedded: embedded) - .new(project, user, environment: environment, dashboard_path: dashboard_path) + def find(project, user, environment, options = {}) + service_for(options) + .new(project, user, options.merge(environment: environment)) .get_dashboard end + # Returns a dashboard without any supplemental info. + # Returns only full, yml-defined dashboards. + # @return [Hash] + def find_raw(project, dashboard_path: nil) + service_for(dashboard_path: dashboard_path) + .new(project, nil, dashboard_path: dashboard_path) + .raw_dashboard + end + # Summary of all known dashboards. # @return [Array] ex) [{ path: String, # display_name: String, @@ -46,13 +62,6 @@ module Gitlab private - def service_for_path(dashboard_path, embedded:) - return embed_service if embedded - return system_service if system_dashboard?(dashboard_path) - - project_service - end - def system_service ::Metrics::Dashboard::SystemDashboardService end @@ -61,12 +70,8 @@ module Gitlab ::Metrics::Dashboard::ProjectDashboardService end - def embed_service - ::Metrics::Dashboard::DefaultEmbedService - end - - def system_dashboard?(filepath) - !filepath || system_service.system_dashboard?(filepath) + def service_for(options) + Gitlab::Metrics::Dashboard::ServiceSelector.call(options) end end end diff --git a/lib/gitlab/metrics/dashboard/service_selector.rb b/lib/gitlab/metrics/dashboard/service_selector.rb new file mode 100644 index 00000000000..7ebd8187906 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/service_selector.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +# Responsible for determining which dashboard service should +# be used to fetch or generate a dashboard hash. +# The services can be considered in two categories - embeds +# and dashboards. Embeds are all portions of dashboards. +module Gitlab + module Metrics + module Dashboard + class ServiceSelector + SERVICES = ::Metrics::Dashboard + + class << self + include Gitlab::Utils::StrongMemoize + + # Returns a class which inherits from the BaseService + # class that can be used to obtain a dashboard. + # @return [Gitlab::Metrics::Dashboard::Services::BaseService] + def call(params) + return SERVICES::CustomMetricEmbedService if custom_metrics_embed?(params) + return SERVICES::DynamicEmbedService if all_embed_params_present?(params) + return SERVICES::DefaultEmbedService if params[:embedded] + return SERVICES::SystemDashboardService if system_dashboard?(params[:dashboard_path]) + return SERVICES::ProjectDashboardService if params[:dashboard_path] + + default_service + end + + private + + def default_service + SERVICES::SystemDashboardService + end + + def system_dashboard?(filepath) + SERVICES::SystemDashboardService.system_dashboard?(filepath) + end + + # Custom metrics are added to the system dashboard + # in ProjectMetricsInserter, so we consider them + # to be a part of the system dashboard, even though + # they are exclustely stored in the database. + def custom_metrics_embed?(params) + all_embed_params_present?(params) && + system_dashboard?(params[:dashboard_path]) && + custom_metrics_group?(params[:group]) + end + + def custom_metrics_group?(group) + custom_metrics_group_titles.include?(group) + end + + # These are the attributes required to uniquely + # identify a panel on a dashboard for embedding. + def all_embed_params_present?(params) + [ + params[:embedded], + params[:dashboard_path], + params[:group], + params[:title], + params[:y_label] + ].all? + end + + def custom_metrics_group_titles + strong_memoize(:custom_metrics_group_titles) do + PrometheusMetricEnums + .custom_group_details + .map { |_, details| details[:group_title] } + end + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/stages/base_stage.rb b/lib/gitlab/metrics/dashboard/stages/base_stage.rb index 0db7b176e8d..514ed50e58d 100644 --- a/lib/gitlab/metrics/dashboard/stages/base_stage.rb +++ b/lib/gitlab/metrics/dashboard/stages/base_stage.rb @@ -5,11 +5,11 @@ module Gitlab module Dashboard module Stages class BaseStage + include Gitlab::Metrics::Dashboard::Defaults + DashboardProcessingError = Class.new(StandardError) LayoutError = Class.new(DashboardProcessingError) - DEFAULT_PANEL_TYPE = 'area-chart' - attr_reader :project, :environment, :dashboard def initialize(project, environment, dashboard) diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 8872e8d38e7..b3852355d77 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -518,10 +518,10 @@ describe Projects::EnvironmentsController do end end - shared_examples_for 'the default dynamic dashboard' do + shared_examples_for 'specified dashboard embed' do |expected_titles| it_behaves_like '200 response' - it 'contains only the Memory and CPU charts' do + it 'contains only the specified charts' do get :metrics_dashboard, params: environment_params(dashboard_params) dashboard = json_response['dashboard'] @@ -531,10 +531,14 @@ describe Projects::EnvironmentsController do expect(dashboard['dashboard']).to be_nil expect(dashboard['panel_groups'].length).to eq 1 expect(panel_group['group']).to be_nil - expect(titles).to eq ['Memory Usage (Total)', 'Core Usage (Total)'] + expect(titles).to eq expected_titles end end + shared_examples_for 'the default dynamic dashboard' do + it_behaves_like 'specified dashboard embed', ['Memory Usage (Total)', 'Core Usage (Total)'] + end + shared_examples_for 'dashboard can be specified' do context 'when dashboard is specified' do let(:dashboard_path) { '.gitlab/dashboards/test.yml' } @@ -551,7 +555,7 @@ describe Projects::EnvironmentsController do end context 'when the specified dashboard is the default dashboard' do - let(:dashboard_path) { ::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH } + let(:dashboard_path) { system_dashboard_path } it_behaves_like 'the default dashboard' end @@ -564,12 +568,40 @@ describe Projects::EnvironmentsController do it_behaves_like 'the default dynamic dashboard' - context 'when the dashboard is specified' do - let(:dashboard_params) { { format: :json, embedded: true, dashboard: '.gitlab/dashboards/fake.yml' } } + context 'when incomplete dashboard params are provided' do + let(:dashboard_params) { { format: :json, embedded: true, title: 'Title' } } + + # The title param should be ignored. + it_behaves_like 'the default dynamic dashboard' + end + + context 'when invalid params are provided' do + let(:dashboard_params) { { format: :json, embedded: true, metric_id: 16 } } - # The dashboard param should be ignored. + # The superfluous param should be ignored. it_behaves_like 'the default dynamic dashboard' end + + context 'when the dashboard is correctly specified' do + let(:dashboard_params) do + { + format: :json, + embedded: true, + dashboard: system_dashboard_path, + group: business_metric_title, + title: 'title', + y_label: 'y_label' + } + end + + it_behaves_like 'error response', :not_found + + context 'and exists' do + let!(:metric) { create(:prometheus_metric, project: project) } + + it_behaves_like 'specified dashboard embed', ['title'] + end + end 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 33393805464..9c1be32645a 100644 --- a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json @@ -16,7 +16,8 @@ "unit": { "type": "string" }, "label": { "type": "string" }, "track": { "type": "string" }, - "prometheus_endpoint_path": { "type": "string" } + "prometheus_endpoint_path": { "type": "string" }, + "metric_id": { "type": "number" } }, "additionalProperties": false } diff --git a/spec/lib/gitlab/metrics/dashboard/defaults_spec.rb b/spec/lib/gitlab/metrics/dashboard/defaults_spec.rb new file mode 100644 index 00000000000..420b246b3f5 --- /dev/null +++ b/spec/lib/gitlab/metrics/dashboard/defaults_spec.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::Dashboard::Defaults do + it { is_expected.to be_const_defined(:DEFAULT_PANEL_TYPE) } + it { is_expected.to be_const_defined(:DEFAULT_PANEL_WEIGHT) } +end diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index e57c7326320..ce1bb49f5c9 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -5,10 +5,9 @@ require 'spec_helper' describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_caching do include MetricsDashboardHelpers - set(:project) { build(:project) } + set(:project) { create(:project) } set(:user) { create(:user) } set(:environment) { create(:environment, project: project) } - let(:system_dashboard_path) { ::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH} before do project.add_maintainer(user) @@ -52,9 +51,80 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi end context 'when the dashboard is expected to be embedded' do - let(:service_call) { described_class.find(project, user, environment, dashboard_path: nil, embedded: true) } + let(:service_call) { described_class.find(project, user, environment, **params) } + let(:params) { { embedded: true } } it_behaves_like 'valid embedded dashboard service response' + + context 'when params are incomplete' do + let(:params) { { embedded: true, dashboard_path: system_dashboard_path } } + + it_behaves_like 'valid embedded dashboard service response' + end + + context 'when the panel is specified' do + context 'as a custom metric' do + let(:params) do + { embedded: true, + dashboard_path: system_dashboard_path, + group: business_metric_title, + title: 'title', + y_label: 'y_label' } + end + + it_behaves_like 'misconfigured dashboard service response', :not_found + + context 'when the metric exists' do + before do + create(:prometheus_metric, project: project) + end + + it_behaves_like 'valid embedded dashboard service response' + end + end + + context 'as a project-defined panel' do + let(:dashboard_path) { '.gitlab/dashboard/test.yml' } + let(:params) do + { embedded: true, + dashboard_path: dashboard_path, + group: 'Group A', + title: 'Super Chart A1', + y_label: 'y_label' } + end + + it_behaves_like 'misconfigured dashboard service response', :not_found + + context 'when the metric exists' do + let(:project) { project_with_dashboard(dashboard_path) } + + it_behaves_like 'valid embedded dashboard service response' + end + end + end + end + end + + describe '.find_raw' do + let(:dashboard) { YAML.load_file(Rails.root.join('config', 'prometheus', 'common_metrics.yml')) } + let(:params) { {} } + + subject { described_class.find_raw(project, **params) } + + it { is_expected.to eq dashboard } + + context 'when the system dashboard is specified' do + let(:params) { { dashboard_path: system_dashboard_path } } + + it { is_expected.to eq dashboard } + end + + context 'when an existing project dashboard is specified' do + let(:dashboard) { YAML.safe_load(fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml')) } + let(:params) { { dashboard_path: '.gitlab/dashboards/test.yml' } } + let(:project) { project_with_dashboard(params[:dashboard_path]) } + + it { is_expected.to eq dashboard } end end diff --git a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb new file mode 100644 index 00000000000..6c6079f6091 --- /dev/null +++ b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::Dashboard::ServiceSelector do + include MetricsDashboardHelpers + + describe '#call' do + let(:arguments) { {} } + + subject { described_class.call(arguments) } + + it { is_expected.to be Metrics::Dashboard::SystemDashboardService } + + context 'when just the dashboard path is provided' do + let(:arguments) { { dashboard_path: '.gitlab/dashboards/test.yml' } } + + it { is_expected.to be Metrics::Dashboard::ProjectDashboardService } + + context 'when the path is for the system dashboard' do + let(:arguments) { { dashboard_path: system_dashboard_path } } + + it { is_expected.to be Metrics::Dashboard::SystemDashboardService } + end + end + + context 'when the embedded flag is provided' do + let(:arguments) { { embedded: true } } + + it { is_expected.to be Metrics::Dashboard::DefaultEmbedService } + + context 'when an incomplete set of dashboard identifiers are provided' do + let(:arguments) { { embedded: true, dashboard_path: '.gitlab/dashboards/test.yml' } } + + it { is_expected.to be Metrics::Dashboard::DefaultEmbedService } + end + + context 'when all the dashboard identifiers are provided' do + let(:arguments) do + { + embedded: true, + dashboard_path: '.gitlab/dashboards/test.yml', + group: 'Important Metrics', + title: 'Total Requests', + y_label: 'req/sec' + } + end + + it { is_expected.to be Metrics::Dashboard::DynamicEmbedService } + end + + context 'with a system dashboard and "custom" group' do + let(:arguments) do + { + embedded: true, + dashboard_path: system_dashboard_path, + group: business_metric_title, + title: 'Total Requests', + y_label: 'req/sec' + } + end + + it { is_expected.to be Metrics::Dashboard::CustomMetricEmbedService } + end + end + end +end diff --git a/spec/services/metrics/dashboard/custom_metric_embed_service_spec.rb b/spec/services/metrics/dashboard/custom_metric_embed_service_spec.rb new file mode 100644 index 00000000000..a1ad5b48ca5 --- /dev/null +++ b/spec/services/metrics/dashboard/custom_metric_embed_service_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Metrics::Dashboard::CustomMetricEmbedService do + include MetricsDashboardHelpers + + set(:project) { build(:project) } + set(:user) { create(:user) } + set(:environment) { create(:environment, project: project) } + + before do + project.add_maintainer(user) + end + + describe '#get_dashboard' do + let(:dashboard_path) { system_dashboard_path } + let(:group) { business_metric_title } + let(:title) { 'title' } + let(:y_label) { 'y_label' } + + let(:service_params) do + [ + project, + user, + { + environment: environment, + dashboard_path: dashboard_path, + group: group, + title: title, + y_label: y_label + } + ] + end + + let(:service_call) { described_class.new(*service_params).get_dashboard } + + it_behaves_like 'misconfigured dashboard service response', :not_found + it_behaves_like 'raises error for users with insufficient permissions' + + context 'the custom metric exists' do + let!(:metric) { create(:prometheus_metric, project: project) } + + it_behaves_like 'valid embedded dashboard service response' + + it 'does not cache the unprocessed dashboard' do + expect(Gitlab::Metrics::Dashboard::Cache).not_to receive(:fetch) + + described_class.new(*service_params).get_dashboard + end + + context 'multiple metrics meet criteria' do + let!(:metric_2) { create(:prometheus_metric, project: project, query: 'avg(metric_2)') } + + it_behaves_like 'valid embedded dashboard service response' + + it 'includes both metrics' do + result = service_call + included_queries = all_queries(result[:dashboard]) + + expect(included_queries).to include('avg(metric_2)', 'avg(metric)') + end + end + end + + context 'when the metric exists in another project' do + let!(:metric) { create(:prometheus_metric, project: create(:project)) } + + it_behaves_like 'misconfigured dashboard service response', :not_found + end + end + + private + + def all_queries(dashboard) + dashboard[:panel_groups].flat_map do |group| + group[:panels].flat_map do |panel| + panel[:metrics].map do |metric| + metric[:query_range] + end + end + end + end +end diff --git a/spec/services/metrics/dashboard/dynamic_embed_service_spec.rb b/spec/services/metrics/dashboard/dynamic_embed_service_spec.rb new file mode 100644 index 00000000000..bf5e90f20f4 --- /dev/null +++ b/spec/services/metrics/dashboard/dynamic_embed_service_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Metrics::Dashboard::DynamicEmbedService, :use_clean_rails_memory_store_caching do + include MetricsDashboardHelpers + + set(:project) { build(:project) } + set(:user) { create(:user) } + set(:environment) { create(:environment, project: project) } + + before do + project.add_maintainer(user) + end + + describe '#get_dashboard' do + let(:dashboard_path) { '.gitlab/dashboards/test.yml' } + let(:group) { 'Group A' } + let(:title) { 'Super Chart A1' } + let(:y_label) { 'y_label' } + + let(:service_params) do + [ + project, + user, + { + environment: environment, + dashboard_path: dashboard_path, + group: group, + title: title, + y_label: y_label + } + ] + end + + let(:service_call) { described_class.new(*service_params).get_dashboard } + + context 'when the dashboard does not exist' do + it_behaves_like 'misconfigured dashboard service response', :not_found + end + + context 'when the dashboard is exists' do + let(:project) { project_with_dashboard(dashboard_path) } + + it_behaves_like 'valid embedded dashboard service response' + it_behaves_like 'raises error for users with insufficient permissions' + + it 'caches the unprocessed dashboard for subsequent calls' do + expect(YAML).to receive(:safe_load).once.and_call_original + + described_class.new(*service_params).get_dashboard + described_class.new(*service_params).get_dashboard + end + + context 'when the specified group is not present on the dashboard' do + let(:group) { 'Group Not Found' } + + it_behaves_like 'misconfigured dashboard service response', :not_found + end + + context 'when the specified title is not present on the dashboard' do + let(:title) { 'Title Not Found' } + + it_behaves_like 'misconfigured dashboard service response', :not_found + end + + context 'when the specified y-axis label is not present on the dashboard' do + let(:y_label) { 'Y-Axis Not Found' } + + it_behaves_like 'misconfigured dashboard service response', :not_found + end + end + end +end diff --git a/spec/support/helpers/metrics_dashboard_helpers.rb b/spec/support/helpers/metrics_dashboard_helpers.rb index 1511a2f6b49..0e86b6dfda7 100644 --- a/spec/support/helpers/metrics_dashboard_helpers.rb +++ b/spec/support/helpers/metrics_dashboard_helpers.rb @@ -18,6 +18,14 @@ module MetricsDashboardHelpers project.repository.refresh_method_caches([:metrics_dashboard]) end + def system_dashboard_path + Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH + end + + def business_metric_title + PrometheusMetricEnums.group_details[:business][:group_title] + end + shared_examples_for 'misconfigured dashboard service response' do |status_code| it 'returns an appropriate message and status code' do result = service_call -- cgit v1.2.1