From 6dac0aa99ac4aa4e041033f40ca53a35dbdecc4a Mon Sep 17 00:00:00 2001 From: kshiflett88 Date: Tue, 8 Aug 2023 08:22:01 -0400 Subject: [PATCH 1/8] APPEALS-26109: Metric Service Sentry Updates --- Gemfile | 2 +- app/controllers/help_controller.rb | 2 +- app/controllers/intakes_controller.rb | 2 +- app/views/certifications/v2.html.erb | 2 +- app/views/decision_reviews/index.html.erb | 2 +- app/views/dispatch/establish_claims/index.html.erb | 2 +- app/views/hearings/index.html.erb | 2 +- app/views/inbox/index.html.erb | 2 +- app/views/intake_manager/index.html.erb | 2 +- app/views/queue/index.html.erb | 2 +- app/views/reader/appeal/index.html.erb | 14 +++++++------- app/views/test/users/index.html.erb | 2 +- 12 files changed, 18 insertions(+), 18 deletions(-) diff --git a/Gemfile b/Gemfile index d700008e245..d334b2634f8 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,7 @@ gem "bgs", git: "https://github.com/department-of-veterans-affairs/ruby-bgs.git" gem "bootsnap", require: false gem "browser" gem "business_time", "~> 0.9.3" -gem "caseflow", git: "https://github.com/department-of-veterans-affairs/caseflow-commons", ref: "91019427bbeac8f0210f23af3e4f104be85ebdf2" +gem "caseflow", git: "https://github.com/department-of-veterans-affairs/caseflow-commons", ref: "6377b46c2639248574673adc6a708d2568c6958c" gem "connect_mpi", git: "https://github.com/department-of-veterans-affairs/connect-mpi.git", ref: "a3a58c64f85b980a8b5ea6347430dd73a99ea74c" gem "connect_vbms", git: "https://github.com/department-of-veterans-affairs/connect_vbms.git", ref: "98b1f9f8aa368189a59af74d91cb0aa4c55006af" gem "console_tree_renderer", git: "https://github.com/department-of-veterans-affairs/console-tree-renderer.git", tag: "v0.1.1" diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index bcd4b1f84d8..b03af1f4e3d 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -6,7 +6,7 @@ class HelpController < ApplicationController def feature_toggle_ui_hash(user = current_user) { programOfficeTeamManagement: FeatureToggle.enabled?(:program_office_team_management, user: user), - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } end diff --git a/app/controllers/intakes_controller.rb b/app/controllers/intakes_controller.rb index d1831b82fbf..4bb2df9afea 100644 --- a/app/controllers/intakes_controller.rb +++ b/app/controllers/intakes_controller.rb @@ -153,7 +153,7 @@ def feature_toggle_ui_hash updatedAppealForm: FeatureToggle.enabled?(:updated_appeal_form, user: current_user), hlrScUnrecognizedClaimants: FeatureToggle.enabled?(:hlr_sc_unrecognized_claimants, user: current_user), vhaClaimReviewEstablishment: FeatureToggle.enabled?(:vha_claim_review_establishment, user: current_user), - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } end diff --git a/app/views/certifications/v2.html.erb b/app/views/certifications/v2.html.erb index 86abe688bf7..8634f07ea5d 100644 --- a/app/views/certifications/v2.html.erb +++ b/app/views/certifications/v2.html.erb @@ -6,7 +6,7 @@ buildDate: build_date, vacolsId: @certification.vacols_id, featureToggles: { - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/decision_reviews/index.html.erb b/app/views/decision_reviews/index.html.erb index 53d2e9ef2ae..e377f8dce05 100644 --- a/app/views/decision_reviews/index.html.erb +++ b/app/views/decision_reviews/index.html.erb @@ -11,7 +11,7 @@ businessLineUrl: business_line.url, featureToggles: { decisionReviewQueueSsnColumn: FeatureToggle.enabled?(:decision_review_queue_ssn_column, user: current_user), - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) }, baseTasksUrl: business_line.tasks_url, taskFilterDetails: task_filter_details diff --git a/app/views/dispatch/establish_claims/index.html.erb b/app/views/dispatch/establish_claims/index.html.erb index 1084ca8126e..3c8c256783a 100644 --- a/app/views/dispatch/establish_claims/index.html.erb +++ b/app/views/dispatch/establish_claims/index.html.erb @@ -10,7 +10,7 @@ userQuota: user_quota && user_quota.to_hash, currentUserHistoricalTasks: current_user_historical_tasks.map(&:to_hash), featureToggles: { - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/hearings/index.html.erb b/app/views/hearings/index.html.erb index a86792326ac..55241926043 100644 --- a/app/views/hearings/index.html.erb +++ b/app/views/hearings/index.html.erb @@ -31,7 +31,7 @@ userIsBoardAttorney: current_user.attorney?, userIsHearingAdmin: current_user.in_hearing_admin_team?, featureToggles: { - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/inbox/index.html.erb b/app/views/inbox/index.html.erb index 0e551431277..dba5d4f67ae 100644 --- a/app/views/inbox/index.html.erb +++ b/app/views/inbox/index.html.erb @@ -10,7 +10,7 @@ pagination: pagination }, featureToggles: { - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/intake_manager/index.html.erb b/app/views/intake_manager/index.html.erb index bb52177d28b..9659d728be5 100644 --- a/app/views/intake_manager/index.html.erb +++ b/app/views/intake_manager/index.html.erb @@ -6,7 +6,7 @@ feedbackUrl: feedback_url, buildDate: build_date featureToggles: { - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/queue/index.html.erb b/app/views/queue/index.html.erb index 7e96ef7f0d1..5fa1ce56ec4 100644 --- a/app/views/queue/index.html.erb +++ b/app/views/queue/index.html.erb @@ -53,7 +53,7 @@ cavc_remand_granted_substitute_appellant: FeatureToggle.enabled?(:cavc_remand_granted_substitute_appellant, user: current_user), cavc_dashboard_workflow: FeatureToggle.enabled?(:cavc_dashboard_workflow, user: current_user), cc_appeal_workflow: FeatureToggle.enabled?(:cc_appeal_workflow, user: current_user), - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user), + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user), cc_vacatur_visibility: FeatureToggle.enabled?(:cc_vacatur_visibility, user: current_user) } }) %> diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index b640ad9a70c..f0689983c98 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -12,13 +12,13 @@ windowSlider: FeatureToggle.enabled?(:window_slider, user: current_user), readerSelectorsMemoized: FeatureToggle.enabled?(:bulk_upload_documents, user: current_user), readerGetDocumentLogging: FeatureToggle.enabled?(:reader_get_document_logging, user: current_user), - metricsLogRestError: FeatureToggle.enabled_metric?(:metrics_log_rest_error, user: current_user), - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user), - metricsLoadScreen: FeatureToggle.enabled_metric?(:metrics_load_screen, user: current_user), - metricsRecordPDFJSGetDocument: FeatureToggle.enabled_metric?(:metrics_get_pdfjs_doc, user: current_user), - metricsReaderRenderText: FeatureToggle.enabled_metric?(:metrics_reader_render_text, user: current_user), - metricsLogRestSuccess: FeatureToggle.enabled_metric?(:metrics_log_rest_success, user: current_user), - metricsPdfStorePages: FeatureToggle.enabled_metric?(:metrics_pdf_store_pages, user: current_user) + metricsLogRestError: FeatureToggle.enabled?(:metrics_log_rest_error, user: current_user), + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user), + metricsLoadScreen: FeatureToggle.enabled?(:metrics_load_screen, user: current_user), + metricsRecordPDFJSGetDocument: FeatureToggle.enabled?(:metrics_get_pdfjs_doc, user: current_user), + metricsReaderRenderText: FeatureToggle.enabled?(:metrics_reader_render_text, user: current_user), + metricsLogRestSuccess: FeatureToggle.enabled?(:metrics_log_rest_success, user: current_user), + metricsPdfStorePages: FeatureToggle.enabled?(:metrics_pdf_store_pages, user: current_user) }, buildDate: build_date }) %> diff --git a/app/views/test/users/index.html.erb b/app/views/test/users/index.html.erb index 0ac3fbdee9c..3bb0dff6ff5 100644 --- a/app/views/test/users/index.html.erb +++ b/app/views/test/users/index.html.erb @@ -17,7 +17,7 @@ epTypes: ep_types, featureToggles: { interfaceVersion2: FeatureToggle.enabled?(:interface_version_2, user: current_user), - metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } }) %> <% end %> From 340dd2b82fad849b9b5181928bc0072efd59bae0 Mon Sep 17 00:00:00 2001 From: Chris-Martine Date: Tue, 22 Aug 2023 15:02:15 -0400 Subject: [PATCH 2/8] Add a default user for metric creation to fix tests --- Gemfile.lock | 4 ++-- app/controllers/metrics/v2/logs_controller.rb | 12 ++++++++++++ app/models/metric.rb | 2 +- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 8f18a4cb67c..f2207547bd3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -9,8 +9,8 @@ GIT GIT remote: https://github.com/department-of-veterans-affairs/caseflow-commons - revision: 91019427bbeac8f0210f23af3e4f104be85ebdf2 - ref: 91019427bbeac8f0210f23af3e4f104be85ebdf2 + revision: 6377b46c2639248574673adc6a708d2568c6958c + ref: 6377b46c2639248574673adc6a708d2568c6958c specs: caseflow (0.4.8) aws-sdk (~> 2.10) diff --git a/app/controllers/metrics/v2/logs_controller.rb b/app/controllers/metrics/v2/logs_controller.rb index bb0db7bfdd7..36a00efdf18 100644 --- a/app/controllers/metrics/v2/logs_controller.rb +++ b/app/controllers/metrics/v2/logs_controller.rb @@ -9,6 +9,18 @@ def create failed_metric_info = metric&.errors.inspect || allowed_params[:message] Rails.logger.info("Failed to create metric #{failed_metric_info}") unless metric&.valid? + if (metric.metric_type === 'error') + error_info = { + name: metric.metric_name, + class: metric.metric_class, + attrs: metric.metric_attributes, + created_at: metric.created_at, + uuid: metric.uuid, + } + error = StandardError.new(error_info) + Raven.capture_exception(error) + end + head :ok end diff --git a/app/models/metric.rb b/app/models/metric.rb index 9022b493b6e..fadb29e86c1 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -77,7 +77,7 @@ def css_id def self.default_object(klass, params, user) { uuid: params[:uuid], - user: user, + user: user|| User.new(full_name: "Stand in user for testing", css_id: SecureRandom.uuid, station_id: 'Metrics'), metric_name: params[:name] || METRIC_TYPES[:log], metric_class: klass&.try(:name) || klass&.class.name || self.name, metric_group: params[:group] || METRIC_GROUPS[:service], From d1a49c19906f805a4157b87f4f94de0e24b41a9d Mon Sep 17 00:00:00 2001 From: Chris-Martine Date: Tue, 22 Aug 2023 15:16:01 -0400 Subject: [PATCH 3/8] Revert "Add a default user for metric creation to fix tests" This reverts commit 340dd2b82fad849b9b5181928bc0072efd59bae0. --- Gemfile.lock | 4 ++-- app/controllers/metrics/v2/logs_controller.rb | 12 ------------ app/models/metric.rb | 2 +- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f2207547bd3..8f18a4cb67c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -9,8 +9,8 @@ GIT GIT remote: https://github.com/department-of-veterans-affairs/caseflow-commons - revision: 6377b46c2639248574673adc6a708d2568c6958c - ref: 6377b46c2639248574673adc6a708d2568c6958c + revision: 91019427bbeac8f0210f23af3e4f104be85ebdf2 + ref: 91019427bbeac8f0210f23af3e4f104be85ebdf2 specs: caseflow (0.4.8) aws-sdk (~> 2.10) diff --git a/app/controllers/metrics/v2/logs_controller.rb b/app/controllers/metrics/v2/logs_controller.rb index 36a00efdf18..bb0db7bfdd7 100644 --- a/app/controllers/metrics/v2/logs_controller.rb +++ b/app/controllers/metrics/v2/logs_controller.rb @@ -9,18 +9,6 @@ def create failed_metric_info = metric&.errors.inspect || allowed_params[:message] Rails.logger.info("Failed to create metric #{failed_metric_info}") unless metric&.valid? - if (metric.metric_type === 'error') - error_info = { - name: metric.metric_name, - class: metric.metric_class, - attrs: metric.metric_attributes, - created_at: metric.created_at, - uuid: metric.uuid, - } - error = StandardError.new(error_info) - Raven.capture_exception(error) - end - head :ok end diff --git a/app/models/metric.rb b/app/models/metric.rb index fadb29e86c1..9022b493b6e 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -77,7 +77,7 @@ def css_id def self.default_object(klass, params, user) { uuid: params[:uuid], - user: user|| User.new(full_name: "Stand in user for testing", css_id: SecureRandom.uuid, station_id: 'Metrics'), + user: user, metric_name: params[:name] || METRIC_TYPES[:log], metric_class: klass&.try(:name) || klass&.class.name || self.name, metric_group: params[:group] || METRIC_GROUPS[:service], From d6d8ec85e4185eef3b12f285d0e2ae6982fcaf13 Mon Sep 17 00:00:00 2001 From: Chris-Martine Date: Tue, 22 Aug 2023 15:19:54 -0400 Subject: [PATCH 4/8] Add default user for metric testing --- app/models/metric.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/metric.rb b/app/models/metric.rb index 9022b493b6e..8f5c3795c2c 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -77,7 +77,7 @@ def css_id def self.default_object(klass, params, user) { uuid: params[:uuid], - user: user, + user: user, || User.new(full_name: "Stand in user for testing", css_id: SecureRandom.uuid, station_id: 'Metrics') metric_name: params[:name] || METRIC_TYPES[:log], metric_class: klass&.try(:name) || klass&.class.name || self.name, metric_group: params[:group] || METRIC_GROUPS[:service], From d8b8306c126371d099c43ea4c441e20591c32c96 Mon Sep 17 00:00:00 2001 From: kshiflett88 Date: Wed, 23 Aug 2023 09:03:32 -0400 Subject: [PATCH 5/8] revert commons ref numnber --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index d700008e245..d334b2634f8 100644 --- a/Gemfile +++ b/Gemfile @@ -17,7 +17,7 @@ gem "bgs", git: "https://github.com/department-of-veterans-affairs/ruby-bgs.git" gem "bootsnap", require: false gem "browser" gem "business_time", "~> 0.9.3" -gem "caseflow", git: "https://github.com/department-of-veterans-affairs/caseflow-commons", ref: "91019427bbeac8f0210f23af3e4f104be85ebdf2" +gem "caseflow", git: "https://github.com/department-of-veterans-affairs/caseflow-commons", ref: "6377b46c2639248574673adc6a708d2568c6958c" gem "connect_mpi", git: "https://github.com/department-of-veterans-affairs/connect-mpi.git", ref: "a3a58c64f85b980a8b5ea6347430dd73a99ea74c" gem "connect_vbms", git: "https://github.com/department-of-veterans-affairs/connect_vbms.git", ref: "98b1f9f8aa368189a59af74d91cb0aa4c55006af" gem "console_tree_renderer", git: "https://github.com/department-of-veterans-affairs/console-tree-renderer.git", tag: "v0.1.1" From 62374f40a7878c1fd73042d348eb57db61d86884 Mon Sep 17 00:00:00 2001 From: kshiflett88 Date: Wed, 23 Aug 2023 10:17:15 -0400 Subject: [PATCH 6/8] fix syntax error in metric.rb --- app/models/metric.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/metric.rb b/app/models/metric.rb index 513e0503002..f28623bb62e 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -77,7 +77,7 @@ def css_id def self.default_object(klass, params, user) { uuid: params[:uuid], - user: user, || User.new(full_name: "Stand in user for testing", css_id: SecureRandom.uuid, station_id: 'Metrics'), + user: user || User.new(full_name: "Stand in user for testing", css_id: SecureRandom.uuid, station_id: 'Metrics'), metric_name: params[:name] || METRIC_TYPES[:log], metric_class: klass&.try(:name) || klass&.class.name || self.name, metric_group: params[:group] || METRIC_GROUPS[:service], From 3ec08657506e9f31ef59a7f4a086df96e9b12605 Mon Sep 17 00:00:00 2001 From: Chris-Martine Date: Thu, 24 Aug 2023 15:33:41 -0400 Subject: [PATCH 7/8] Added default metric_params to account for failing tests --- Gemfile.lock | 4 ++-- app/services/metrics_service.rb | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 8f18a4cb67c..f2207547bd3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -9,8 +9,8 @@ GIT GIT remote: https://github.com/department-of-veterans-affairs/caseflow-commons - revision: 91019427bbeac8f0210f23af3e4f104be85ebdf2 - ref: 91019427bbeac8f0210f23af3e4f104be85ebdf2 + revision: 6377b46c2639248574673adc6a708d2568c6958c + ref: 6377b46c2639248574673adc6a708d2568c6958c specs: caseflow (0.4.8) aws-sdk (~> 2.10) diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index cf00098a244..6a772d3936d 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -64,7 +64,22 @@ def self.record(description, service: nil, name: "unknown", caller: nil) increment_datadog_counter("request_error", service, name, app) if service - metric_params[:type] = Metric::METRIC_TYPES[:error] + metric_params = metric_params || { + name: "Stand in object if metrics_service.record fails", + message: "Variables not initialized before failure", + type: Metric::METRIC_TYPES[:error], + product: "", + attrs: { + service: "", + endpoint: "" + }, + sent_to: [[Metric::LOG_SYSTEMS[:rails_console]]], + sent_to_info: "", + start: 'Time not recorded', + end: 'Time not recorded', + duration: 'Time not recorded' + } + store_record_metric(uuid, metric_params, caller) # Re-raise the same error. We don't want to interfere at all in normal error handling. From 1c1810665bdbf2bb35f5c8f0e1511f12679bcb17 Mon Sep 17 00:00:00 2001 From: kshiflett88 Date: Thu, 24 Aug 2023 17:41:12 -0400 Subject: [PATCH 8/8] Spec fix/update for update_appellant_representation_job --- app/jobs/update_appellant_representation_job.rb | 1 - app/services/metrics_service.rb | 2 +- spec/jobs/update_appellant_representation_job_spec.rb | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/jobs/update_appellant_representation_job.rb b/app/jobs/update_appellant_representation_job.rb index 081741c104b..36ee5b65857 100644 --- a/app/jobs/update_appellant_representation_job.rb +++ b/app/jobs/update_appellant_representation_job.rb @@ -7,7 +7,6 @@ class UpdateAppellantRepresentationJob < CaseflowJob include ActionView::Helpers::DateHelper queue_with_priority :low_priority application_attr :queue - APP_NAME = "caseflow_job" METRIC_GROUP_NAME = UpdateAppellantRepresentationJob.name.underscore TOTAL_NUMBER_OF_APPEALS_TO_UPDATE = 1000 diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index 6a772d3936d..ee36efb98aa 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -64,7 +64,7 @@ def self.record(description, service: nil, name: "unknown", caller: nil) increment_datadog_counter("request_error", service, name, app) if service - metric_params = metric_params || { + metric_params = { name: "Stand in object if metrics_service.record fails", message: "Variables not initialized before failure", type: Metric::METRIC_TYPES[:error], diff --git a/spec/jobs/update_appellant_representation_job_spec.rb b/spec/jobs/update_appellant_representation_job_spec.rb index 107e2975451..5bfe84c9db4 100644 --- a/spec/jobs/update_appellant_representation_job_spec.rb +++ b/spec/jobs/update_appellant_representation_job_spec.rb @@ -43,7 +43,7 @@ ) expect(DataDogService).to receive(:emit_gauge).with( app_name: "queue_job", - attrs: { endpoint: "AppellantNotification.appeal_mapper", service: "queue_job" }, + attrs: { endpoint: "AppellantNotification.appeal_mapper", service: "queue_job", uuid: anything }, metric_group: "service", metric_name: "request_latency", metric_value: anything