From f49dd76a44ec04d35fbf3f08bd3c950d2df4de5b Mon Sep 17 00:00:00 2001 From: Sarah Yasonik Date: Thu, 20 Jun 2019 14:06:18 +0000 Subject: Add embedding flag and filter to CPU/Mem This commits adds support for metrics dashboards for embedding. If the flag 'embedded' is provided to the environments/id/metrics_dashboard endpoint, the response will be suitable for embedding in issues or other content. This is a precursor for support for embedding metrics in GFM. --- .../projects/environments_controller.rb | 14 ++++- lib/gitlab/metrics/dashboard/base_service.rb | 11 ++-- .../metrics/dashboard/dynamic_dashboard_service.rb | 65 ++++++++++++++++++++++ lib/gitlab/metrics/dashboard/finder.rb | 32 +++++++++-- .../projects/environments_controller_spec.rb | 13 +++++ .../dashboard/schemas/embedded_dashboard.json | 13 +++++ .../dashboard/schemas/embedded_panel_groups.json | 11 ++++ .../dashboard/dynamic_dashboard_service_spec.rb | 30 ++++++++++ spec/lib/gitlab/metrics/dashboard/finder_spec.rb | 8 ++- spec/support/helpers/metrics_dashboard_helpers.rb | 16 +++++- 10 files changed, 196 insertions(+), 17 deletions(-) create mode 100644 lib/gitlab/metrics/dashboard/dynamic_dashboard_service.rb create mode 100644 spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json create mode 100644 spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_panel_groups.json create mode 100644 spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index ae46a234aa6..ecf05e6ea64 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -162,9 +162,17 @@ class Projects::EnvironmentsController < Projects::ApplicationController return render_403 unless Feature.enabled?(:environment_metrics_use_prometheus_endpoint, project) if Feature.enabled?(:environment_metrics_show_multiple_dashboards, project) - result = dashboard_finder.find(project, current_user, environment, params[:dashboard]) - - result[:all_dashboards] = dashboard_finder.find_all_paths(project) + result = dashboard_finder.find( + project, + current_user, + environment, + dashboard_path: params[:dashboard], + embedded: params[:embedded] + ) + + unless params[:embedded] + result[:all_dashboards] = dashboard_finder.find_all_paths(project) + end else result = dashboard_finder.find(project, current_user, environment) end diff --git a/lib/gitlab/metrics/dashboard/base_service.rb b/lib/gitlab/metrics/dashboard/base_service.rb index 090014aa597..90895eb237a 100644 --- a/lib/gitlab/metrics/dashboard/base_service.rb +++ b/lib/gitlab/metrics/dashboard/base_service.rb @@ -23,6 +23,11 @@ module Gitlab raise NotImplementedError end + # Returns an un-processed dashboard from the cache. + def raw_dashboard + Gitlab::Metrics::Dashboard::Cache.fetch(cache_key) { get_raw_dashboard } + end + private # Returns a new dashboard Hash, supplemented with DB info @@ -37,11 +42,6 @@ module Gitlab params[:dashboard_path] end - # Returns an un-processed dashboard from the cache. - def raw_dashboard - Gitlab::Metrics::Dashboard::Cache.fetch(cache_key) { get_raw_dashboard } - end - # @return [Hash] an unmodified dashboard def get_raw_dashboard raise NotImplementedError @@ -54,6 +54,7 @@ module Gitlab # Determines whether custom metrics should be included # in the processed output. + # @return [Boolean] def insert_project_metrics? false end diff --git a/lib/gitlab/metrics/dashboard/dynamic_dashboard_service.rb b/lib/gitlab/metrics/dashboard/dynamic_dashboard_service.rb new file mode 100644 index 00000000000..81ed8922e17 --- /dev/null +++ b/lib/gitlab/metrics/dashboard/dynamic_dashboard_service.rb @@ -0,0 +1,65 @@ +# 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. +# +# Why isn't this filtering in a processing stage? By filtering +# here, we ensure the dynamically-determined dashboard is cached. +# +# Use Gitlab::Metrics::Dashboard::Finder to retrive dashboards. +module Gitlab + module Metrics + module Dashboard + class DynamicDashboardService < Gitlab::Metrics::Dashboard::BaseService + # For the default filtering for embedded metrics, + # uses the 'id' key in dashboard-yml definition for + # identification. + DEFAULT_EMBEDDED_METRICS_IDENTIFIERS = %w( + system_metrics_kubernetes_container_memory_total + system_metrics_kubernetes_container_cores_total + ).freeze + + # Returns a new dashboard with only the matching + # metrics from the system dashboard, stripped of groups. + # @return [Hash] + def raw_dashboard + panels = panel_groups.each_with_object([]) do |group, panels| + matched_panels = group['panels'].select { |panel| matching_panel?(panel) } + + panels.concat(matched_panels) + end + + { '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 + # system dashboard + def panel_groups + Gitlab::Metrics::Dashboard::SystemDashboardService + .new(project, nil) + .raw_dashboard['panel_groups'] + end + + # Identifies a panel as "matching" if any metric ids in + # the panel is in the list of identifiers to collect. + def matching_panel?(panel) + panel['metrics'].any? do |metric| + metric_identifiers.include?(metric['id']) + end + end + + def metric_identifiers + DEFAULT_EMBEDDED_METRICS_IDENTIFIERS + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/finder.rb b/lib/gitlab/metrics/dashboard/finder.rb index 49ea5c7d4f2..d7491d1553d 100644 --- a/lib/gitlab/metrics/dashboard/finder.rb +++ b/lib/gitlab/metrics/dashboard/finder.rb @@ -9,17 +9,28 @@ module Gitlab class Finder class << self # Returns a formatted dashboard packed with DB info. + # @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 + # 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. # @return [Hash] - def find(project, user, environment, dashboard_path = nil) - service = system_dashboard?(dashboard_path) ? system_service : project_service - - service + 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) .get_dashboard end # Summary of all known dashboards. - # @return [Array] ex) [{ path: String, default: Boolean }] + # @return [Array] ex) [{ path: String, + # display_name: String, + # default: Boolean }] def find_all_paths(project) project.repository.metrics_dashboard_paths end @@ -35,6 +46,13 @@ module Gitlab private + def service_for_path(dashboard_path, embedded:) + return dynamic_service if embedded + return system_service if system_dashboard?(dashboard_path) + + project_service + end + def system_service Gitlab::Metrics::Dashboard::SystemDashboardService end @@ -43,6 +61,10 @@ module Gitlab Gitlab::Metrics::Dashboard::ProjectDashboardService end + def dynamic_service + Gitlab::Metrics::Dashboard::DynamicDashboardService + end + def system_dashboard?(filepath) !filepath || system_service.system_dashboard?(filepath) end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 9699f2952f2..4c2c6160c62 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -556,6 +556,19 @@ describe Projects::EnvironmentsController do it_behaves_like 'has all dashboards' end end + + context 'when the dashboard is intended for embedding' do + let(:dashboard_params) { { format: :json, embedded: true } } + + it_behaves_like '200 response' + + context 'when a dashboard path is provided' do + let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/test.yml', embedded: true } } + + # The dashboard path should simple be ignored. + it_behaves_like '200 response' + end + end end end diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json new file mode 100644 index 00000000000..7d2b409a0f6 --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json @@ -0,0 +1,13 @@ +{ + "type": "object", + "required": ["panel_groups"], + "properties": { + "panel_groups": { + "type": "array", + "items": { + "$ref": "spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_panel_groups.json" + } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_panel_groups.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_panel_groups.json new file mode 100644 index 00000000000..bf05c054e2f --- /dev/null +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/embedded_panel_groups.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required": ["panels"], + "properties": { + "panels": { + "type": "array", + "items": { "$ref": "panels.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb new file mode 100644 index 00000000000..eecd257b38d --- /dev/null +++ b/spec/lib/gitlab/metrics/dashboard/dynamic_dashboard_service_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Metrics::Dashboard::DynamicDashboardService, :use_clean_rails_memory_store_caching do + include MetricsDashboardHelpers + + set(:project) { build(:project) } + set(:environment) { create(:environment, project: project) } + + describe '#get_dashboard' do + let(:service_params) { [project, nil, { environment: environment, dashboard_path: nil }] } + let(:service_call) { described_class.new(*service_params).get_dashboard } + + it_behaves_like 'valid embedded dashboard service response' + + 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 called with a non-system dashboard' do + let(:dashboard_path) { 'garbage/dashboard/path' } + + it_behaves_like 'valid embedded dashboard service response' + end + end +end diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index 29fe1ae8d9c..b9a5ee9c2b3 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -11,7 +11,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi describe '.find' do let(:dashboard_path) { '.gitlab/dashboards/test.yml' } - let(:service_call) { described_class.find(project, nil, environment, dashboard_path) } + let(:service_call) { described_class.find(project, nil, environment, dashboard_path: dashboard_path) } it_behaves_like 'misconfigured dashboard service response', :not_found @@ -45,6 +45,12 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi it_behaves_like 'valid dashboard service response' end + + context 'when the dashboard is expected to be embedded' do + let(:service_call) { described_class.find(project, nil, environment, dashboard_path: nil, embedded: true) } + + it_behaves_like 'valid embedded dashboard service response' + end end describe '.find_all_paths' do diff --git a/spec/support/helpers/metrics_dashboard_helpers.rb b/spec/support/helpers/metrics_dashboard_helpers.rb index 1f36b0e217c..6de00eea474 100644 --- a/spec/support/helpers/metrics_dashboard_helpers.rb +++ b/spec/support/helpers/metrics_dashboard_helpers.rb @@ -28,9 +28,7 @@ module MetricsDashboardHelpers end end - shared_examples_for 'valid dashboard service response' do - let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/dashboard.json')) } - + shared_examples_for 'valid dashboard service response for schema' do it 'returns a json representation of the dashboard' do result = service_call @@ -40,4 +38,16 @@ module MetricsDashboardHelpers expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty end end + + shared_examples_for 'valid dashboard service response' do + let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/dashboard.json')) } + + it_behaves_like 'valid dashboard service response for schema' + end + + shared_examples_for 'valid embedded dashboard service response' do + let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json')) } + + it_behaves_like 'valid dashboard service response for schema' + end end -- cgit v1.2.1