Commit 6438ff1a authored by syasonik's avatar syasonik

Normalize presentation of alert payload with complete info

Removes limits on the filtering of payload attributes
for presentation of alerts in UI and as part of incidents
descriptions.
parent 79a03f42
...@@ -32,8 +32,6 @@ module AlertManagement ...@@ -32,8 +32,6 @@ module AlertManagement
:acknowledged :acknowledged
].freeze ].freeze
DETAILS_IGNORED_PARAMS = %w(start_time).freeze
belongs_to :project belongs_to :project
belongs_to :issue, optional: true belongs_to :issue, optional: true
belongs_to :prometheus_alert, optional: true belongs_to :prometheus_alert, optional: true
...@@ -119,7 +117,7 @@ module AlertManagement ...@@ -119,7 +117,7 @@ module AlertManagement
end end
delegate :iid, to: :issue, prefix: true, allow_nil: true 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_iid, -> (iid) { where(iid: iid) }
scope :for_status, -> (status) { where(status: status) } scope :for_status, -> (status) { where(status: status) }
...@@ -172,12 +170,6 @@ module AlertManagement ...@@ -172,12 +170,6 @@ module AlertManagement
with_prometheus_alert.where(id: ids) with_prometheus_alert.where(id: ids)
end end
def details
details_payload = payload.except(*attributes.keys, *DETAILS_IGNORED_PARAMS)
Gitlab::Utils::InlineHash.merge_keys(details_payload)
end
def prometheus? def prometheus?
monitoring_tool == Gitlab::AlertManagement::AlertParams::MONITORING_TOOLS[:prometheus] monitoring_tool == Gitlab::AlertManagement::AlertParams::MONITORING_TOOLS[:prometheus]
end end
......
...@@ -42,6 +42,10 @@ module AlertManagement ...@@ -42,6 +42,10 @@ module AlertManagement
details_project_alert_management_url(project, alert.iid) details_project_alert_management_url(project, alert.iid)
end end
def details
Gitlab::Utils::InlineHash.merge_keys(payload)
end
private private
attr_reader :alert, :project attr_reader :alert, :project
...@@ -81,7 +85,7 @@ module AlertManagement ...@@ -81,7 +85,7 @@ module AlertManagement
end end
def details_list def details_list
alert.details details
.map { |label, value| list_item(label, value) } .map { |label, value| list_item(label, value) }
.join(MARKDOWN_LINE_BREAK) .join(MARKDOWN_LINE_BREAK)
end end
......
...@@ -12,10 +12,6 @@ module AlertManagement ...@@ -12,10 +12,6 @@ module AlertManagement
alerting_alert.alert_markdown alerting_alert.alert_markdown
end end
def details_list
alerting_alert.annotation_list
end
def metric_embed_for_alert def metric_embed_for_alert
alerting_alert.metric_embed_for_alert alerting_alert.metric_embed_for_alert
end end
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
module Projects module Projects
module Prometheus module Prometheus
class AlertPresenter < Gitlab::View::Presenter::Delegated 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 GENERIC_ALERT_SUMMARY_ANNOTATIONS = %w(monitoring_tool service hosts).freeze
MARKDOWN_LINE_BREAK = " \n".freeze MARKDOWN_LINE_BREAK = " \n".freeze
INCIDENT_LABEL_NAME = ::IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES[:title].freeze INCIDENT_LABEL_NAME = ::IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES[:title].freeze
...@@ -56,11 +55,10 @@ module Projects ...@@ -56,11 +55,10 @@ module Projects
MARKDOWN MARKDOWN
end end
def annotation_list def details_list
strong_memoize(:annotation_list) do strong_memoize(:details_list) do
annotations details
.reject { |annotation| annotation.label.in?(RESERVED_ANNOTATIONS | GENERIC_ALERT_SUMMARY_ANNOTATIONS) } .map { |label, value| list_item(label, value) }
.map { |annotation| list_item(annotation.label, annotation.value) }
.join(MARKDOWN_LINE_BREAK) .join(MARKDOWN_LINE_BREAK)
end end
end end
...@@ -109,13 +107,17 @@ module Projects ...@@ -109,13 +107,17 @@ module Projects
metadata.join(MARKDOWN_LINE_BREAK) metadata.join(MARKDOWN_LINE_BREAK)
end end
def details
Gitlab::Utils::InlineHash.merge_keys(payload)
end
def alert_details def alert_details
if annotation_list.present? if details.present?
<<~MARKDOWN.chomp <<~MARKDOWN.chomp
#### Alert Details #### Alert Details
#{annotation_list} #{details_list}
MARKDOWN MARKDOWN
end end
end end
......
---
title: Present complete alert payload in detail and incident views
merge_request: 42140
author:
type: changed
...@@ -332,33 +332,6 @@ RSpec.describe AlertManagement::Alert do ...@@ -332,33 +332,6 @@ RSpec.describe AlertManagement::Alert do
end end
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 describe '#to_reference' do
it { expect(triggered_alert.to_reference).to eq('') } it { expect(triggered_alert.to_reference).to eq('') }
end end
......
...@@ -9,7 +9,14 @@ RSpec.describe AlertManagement::AlertPresenter do ...@@ -9,7 +9,14 @@ RSpec.describe AlertManagement::AlertPresenter do
{ {
'title' => 'Alert title', 'title' => 'Alert title',
'start_time' => '2020-04-27T10:10:22.265949279Z', 'start_time' => '2020-04-27T10:10:22.265949279Z',
'custom' => { 'param' => 73 } 'custom' => {
'alert' => {
'fields' => %w[one two]
}
},
'yet' => {
'another' => 73
}
} }
end end
...@@ -37,7 +44,10 @@ RSpec.describe AlertManagement::AlertPresenter do ...@@ -37,7 +44,10 @@ RSpec.describe AlertManagement::AlertPresenter do
#### Alert Details #### 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 MARKDOWN
) )
end end
...@@ -54,4 +64,17 @@ RSpec.describe AlertManagement::AlertPresenter do ...@@ -54,4 +64,17 @@ RSpec.describe AlertManagement::AlertPresenter do
expect(presenter.details_url).to match(%r{#{project.web_url}/-/alert_management/#{alert.iid}/details}) expect(presenter.details_url).to match(%r{#{project.web_url}/-/alert_management/#{alert.iid}/details})
end end
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 end
...@@ -38,7 +38,11 @@ RSpec.describe AlertManagement::PrometheusAlertPresenter do ...@@ -38,7 +38,11 @@ RSpec.describe AlertManagement::PrometheusAlertPresenter do
#### Alert Details #### 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
--- ---
......
...@@ -60,21 +60,6 @@ RSpec.describe Projects::Prometheus::AlertPresenter do ...@@ -60,21 +60,6 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
subject { presenter.issue_summary_markdown } subject { presenter.issue_summary_markdown }
context 'without default payload' do 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 it do
is_expected.to eq( is_expected.to eq(
<<~MARKDOWN.chomp <<~MARKDOWN.chomp
...@@ -82,58 +67,50 @@ RSpec.describe Projects::Prometheus::AlertPresenter do ...@@ -82,58 +67,50 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
#### Alert Details #### Alert Details
**foo:** value1#{markdown_line_break} **startsAt:** #{presenter.starts_at_raw}
**bar:** value2
MARKDOWN MARKDOWN
) )
end end
end end
context 'with full query' do context 'with optional attributes' do
before do before do
payload['generatorURL'] = 'http://host?g0.expr=query' payload['annotations'] = {
end 'title' => 'Alert Title',
'foo' => 'value1',
it do 'bar' => 'value2',
is_expected.to eq( 'description' => 'Alert Description',
<<~MARKDOWN.chomp
**Start time:** #{presenter.start_time}#{markdown_line_break}
**full_query:** `query`
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', 'monitoring_tool' => 'monitoring_tool_name',
'service' => 'service_name', 'service' => 'service_name',
'hosts' => ['http://localhost:3000', 'http://localhost:3001'] 'hosts' => ['http://localhost:3000', 'http://localhost:3001']
} }
end payload['generatorURL'] = 'http://host?g0.expr=query'
before do
payload['annotations'] = generic_alert_params
end end
it do it do
is_expected.to eq( is_expected.to eq(
<<~MARKDOWN.chomp <<~MARKDOWN.chomp
**Start time:** #{presenter.start_time}#{markdown_line_break} **Start time:** #{presenter.start_time}#{markdown_line_break}
**full_query:** `query`#{markdown_line_break}
**Service:** service_name#{markdown_line_break} **Service:** service_name#{markdown_line_break}
**Monitoring tool:** monitoring_tool_name#{markdown_line_break} **Monitoring tool:** monitoring_tool_name#{markdown_line_break}
**Hosts:** http://localhost:3000 http://localhost:3001 **Hosts:** http://localhost:3000 http://localhost:3001
#### Alert Details #### Alert Details
**description:** The Generic Alert Description **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 MARKDOWN
) )
end end
end
context 'when hosts is a string' do context 'when hosts is a string' do
before do before do
...@@ -146,24 +123,20 @@ RSpec.describe Projects::Prometheus::AlertPresenter do ...@@ -146,24 +123,20 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
**Start time:** #{presenter.start_time}#{markdown_line_break} **Start time:** #{presenter.start_time}#{markdown_line_break}
**Hosts:** http://localhost:3000 **Hosts:** http://localhost:3000
#### Alert Details
**annotations.hosts:** http://localhost:3000#{markdown_line_break}
**startsAt:** #{presenter.starts_at_raw}
MARKDOWN MARKDOWN
) )
end end
end end
end
context 'with embedded metrics' do context 'with embedded metrics' do
let(:starts_at) { '2018-03-12T09:06:00Z' } let(:starts_at) { '2018-03-12T09:06:00Z' }
shared_examples_for 'markdown with metrics embed' do shared_examples_for 'markdown with metrics embed' do
let(:expected_markdown) do let(:embed_regex) { /\n\[\]\(#{Regexp.quote(presenter.metrics_dashboard_url)}\)\z/ }
<<~MARKDOWN.chomp
**Start time:** #{presenter.start_time}#{markdown_line_break}
**full_query:** `avg(metric) > 1.0`
[](#{presenter.metrics_dashboard_url})
MARKDOWN
end
context 'without a starting time available' do context 'without a starting time available' do
around do |example| around do |example|
...@@ -174,11 +147,11 @@ RSpec.describe Projects::Prometheus::AlertPresenter do ...@@ -174,11 +147,11 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
payload.delete('startsAt') payload.delete('startsAt')
end end
it { is_expected.to eq(expected_markdown) } it { is_expected.to match(embed_regex) }
end end
context 'with a starting time available' do context 'with a starting time available' do
it { is_expected.to eq(expected_markdown) } it { is_expected.to match(embed_regex) }
end end
end end
...@@ -208,12 +181,8 @@ RSpec.describe Projects::Prometheus::AlertPresenter do ...@@ -208,12 +181,8 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
end end
context 'when not enough information is present for an embed' do context 'when not enough information is present for an embed' do
let(:expected_markdown) do shared_examples_for 'does not include an embed' do
<<~MARKDOWN.chomp it { is_expected.not_to match(/\[\]\(.+\)/) }
**Start time:** #{presenter.start_time}#{markdown_line_break}
**full_query:** `avg(metric) > 1.0`
MARKDOWN
end end
context 'without title' do context 'without title' do
...@@ -221,7 +190,7 @@ RSpec.describe Projects::Prometheus::AlertPresenter do ...@@ -221,7 +190,7 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
payload['annotations'].delete('title') payload['annotations'].delete('title')
end end
it { is_expected.to eq(expected_markdown) } it_behaves_like 'does not include an embed'
end end
context 'without environment' do context 'without environment' do
...@@ -229,22 +198,15 @@ RSpec.describe Projects::Prometheus::AlertPresenter do ...@@ -229,22 +198,15 @@ RSpec.describe Projects::Prometheus::AlertPresenter do
payload['labels'].delete('gitlab_environment_name') payload['labels'].delete('gitlab_environment_name')
end end
it { is_expected.to eq(expected_markdown) } it_behaves_like 'does not include an embed'
end end
context 'without full_query' do context 'without full_query' do
let(:expected_markdown) do
<<~MARKDOWN.chomp
**Start time:** #{presenter.start_time}
MARKDOWN
end
before do before do
payload.delete('generatorURL') payload.delete('generatorURL')
end end
it { is_expected.to eq(expected_markdown) } it_behaves_like 'does not include an embed'
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment