Merge branch 'sy-unhide-alert-payload-attrs' into 'master'

Normalize presentation of alert payload with complete info

See merge request gitlab-org/gitlab!42140

app/models/alert_management/alert.rb

@@ -32,8 +32,6 @@ module AlertManagement
       :acknowledged
     ].freeze
 
-    DETAILS_IGNORED_PARAMS = %w(start_time).freeze
-
     belongs_to :project
     belongs_to :issue, optional: true
     belongs_to :prometheus_alert, optional: true
@@ -119,7 +117,7 @@ module AlertManagement
     end
 
     delegate :iid, to: :issue, prefix: true, allow_nil: true
-    delegate :metrics_dashboard_url, :details_url, to: :present
+    delegate :metrics_dashboard_url, :details_url, :details, to: :present
 
     scope :for_iid, -> (iid) { where(iid: iid) }
     scope :for_status, -> (status) { where(status: status) }
@@ -172,12 +170,6 @@ module AlertManagement
       with_prometheus_alert.where(id: ids)
     end
 
-    def details
-      details_payload = payload.except(*attributes.keys, *DETAILS_IGNORED_PARAMS)
-
-      Gitlab::Utils::InlineHash.merge_keys(details_payload)
-    end
-
     def prometheus?
       monitoring_tool == Gitlab::AlertManagement::AlertParams::MONITORING_TOOLS[:prometheus]
     end

app/presenters/alert_management/alert_presenter.rb

@@ -42,6 +42,10 @@ module AlertManagement
       details_project_alert_management_url(project, alert.iid)
     end
 
+    def details
+      Gitlab::Utils::InlineHash.merge_keys(payload)
+    end
+
     private
 
     attr_reader :alert, :project
@@ -81,7 +85,7 @@ module AlertManagement
     end
 
     def details_list
-      alert.details
+      details
         .map { |label, value| list_item(label, value) }
         .join(MARKDOWN_LINE_BREAK)
     end

app/presenters/alert_management/prometheus_alert_presenter.rb

@@ -12,10 +12,6 @@ module AlertManagement
       alerting_alert.alert_markdown
     end
 
-    def details_list
-      alerting_alert.annotation_list
-    end
-
     def metric_embed_for_alert
       alerting_alert.metric_embed_for_alert
     end

app/presenters/projects/prometheus/alert_presenter.rb

@@ -3,7 +3,6 @@
 module Projects
   module Prometheus
     class AlertPresenter < Gitlab::View::Presenter::Delegated
-      RESERVED_ANNOTATIONS = %w(gitlab_incident_markdown gitlab_y_label title).freeze
       GENERIC_ALERT_SUMMARY_ANNOTATIONS = %w(monitoring_tool service hosts).freeze
       MARKDOWN_LINE_BREAK = "  \n".freeze
       INCIDENT_LABEL_NAME = ::IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES[:title].freeze
@@ -56,11 +55,10 @@ module Projects
         MARKDOWN
       end
 
-      def annotation_list
-        strong_memoize(:annotation_list) do
-          annotations
-            .reject { |annotation| annotation.label.in?(RESERVED_ANNOTATIONS | GENERIC_ALERT_SUMMARY_ANNOTATIONS) }
-            .map { |annotation| list_item(annotation.label, annotation.value) }
+      def details_list
+        strong_memoize(:details_list) do
+          details
+            .map { |label, value| list_item(label, value) }
             .join(MARKDOWN_LINE_BREAK)
         end
       end
@@ -109,13 +107,17 @@ module Projects
         metadata.join(MARKDOWN_LINE_BREAK)
       end
 
+      def details
+        Gitlab::Utils::InlineHash.merge_keys(payload)
+      end
+
       def alert_details
-        if annotation_list.present?
+        if details.present?
           <<~MARKDOWN.chomp
 
             #### Alert Details
 
-            #{annotation_list}
+            #{details_list}
           MARKDOWN
         end
       end

changelogs/unreleased/sy-unhide-alert-payload-attrs.yml

+---
+title: Present complete alert payload in detail and incident views
+merge_request: 42140
+author:
+type: changed

spec/models/alert_management/alert_spec.rb

@@ -332,33 +332,6 @@ RSpec.describe AlertManagement::Alert do
     end
   end
 
-  describe '#details' do
-    let(:payload) do
-      {
-        'title' => 'Details title',
-        'custom' => {
-          'alert' => {
-            'fields' => %w[one two]
-          }
-        },
-        'yet' => {
-          'another' => 'field'
-        }
-      }
-    end
-
-    let(:alert) { build(:alert_management_alert, project: project, title: 'Details title', payload: payload) }
-
-    subject { alert.details }
-
-    it 'renders the payload as inline hash' do
-      is_expected.to eq(
-        'custom.alert.fields' => %w[one two],
-        'yet.another' => 'field'
-      )
-    end
-  end
-
   describe '#to_reference' do
     it { expect(triggered_alert.to_reference).to eq('') }
   end

