From 48f27e30c26cbf760bceaab497396ce20fa93b9d Mon Sep 17 00:00:00 2001
From: Rajendra Kadam <rkadam@gitlab.com>
Date: Tue, 4 Jul 2023 19:54:31 +0000
Subject: [PATCH] Remove/update prometheus payload for metric removals
Add/update specs for the removal changes
Update GQL docs for removed type
Changelog: removed
MR: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/125209
---
.../types/alert_management/alert_type.rb | 13 ---
.../alert_management/alert_presenter.rb | 9 +--
doc/api/graphql/reference/index.md | 1 -
lib/gitlab/alert_management/payload/base.rb | 1 -
.../payload/managed_prometheus.rb | 12 ---
.../alert_management/payload/prometheus.rb | 23 ------
.../types/alert_management/alert_type_spec.rb | 1 -
.../payload/managed_prometheus_spec.rb | 16 ----
.../payload/prometheus_spec.rb | 26 ------
.../alert_management/alert_presenter_spec.rb | 18 -----
.../alert/metrics_dashboard_url_spec.rb | 79 -------------------
.../project/alert_management/alerts_spec.rb | 1 -
12 files changed, 2 insertions(+), 198 deletions(-)
delete mode 100644 spec/requests/api/graphql/project/alert_management/alert/metrics_dashboard_url_spec.rb
diff --git a/app/graphql/types/alert_management/alert_type.rb b/app/graphql/types/alert_management/alert_type.rb
index 36dd930c3d9d0..c17406b3e5696 100644
--- a/app/graphql/types/alert_management/alert_type.rb
+++ b/app/graphql/types/alert_management/alert_type.rb
@@ -111,13 +111,6 @@ class AlertType < BaseObject
null: true,
description: 'Assignees of the alert.'
- field :metrics_dashboard_url,
- GraphQL::Types::String,
- null: true,
- description: 'URL for metrics embed for the alert.',
- deprecated: { reason: 'Returns no data. Underlying feature was removed in 16.0',
- milestone: '16.0' }
-
field :runbook,
GraphQL::Types::String,
null: true,
@@ -143,12 +136,6 @@ class AlertType < BaseObject
method: :details_url,
null: false,
description: 'URL of the alert.'
-
- def metrics_dashboard_url
- return if Feature.enabled?(:remove_monitor_metrics)
-
- object.metrics_dashboard_url
- end
end
end
end
diff --git a/app/presenters/alert_management/alert_presenter.rb b/app/presenters/alert_management/alert_presenter.rb
index 41831d50692a7..60fa351b4494d 100644
--- a/app/presenters/alert_management/alert_presenter.rb
+++ b/app/presenters/alert_management/alert_presenter.rb
@@ -10,7 +10,7 @@ class AlertPresenter < Gitlab::View::Presenter::Delegated
MARKDOWN_LINE_BREAK = " \n"
HORIZONTAL_LINE = "\n\n---\n\n"
- delegate :metrics_dashboard_url, :runbook, to: :parsed_payload
+ delegate :runbook, to: :parsed_payload
def initialize(alert, **attributes)
super
@@ -60,8 +60,7 @@ def email_title
def issue_summary_markdown
<<~MARKDOWN.chomp
- #{metadata_list}
- #{metric_embed_for_alert}
+ #{metadata_list}\n
MARKDOWN
end
@@ -80,10 +79,6 @@ def metadata_list
metadata.join(MARKDOWN_LINE_BREAK)
end
- def metric_embed_for_alert
- "\n[](#{metrics_dashboard_url})" if metrics_dashboard_url
- end
-
def list_item(key, value)
"**#{key}:** #{value}".strip
end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 5d897cfc923b3..8d5cd68915008 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -12109,7 +12109,6 @@ Describes an alert from the project's Alert Management.
| <a id="alertmanagementalertiid"></a>`iid` | [`ID!`](#id) | Internal ID of the alert. |
| <a id="alertmanagementalertissue"></a>`issue` | [`Issue`](#issue) | Issue attached to the alert. |
| <a id="alertmanagementalertissueiid"></a>`issueIid` **{warning-solid}** | [`ID`](#id) | **Deprecated** in 13.10. Use issue field. |
-| <a id="alertmanagementalertmetricsdashboardurl"></a>`metricsDashboardUrl` **{warning-solid}** | [`String`](#string) | **Deprecated** in 16.0. Returns no data. Underlying feature was removed in 16.0. |
| <a id="alertmanagementalertmonitoringtool"></a>`monitoringTool` | [`String`](#string) | Monitoring tool the alert came from. |
| <a id="alertmanagementalertnotes"></a>`notes` | [`NoteConnection!`](#noteconnection) | All notes on this noteable. (see [Connections](#connections)) |
| <a id="alertmanagementalertprometheusalert"></a>`prometheusAlert` | [`PrometheusAlert`](#prometheusalert) | Alert condition for Prometheus. |
diff --git a/lib/gitlab/alert_management/payload/base.rb b/lib/gitlab/alert_management/payload/base.rb
index 5b136431ce724..3840a560c57cd 100644
--- a/lib/gitlab/alert_management/payload/base.rb
+++ b/lib/gitlab/alert_management/payload/base.rb
@@ -33,7 +33,6 @@ class Base
:has_required_attributes?,
:hosts,
:metric_id,
- :metrics_dashboard_url,
:monitoring_tool,
:resolved?,
:runbook,
diff --git a/lib/gitlab/alert_management/payload/managed_prometheus.rb b/lib/gitlab/alert_management/payload/managed_prometheus.rb
index 2236e60a0c62f..4ed21108d3eaf 100644
--- a/lib/gitlab/alert_management/payload/managed_prometheus.rb
+++ b/lib/gitlab/alert_management/payload/managed_prometheus.rb
@@ -35,18 +35,6 @@ def environment
gitlab_alert&.environment || super
end
- def metrics_dashboard_url
- return unless gitlab_alert
-
- metrics_dashboard_project_prometheus_alert_url(
- project,
- gitlab_alert.prometheus_metric_id,
- environment_id: environment.id,
- embedded: true,
- **alert_embed_window_params
- )
- end
-
private
def plain_gitlab_fingerprint
diff --git a/lib/gitlab/alert_management/payload/prometheus.rb b/lib/gitlab/alert_management/payload/prometheus.rb
index cf6fe4493986f..15fa91646c802 100644
--- a/lib/gitlab/alert_management/payload/prometheus.rb
+++ b/lib/gitlab/alert_management/payload/prometheus.rb
@@ -96,29 +96,6 @@ def severity_mapping
def plain_gitlab_fingerprint
[starts_at_raw, title, full_query].join('/')
end
-
- # Formatted for parsing by JS
- def alert_embed_window_params
- {
- start: (starts_at - METRIC_TIME_WINDOW).utc.strftime('%FT%TZ'),
- end: (starts_at + METRIC_TIME_WINDOW).utc.strftime('%FT%TZ')
- }
- end
-
- def dashboard_json
- {
- panel_groups: [{
- panels: [{
- type: 'area-chart',
- title: title,
- y_label: gitlab_y_label,
- metrics: [{
- query_range: full_query
- }]
- }]
- }]
- }.to_json
- end
end
end
end
diff --git a/spec/graphql/types/alert_management/alert_type_spec.rb b/spec/graphql/types/alert_management/alert_type_spec.rb
index 4428fc0683a46..92e8104fc4d92 100644
--- a/spec/graphql/types/alert_management/alert_type_spec.rb
+++ b/spec/graphql/types/alert_management/alert_type_spec.rb
@@ -31,7 +31,6 @@
assignees
notes
discussions
- metrics_dashboard_url
runbook
todos
details_url
diff --git a/spec/lib/gitlab/alert_management/payload/managed_prometheus_spec.rb b/spec/lib/gitlab/alert_management/payload/managed_prometheus_spec.rb
index fa8afd47c53e2..d7184c899332c 100644
--- a/spec/lib/gitlab/alert_management/payload/managed_prometheus_spec.rb
+++ b/spec/lib/gitlab/alert_management/payload/managed_prometheus_spec.rb
@@ -150,20 +150,4 @@
it { is_expected.to eq(environment) }
end
end
-
- describe '#metrics_dashboard_url' do
- subject { parsed_payload.metrics_dashboard_url }
-
- context 'without alert' do
- it { is_expected.to be_nil }
- end
-
- context 'with gitlab alert' do
- include_context 'gitlab-managed prometheus alert attributes' do
- let(:raw_payload) { payload }
- end
-
- it { is_expected.to eq(dashboard_url_for_alert) }
- end
- end
end
diff --git a/spec/lib/gitlab/alert_management/payload/prometheus_spec.rb b/spec/lib/gitlab/alert_management/payload/prometheus_spec.rb
index cf525801aa040..92836915f7b80 100644
--- a/spec/lib/gitlab/alert_management/payload/prometheus_spec.rb
+++ b/spec/lib/gitlab/alert_management/payload/prometheus_spec.rb
@@ -178,32 +178,6 @@
end
end
- describe '#metrics_dashboard_url' do
- include_context 'self-managed prometheus alert attributes' do
- let(:raw_payload) { payload }
- end
-
- subject { parsed_payload.metrics_dashboard_url }
-
- context 'without environment' do
- let(:raw_payload) { payload.except('labels') }
-
- it { is_expected.to be_nil }
- end
-
- context 'without full query' do
- let(:raw_payload) { payload.except('generatorURL') }
-
- it { is_expected.to be_nil }
- end
-
- context 'without title' do
- let(:raw_payload) { payload.except('annotations') }
-
- it { is_expected.to be_nil }
- end
- end
-
describe '#has_required_attributes?' do
let(:starts_at) { Time.current.change(usec: 0).utc }
let(:raw_payload) { { 'annotations' => { 'title' => 'title' }, 'startsAt' => starts_at.rfc3339 } }
diff --git a/spec/presenters/alert_management/alert_presenter_spec.rb b/spec/presenters/alert_management/alert_presenter_spec.rb
index fe228f174fed9..eedb2e07fb69b 100644
--- a/spec/presenters/alert_management/alert_presenter_spec.rb
+++ b/spec/presenters/alert_management/alert_presenter_spec.rb
@@ -91,24 +91,6 @@
)
end
end
-
- context 'with metrics_dashboard_url' do
- before do
- allow(alert.parsed_payload).to receive(:metrics_dashboard_url).and_return('https://gitlab.com/metrics')
- end
-
- it do
- is_expected.to eq(
- <<~MARKDOWN.chomp
- **Start time:** #{presenter.start_time}#{markdown_line_break}
- **Severity:** #{presenter.severity}#{markdown_line_break}
- **GitLab alert:** #{alert_url}
-
- [](https://gitlab.com/metrics)
- MARKDOWN
- )
- end
- end
end
describe '#start_time' do
diff --git a/spec/requests/api/graphql/project/alert_management/alert/metrics_dashboard_url_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/metrics_dashboard_url_spec.rb
deleted file mode 100644
index 2c49c9bc47be1..0000000000000
--- a/spec/requests/api/graphql/project/alert_management/alert/metrics_dashboard_url_spec.rb
+++ /dev/null
@@ -1,79 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe 'getting Alert Management Alert Assignees', feature_category: :incident_management do
- include GraphqlHelpers
-
- let_it_be(:project) { create(:project) }
- let_it_be(:current_user) { create(:user) }
-
- let(:fields) do
- <<~QUERY
- nodes {
- iid
- metricsDashboardUrl
- }
- QUERY
- end
-
- let(:graphql_query) do
- graphql_query_for(
- 'project',
- { 'fullPath' => project.full_path },
- query_graphql_field('alertManagementAlerts', {}, fields)
- )
- end
-
- let(:alerts) { graphql_data.dig('project', 'alertManagementAlerts', 'nodes') }
- let(:first_alert) { alerts.first }
-
- before do
- stub_feature_flags(remove_monitor_metrics: false)
- project.add_developer(current_user)
- end
-
- context 'with self-managed prometheus payload' do
- include_context 'self-managed prometheus alert attributes'
-
- before do
- create(:alert_management_alert, :prometheus, project: project, payload: payload)
- end
-
- context 'when metrics dashboard feature is unavailable' do
- before do
- stub_feature_flags(remove_monitor_metrics: true)
- end
-
- it 'returns nil' do
- post_graphql(graphql_query, current_user: current_user)
- expect(first_alert['metricsDashboardUrl']).to be_nil
- end
- end
- end
-
- context 'with gitlab-managed prometheus payload' do
- include_context 'gitlab-managed prometheus alert attributes'
-
- before do
- create(:alert_management_alert, :prometheus, project: project, payload: payload, prometheus_alert: prometheus_alert)
- end
-
- it 'includes the correct metrics dashboard url' do
- post_graphql(graphql_query, current_user: current_user)
-
- expect(first_alert).to include('metricsDashboardUrl' => dashboard_url_for_alert)
- end
-
- context 'when metrics dashboard feature is unavailable' do
- before do
- stub_feature_flags(remove_monitor_metrics: true)
- end
-
- it 'returns nil' do
- post_graphql(graphql_query, current_user: current_user)
- expect(first_alert['metricsDashboardUrl']).to be_nil
- end
- end
- end
-end
diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb
index 55d223daf278f..7f586edd51006 100644
--- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb
+++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb
@@ -74,7 +74,6 @@
'details' => { 'custom.alert' => 'payload', 'runbook' => 'runbook' },
'createdAt' => triggered_alert.created_at.strftime('%Y-%m-%dT%H:%M:%SZ'),
'updatedAt' => triggered_alert.updated_at.strftime('%Y-%m-%dT%H:%M:%SZ'),
- 'metricsDashboardUrl' => nil,
'detailsUrl' => triggered_alert.details_url,
'prometheusAlert' => nil,
'runbook' => 'runbook'
--
GitLab