diff options
11 files changed, 222 insertions, 43 deletions
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/doc/development/contributing/issue_workflow.md b/doc/development/contributing/issue_workflow.md index 0396f7ebc45..3d36a7bf3b1 100644 --- a/doc/development/contributing/issue_workflow.md +++ b/doc/development/contributing/issue_workflow.md @@ -172,35 +172,35 @@ This label documents the planned timeline & urgency which is used to measure aga | ~P3 | Medium Priority | Within the next 3 releases (approx one quarter or 90 days) | | ~P4 | Low Priority | Anything outside the next 3 releases (more than one quarter or 120 days) | -If an issue seems to fall between two priority labels, assign it to the higher- -priority label. - ## Severity labels Severity labels help us clearly communicate the impact of a ~bug on users. - -| Label | Meaning | Impact on Functionality | Example | -|-------|-------------------|-------------------------------------------------------|---------| -| ~S1 | Blocker | Outage, broken feature with no workaround | Unable to create an issue. Data corruption/loss. Security breach. | -| ~S2 | Critical Severity | Broken Feature, workaround too complex & unacceptable | Can push commits, but only via the command line. | -| ~S3 | Major Severity | Broken Feature, workaround acceptable | Can create merge requests only from the Merge Requests page, not through the Issue. | -| ~S4 | Low Severity | Functionality inconvenience or cosmetic issue | Label colors are incorrect / not being displayed. | - -If an issue seems to fall between two severity labels, even taking the -[severity impact guidance](#severity-impact-guidance) into account, assign -it to the higher-severity label. - -### Severity impact guidance - -Severity levels can be applied further depending on the facet of the impact; e.g. Affected customers, GitLab.com availability, performance and etc. The below is a guideline. - -| Severity | Affected Customers/Users | GitLab.com Availability | Performance Degradation | -|----------|---------------------------------------------------------------------|----------------------------------------------------|------------------------------| -| ~S1 | >50% users affected (possible company extinction level event) | Significant impact on all of GitLab.com | | -| ~S2 | Many users or multiple paid customers affected (but not apocalyptic)| Significant impact on large portions of GitLab.com | Degradation is guaranteed to occur in the near future | -| ~S3 | A few users or a single paid customer affected | Limited impact on important portions of GitLab.com | Degradation is likely to occur in the near future | -| ~S4 | No paid users/customer affected, or expected to in the near future | Minor impact on GitLab.com | Degradation _may_ occur but it's not likely | - +There can be multiple facets of the impact. The below is a guideline. + +| Label | Meaning | Functionality | Affected Users | GitLab.com Availability | Performance Degradation | +|-------|-------------------|-------------------------------------------------------|----------------------------------|----------------------------------------------------|------------------------------| +| ~S1 | Blocker | Unusable feature with no workaround, user is blocked | Impacts 50% or more of users | Outage, Significant impact on all of GitLab.com | | +| ~S2 | Critical Severity | Broken Feature, workaround too complex & unacceptable | Impacts between 25%-50% of users | Significant impact on large portions of GitLab.com | Degradation is guaranteed to occur in the near future | +| ~S3 | Major Severity | Broken feature with an acceptable workaround | Impacts up to 25% of users | Limited impact on important portions of GitLab.com | Degradation is likely to occur in the near future | +| ~S4 | Low Severity | Functionality inconvenience or cosmetic issue | Impacts less than 5% of users | Minor impact on GitLab.com | Degradation _may_ occur but it's not likely | + +If a bug seems to fall between two severity labels, assign it to the higher-severity label. + +* Example(s) of ~S1 + * Data corruption/loss. + * Security breach. + * Unable to create an issue or merge request. + * Unable to add a comment or discussion to the issue or merge request. +* Example(s) of ~S2 + * Cannot submit changes through the web IDE but the commandline works. + * A status widget on the merge request page is not working but information can be seen in the test pipeline page. +* Example(s) of ~S3 + * Can create merge requests only from the Merge Requests list view, not from an Issue page. + * Status is not updated in real time and needs a page refresh. +* Example(s) of ~S4 + * Label colors are incorrect. + * UI elements are not fully aligned. + ## Label for community contributors Issues that are beneficial to our users, 'nice to haves', that we currently do 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<Hash>] ex) [{ path: String, default: Boolean }] + # @return [Array<Hash>] 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 |