spec/presenters/alert_management/alert_presenter_spec.rb

@@ -9,7 +9,14 @@ RSpec.describe AlertManagement::AlertPresenter do
     {
       'title' => 'Alert title',
       'start_time' => '2020-04-27T10:10:22.265949279Z',
-      'custom' => { 'param' => 73 }
+      'custom' => {
+        'alert' => {
+          'fields' => %w[one two]
+        }
+      },
+      'yet' => {
+        'another' => 73
+      }
     }
   end
 
@@ -37,7 +44,10 @@ RSpec.describe AlertManagement::AlertPresenter do
 
           #### Alert Details
 
-          **custom.param:** 73
+          **title:** Alert title#{markdown_line_break}
+          **start_time:** 2020-04-27T10:10:22.265949279Z#{markdown_line_break}
+          **custom.alert.fields:** ["one", "two"]#{markdown_line_break}
+          **yet.another:** 73
         MARKDOWN
       )
     end
@@ -54,4 +64,17 @@ RSpec.describe AlertManagement::AlertPresenter do
       expect(presenter.details_url).to match(%r{#{project.web_url}/-/alert_management/#{alert.iid}/details})
     end
   end
+
+  describe '#details' do
+    subject { presenter.details }
+
+    it 'renders the payload as inline hash' do
+      is_expected.to eq(
+        'title' => 'Alert title',
+        'start_time' => '2020-04-27T10:10:22.265949279Z',
+        'custom.alert.fields' => %w[one two],
+        'yet.another' => 73
+      )
+    end
+  end
 end

spec/presenters/alert_management/prometheus_alert_presenter_spec.rb

@@ -38,7 +38,11 @@ RSpec.describe AlertManagement::PrometheusAlertPresenter do
 
           #### Alert Details
 
-          **custom annotation:** custom annotation value
+          **annotations.custom annotation:** custom annotation value#{markdown_line_break}
+          **annotations.gitlab_incident_markdown:** **`markdown example`**#{markdown_line_break}
+          **annotations.title:** Alert title#{markdown_line_break}
+          **startsAt:** 2020-04-27T10:10:22.265949279Z#{markdown_line_break}
+          **generatorURL:** http://8d467bd4607a:9090/graph?g0.expr=vector%281%29&g0.tab=1
 
           ---
 

spec/presenters/projects/prometheus/alert_presenter_spec.rb

@@ -60,21 +60,6 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
     subject { presenter.issue_summary_markdown }
 
     context 'without default payload' do
-      it do
-        is_expected.to eq(
-          <<~MARKDOWN.chomp
-            **Start time:** #{presenter.start_time}
-
-          MARKDOWN
-        )
-      end
-    end
-
-    context 'with annotations' do
-      before do
-        payload['annotations'] = { 'title' => 'Alert Title', 'foo' => 'value1', 'bar' => 'value2' }
-      end
-
       it do
         is_expected.to eq(
           <<~MARKDOWN.chomp
@@ -82,15 +67,23 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
 
             #### Alert Details
 
-            **foo:** value1#{markdown_line_break}
-            **bar:** value2
+            **startsAt:** #{presenter.starts_at_raw}
           MARKDOWN
         )
       end
     end
 
-    context 'with full query' do
+    context 'with optional attributes' do
       before do
+        payload['annotations'] = {
+          'title' => 'Alert Title',
+          'foo' => 'value1',
+          'bar' => 'value2',
+          'description' => 'Alert Description',
+          'monitoring_tool' => 'monitoring_tool_name',
+          'service' => 'service_name',
+          'hosts' => ['http://localhost:3000', 'http://localhost:3001']
+        }
         payload['generatorURL'] = 'http://host?g0.expr=query'
       end
 
@@ -98,72 +91,52 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
         is_expected.to eq(
           <<~MARKDOWN.chomp
             **Start time:** #{presenter.start_time}#{markdown_line_break}
-            **full_query:** `query`
+            **full_query:** `query`#{markdown_line_break}
+            **Service:** service_name#{markdown_line_break}
+            **Monitoring tool:** monitoring_tool_name#{markdown_line_break}
+            **Hosts:** http://localhost:3000 http://localhost:3001
 
+            #### Alert Details
+
+            **annotations.hosts:** ["http://localhost:3000", "http://localhost:3001"]#{markdown_line_break}
+            **annotations.service:** service_name#{markdown_line_break}
+            **annotations.monitoring_tool:** monitoring_tool_name#{markdown_line_break}
+            **annotations.description:** Alert Description#{markdown_line_break}
+            **annotations.bar:** value2#{markdown_line_break}
+            **annotations.foo:** value1#{markdown_line_break}
+            **annotations.title:** Alert Title#{markdown_line_break}
+            **generatorURL:** http://host?g0.expr=query#{markdown_line_break}
+            **startsAt:** #{presenter.starts_at_raw}
           MARKDOWN
         )
       end
     end
 
-    context 'with the Generic Alert parameters' do
-      let(:generic_alert_params) do
-        {
-          'title' => 'The Generic Alert Title',
-          'description' => 'The Generic Alert Description',
-          'monitoring_tool' => 'monitoring_tool_name',
-          'service' => 'service_name',
-          'hosts' => ['http://localhost:3000', 'http://localhost:3001']
-        }
-      end
-
+    context 'when hosts is a string' do
       before do
-        payload['annotations'] = generic_alert_params
+        payload['annotations'] = { 'hosts' => 'http://localhost:3000' }
       end
 
       it do
         is_expected.to eq(
           <<~MARKDOWN.chomp
-            **Start time:** #{presenter.start_time}#{markdown_line_break}
-            **Service:** service_name#{markdown_line_break}
-            **Monitoring tool:** monitoring_tool_name#{markdown_line_break}
-            **Hosts:** http://localhost:3000 http://localhost:3001
+          **Start time:** #{presenter.start_time}#{markdown_line_break}
+          **Hosts:** http://localhost:3000
 
-            #### Alert Details
+          #### Alert Details
 
-            **description:** The Generic Alert Description
+          **annotations.hosts:** http://localhost:3000#{markdown_line_break}
+          **startsAt:** #{presenter.starts_at_raw}
           MARKDOWN
         )
       end
-
-      context 'when hosts is a string' do
-        before do
-          payload['annotations'] = { 'hosts' => 'http://localhost:3000' }
-        end
-
-        it do
-          is_expected.to eq(
-            <<~MARKDOWN.chomp
-            **Start time:** #{presenter.start_time}#{markdown_line_break}
-            **Hosts:** http://localhost:3000
-
-            MARKDOWN
-          )
-        end
-      end
     end
 
     context 'with embedded metrics' do
       let(:starts_at) { '2018-03-12T09:06:00Z' }
 
       shared_examples_for 'markdown with metrics embed' do
-        let(:expected_markdown) do
-          <<~MARKDOWN.chomp
-          **Start time:** #{presenter.start_time}#{markdown_line_break}
-          **full_query:** `avg(metric) > 1.0`
-
-          [](#{presenter.metrics_dashboard_url})
-          MARKDOWN
-        end
+        let(:embed_regex) { /\n\[\]\(#{Regexp.quote(presenter.metrics_dashboard_url)}\)\z/ }
 
         context 'without a starting time available' do
           around do |example|
@@ -174,11 +147,11 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
             payload.delete('startsAt')
           end
 
-          it { is_expected.to eq(expected_markdown) }
+          it { is_expected.to match(embed_regex) }
         end
 
         context 'with a starting time available' do
-          it { is_expected.to eq(expected_markdown) }
+          it { is_expected.to match(embed_regex) }
         end
       end
 
@@ -208,12 +181,8 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
         end
 
         context 'when not enough information is present for an embed' do
-          let(:expected_markdown) do
-            <<~MARKDOWN.chomp
-            **Start time:** #{presenter.start_time}#{markdown_line_break}
-            **full_query:** `avg(metric) > 1.0`
-
-            MARKDOWN
+          shared_examples_for 'does not include an embed' do
+            it { is_expected.not_to match(/\[\]\(.+\)/) }
           end
 
           context 'without title' do
@@ -221,7 +190,7 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
               payload['annotations'].delete('title')
             end
 
-            it { is_expected.to eq(expected_markdown) }
+            it_behaves_like 'does not include an embed'
           end
 
           context 'without environment' do
@@ -229,22 +198,15 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
               payload['labels'].delete('gitlab_environment_name')
             end
 
-            it { is_expected.to eq(expected_markdown) }
+            it_behaves_like 'does not include an embed'
           end
 
           context 'without full_query' do
-            let(:expected_markdown) do
-              <<~MARKDOWN.chomp
-              **Start time:** #{presenter.start_time}
-
-              MARKDOWN
-            end
-
             before do
               payload.delete('generatorURL')
             end
 
-            it { is_expected.to eq(expected_markdown) }
+            it_behaves_like 'does not include an embed'
           end
         end
       end