diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index f8fa905e3f7469526d8f272d6d7abc69fea4e1ed..f92c7206c23a9f45e76247f4d12fb1896519f7b0 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -121,7 +121,7 @@ def new build_params = issue_params.merge( merge_request_to_resolve_discussions_of: params[:merge_request_to_resolve_discussions_of], discussion_to_resolve: params[:discussion_to_resolve], - observability_links: params[:observability_metric_details], + observability_links: { metrics: params[:observability_metric_details], logs: params[:observability_log_details] }, confidential: !!Gitlab::Utils.to_boolean(issue_params[:confidential]) ) service = ::Issues::BuildService.new(container: project, current_user: current_user, params: build_params) diff --git a/ee/app/helpers/observability/logs_issues_helper.rb b/ee/app/helpers/observability/logs_issues_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..c8c6e1a12e49904aedf57d6e934e34b0b504a18e --- /dev/null +++ b/ee/app/helpers/observability/logs_issues_helper.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Observability + module LogsIssuesHelper + def observability_logs_issues_params(params) + return {} if params.blank? + + { + title: "Issue created from log of '#{params['service']}' service at #{params['timestamp']}", + description: observability_logs_issue_description(params) + } + end + + private + + def observability_logs_issue_description(params) + <<~TEXT + [Log details](#{params['fullUrl']}) \\ + Service: `#{params['service']}` \\ + Trace ID: `#{params['traceId']}` \\ + Severity Number: `#{params['severityNumber']}` \\ + Timestamp: `#{params['timestamp']}` \\ + Message: + ``` + #{params['body']} + ``` + TEXT + end + end +end diff --git a/ee/app/helpers/observability/metrics_issues_helper.rb b/ee/app/helpers/observability/metrics_issues_helper.rb index 00e4bc51ac3bfb77aa15d25ab4addde7b0e6fd83..82ba0217289a82df392e5cf70566abf3af2e73f4 100644 --- a/ee/app/helpers/observability/metrics_issues_helper.rb +++ b/ee/app/helpers/observability/metrics_issues_helper.rb @@ -2,30 +2,24 @@ module Observability module MetricsIssuesHelper - def observability_metric_issue_description(params) - <<-TEXT -[Metric details](#{params['fullUrl']}) \\ -Name: `#{params['name']}` \\ -Type: `#{params['type']}` \\ -Timeframe: `#{params.dig('timeframe', 0)} - #{params.dig('timeframe', 1)}` - TEXT - end - - def observability_issue_params - return {} unless can?(current_user, :read_observability, container) + def observability_metrics_issues_params(params) + return {} if params.blank? - begin - links_params = ::Gitlab::Json.parse(CGI.unescape(params[:observability_links])) + { + title: "Issue created from #{params['name']}", + description: observability_metrics_issue_description(params) + } + end - return {} if links_params.blank? + private - { - title: "Issue created from #{links_params['name']}", - description: observability_metric_issue_description(links_params) - } - rescue JSON::ParserError, TypeError - {} - end + def observability_metrics_issue_description(params) + <<~TEXT + [Metric details](#{params['fullUrl']}) \\ + Name: `#{params['name']}` \\ + Type: `#{params['type']}` \\ + Timeframe: `#{params.dig('timeframe', 0)} - #{params.dig('timeframe', 1)}` + TEXT end end end diff --git a/ee/app/helpers/observability/observability_issues_helper.rb b/ee/app/helpers/observability/observability_issues_helper.rb new file mode 100644 index 0000000000000000000000000000000000000000..6ec31ff750fc2b3c9a7f7c759f178de6ef4a5c07 --- /dev/null +++ b/ee/app/helpers/observability/observability_issues_helper.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Observability + module ObservabilityIssuesHelper + include ::Observability::MetricsIssuesHelper + include ::Observability::LogsIssuesHelper + + def observability_issue_params + return {} unless can?(current_user, :read_observability, container) + + links_params = parsed_link_params(params[:observability_links]) + + if links_params[:metrics].present? + observability_metrics_issues_params(links_params[:metrics]) + elsif links_params[:logs].present? + observability_logs_issues_params(links_params[:logs]) + else + {} + end + end + + private + + def parsed_link_params(links_params) + return {} unless links_params.present? + + { + metrics: safe_parse_json(links_params[:metrics]), + logs: safe_parse_json(links_params[:logs]) + } + end + + def safe_parse_json(stringified_json) + return {} if stringified_json.blank? + + ::Gitlab::Json.parse(CGI.unescape(stringified_json)) + rescue JSON::ParserError, TypeError + {} + end + end +end diff --git a/ee/app/services/ee/issues/build_service.rb b/ee/app/services/ee/issues/build_service.rb index 3ffa17b2328301c97d7f178a3d384447779b1fcb..ddef049519212de4c0101fff69802f1a333864f4 100644 --- a/ee/app/services/ee/issues/build_service.rb +++ b/ee/app/services/ee/issues/build_service.rb @@ -4,7 +4,7 @@ module EE module Issues module BuildService extend ::Gitlab::Utils::Override - include ::Observability::MetricsIssuesHelper + include ::Observability::ObservabilityIssuesHelper def issue_params_from_template return {} unless container.feature_available?(:issuable_default_templates) diff --git a/ee/spec/controllers/projects/issues_controller_spec.rb b/ee/spec/controllers/projects/issues_controller_spec.rb index d374481797341ded6f4a7b1ad6ffbdc4e50d1fd5..bd93e85118a66bd638757dffdbbc9ae9a4fef6f4 100644 --- a/ee/spec/controllers/projects/issues_controller_spec.rb +++ b/ee/spec/controllers/projects/issues_controller_spec.rb @@ -312,6 +312,160 @@ def send_request end end + describe 'GET #new' do + before do + project.add_developer(user) + sign_in(user) + end + + context 'when passing observability metrics' do + let(:metric_params) { '%7B%22fullUrl%22%3A%22http%3A%2F%2Fgdk.test%3A3443%2Fflightjs%2FFlight%2F-%2Fmetrics%2Fapp.ads.ad_requests%3Ftype%3DSum%26date_range%3Dcustom%26date_start%3D2024-08-14T16%253A02%253A49.400Z%26date_end%3D2024-08-14T17%253A02%253A49.400Z%22%2C%22name%22%3A%22app.ads.ad_requests%22%2C%22type%22%3A%22Sum%22%2C%22timeframe%22%3A%5B%22Wed%2C%2014%20Aug%202024%2016%3A02%3A49%20GMT%22%2C%22Wed%2C%2014%20Aug%202024%2017%3A02%3A49%20GMT%22%5D%7D' } + + subject do + get :new, params: { + namespace_id: project.namespace, + project_id: project, + observability_metric_details: metric_params + } + end + + context 'when read_observability is prevented' do + before do + stub_feature_flags(observability_features: false) + stub_licensed_features(observability: false) + end + + context 'when observability_metric_details parameters exist' do + it 'does not populate observability_values' do + subject + + expect(assigns(:observability_values)).to be_nil + end + end + + context 'when observability_metric_details parameters do not exist' do + let(:metric_params) { {} } + + it 'does not populate observability_values' do + subject + + expect(assigns(:observability_values)).to be_nil + end + end + end + + context 'when read_observability is allowed' do + before do + stub_licensed_features(observability: true) + end + + context 'when observability_metric_details parameters exist' do + it 'does prefill the issue title and description' do + subject + + expect(assigns(:issue).title).to eq('Issue created from app.ads.ad_requests') + expect(assigns(:issue).description).to eq( + <<~TEXT + [Metric details](http://gdk.test:3443/flightjs/Flight/-/metrics/app.ads.ad_requests?type=Sum&date_range=custom&date_start=2024-08-14T16%3A02%3A49.400Z&date_end=2024-08-14T17%3A02%3A49.400Z) \\ + Name: `app.ads.ad_requests` \\ + Type: `Sum` \\ + Timeframe: `Wed, 14 Aug 2024 16:02:49 GMT - Wed, 14 Aug 2024 17:02:49 GMT` + TEXT + ) + end + end + + context 'when observability_metric_details parameters do not exist' do + let(:metric_params) { {} } + + it 'does not prefill the issue title and description' do + subject + + expect(assigns(:issue).title).to be_nil + expect(assigns(:issue).description).to be_nil + end + end + end + end + + context 'when passing observability logs' do + let(:log_params) { '%7B"body"%3A"Consumed%20record%20with%20orderId%3A%200522613b-3a15-11ef-85dd-0242ac120016%2C%20and%20updated%20total%20count%20to%3A%201353"%2C"fingerprint"%3A"8d6c44aebc683e3c"%2C"fullUrl"%3A"http%3A%2F%2Fgdk.test%3A3443%2Fflightjs%2FFlight%2F-%2Flogs%3Fsearch%3D%26service%5B%5D%3Dfrauddetectionservice%26severityNumber%5B%5D%3D9%26traceId%5B%5D%3D72b72def-09b3-e29f-e195-7c6db5ee599f%26fingerprint%5B%5D%3D8d6c44aebc683e3c%26timestamp%3D2024-07-04T14%253A52%253A22.693752628Z%26drawerOpen%3Dtrue"%2C"service"%3A"frauddetectionservice"%2C"severityNumber"%3A9%2C"timestamp"%3A"2024-07-04T14%3A52%3A22.693752628Z"%2C"traceId"%3A"72b72def-09b3-e29f-e195-7c6db5ee599f"%7D' } + + subject do + get :new, params: { + namespace_id: project.namespace, + project_id: project, + observability_log_details: log_params + } + end + + context 'when read_observability is prevented' do + before do + stub_feature_flags(observability_features: false) + stub_licensed_features(observability: false) + end + + context 'when observability_log_details parameters exist' do + it 'does not prefill the issue title and description' do + subject + + expect(assigns(:issue).title).to be_nil + expect(assigns(:issue).description).to be_nil + end + end + + context 'when observability_log_details parameters do not exist' do + let(:log_params) { {} } + + it 'does not prefill the issue title and description' do + subject + + expect(assigns(:issue).title).to be_nil + expect(assigns(:issue).description).to be_nil + end + end + end + + context 'when read_observability is allowed' do + before do + stub_licensed_features(observability: true) + end + + context 'when observability_log_details parameters exist' do + it 'does prefill the issue title and description' do + subject + + expect(assigns(:issue).title).to eq("Issue created from log of 'frauddetectionservice' service at 2024-07-04T14:52:22.693752628Z") + expect(assigns(:issue).description).to eq( + <<~TEXT + [Log details](http://gdk.test:3443/flightjs/Flight/-/logs?search=&service[]=frauddetectionservice&severityNumber[]=9&traceId[]=72b72def-09b3-e29f-e195-7c6db5ee599f&fingerprint[]=8d6c44aebc683e3c×tamp=2024-07-04T14%3A52%3A22.693752628Z&drawerOpen=true) \\ + Service: `frauddetectionservice` \\ + Trace ID: `72b72def-09b3-e29f-e195-7c6db5ee599f` \\ + Severity Number: `9` \\ + Timestamp: `2024-07-04T14:52:22.693752628Z` \\ + Message: + ``` + Consumed record with orderId: 0522613b-3a15-11ef-85dd-0242ac120016, and updated total count to: 1353 + ``` + TEXT + ) + end + end + + context 'when observability_log_details parameters do not exist' do + let(:log_params) { {} } + + it 'does not prefill the issue title and description' do + subject + + expect(assigns(:issue).title).to be_nil + expect(assigns(:issue).description).to be_nil + end + end + end + end + end + describe 'GET #discussions' do let(:issue) { create(:issue, project: project) } let!(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } diff --git a/ee/spec/helpers/observability/logs_issues_helper_spec.rb b/ee/spec/helpers/observability/logs_issues_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..58b11d3cbe6d0b68e62b2fa8fa944fd6cf00680a --- /dev/null +++ b/ee/spec/helpers/observability/logs_issues_helper_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Observability::LogsIssuesHelper, feature_category: :metrics do + describe '#observability_logs_issues_params' do + let(:params) do + { + 'service' => 'UserService', + 'timestamp' => '2024-08-14T12:34:56Z', + 'fullUrl' => 'http://example.com/log/123', + 'traceId' => 'abc123', + 'severityNumber' => '5', + 'body' => 'An error occurred' + } + end + + context 'when params are blank' do + it 'returns an empty hash' do + expect(helper.observability_logs_issues_params({})).to eq({}) + expect(helper.observability_logs_issues_params(nil)).to eq({}) + end + end + + context 'when params are present' do + it 'returns the correct hash' do + result = helper.observability_logs_issues_params(params) + + expect(result[:title]).to eq("Issue created from log of 'UserService' service at 2024-08-14T12:34:56Z") + expect(result[:description]).to eq( + <<~TEXT + [Log details](http://example.com/log/123) \\ + Service: `UserService` \\ + Trace ID: `abc123` \\ + Severity Number: `5` \\ + Timestamp: `2024-08-14T12:34:56Z` \\ + Message: + ``` + An error occurred + ``` + TEXT + ) + end + end + end +end diff --git a/ee/spec/helpers/observability/metrics_issues_helper_spec.rb b/ee/spec/helpers/observability/metrics_issues_helper_spec.rb index 16a498500c6ee06fc5c88b6efe3b2fd1bed7ef89..12a26afbb4d08fec13efcc1b342ef57b0df14064 100644 --- a/ee/spec/helpers/observability/metrics_issues_helper_spec.rb +++ b/ee/spec/helpers/observability/metrics_issues_helper_spec.rb @@ -3,92 +3,37 @@ require 'spec_helper' RSpec.describe Observability::MetricsIssuesHelper, feature_category: :metrics do - using RSpec::Parameterized::TableSyntax - - let_it_be(:project) { build_stubbed(:project) } # rubocop:disable RSpec/FactoryBot/AvoidCreate -- we need a project with a repository - let_it_be(:developer) { build_stubbed(:user) } - - let(:user) { developer } - - before_all do - project.add_developer(developer) - end - - describe '#observability_issue_params' do - let(:params) { {} } - - subject(:service) do - ::Issues::BuildService.new(container: project, current_user: user, params: params).observability_issue_params + describe '#observability_metrics_issues_params' do + let(:params) do + { + 'name' => 'CPU Usage High', + 'fullUrl' => 'http://example.com/metric/123', + 'type' => 'gauge', + 'timeframe' => ['2024-08-14T00:00:00Z', '2024-08-14T23:59:59Z'] + } end - context 'when feature flag or licence flag is disabled' do - where(:feature_flag_enabled, :licence_flag_enabled) do - true | false - false | true - false | false - end - - with_them do - before do - stub_feature_flags(observability_features: licence_flag_enabled) - stub_licensed_features(observability: feature_flag_enabled) - end - - it { is_expected.to eq({}) } + context 'when params are blank' do + it 'returns an empty hash' do + expect(helper.observability_metrics_issues_params({})).to eq({}) + expect(helper.observability_metrics_issues_params(nil)).to eq({}) end end - context 'when feature flag and licence flag are enabled' do - before do - stub_feature_flags(observability_features: true) - stub_licensed_features(observability: true) - end - - context 'when observabiilty_links params are empty' do - it { is_expected.to eq({}) } - end - - context 'when observability_links params are invalid' do - let(:params) { { observability_links: 'this is not valid at all' } } - - it { is_expected.to eq({}) } - end - - context 'when observability_links params are valid', :aggregate_failures do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_observability, project).and_return(true) - end - - # rubocop:disable Layout/LineLength -- urls with params will be too long - let(:params) do - { observability_links: CGI.escape( - %({"fullUrl":"http://gdk.test:3000/gitlab-org/gitlab-test/-/metrics/app.ads.ad_requests?type=Sum&date_range=1h&group_by_fn=sum&group_by_attrs[]=app.ads.ad_request_type&group_by_attrs[]=app.ads.ad_response_type", "name": "app.ads.ad_requests", "type": "Sum", "timeframe":["2024-07-2504:47:00UTC","2024-07-2505:47:00UTC"]}) - ) } - end - # rubocop:enable Layout/LineLength - - it 'has the correct output' do - expect(service[:description]).to include("Name: `app.ads.ad_requests`") - expect(service[:description]).to include("Type: `Sum`") - expect(service[:description]).to include("Timeframe: `2024-07-2504:47:00UTC - 2024-07-2505:47:00UTC`") - expect(service[:title]).to eq("Issue created from app.ads.ad_requests") - end - end - - context 'when observability_links params are invalid JSON' do - before do - allow(Ability).to receive(:allowed?).and_call_original - allow(Ability).to receive(:allowed?).with(user, :read_observability, project).and_return(true) - end - - # rubocop:disable Layout/LineLength -- urls with params will be too long - let(:params) do - { observability_links: "thisisnotjson" } - end - # rubocop:enable Layout/LineLength - - it { is_expected.to eq({}) } + context 'when params are present' do + it 'returns the correct hash' do + result = helper.observability_metrics_issues_params(params) + + expect(result).to be_a(Hash) + expect(result[:title]).to eq("Issue created from CPU Usage High") + expect(result[:description]).to eq( + <<~TEXT + [Metric details](http://example.com/metric/123) \\ + Name: `CPU Usage High` \\ + Type: `gauge` \\ + Timeframe: `2024-08-14T00:00:00Z - 2024-08-14T23:59:59Z` + TEXT + ) end end end diff --git a/ee/spec/helpers/observability/observability_issues_helper_spec.rb b/ee/spec/helpers/observability/observability_issues_helper_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..09d044ca6c9898b89d9a6cb28e1eaddede42862d --- /dev/null +++ b/ee/spec/helpers/observability/observability_issues_helper_spec.rb @@ -0,0 +1,108 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Observability::ObservabilityIssuesHelper, feature_category: :metrics do + using RSpec::Parameterized::TableSyntax + + let(:project) { build_stubbed(:project) } + let(:user) { build_stubbed(:user) } + + describe '#observability_issue_params' do + let(:params) do + { + observability_links: { + metrics: CGI.escape('{"a":"b"}'), + logs: CGI.escape('{"a":"b"}') + } + } + end + + let(:service) do + ::Issues::BuildService.new(container: project, current_user: user, params: params) + end + + subject do + service.observability_issue_params + end + + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(user, :read_observability, project).and_return(true) + end + + context 'when user does not have permissions' do + before do + allow(Ability).to receive(:allowed?).with(user, :read_observability, project).and_return(false) + end + + it { is_expected.to eq({}) } + end + + context 'when feature flag and licence flag are enabled and user have permissions' do + context 'when params are empty' do + let(:params) { {} } + + it { is_expected.to eq({}) } + end + + context 'when observability_links params is empty' do + let(:params) { { observability_links: {} } } + + it { is_expected.to eq({}) } + end + + context 'when observability_links[:logs] is invalid JSON' do + let(:params) do + { observability_links: { logs: 'invalidjson' } } + end + + it { is_expected.to eq({}) } + end + + context 'when observability_links[:metrics] is invalid JSON' do + let(:params) do + { observability_links: { metrics: 'invalidjson' } } + end + + it { is_expected.to eq({}) } + end + + context 'when observability_links[:logs] is valid stringified JSON' do + let(:params) do + { observability_links: { logs: '{"a":"b"}' } } + end + + let(:expected_params) do + { + foo: 'bar' + } + end + + before do + allow(service).to receive(:observability_logs_issues_params).and_return(expected_params) + end + + it { is_expected.to eq(expected_params) } + end + + context 'when observability_links[:metrics] is valid stringified JSON' do + let(:params) do + { observability_links: { metrics: '{"a":"b"}' } } + end + + let(:expected_params) do + { + foo: 'baz' + } + end + + before do + allow(service).to receive(:observability_metrics_issues_params).and_return(expected_params) + end + + it { is_expected.to eq(expected_params) } + end + end + end +end