From 6269627063c7863c6388813785a62f52c444f293 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Wed, 7 Jun 2023 09:24:55 -0400 Subject: [PATCH 01/39] Add error handling to GET requests (#18679) * add catch to get document call * Update documentViewer.js * generic error handling and custom error handling * Update ApiUtil.js * send error logs to backend * Update Metrics.js * add uuid to error logging * add metrics table * revert * send metrics to backend * store metrics in table * update info for javascript metrics * cleanup * add source parameter * add ability to mark as performance * add feature toggle * log_controller unit test and fixes * Update schema.rb * Update schema.rb * Update metric.rb * more and fix unit tests * update metrics table for new fields and update ApiUtils error handling * update Metric.js for usage * record metrics from MetricsService * Update documentViewer.js * Update metrics_service.rb * metric_service fixes * fixing issues * can view metrics on demo * Update dashboard_controller.rb * reduce to 7 days, newest first * Update dashboard_controller.rb * 1 day at a time * add indexes --- .../metrics/dashboard_controller.rb | 35 ++++++ app/controllers/metrics/v2/logs_controller.rb | 33 ++++++ app/controllers/test/users_controller.rb | 3 +- app/models/metric.rb | 101 ++++++++++++++++++ app/services/metrics_service.rb | 63 +++++++++-- app/views/metrics/dashboard/show.html.erb | 54 ++++++++++ app/views/reader/appeal/index.html.erb | 1 + .../reader/DocumentViewer/PDF/index.jsx | 4 +- .../app/2.0/screens/reader/DocumentViewer.jsx | 18 +++- client/app/2.0/store/reader/documentViewer.js | 30 +++--- client/app/util/ApiUtil.js | 49 ++++++++- client/app/util/Metrics.js | 47 ++++++++ client/test/app/util/ApiUtil.test.js | 3 +- config/routes.rb | 5 + .../20230523174750_create_metrics_table.rb | 29 +++++ db/schema.rb | 36 ++++++- .../metrics/v2/logs_controller_spec.rb | 40 +++++++ spec/models/metric_spec.rb | 50 +++++++++ 18 files changed, 564 insertions(+), 37 deletions(-) create mode 100644 app/controllers/metrics/dashboard_controller.rb create mode 100644 app/controllers/metrics/v2/logs_controller.rb create mode 100644 app/models/metric.rb create mode 100644 app/views/metrics/dashboard/show.html.erb create mode 100644 db/migrate/20230523174750_create_metrics_table.rb create mode 100644 spec/controllers/metrics/v2/logs_controller_spec.rb create mode 100644 spec/models/metric_spec.rb diff --git a/app/controllers/metrics/dashboard_controller.rb b/app/controllers/metrics/dashboard_controller.rb new file mode 100644 index 00000000000..f3e8d6a394a --- /dev/null +++ b/app/controllers/metrics/dashboard_controller.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +class Metrics::DashboardController < ApplicationController + skip_before_action :verify_authentication + + def show + return render_access_error unless access_allowed? + + no_cache + + @metrics = Metric.where('created_at > ?', 1.day.ago).order(created_at: :desc) + + begin + render :show, layout: "plain_application" + rescue StandardError => error + Rails.logger.error(error.full_message) + raise error.full_message + end + end + + private + + def access_allowed? + current_user.admin? || + BoardProductOwners.singleton.user_has_access?(current_user) || + CaseflowSupport.singleton.user_has_access?(current_user) || + Rails.env.development? + end + + def render_access_error + render(Caseflow::Error::ActionForbiddenError.new( + message: COPY::ACCESS_DENIED_TITLE + ).serialize_response) + end +end diff --git a/app/controllers/metrics/v2/logs_controller.rb b/app/controllers/metrics/v2/logs_controller.rb new file mode 100644 index 00000000000..25f43e44158 --- /dev/null +++ b/app/controllers/metrics/v2/logs_controller.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class Metrics::V2::LogsController < ApplicationController + skip_before_action :verify_authentication + + def create + metric = Metric.create_metric_from_rest(self, allowed_params, current_user) + + failed_metric_info = metric&.errors.inspect || allowed_params[:message] + Rails.logger.info("Failed to create metric #{failed_metric_info}") unless metric&.valid? + + head :ok + end + + def allowed_params + params.require(:metric).permit(:uuid, + :name, + :group, + :message, + :type, + :product, + :app_name, + :metric_attributes, + :additional_info, + :sent_to, + :sent_to_info, + :relevant_tables_info, + :start, + :end, + :duration + ) + end +end diff --git a/app/controllers/test/users_controller.rb b/app/controllers/test/users_controller.rb index bf6f2d93bb8..f8e5b214f4d 100644 --- a/app/controllers/test/users_controller.rb +++ b/app/controllers/test/users_controller.rb @@ -62,7 +62,8 @@ class Test::UsersController < ApplicationController stats: "/stats", jobs: "/jobs", admin: "/admin", - test_veterans: "/test/data" + test_veterans: "/test/data", + metrics_dashboard: "/metrics/dashboard" } } ].freeze diff --git a/app/models/metric.rb b/app/models/metric.rb new file mode 100644 index 00000000000..c91f16f0db3 --- /dev/null +++ b/app/models/metric.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +class Metric < CaseflowRecord + belongs_to :user + + METRIC_TYPES = { error: 'error', log: 'log', performance: 'performance' } + LOG_SYSTEMS = { datadog: 'datadog', rails_console: 'rails_console', javascript_console: 'javascript_console' } + PRODUCT_TYPES = { + queue: 'queue', + hearings: 'hearings', + intake: 'intake', + vha: 'vha', + efolder: 'efolder', + reader: 'reader', + caseflow: 'caseflow', # Default product + # Added below because MetricService has usages of this as a service + vacols: 'vacols', + bgs: 'bgs', + gov_delivery: 'gov_delivery', + mpi: 'mpi', + pexip: 'pexip', + va_dot_gov: 'va_dot_gov', + va_notify: 'va_notify', + vbms: 'vbms', + } + APP_NAMES = { caseflow: 'caseflow', efolder: 'efolder' } + METRIC_GROUPS = { service: 'service' } + + validates :metric_type, inclusion: { in: METRIC_TYPES.values} + validates :metric_product, inclusion: { in: PRODUCT_TYPES.values } + validates :metric_group, inclusion: { in: METRIC_GROUPS.values } + validates :app_name, inclusion: { in: APP_NAMES.values } + validate :sent_to_in_log_systems + + def self.create_metric(caller, params, user) + create( default_object(caller, params, user) ) + end + + def self.create_metric_from_rest(caller, params, user) + params[:metric_attributes] = JSON.parse(params[:metric_attributes]) if params[:metric_attributes] + params[:additional_info] = JSON.parse(params[:additional_info]) if params[:additional_info] + params[:sent_to_info] = JSON.parse(params[:sent_to_info]) if params[:sent_to_info] + params[:relevant_tables_info] = JSON.parse(params[:relevant_tables_info]) if params[:relevant_tables_info] + + create( default_object(caller, params, user) ) + end + + def sent_to_in_log_systems + invalid_systems = sent_to - LOG_SYSTEMS.values + msg = "contains invalid log systems. The following are valid log systems #{LOG_SYSTEMS.values}" + errors.add(:sent_to, msg) if invalid_systems.size > 0 + end + + private + + # Returns an object with defaults set if below symbols are not found in params default object. + # Looks for these symbols in params parameter + # - uuid + # - name + # - group + # - message + # - type + # - product + # - app_name + # - metric_attributes + # - additional_info + # - sent_to + # - sent_to_info + # - relevant_tables_info + # - start + # - end + # - duration + def self.default_object(caller, params, user) + { + uuid: params[:uuid], + user: user, + metric_name: params[:name] || METRIC_TYPES[:log], + metric_class: caller&.name || self.name, + metric_group: params[:group] || METRIC_GROUPS[:service], + metric_message: params[:message] || METRIC_TYPES[:log], + metric_type: params[:type] || METRIC_TYPES[:log], + metric_product: PRODUCT_TYPES[params[:product]] || PRODUCT_TYPES[:caseflow], + app_name: params[:app_name] || APP_NAMES[:caseflow], + metric_attributes: params[:metric_attributes], + additional_info: params[:additional_info], + sent_to: Array(params[:sent_to]).flatten, + sent_to_info: params[:sent_to_info], + relevant_tables_info: params[:relevant_tables_info], + start: params[:start], + end: params[:end], + duration: calculate_duration(params[:start], params[:end], params[:duration]), + } + end + + def self.calculate_duration(start, end_time, duration) + return duration if duration || !start || !end_time + + end_time - start + end + +end diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index b9b83f77df1..77d7e59248c 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -4,10 +4,14 @@ # see https://dropwizard.github.io/metrics/3.1.0/getting-started/ for abstractions on metric types class MetricsService - def self.record(description, service: nil, name: "unknown") + def self.record(description, service: nil, name: "unknown", caller: nil) return_value = nil app = RequestStore[:application] || "other" service ||= app + uuid = SecureRandom.uuid + metric_name= 'request_latency' + sent_to = [[Metric::LOG_SYSTEMS[:rails_console]]] + sent_to_info = nil Rails.logger.info("STARTED #{description}") stopwatch = Benchmark.measure do @@ -16,23 +20,49 @@ def self.record(description, service: nil, name: "unknown") if service latency = stopwatch.real - DataDogService.emit_gauge( + sent_to_info = { metric_group: "service", - metric_name: "request_latency", + metric_name: metric_name, metric_value: latency, app_name: app, attrs: { service: service, - endpoint: name + endpoint: name, + uuid: uuid } - ) + } + DataDogService.emit_gauge(sent_to_info) + + sent_to << Metric::LOG_SYSTEMS[:datadog] end Rails.logger.info("FINISHED #{description}: #{stopwatch}") + + metric_params = { + name: metric_name, + message: description, + type: Metric::METRIC_TYPES[:performance], + product: service, + attrs: { + service: service, + endpoint: name + }, + sent_to: sent_to, + sent_to_info: sent_to_info, + duration: stopwatch.total + } + store_record_metric(uuid, metric_params, caller) + return_value - rescue StandardError + rescue StandardError => error + Rails.logger.error("#{error.message}\n#{error.backtrace.join("\n")}") + Raven.capture_exception(error, extra: { type: "request_error", service: service, name: name, app: app }) + increment_datadog_counter("request_error", service, name, app) if service + metric_params[:type] = Metric::METRIC_TYPES[:error] + store_record_metric(uuid, metric_params, caller) + # Re-raise the same error. We don't want to interfere at all in normal error handling. # This is just to capture the metric. raise @@ -51,4 +81,25 @@ def self.record(description, service: nil, name: "unknown") } ) end + + private + + def self.store_record_metric(uuid, params, caller) + + name ="caseflow.server.metric.#{params[:name]&.downcase.gsub(/::/, '.')}" + params = { + uuid: uuid, + name: name, + message: params[:message], + type: params[:type], + product: params[:product], + metric_attributes: params[:attrs], + sent_to: params[:sent_to], + sent_to_info: params[:sent_to_info], + duration: params[:duration], + } + metric = Metric.create_metric(caller || self, params, RequestStore[:current_user]) + failed_metric_info = metric&.errors.inspect + Rails.logger.info("Failed to create metric #{failed_metric_info}") unless metric&.valid? + end end diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb new file mode 100644 index 00000000000..49dddb80c7d --- /dev/null +++ b/app/views/metrics/dashboard/show.html.erb @@ -0,0 +1,54 @@ +

Metrics Dashboard

+

Shows metrics created in past day

+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + <% @metrics.each do |metric| %> + + + + + + + + + + + + + + + + + + + + + <% end %> + +
uuidnameclassgroupmessagetypeproductappattributesadditional_infosent_tosent_to_inforelevant_tables_infostartenddurationuser_idcreated_at
<%= metric.uuid %><%= metric.metric_name %><%= metric.metric_class %><%= metric.metric_group %><%= metric.metric_message %><%= metric.metric_type %><%= metric.metric_product %><%= metric.app_name %><%= metric.metric_attributes %><%= metric.additional_info %><%= metric.sent_to %><%= metric.sent_to_info %><%= metric.relevant_tables_info %><%= metric.start %><%= metric.end %><%= metric.duration %><%= metric.user_id %><%= metric.created_at %>
+
diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index 602c0157730..8430950a0a1 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -11,6 +11,7 @@ interfaceVersion2: FeatureToggle.enabled?(:interface_version_2, user: current_user), windowSlider: FeatureToggle.enabled?(:window_slider, user: current_user), readerSelectorsMemoized: FeatureToggle.enabled?(:bulk_upload_documents, user: current_user), + logErrorMetrics: FeatureToggle.enabled?(:log_error_metrics, user: current_user), }, buildDate: build_date }) %> diff --git a/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx b/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx index e79cd22c61a..108d1a45e9b 100644 --- a/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx +++ b/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx @@ -67,8 +67,8 @@ export const Pdf = ({ doc, clickPage, ...props }) => { Caseflow is experiencing technical difficulties and cannot load {doc.type}.
- You can try downloading the document - or try again later. + You can try + downloading the document or try again later.
diff --git a/client/app/2.0/screens/reader/DocumentViewer.jsx b/client/app/2.0/screens/reader/DocumentViewer.jsx index f396c9db552..3923d45a079 100644 --- a/client/app/2.0/screens/reader/DocumentViewer.jsx +++ b/client/app/2.0/screens/reader/DocumentViewer.jsx @@ -173,7 +173,8 @@ const DocumentViewer = (props) => { showPdf: (currentPage, currentDocument, scale) => dispatch(showPdf({ currentDocument, pageNumber: currentPage, - scale + scale, + featureToggles: props.featureToggles, })), toggleKeyboardInfo: (val) => dispatch(toggleKeyboardInfo(val)), startMove: (commentId) => dispatch(startMove(commentId)), @@ -292,14 +293,16 @@ const DocumentViewer = (props) => { dispatch(showPdf({ currentDocument: state.currentDocument, - scale: 1 + scale: 1, + featureToggles: props.featureToggles, })); }, rotateDocument: () => { dispatch(showPdf({ currentDocument: state.currentDocument, rotation: state.currentDocument.rotation, - scale: state.scale + scale: state.scale, + featureToggles: props.featureToggles, })); }, zoom: (direction) => { @@ -311,7 +314,11 @@ const DocumentViewer = (props) => { window.analyticsEvent(CATEGORIES.VIEW_DOCUMENT_PAGE, `zoom ${direction}`, scale); - dispatch(showPdf({ currentDocument: state.currentDocument, scale })); + dispatch(showPdf({ + currentDocument: state.currentDocument, + scale, + featureToggles: props.featureToggles, + })); }, setPageNumber: (pageNumber) => { // Add the analytics event @@ -357,7 +364,8 @@ const DocumentViewer = (props) => { if (currentDocument?.id) { dispatch(showPdf({ currentDocument, - scale: state.scale + scale: state.scale, + featureToggles: props.featureToggles, })); } else { // Load the Documents diff --git a/client/app/2.0/store/reader/documentViewer.js b/client/app/2.0/store/reader/documentViewer.js index 605ffc0a713..8a64bbf6a39 100644 --- a/client/app/2.0/store/reader/documentViewer.js +++ b/client/app/2.0/store/reader/documentViewer.js @@ -18,6 +18,7 @@ import { import { addMetaLabel, formatCategoryName } from 'utils/reader'; import { removeComment } from 'store/reader/annotationLayer'; import { markDocAsRead } from 'store/reader/documentList'; +import { recordMetrics } from '../../../util/Metrics'; // Set the PDFJS service worker PDF.GlobalWorkerOptions.workerSrc = pdfjsWorker; @@ -233,7 +234,7 @@ export const showPage = async (params) => { export const showPdf = createAsyncThunk( 'documentViewer/show', async ( - { rotation = null, pageNumber, currentDocument, scale }, + { rotation = null, pageNumber, currentDocument, scale, featureToggles = {} }, { dispatch } ) => { // Update the Document as read if not already @@ -248,20 +249,23 @@ export const showPdf = createAsyncThunk( withCredentials: true, timeout: true, responseType: 'arraybuffer', + logErrorMetrics: featureToggles.logErrorMetrics }); - // Store the Document in-memory so that we do not serialize through Redux, but still persist - pdfDocuments[currentDocument.id] = { - pdf: await PDF.getDocument({ data: body }).promise, - }; - - // Store the pages for the PDF - pdfDocuments[currentDocument.id].pages = await Promise.all( - range(0, pdfDocuments[currentDocument.id].pdf.numPages).map( - (pageIndex) => - pdfDocuments[currentDocument.id].pdf.getPage(pageIndex + 1) - ) - ); + if (body) { + // Store the Document in-memory so that we do not serialize through Redux, but still persist + pdfDocuments[currentDocument.id] = { + pdf: await PDF.getDocument({ data: body }).promise, + }; + + // Store the pages for the PDF + pdfDocuments[currentDocument.id].pages = await Promise.all( + range(0, pdfDocuments[currentDocument.id].pdf.numPages).map( + (pageIndex) => + pdfDocuments[currentDocument.id].pdf.getPage(pageIndex + 1) + ) + ); + } } // Store the Viewport diff --git a/client/app/util/ApiUtil.js b/client/app/util/ApiUtil.js index a1274bc2d4c..e777e7c8422 100644 --- a/client/app/util/ApiUtil.js +++ b/client/app/util/ApiUtil.js @@ -2,6 +2,7 @@ import request from 'superagent'; import nocache from 'superagent-no-cache'; import ReactOnRails from 'react-on-rails'; import StringUtil from './StringUtil'; +import uuid from 'uuid'; import _ from 'lodash'; import { timeFunctionPromise } from '../util/PerfDebug'; @@ -40,13 +41,47 @@ export const getHeadersObject = (options = {}) => { return headers; }; +const errorHandling = (url, error, method, options = {}) => { + const id = uuid.v4(); + const message = `UUID: ${id}.\nProblem with ${method} ${url}.\n${error}`; + + console.error(new Error(message)); + + if (options?.logErrorMetrics) { + const data = { + metric: { + uuid: id, + name: `caseflow.client.rest.${method.toLowerCase()}.error`, + message, + type: 'error', + product: 'caseflow', + metric_attributes: JSON.stringify({ + method, + url, + error + }), + sent_to: 'javascript_console', + } + }; + + request. + post('/metrics/v2/logs'). + set(getHeadersObject()). + send(data). + use(nocache). + on('error', (err) => console.error(`DANGER DANGER DANGER\nUUID: ${uuid.v4()}.\n: ${err}`)). + end(); + } +}; + const httpMethods = { delete(url, options = {}) { return request. delete(url). set(getHeadersObject(options.headers)). send(options.data). - use(nocache); + use(nocache). + on('error', (err) => errorHandling(url, err, 'DELETE', options)); }, get(url, options = {}) { @@ -56,7 +91,8 @@ const httpMethods = { get(url). set(getHeadersObject(options.headers)). query(options.query). - timeout(timeoutSettings); + timeout(timeoutSettings). + on('error', (err) => errorHandling(url, err, 'GET', options)); if (options.responseType) { promise.responseType(options.responseType); @@ -79,7 +115,8 @@ const httpMethods = { post(url). set(getHeadersObject({ 'X-HTTP-METHOD-OVERRIDE': 'patch' })). send(options.data). - use(nocache); + use(nocache). + on('error', (err) => errorHandling(url, err, 'PATCH', options)); }, post(url, options = {}) { @@ -87,7 +124,8 @@ const httpMethods = { post(url). set(getHeadersObject(options.headers)). send(options.data). - use(nocache); + use(nocache). + on('error', (err) => errorHandling(url, err, 'POST', options)); }, put(url, options = {}) { @@ -95,7 +133,8 @@ const httpMethods = { put(url). set(getHeadersObject(options.headers)). send(options.data). - use(nocache); + use(nocache). + on('error', (err) => errorHandling(url, err, 'PUT', options)); } }; diff --git a/client/app/util/Metrics.js b/client/app/util/Metrics.js index 029aeeaafb3..16675182bbc 100644 --- a/client/app/util/Metrics.js +++ b/client/app/util/Metrics.js @@ -1,6 +1,7 @@ import ApiUtil from './ApiUtil'; import _ from 'lodash'; import moment from 'moment'; +import uuid from 'uuid'; const INTERVAL_TO_SEND_METRICS_MS = moment.duration(60, 'seconds'); @@ -31,3 +32,49 @@ export const collectHistogram = (data) => { histograms.push(ApiUtil.convertToSnakeCase(data)); }; + +/** + * uniqueId should be V4 UUID + * If a uniqueId is not presented one will be generated for it + * + * Data is an object containing information that will be stored in metric_attributes + * + * If a message is not provided one will be created based on the data passed in + * + * Product is which area of Caseflow did the metric come from: queue, hearings, intake, vha, case_distribution, reader + * + */ +export const recordMetrics = (uniqueId, data, isError = false, { message, product, start, end, duration }) => { + let id = uniqueId; + const type = isError ? 'error' : 'log'; + let metricMessage = message ? message : `${id}\n${data}`; + const productArea = product ? product : 'caseflow'; + + // If a uuid wasn't provided assume that metric also wasn't sent to javascript console and send with UUID to console + if (!uniqueId) { + id = uuid.v4(); + if (isError) { + console.error(metricMessage); + } else { + // eslint-disable-next-line no-console + console.log(metricMessage); + } + } + + const postData = { + metric: { + uuid: id, + name: `caseflow.client.${productArea}.${type}`, + message: metricMessage, + type, + product: productArea, + metric_attributes: JSON.stringify(data), + sent_to: 'javascript_console', + start, + end, + duration + } + }; + + ApiUtil.post('/metrics/v2/logs', { data: postData }); +}; diff --git a/client/test/app/util/ApiUtil.test.js b/client/test/app/util/ApiUtil.test.js index bad4a427729..26e426b36a5 100644 --- a/client/test/app/util/ApiUtil.test.js +++ b/client/test/app/util/ApiUtil.test.js @@ -14,7 +14,8 @@ jest.mock('superagent', () => ({ set: jest.fn().mockReturnThis(), accept: jest.fn().mockReturnThis(), timeout: jest.fn().mockReturnThis(), - use: jest.fn().mockReturnThis() + use: jest.fn().mockReturnThis(), + on: jest.fn().mockReturnThis(), })); const defaultHeaders = { diff --git a/config/routes.rb b/config/routes.rb index b3390b552a2..5d95cf261fa 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -88,8 +88,13 @@ namespace :v1 do resources :histogram, only: :create end + namespace :v2 do + resources :logs, only: :create + end + get 'dashboard' => 'dashboard#show' end + namespace :dispatch do get "/", to: redirect("/dispatch/establish-claim") get 'missing-decision', to: 'establish_claims#unprepared_tasks' diff --git a/db/migrate/20230523174750_create_metrics_table.rb b/db/migrate/20230523174750_create_metrics_table.rb new file mode 100644 index 00000000000..10e8aeb0598 --- /dev/null +++ b/db/migrate/20230523174750_create_metrics_table.rb @@ -0,0 +1,29 @@ +class CreateMetricsTable < ActiveRecord::Migration[5.2] + def change + create_table :metrics do |t| + t.uuid :uuid, default: -> { "uuid_generate_v4()" }, null: false, comment: "Unique ID for the metric, can be used to search within various systems for the logging" + t.references :user, null: false, foreign_key: true, comment: "The ID of the user who generated metric." + t.string :metric_name, null: false, comment: "Name of metric" + t.string :metric_class, null: false, comment: "Class of metric, use reflection to find value to populate this" + t.string :metric_group, null: false, default: "service", comment: "Metric group: service, etc" + t.string :metric_message, null: false, comment: "Message or log for metric" + t.string :metric_type, null: false, comment: "Type of metric: ERROR, LOG, PERFORMANCE, etc" + t.string :metric_product, null: false, comment: "Where in application: Queue, Hearings, Intake, VHA, etc" + t.string :app_name, null: false, comment: "Application name: caseflow or efolder" + t.json :metric_attributes, comment: "Store attributes relevant to the metric: OS, browser, etc" + t.json :additional_info, comment: "additional data to store for the metric" + t.string :sent_to, array: true, comment: "Which system metric was sent to: Datadog, Rails Console, Javascript Console, etc " + t.json :sent_to_info, comment: "Additional information for which system metric was sent to" + t.json :relevant_tables_info, comment: "Store information to tie metric to database table(s)" + t.timestamp :start, comment: "When metric recording started" + t.timestamp :end, comment: "When metric recording stopped" + t.float :duration, comment: "Time in milliseconds from start to end" + t.timestamps + end + + add_index :metrics, :metric_name + add_index :metrics, :metric_product + add_index :metrics, :app_name + add_index :metrics, :sent_to + end +end diff --git a/db/schema.rb b/db/schema.rb index eafc2b8ab21..97a84730ec8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_03_17_164013) do +ActiveRecord::Schema.define(version: 2023_05_23_174750) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -91,7 +91,7 @@ t.boolean "appeal_docketed", default: false, null: false, comment: "When true, appeal has been docketed" t.bigint "appeal_id", null: false, comment: "AMA or Legacy Appeal ID" t.string "appeal_type", null: false, comment: "Appeal Type (Appeal or LegacyAppeal)" - t.datetime "created_at", null: false, comment: "Date and Time the record was inserted into the table" + t.datetime "created_at", null: false t.bigint "created_by_id", null: false, comment: "User id of the user that inserted the record" t.boolean "decision_mailed", default: false, null: false, comment: "When true, appeal has decision mail request complete" t.boolean "hearing_postponed", default: false, null: false, comment: "When true, appeal has hearing postponed and no hearings scheduled" @@ -100,7 +100,7 @@ t.boolean "privacy_act_complete", default: false, null: false, comment: "When true, appeal has a privacy act request completed" t.boolean "privacy_act_pending", default: false, null: false, comment: "When true, appeal has a privacy act request still open" t.boolean "scheduled_in_error", default: false, null: false, comment: "When true, hearing was scheduled in error and none scheduled" - t.datetime "updated_at", comment: "Date and time the record was last updated" + t.datetime "updated_at" t.bigint "updated_by_id", comment: "User id of the last user that updated the record" t.boolean "vso_ihp_complete", default: false, null: false, comment: "When true, appeal has a VSO IHP request completed" t.boolean "vso_ihp_pending", default: false, null: false, comment: "When true, appeal has a VSO IHP request pending" @@ -1221,6 +1221,33 @@ t.index ["updated_at"], name: "index_messages_on_updated_at" end + create_table "metrics", force: :cascade do |t| + t.json "additional_info", comment: "additional data to store for the metric" + t.string "app_name", null: false, comment: "Application name: caseflow or efolder" + t.datetime "created_at", null: false + t.float "duration", comment: "Time in milliseconds from start to end" + t.datetime "end", comment: "When metric recording stopped" + t.json "metric_attributes", comment: "Store attributes relevant to the metric: OS, browser, etc" + t.string "metric_class", null: false, comment: "Class of metric, use reflection to find value to populate this" + t.string "metric_group", default: "service", null: false, comment: "Metric group: service, etc" + t.string "metric_message", null: false, comment: "Message or log for metric" + t.string "metric_name", null: false, comment: "Name of metric" + t.string "metric_product", null: false, comment: "Where in application: Queue, Hearings, Intake, VHA, etc" + t.string "metric_type", null: false, comment: "Type of metric: ERROR, LOG, PERFORMANCE, etc" + t.json "relevant_tables_info", comment: "Store information to tie metric to database table(s)" + t.string "sent_to", comment: "Which system metric was sent to: Datadog, Rails Console, Javascript Console, etc ", array: true + t.json "sent_to_info", comment: "Additional information for which system metric was sent to" + t.datetime "start", comment: "When metric recording started" + t.datetime "updated_at", null: false + t.bigint "user_id", null: false, comment: "The ID of the user who generated metric." + t.uuid "uuid", default: -> { "uuid_generate_v4()" }, null: false, comment: "Unique ID for the metric, can be used to search within various systems for the logging" + t.index ["app_name"], name: "index_metrics_on_app_name" + t.index ["metric_name"], name: "index_metrics_on_metric_name" + t.index ["metric_product"], name: "index_metrics_on_metric_product" + t.index ["sent_to"], name: "index_metrics_on_sent_to" + t.index ["user_id"], name: "index_metrics_on_user_id" + end + create_table "mpi_update_person_events", force: :cascade do |t| t.bigint "api_key_id", null: false, comment: "API Key used to initiate the event" t.datetime "completed_at", comment: "Timestamp of when update was completed, regardless of success or failure" @@ -1275,7 +1302,7 @@ t.string "recipient_email", comment: "Participant's Email Address" t.string "recipient_phone_number", comment: "Participants Phone Number" t.text "sms_notification_content", comment: "Full SMS Text Content of Notification" - t.string "sms_notification_external_id" + t.string "sms_notification_external_id", comment: "VA Notify Notification Id for the sms notification send through their API " t.string "sms_notification_status", comment: "Status of SMS/Text Notification" t.datetime "updated_at", comment: "TImestamp of when Notification was Updated" t.index ["appeals_id", "appeals_type"], name: "index_appeals_notifications_on_appeals_id_and_appeals_type" @@ -2028,6 +2055,7 @@ add_foreign_key "membership_requests", "users", column: "decider_id" add_foreign_key "membership_requests", "users", column: "requestor_id" add_foreign_key "messages", "users" + add_foreign_key "metrics", "users" add_foreign_key "mpi_update_person_events", "api_keys" add_foreign_key "nod_date_updates", "appeals" add_foreign_key "nod_date_updates", "users" diff --git a/spec/controllers/metrics/v2/logs_controller_spec.rb b/spec/controllers/metrics/v2/logs_controller_spec.rb new file mode 100644 index 00000000000..5918a9c64aa --- /dev/null +++ b/spec/controllers/metrics/v2/logs_controller_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +describe Metrics::V2::LogsController, type: :controller do + + let(:request_params_javascript) do + { + metric: { + method: "123456789", + uuid: "PAT123456^CFL200^A", + url: '', + message: '', + isError: false, + isPerformance: false, + source: 'javascript' + } + } + end + + let(:request_params_min) do + { + metric: { + message: 'min' + } + } + end + + + context "with good request" do + it "returns 200 for javascript source" do + expect(Metric).to receive(:create_javascript_metric).and_return(nil) + post :create, params: request_params_javascript + expect(response.status).to eq(200) + end + + it "returns 200 for min params" do + post :create, params: request_params_min + expect(response.status).to eq(200) + end + end +end diff --git a/spec/models/metric_spec.rb b/spec/models/metric_spec.rb new file mode 100644 index 00000000000..f46c5163942 --- /dev/null +++ b/spec/models/metric_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +describe Metric do + let(:user) { create(:user) } + + describe "create_javascript_metric" do + let!(:params) do + { + method: "123456789", + uuid: "PAT123456^CFL200^A", + url: '', + message: '', + isError: false, + isPerformance: false, + source: 'javascript' + } + end + + it "creates a javascript metric for log" do + options = {is_error: false, performance: false} + metric = Metric.create_javascript_metric(params, user, options) + + expect(metric.valid?).to be true + expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:log]) + end + + it "creates a javascript metric for error" do + options = {is_error: true, performance: false} + metric = Metric.create_javascript_metric(params, user, options) + + expect(metric.valid?).to be true + expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:error]) + end + + it "creates a javascript metric for performance" do + options = {is_error: false, performance: true} + metric = Metric.create_javascript_metric(params, user, options) + + expect(metric.valid?).to be true + expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:performance]) + end + + it "creates a javascript metric with invalid sent_to" do + options = {is_error: false, performance: false} + metric = Metric.create_javascript_metric(params.merge({sent_to: 'fake'}), user, options) + + expect(metric.valid?).to be false + end + end +end From cb9927466413c19e7d860e33f1ed804b8c6b74b7 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Wed, 7 Jun 2023 10:25:29 -0400 Subject: [PATCH 02/39] rubocop fixes --- .../metrics/dashboard_controller.rb | 4 +- app/controllers/metrics/v2/logs_controller.rb | 30 ++++++------ app/models/metric.rb | 48 +++++++++---------- 3 files changed, 41 insertions(+), 41 deletions(-) diff --git a/app/controllers/metrics/dashboard_controller.rb b/app/controllers/metrics/dashboard_controller.rb index f3e8d6a394a..9f040c049dd 100644 --- a/app/controllers/metrics/dashboard_controller.rb +++ b/app/controllers/metrics/dashboard_controller.rb @@ -8,10 +8,10 @@ def show no_cache - @metrics = Metric.where('created_at > ?', 1.day.ago).order(created_at: :desc) + @metrics = Metric.where("created_at > ?", 1.day.ago).order(created_at: :desc) begin - render :show, layout: "plain_application" + render :show, layout: "plain_application" rescue StandardError => error Rails.logger.error(error.full_message) raise error.full_message diff --git a/app/controllers/metrics/v2/logs_controller.rb b/app/controllers/metrics/v2/logs_controller.rb index 25f43e44158..bb0db7bfdd7 100644 --- a/app/controllers/metrics/v2/logs_controller.rb +++ b/app/controllers/metrics/v2/logs_controller.rb @@ -14,20 +14,20 @@ def create def allowed_params params.require(:metric).permit(:uuid, - :name, - :group, - :message, - :type, - :product, - :app_name, - :metric_attributes, - :additional_info, - :sent_to, - :sent_to_info, - :relevant_tables_info, - :start, - :end, - :duration - ) + :name, + :group, + :message, + :type, + :product, + :app_name, + :metric_attributes, + :additional_info, + :sent_to, + :sent_to_info, + :relevant_tables_info, + :start, + :end, + :duration + ) end end diff --git a/app/models/metric.rb b/app/models/metric.rb index c91f16f0db3..ccba42c80b4 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -3,37 +3,37 @@ class Metric < CaseflowRecord belongs_to :user - METRIC_TYPES = { error: 'error', log: 'log', performance: 'performance' } - LOG_SYSTEMS = { datadog: 'datadog', rails_console: 'rails_console', javascript_console: 'javascript_console' } + METRIC_TYPES = { error: "error", log: "log", performance: "performance" }.freeze + LOG_SYSTEMS = { datadog: "datadog", rails_console: "rails_console", javascript_console: "javascript_console" } PRODUCT_TYPES = { - queue: 'queue', - hearings: 'hearings', - intake: 'intake', - vha: 'vha', - efolder: 'efolder', - reader: 'reader', - caseflow: 'caseflow', # Default product + queue: "queue", + hearings: "hearings", + intake: "intake", + vha: "vha", + efolder: "efolder", + reader: "reader", + caseflow: "caseflow", # Default product # Added below because MetricService has usages of this as a service - vacols: 'vacols', - bgs: 'bgs', - gov_delivery: 'gov_delivery', - mpi: 'mpi', - pexip: 'pexip', - va_dot_gov: 'va_dot_gov', - va_notify: 'va_notify', - vbms: 'vbms', - } - APP_NAMES = { caseflow: 'caseflow', efolder: 'efolder' } - METRIC_GROUPS = { service: 'service' } + vacols: "vacols", + bgs: "bgs", + gov_delivery: "gov_delivery", + mpi: "mpi", + pexip: "pexip", + va_dot_gov: "va_dot_gov", + va_notify: "va_notify", + vbms: "vbms", + }.freeze + APP_NAMES = { caseflow: "caseflow", efolder: "efolder" }.freeze + METRIC_GROUPS = { service: "service" }.freeze - validates :metric_type, inclusion: { in: METRIC_TYPES.values} + validates :metric_type, inclusion: { in: METRIC_TYPES.values } validates :metric_product, inclusion: { in: PRODUCT_TYPES.values } validates :metric_group, inclusion: { in: METRIC_GROUPS.values } validates :app_name, inclusion: { in: APP_NAMES.values } validate :sent_to_in_log_systems def self.create_metric(caller, params, user) - create( default_object(caller, params, user) ) + create(default_object(caller, params, user)) end def self.create_metric_from_rest(caller, params, user) @@ -42,13 +42,13 @@ def self.create_metric_from_rest(caller, params, user) params[:sent_to_info] = JSON.parse(params[:sent_to_info]) if params[:sent_to_info] params[:relevant_tables_info] = JSON.parse(params[:relevant_tables_info]) if params[:relevant_tables_info] - create( default_object(caller, params, user) ) + create(default_object(caller, params, user)) end def sent_to_in_log_systems invalid_systems = sent_to - LOG_SYSTEMS.values msg = "contains invalid log systems. The following are valid log systems #{LOG_SYSTEMS.values}" - errors.add(:sent_to, msg) if invalid_systems.size > 0 + errors.add(:sent_to, msg) if !invalid_systems.empty? end private From 0fc63883b0467bd2e896ddcf4060b9eeb7bd3628 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Wed, 7 Jun 2023 11:18:53 -0400 Subject: [PATCH 03/39] Update metrics_service.rb --- app/services/metrics_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index 77d7e59248c..80a93a78f85 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -49,7 +49,7 @@ def self.record(description, service: nil, name: "unknown", caller: nil) }, sent_to: sent_to, sent_to_info: sent_to_info, - duration: stopwatch.total + duration: stopwatch.total * 1000 # values is in seconds and we want milliseconds } store_record_metric(uuid, metric_params, caller) From fef4d9fcdfc268dce42843fc3fa9023486470f7d Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Wed, 7 Jun 2023 11:19:16 -0400 Subject: [PATCH 04/39] only show metrics for past hour and only in demo --- .../metrics/dashboard_controller.rb | 20 ++++--------------- app/views/metrics/dashboard/show.html.erb | 2 +- .../reader/DocumentViewer/PDF/index.jsx | 4 ++-- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/app/controllers/metrics/dashboard_controller.rb b/app/controllers/metrics/dashboard_controller.rb index 9f040c049dd..85e682834f1 100644 --- a/app/controllers/metrics/dashboard_controller.rb +++ b/app/controllers/metrics/dashboard_controller.rb @@ -1,14 +1,12 @@ # frozen_string_literal: true class Metrics::DashboardController < ApplicationController - skip_before_action :verify_authentication + before_action :require_demo def show - return render_access_error unless access_allowed? - no_cache - @metrics = Metric.where("created_at > ?", 1.day.ago).order(created_at: :desc) + @metrics = Metric.where("created_at > ?", 1.hour.ago).order(created_at: :desc) begin render :show, layout: "plain_application" @@ -19,17 +17,7 @@ def show end private - - def access_allowed? - current_user.admin? || - BoardProductOwners.singleton.user_has_access?(current_user) || - CaseflowSupport.singleton.user_has_access?(current_user) || - Rails.env.development? - end - - def render_access_error - render(Caseflow::Error::ActionForbiddenError.new( - message: COPY::ACCESS_DENIED_TITLE - ).serialize_response) + def require_demo + redirect_to "/unauthorized" unless Rails.deploy_env?(:demo) end end diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb index 49dddb80c7d..4677d936ba2 100644 --- a/app/views/metrics/dashboard/show.html.erb +++ b/app/views/metrics/dashboard/show.html.erb @@ -1,5 +1,5 @@

Metrics Dashboard

-

Shows metrics created in past day

+

Shows metrics created in past hour


diff --git a/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx b/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx index 108d1a45e9b..e79cd22c61a 100644 --- a/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx +++ b/client/app/2.0/components/reader/DocumentViewer/PDF/index.jsx @@ -67,8 +67,8 @@ export const Pdf = ({ doc, clickPage, ...props }) => { Caseflow is experiencing technical difficulties and cannot load {doc.type}.
- You can try - downloading the document or try again later. + You can try downloading the document + or try again later.
From 5aa705d767ff80785e9acd697e42519f6c641f82 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 08:36:26 -0400 Subject: [PATCH 05/39] disable interface_version_2 for all --- client/app/2.0/store/reader/documentViewer.js | 1 - scripts/enable_features_dev.rb | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/client/app/2.0/store/reader/documentViewer.js b/client/app/2.0/store/reader/documentViewer.js index 8a64bbf6a39..3f6ae9895ac 100644 --- a/client/app/2.0/store/reader/documentViewer.js +++ b/client/app/2.0/store/reader/documentViewer.js @@ -18,7 +18,6 @@ import { import { addMetaLabel, formatCategoryName } from 'utils/reader'; import { removeComment } from 'store/reader/annotationLayer'; import { markDocAsRead } from 'store/reader/documentList'; -import { recordMetrics } from '../../../util/Metrics'; // Set the PDFJS service worker PDF.GlobalWorkerOptions.workerSrc = pdfjsWorker; diff --git a/scripts/enable_features_dev.rb b/scripts/enable_features_dev.rb index 7161df9e379..5a0a2ddf6c3 100644 --- a/scripts/enable_features_dev.rb +++ b/scripts/enable_features_dev.rb @@ -57,7 +57,8 @@ def call disabled_flags = [ "legacy_das_deprecation", "cavc_dashboard_workflow", - "poa_auto_refresh" + "poa_auto_refresh", + "interface_version_2" ] all_features = AllFeatureToggles.new.call.flatten.uniq From c091c0e513c954170c0c2f5b84e9c77a06b4ccfb Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 12:01:58 -0400 Subject: [PATCH 06/39] scrollbar on top --- app/views/metrics/dashboard/show.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb index 4677d936ba2..8310a0279fe 100644 --- a/app/views/metrics/dashboard/show.html.erb +++ b/app/views/metrics/dashboard/show.html.erb @@ -1,8 +1,8 @@

Metrics Dashboard

Shows metrics created in past hour


-
-
+
+
From 2c86f38e382c8277bda18872ba805e059152b6fd Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 12:03:16 -0400 Subject: [PATCH 07/39] display css_id for metrics dashboard --- app/models/metric.rb | 4 ++++ app/views/metrics/dashboard/show.html.erb | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/metric.rb b/app/models/metric.rb index ccba42c80b4..12f4d84c0ae 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -51,6 +51,10 @@ def sent_to_in_log_systems errors.add(:sent_to, msg) if !invalid_systems.empty? end + def css_id + user.css_id + end + private # Returns an object with defaults set if below symbols are not found in params default object. diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb index 8310a0279fe..8d10df4a0bd 100644 --- a/app/views/metrics/dashboard/show.html.erb +++ b/app/views/metrics/dashboard/show.html.erb @@ -21,7 +21,7 @@ - + @@ -45,7 +45,7 @@ - + <% end %> From e7e8226e2ccf7b449918b075749eb0584b8c5656 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 13:20:38 -0400 Subject: [PATCH 08/39] Update show.html.erb --- app/views/metrics/dashboard/show.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb index 8d10df4a0bd..3f591101e12 100644 --- a/app/views/metrics/dashboard/show.html.erb +++ b/app/views/metrics/dashboard/show.html.erb @@ -18,9 +18,9 @@ - - - + + + From d94efd28e5d932f72ddaf7f67ca312125a4dba17 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 13:32:39 -0400 Subject: [PATCH 09/39] include start and end for MetricsService metrics --- app/controllers/metrics/dashboard_controller.rb | 3 ++- app/services/metrics_service.rb | 6 ++++++ app/views/metrics/dashboard/show.html.erb | 6 +++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/controllers/metrics/dashboard_controller.rb b/app/controllers/metrics/dashboard_controller.rb index 85e682834f1..232aeaa9d1b 100644 --- a/app/controllers/metrics/dashboard_controller.rb +++ b/app/controllers/metrics/dashboard_controller.rb @@ -6,7 +6,7 @@ class Metrics::DashboardController < ApplicationController def show no_cache - @metrics = Metric.where("created_at > ?", 1.hour.ago).order(created_at: :desc) + @metrics = Metric.includes(:user).where("created_at > ?", 1.hour.ago).order(created_at: :desc) begin render :show, layout: "plain_application" @@ -17,6 +17,7 @@ def show end private + def require_demo redirect_to "/unauthorized" unless Rails.deploy_env?(:demo) end diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index 80a93a78f85..cf00098a244 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -13,10 +13,12 @@ def self.record(description, service: nil, name: "unknown", caller: nil) sent_to = [[Metric::LOG_SYSTEMS[:rails_console]]] sent_to_info = nil + start = Time.now Rails.logger.info("STARTED #{description}") stopwatch = Benchmark.measure do return_value = yield end + stopped = Time.now if service latency = stopwatch.real @@ -49,6 +51,8 @@ def self.record(description, service: nil, name: "unknown", caller: nil) }, sent_to: sent_to, sent_to_info: sent_to_info, + start: start, + end: stopped, duration: stopwatch.total * 1000 # values is in seconds and we want milliseconds } store_record_metric(uuid, metric_params, caller) @@ -96,6 +100,8 @@ def self.store_record_metric(uuid, params, caller) metric_attributes: params[:attrs], sent_to: params[:sent_to], sent_to_info: params[:sent_to_info], + start: params[:start], + end: params[:end], duration: params[:duration], } metric = Metric.create_metric(caller || self, params, RequestStore[:current_user]) diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb index 3f591101e12..8d10df4a0bd 100644 --- a/app/views/metrics/dashboard/show.html.erb +++ b/app/views/metrics/dashboard/show.html.erb @@ -18,9 +18,9 @@ - - - + + + From 9cc5cef69de3c2eb72e667de9b8ae535638a5a85 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 15:06:58 -0400 Subject: [PATCH 10/39] Update show.html.erb --- app/views/metrics/dashboard/show.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb index 8d10df4a0bd..157af817199 100644 --- a/app/views/metrics/dashboard/show.html.erb +++ b/app/views/metrics/dashboard/show.html.erb @@ -20,7 +20,7 @@ - + From 2b48fd491e2b94c65c0e41a17d7f76581f80d620 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 16:21:52 -0400 Subject: [PATCH 11/39] Update show.html.erb --- app/views/metrics/dashboard/show.html.erb | 125 +++++++++++++--------- 1 file changed, 77 insertions(+), 48 deletions(-) diff --git a/app/views/metrics/dashboard/show.html.erb b/app/views/metrics/dashboard/show.html.erb index 157af817199..b427d5c3290 100644 --- a/app/views/metrics/dashboard/show.html.erb +++ b/app/views/metrics/dashboard/show.html.erb @@ -1,54 +1,83 @@ + + +

Metrics Dashboard

Shows metrics created in past hour


-
-
uuidstart end durationuser_idcss_id created_at
<%= metric.start %> <%= metric.end %> <%= metric.duration %><%= metric.user_id %><%= metric.css_id %> <%= metric.created_at %>
sent_to sent_to_info relevant_tables_infostartenddurationstart (ms)end (ms)duration (ms) css_id created_at
sent_to sent_to_info relevant_tables_infostart (ms)end (ms)duration (ms)startendduration css_id created_at
relevant_tables_info start enddurationduration (ms) css_id created_at
- - - - - - - - - - - - - - - - - - - - - - - - - <% @metrics.each do |metric| %> +
+
+
uuidnameclassgroupmessagetypeproductappattributesadditional_infosent_tosent_to_inforelevant_tables_infostartendduration (ms)css_idcreated_at
+ - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + - <% end %> - -
<%= metric.uuid %><%= metric.metric_name %><%= metric.metric_class %><%= metric.metric_group %><%= metric.metric_message %><%= metric.metric_type %><%= metric.metric_product %><%= metric.app_name %><%= metric.metric_attributes %><%= metric.additional_info %><%= metric.sent_to %><%= metric.sent_to_info %><%= metric.relevant_tables_info %><%= metric.start %><%= metric.end %><%= metric.duration %><%= metric.css_id %><%= metric.created_at %>uuidnameclassgroupmessagetypeproductappattributesadditional_infosent_tosent_to_inforelevant_tables_infostartendduration (ms)css_idcreated_at
+ + + + <% @metrics.each do |metric| %> + + <%= metric.uuid %> + <%= metric.metric_name %> + <%= metric.metric_class %> + <%= metric.metric_group %> + <%= metric.metric_message %> + <%= metric.metric_type %> + <%= metric.metric_product %> + <%= metric.app_name %> + <%= metric.metric_attributes %> + <%= metric.additional_info %> + <%= metric.sent_to %> + <%= metric.sent_to_info %> + <%= metric.relevant_tables_info %> + <%= metric.start %> + <%= metric.end %> + <%= metric.duration %> + <%= metric.css_id %> + <%= metric.created_at %> + + <% end %> + + +
From 13ba3bfa6f20963bf2e1039091e4457a28f1fda4 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 16:39:16 -0400 Subject: [PATCH 12/39] enable metric capture for all ApiUtil errors --- client/app/reader/PdfFile.jsx | 3 +- client/app/util/ApiUtil.js | 52 ++++++++++++++++++----------------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/client/app/reader/PdfFile.jsx b/client/app/reader/PdfFile.jsx index 54e87872b5b..8d1128c1d38 100644 --- a/client/app/reader/PdfFile.jsx +++ b/client/app/reader/PdfFile.jsx @@ -48,7 +48,8 @@ export class PdfFile extends React.PureComponent { cache: true, withCredentials: true, timeout: true, - responseType: 'arraybuffer' + responseType: 'arraybuffer', + logErrorMetrics: true }; window.addEventListener('keydown', this.keyListener); diff --git a/client/app/util/ApiUtil.js b/client/app/util/ApiUtil.js index e777e7c8422..b523d48175c 100644 --- a/client/app/util/ApiUtil.js +++ b/client/app/util/ApiUtil.js @@ -41,37 +41,39 @@ export const getHeadersObject = (options = {}) => { return headers; }; +// eslint-disable-next-line no-unused-vars const errorHandling = (url, error, method, options = {}) => { const id = uuid.v4(); const message = `UUID: ${id}.\nProblem with ${method} ${url}.\n${error}`; console.error(new Error(message)); - if (options?.logErrorMetrics) { - const data = { - metric: { - uuid: id, - name: `caseflow.client.rest.${method.toLowerCase()}.error`, - message, - type: 'error', - product: 'caseflow', - metric_attributes: JSON.stringify({ - method, - url, - error - }), - sent_to: 'javascript_console', - } - }; - - request. - post('/metrics/v2/logs'). - set(getHeadersObject()). - send(data). - use(nocache). - on('error', (err) => console.error(`DANGER DANGER DANGER\nUUID: ${uuid.v4()}.\n: ${err}`)). - end(); - } + // Need to renable this check before going to master + // if (options?.logErrorMetrics) { + const data = { + metric: { + uuid: id, + name: `caseflow.client.rest.${method.toLowerCase()}.error`, + message, + type: 'error', + product: 'caseflow', + metric_attributes: JSON.stringify({ + method, + url, + error + }), + sent_to: 'javascript_console', + } + }; + + request. + post('/metrics/v2/logs'). + set(getHeadersObject()). + send(data). + use(nocache). + on('error', (err) => console.error(`DANGER DANGER DANGER\nUUID: ${uuid.v4()}.\n: ${err}`)). + end(); + // } }; const httpMethods = { From 06cfd45c4a098b2a7d7071d2d0712cb3d59bdb35 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 8 Jun 2023 16:41:42 -0400 Subject: [PATCH 13/39] revert change --- client/app/reader/PdfFile.jsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/app/reader/PdfFile.jsx b/client/app/reader/PdfFile.jsx index 8d1128c1d38..54e87872b5b 100644 --- a/client/app/reader/PdfFile.jsx +++ b/client/app/reader/PdfFile.jsx @@ -48,8 +48,7 @@ export class PdfFile extends React.PureComponent { cache: true, withCredentials: true, timeout: true, - responseType: 'arraybuffer', - logErrorMetrics: true + responseType: 'arraybuffer' }; window.addEventListener('keydown', this.keyListener); From 8476091cd9092c182e8b77a4844d9eb355f17037 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Fri, 9 Jun 2023 15:39:47 -0400 Subject: [PATCH 14/39] fix metric recording --- app/models/metric.rb | 12 ++++++------ client/app/util/ApiUtil.js | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/models/metric.rb b/app/models/metric.rb index 12f4d84c0ae..af8f2d11ab2 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -32,17 +32,17 @@ class Metric < CaseflowRecord validates :app_name, inclusion: { in: APP_NAMES.values } validate :sent_to_in_log_systems - def self.create_metric(caller, params, user) - create(default_object(caller, params, user)) + def self.create_metric(klass, params, user) + create(default_object(klass, params, user)) end - def self.create_metric_from_rest(caller, params, user) + def self.create_metric_from_rest(klass, params, user) params[:metric_attributes] = JSON.parse(params[:metric_attributes]) if params[:metric_attributes] params[:additional_info] = JSON.parse(params[:additional_info]) if params[:additional_info] params[:sent_to_info] = JSON.parse(params[:sent_to_info]) if params[:sent_to_info] params[:relevant_tables_info] = JSON.parse(params[:relevant_tables_info]) if params[:relevant_tables_info] - create(default_object(caller, params, user)) + create(default_object(klass, params, user)) end def sent_to_in_log_systems @@ -74,12 +74,12 @@ def css_id # - start # - end # - duration - def self.default_object(caller, params, user) + def self.default_object(klass, params, user) { uuid: params[:uuid], user: user, metric_name: params[:name] || METRIC_TYPES[:log], - metric_class: caller&.name || self.name, + metric_class: klass&.class.name || self.class.name, metric_group: params[:group] || METRIC_GROUPS[:service], metric_message: params[:message] || METRIC_TYPES[:log], metric_type: params[:type] || METRIC_TYPES[:log], diff --git a/client/app/util/ApiUtil.js b/client/app/util/ApiUtil.js index b523d48175c..b710a5aa4c1 100644 --- a/client/app/util/ApiUtil.js +++ b/client/app/util/ApiUtil.js @@ -71,7 +71,7 @@ const errorHandling = (url, error, method, options = {}) => { set(getHeadersObject()). send(data). use(nocache). - on('error', (err) => console.error(`DANGER DANGER DANGER\nUUID: ${uuid.v4()}.\n: ${err}`)). + on('error', (err) => console.error(`Metric not recorded\nUUID: ${uuid.v4()}.\n: ${err}`)). end(); // } }; From 6a2ec3ef18121989d8156a7365742d1628b6342f Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Mon, 12 Jun 2023 10:52:24 -0400 Subject: [PATCH 15/39] Improved frontend metrics capturing (#18742) * new frontend metrics setup * improve loading screen metric * record metrics for searching within a document * update feature toggle * add in feature toggles * Update show.html.erb --- app/models/metric.rb | 2 +- app/views/reader/appeal/index.html.erb | 2 +- client/app/2.0/store/reader/documentViewer.js | 2 +- client/app/components/LoadingDataDisplay.jsx | 15 +- client/app/reader/DocumentSearch.jsx | 20 ++- client/app/reader/PdfUI.jsx | 2 +- client/app/util/ApiUtil.js | 2 +- client/app/util/Metrics.js | 128 +++++++++++++++--- 8 files changed, 145 insertions(+), 28 deletions(-) diff --git a/app/models/metric.rb b/app/models/metric.rb index af8f2d11ab2..ebb0a543935 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -79,7 +79,7 @@ def self.default_object(klass, params, user) uuid: params[:uuid], user: user, metric_name: params[:name] || METRIC_TYPES[:log], - metric_class: klass&.class.name || self.class.name, + metric_class: klass&.try(:name) || klass&.class.name || self.name, metric_group: params[:group] || METRIC_GROUPS[:service], metric_message: params[:message] || METRIC_TYPES[:log], metric_type: params[:type] || METRIC_TYPES[:log], diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index 8430950a0a1..374228eefb1 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -11,7 +11,7 @@ interfaceVersion2: FeatureToggle.enabled?(:interface_version_2, user: current_user), windowSlider: FeatureToggle.enabled?(:window_slider, user: current_user), readerSelectorsMemoized: FeatureToggle.enabled?(:bulk_upload_documents, user: current_user), - logErrorMetrics: FeatureToggle.enabled?(:log_error_metrics, user: current_user), + metricsLogRestError: FeatureToggle.enabled?(:metrics_log_rest_error, user: current_user), }, buildDate: build_date }) %> diff --git a/client/app/2.0/store/reader/documentViewer.js b/client/app/2.0/store/reader/documentViewer.js index 3f6ae9895ac..2d33c7ffe02 100644 --- a/client/app/2.0/store/reader/documentViewer.js +++ b/client/app/2.0/store/reader/documentViewer.js @@ -248,7 +248,7 @@ export const showPdf = createAsyncThunk( withCredentials: true, timeout: true, responseType: 'arraybuffer', - logErrorMetrics: featureToggles.logErrorMetrics + metricsLogRestError: featureToggles.metricsLogRestError }); if (body) { diff --git a/client/app/components/LoadingDataDisplay.jsx b/client/app/components/LoadingDataDisplay.jsx index 449e54a0e61..779c91e4696 100644 --- a/client/app/components/LoadingDataDisplay.jsx +++ b/client/app/components/LoadingDataDisplay.jsx @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import LoadingScreen from './LoadingScreen'; import StatusMessage from './StatusMessage'; import COPY from '../../COPY'; +import { recordAsyncMetrics } from '../util/Metrics'; const PROMISE_RESULTS = { SUCCESS: 'SUCCESS', @@ -42,10 +43,22 @@ class LoadingDataDisplay extends React.PureComponent { this.setState({ promiseStartTimeMs: Date.now() }); + const metricData = { + message: this.props.loadingComponentProps?.message || 'loading screen', + type: 'performance', + data: { + failStatusMessageProps: this.props.failStatusMessageProps, + loadingComponentProps: this.props.loadingComponentProps, + slowLoadMessage: this.props.slowLoadMessage, + slowLoadThresholdMs: this.props.slowLoadThresholdMs, + timeoutMs: this.props.timeoutMs + } + }; + // Promise does not give us a way to "un-then" and stop listening // when the component unmounts. So we'll leave this reference dangling, // but at least we can use this._isMounted to avoid taking action if necessary. - promise.then( + recordAsyncMetrics(promise, metricData).then( () => { if (!this._isMounted) { return; diff --git a/client/app/reader/DocumentSearch.jsx b/client/app/reader/DocumentSearch.jsx index 4d35f8908d6..cf4286f7032 100644 --- a/client/app/reader/DocumentSearch.jsx +++ b/client/app/reader/DocumentSearch.jsx @@ -14,6 +14,7 @@ import { searchText, getDocumentText, updateSearchIndex, setSearchIndexToHighlig import _ from 'lodash'; import classNames from 'classnames'; import { LOGO_COLORS } from '../constants/AppConstants'; +import { recordMetrics } from '../util/Metrics'; export class DocumentSearch extends React.PureComponent { constructor() { @@ -41,8 +42,19 @@ export class DocumentSearch extends React.PureComponent { this.getText(); + const metricData = { + message: `Searching within Reader document ${this.props.file} for "${this.searchTerm}"`, + type: 'performance', + product: 'reader', + data: { + searchTerm: this.searchTerm, + file: this.props.file, + }, + }; + // todo: add guard to PdfActions.searchText to abort if !searchTerm.length - this.props.searchText(this.searchTerm); + recordMetrics(this.props.searchText(this.searchTerm), metricData, + this.props.featureToggles.metricsRecordDocumentSearch); } updateSearchIndex = (iterateForwards) => { @@ -198,7 +210,8 @@ DocumentSearch.propTypes = { setSearchIsLoading: PropTypes.func, showSearchBar: PropTypes.func, totalMatchesInFile: PropTypes.number, - updateSearchIndex: PropTypes.func + updateSearchIndex: PropTypes.func, + featureToggles: PropTypes.object, }; const mapStateToProps = (state, props) => ({ @@ -209,7 +222,8 @@ const mapStateToProps = (state, props) => ({ currentMatchIndex: getCurrentMatchIndex(state, props), matchIndexToHighlight: state.searchActionReducer.indexToHighlight, hidden: state.pdfViewer.hideSearchBar, - textExtracted: !_.isEmpty(state.searchActionReducer.extractedText) + textExtracted: !_.isEmpty(state.searchActionReducer.extractedText), + featureToggles: props.featureToggles, }); const mapDispatchToProps = (dispatch) => ({ diff --git a/client/app/reader/PdfUI.jsx b/client/app/reader/PdfUI.jsx index 2d01653a5f9..6099370cb78 100644 --- a/client/app/reader/PdfUI.jsx +++ b/client/app/reader/PdfUI.jsx @@ -317,7 +317,7 @@ export class PdfUI extends React.Component {
- + { console.error(new Error(message)); // Need to renable this check before going to master - // if (options?.logErrorMetrics) { + // if (options?.metricsLogRestError) { const data = { metric: { uuid: id, diff --git a/client/app/util/Metrics.js b/client/app/util/Metrics.js index 16675182bbc..18e15f28446 100644 --- a/client/app/util/Metrics.js +++ b/client/app/util/Metrics.js @@ -3,6 +3,10 @@ import _ from 'lodash'; import moment from 'moment'; import uuid from 'uuid'; +// ------------------------------------------------------------------------------------------ +// Histograms +// ------------------------------------------------------------------------------------------ + const INTERVAL_TO_SEND_METRICS_MS = moment.duration(60, 'seconds'); let histograms = []; @@ -33,6 +37,33 @@ export const collectHistogram = (data) => { histograms.push(ApiUtil.convertToSnakeCase(data)); }; +// ------------------------------------------------------------------------------------------ +// Metric Storage and recording +// ------------------------------------------------------------------------------------------ + +const metricMessage = (uniqueId, data, message) => message ? message : `${uniqueId}\n${data}`; + +/** + * If a uuid wasn't provided assume that metric also wasn't sent to javascript console + * and send with UUID to console + */ +const checkUuid = (uniqueId, data, message, type) => { + let id = uniqueId; + const isError = type === 'error'; + + if (!uniqueId) { + id = uuid.v4(); + if (isError) { + console.error(metricMessage(uniqueId, data, message)); + } else { + // eslint-disable-next-line no-console + console.log(metricMessage(uniqueId, data, message)); + } + } + + return id; +}; + /** * uniqueId should be V4 UUID * If a uniqueId is not presented one will be generated for it @@ -44,29 +75,16 @@ export const collectHistogram = (data) => { * Product is which area of Caseflow did the metric come from: queue, hearings, intake, vha, case_distribution, reader * */ -export const recordMetrics = (uniqueId, data, isError = false, { message, product, start, end, duration }) => { - let id = uniqueId; - const type = isError ? 'error' : 'log'; - let metricMessage = message ? message : `${id}\n${data}`; +export const storeMetrics = (uniqueId, data, { message, type = 'log', product, start, end, duration }) => { + const metricType = ['log', 'error', 'performance'].includes(type) ? type : 'log'; const productArea = product ? product : 'caseflow'; - // If a uuid wasn't provided assume that metric also wasn't sent to javascript console and send with UUID to console - if (!uniqueId) { - id = uuid.v4(); - if (isError) { - console.error(metricMessage); - } else { - // eslint-disable-next-line no-console - console.log(metricMessage); - } - } - const postData = { metric: { - uuid: id, - name: `caseflow.client.${productArea}.${type}`, - message: metricMessage, - type, + uuid: uniqueId, + name: `caseflow.client.${productArea}.${metricType}`, + message: metricMessage(uniqueId, data, message), + type: metricType, product: productArea, metric_attributes: JSON.stringify(data), sent_to: 'javascript_console', @@ -78,3 +96,75 @@ export const recordMetrics = (uniqueId, data, isError = false, { message, produc ApiUtil.post('/metrics/v2/logs', { data: postData }); }; + +export const recordMetrics = (targetFunction, { uniqueId, data, message, type = 'log', product }, + saveMetrics = true) => { + + let id = checkUuid(uniqueId, data, message, type); + + const t0 = performance.now(); + const start = Date.now(); + const name = targetFunction?.name || message; + + // eslint-disable-next-line no-console + console.info(`STARTED: ${id} ${name}`); + const result = () => targetFunction(); + const t1 = performance.now(); + const end = Date.now(); + + const duration = t1 - t0; + + // eslint-disable-next-line no-console + console.info(`FINISHED: ${id} ${name} in ${duration} milliseconds`); + + if (saveMetrics) { + const metricData = { + ...data, + name + }; + + storeMetrics(uniqueId, metricData, { message, type, product, start, end, duration }); + } + + return result; +}; + +/** + * Hopefully this doesn't cause issues and preserves the async of the promise or async function + * + * Might need to split into async and promise versions if issues + */ +export const recordAsyncMetrics = async (asyncFunction, { uniqueId, data, message, type = 'log', product }, + saveMetrics = true) => { + + let id = checkUuid(uniqueId, data, message, type); + + const t0 = performance.now(); + const start = Date.now(); + const name = message || asyncFunction; + + // eslint-disable-next-line no-console + console.info(`STARTED: ${id} ${name}`); + const prom = () => asyncFunction; + const result = await prom(); + const t1 = performance.now(); + const end = Date.now(); + + const duration = t1 - t0; + + // eslint-disable-next-line no-console + console.info(`FINISHED: ${id} ${name} in ${duration} milliseconds`); + + if (saveMetrics) { + const metricData = { + ...data, + name + }; + + storeMetrics(uniqueId, metricData, { message, type, product, start, end, duration }); + } + + + return result; +}; + From 62329163c3e9b9f679ee2aa10b082e4e079df250 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Mon, 12 Jun 2023 11:33:01 -0400 Subject: [PATCH 16/39] add metricsLoadScreen feature toggle --- app/views/reader/appeal/index.html.erb | 1 + client/app/components/LoadingDataDisplay.jsx | 10 +++++++--- client/app/reader/DecisionReviewer.jsx | 3 ++- client/app/reader/ReaderLoadingScreen.jsx | 6 ++++-- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index 374228eefb1..e39927a3b1d 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -12,6 +12,7 @@ windowSlider: FeatureToggle.enabled?(:window_slider, user: current_user), readerSelectorsMemoized: FeatureToggle.enabled?(:bulk_upload_documents, user: current_user), metricsLogRestError: FeatureToggle.enabled?(:metrics_log_rest_error, user: current_user), + metricsLoadScreen: FeatureToggle.enabled?(:metrics_load_screen, user: current_user), }, buildDate: build_date }) %> diff --git a/client/app/components/LoadingDataDisplay.jsx b/client/app/components/LoadingDataDisplay.jsx index 779c91e4696..f9fbb32e876 100644 --- a/client/app/components/LoadingDataDisplay.jsx +++ b/client/app/components/LoadingDataDisplay.jsx @@ -55,10 +55,12 @@ class LoadingDataDisplay extends React.PureComponent { } }; + const shouldRecordMetrics = this.props.metricsLoadScreen; + // Promise does not give us a way to "un-then" and stop listening // when the component unmounts. So we'll leave this reference dangling, // but at least we can use this._isMounted to avoid taking action if necessary. - recordAsyncMetrics(promise, metricData).then( + recordAsyncMetrics(promise, metricData, shouldRecordMetrics).then( () => { if (!this._isMounted) { return; @@ -175,7 +177,8 @@ LoadingDataDisplay.propTypes = { loadingComponentProps: PropTypes.object, slowLoadMessage: PropTypes.string, slowLoadThresholdMs: PropTypes.number, - timeoutMs: PropTypes.number + timeoutMs: PropTypes.number, + metricsLoadScreen: PropTypes.bool, }; LoadingDataDisplay.defaultProps = { @@ -186,7 +189,8 @@ LoadingDataDisplay.defaultProps = { errorComponent: StatusMessage, loadingComponentProps: {}, failStatusMessageProps: {}, - failStatusMessageChildren: DEFAULT_UNKNOWN_ERROR_MSG + failStatusMessageChildren: DEFAULT_UNKNOWN_ERROR_MSG, + metricsLoadScreen: false, }; export default LoadingDataDisplay; diff --git a/client/app/reader/DecisionReviewer.jsx b/client/app/reader/DecisionReviewer.jsx index 2d676eb5e62..b5597e88198 100644 --- a/client/app/reader/DecisionReviewer.jsx +++ b/client/app/reader/DecisionReviewer.jsx @@ -90,7 +90,8 @@ export class DecisionReviewer extends React.PureComponent { return + vacolsId={vacolsId} + featureToggles={this.props.featureToggles}> + failStatusMessageChildren={failStatusMessageChildren} + metricsLoadScreen={this.props.featureToggles.metricsLoadScreen}> {this.props.children} ; @@ -66,7 +67,8 @@ ReaderLoadingScreen.propTypes = { onReceiveAnnotations: PropTypes.func, onReceiveDocs: PropTypes.func, onReceiveManifests: PropTypes.func, - vacolsId: PropTypes.string + vacolsId: PropTypes.string, + featureToggles: PropTypes.object }; const mapStateToProps = (state) => ({ From bde013382059009b957c27843ef86381325b5141 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Mon, 12 Jun 2023 13:00:02 -0400 Subject: [PATCH 17/39] Update DecisionReviewer.jsx --- client/app/reader/DecisionReviewer.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/app/reader/DecisionReviewer.jsx b/client/app/reader/DecisionReviewer.jsx index b5597e88198..3625ad70d9a 100644 --- a/client/app/reader/DecisionReviewer.jsx +++ b/client/app/reader/DecisionReviewer.jsx @@ -110,7 +110,8 @@ export class DecisionReviewer extends React.PureComponent { return + vacolsId={vacolsId} + featureToggles={this.props.featureToggles}> Date: Fri, 16 Jun 2023 08:45:25 -0500 Subject: [PATCH 18/39] Mbeard/appeals 23486 (#18781) * adds feature toggle * adds feature toggle to reader pdfpage which renders readertext document id, type and render time * refactors uuid * fixes file prop --------- Co-authored-by: Matt Roth --- app/views/reader/appeal/index.html.erb | 1 + client/app/reader/Pdf.jsx | 1 + client/app/reader/PdfFile.jsx | 1 + client/app/reader/PdfPage.jsx | 23 +++++++++++++++++++++-- client/app/reader/PdfUI.jsx | 1 + 5 files changed, 25 insertions(+), 2 deletions(-) diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index e39927a3b1d..5bdaffec93d 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -13,6 +13,7 @@ readerSelectorsMemoized: FeatureToggle.enabled?(:bulk_upload_documents, user: current_user), metricsLogRestError: FeatureToggle.enabled?(:metrics_log_rest_error, user: current_user), metricsLoadScreen: FeatureToggle.enabled?(:metrics_load_screen, user: current_user), + metricsReaderRenderText: FeatureToggle.enabled?(:metrics_reader_render_text, user: current_user), }, buildDate: build_date }) %> diff --git a/client/app/reader/Pdf.jsx b/client/app/reader/Pdf.jsx index dbf3a756762..dab645992af 100644 --- a/client/app/reader/Pdf.jsx +++ b/client/app/reader/Pdf.jsx @@ -79,6 +79,7 @@ export class Pdf extends React.PureComponent { isVisible={this.props.file === file} scale={this.props.scale} documentType={this.props.documentType} + featureToggles={this.props.featureToggles} />; }); diff --git a/client/app/reader/PdfFile.jsx b/client/app/reader/PdfFile.jsx index 54e87872b5b..8ba404dfbd4 100644 --- a/client/app/reader/PdfFile.jsx +++ b/client/app/reader/PdfFile.jsx @@ -145,6 +145,7 @@ export class PdfFile extends React.PureComponent { isFileVisible={this.props.isVisible} scale={this.props.scale} pdfDocument={this.props.pdfDocument} + featureToggles={this.props.featureToggles} />
; } diff --git a/client/app/reader/PdfPage.jsx b/client/app/reader/PdfPage.jsx index 8dcb68b7a60..9dc202fb3db 100644 --- a/client/app/reader/PdfPage.jsx +++ b/client/app/reader/PdfPage.jsx @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import Mark from 'mark.js'; +import { v4 as uuidv4 } from 'uuid'; import CommentLayer from './CommentLayer'; import { connect } from 'react-redux'; @@ -12,7 +13,7 @@ import { bindActionCreators } from 'redux'; import { PDF_PAGE_HEIGHT, PDF_PAGE_WIDTH, SEARCH_BAR_HEIGHT, PAGE_DIMENSION_SCALE, PAGE_MARGIN } from './constants'; import { pageNumberOfPageIndex } from './utils'; import * as PDFJS from 'pdfjs-dist'; -import { collectHistogram } from '../util/Metrics'; +import { collectHistogram, recordMetrics } from '../util/Metrics'; import { css } from 'glamor'; import classNames from 'classnames'; @@ -181,6 +182,7 @@ export class PdfPage extends React.PureComponent { }; drawText = (page, text) => { + if (!this.textLayer) { return; } @@ -215,8 +217,24 @@ export class PdfPage extends React.PureComponent { then((page) => { this.page = page; + const uuid = uuidv4(); + + const readerRenderText = { + uuid, + message: 'Searching within Reader document text', + type: 'performance', + product: 'reader', + data: { + documentId: this.props.documentId, + documentType: this.props.documentType, + file: this.props.file + }, + }; + this.getText(page).then((text) => { this.drawText(page, text); + // eslint-disable-next-line max-len + recordMetrics(this.drawText(page, text), readerRenderText, this.props.featureToggles.metricsReaderRenderText); }); this.drawPage(page).then(() => { @@ -356,7 +374,8 @@ PdfPage.propTypes = { searchText: PropTypes.string, setDocScrollPosition: PropTypes.func, setSearchIndexToHighlight: PropTypes.func, - windowingOverscan: PropTypes.string + windowingOverscan: PropTypes.string, + featureToggles: PropTypes.object }; const mapDispatchToProps = (dispatch) => ({ diff --git a/client/app/reader/PdfUI.jsx b/client/app/reader/PdfUI.jsx index 6099370cb78..3b4c28e932c 100644 --- a/client/app/reader/PdfUI.jsx +++ b/client/app/reader/PdfUI.jsx @@ -330,6 +330,7 @@ export class PdfUI extends React.Component { onPageChange={this.onPageChange} prefetchFiles={this.props.prefetchFiles} resetJumpToPage={this.props.resetJumpToPage} + featureToggles={this.props.featureToggles} /> { this.getPdfFooter() } From 3f26b3bd0e8bb821ef00a8c3be93b60d059ec583 Mon Sep 17 00:00:00 2001 From: Kodi Shiflett Date: Fri, 16 Jun 2023 11:47:28 -0400 Subject: [PATCH 19/39] APPEALS-23487: Add additional metrics for Browser captured error (#18725) * APPEALS-23487: Add additional metrics for browser captured error * Added metricsBrowserError feature toggle * Added date time and duration metrics --- app/views/reader/appeal/index.html.erb | 3 ++- client/app/index.js | 32 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index 5bdaffec93d..9be99326886 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -12,8 +12,9 @@ windowSlider: FeatureToggle.enabled?(:window_slider, user: current_user), readerSelectorsMemoized: FeatureToggle.enabled?(:bulk_upload_documents, 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), - metricsReaderRenderText: FeatureToggle.enabled?(:metrics_reader_render_text, user: current_user), + metricsReaderRenderText: FeatureToggle.enabled?(:metrics_reader_render_text, user: current_user) }, buildDate: build_date }) %> diff --git a/client/app/index.js b/client/app/index.js index 49067c2ca5e..b8fc8de52ea 100644 --- a/client/app/index.js +++ b/client/app/index.js @@ -13,6 +13,9 @@ import { render } from 'react-dom'; import { forOwn } from 'lodash'; import { BrowserRouter, Switch } from 'react-router-dom'; +// Internal Dependencies +import { storeMetrics } from './util/Metrics'; + // Redux Store Dependencies import ReduxBase from 'app/components/ReduxBase'; import rootReducer from 'store/root'; @@ -55,6 +58,7 @@ import Inbox from 'app/inbox'; import Explain from 'app/explain'; import MPISearch from 'app/mpi/MPISearch'; import Admin from 'app/admin'; +import uuid from 'uuid'; const COMPONENTS = { // New Version 2.0 Root Component @@ -93,6 +97,34 @@ const COMPONENTS = { }; const componentWrapper = (component) => (props, railsContext, domNodeId) => { + window.onerror = (event, source, lineno, colno, error) => { + if (props.featureToggles?.metricsBrowserError) { + const id = uuid.v4(); + const data = { + event, + source, + lineno, + colno, + error + }; + const t0 = performance.now(); + const start = Date.now(); + const t1 = performance.now(); + const end = Date.now(); + const duration = t1 - t0; + storeMetrics( + id, + data, + { type: 'error', + product: 'browser', + start: start, + end: end, + duration: duration } + ); + } + return true; + }; + /* eslint-disable */ const wrapComponent = (Component) => ( From bd5f764bccf72ced1087c90594d639b614232aed Mon Sep 17 00:00:00 2001 From: SHarshain <133917878+SHarshain@users.noreply.github.com> Date: Tue, 20 Jun 2023 13:55:01 -0400 Subject: [PATCH 20/39] APPEALS-23475. Record Successhandling in the metrics (#18774) * APPEALS-23475. Record Successhandling in the metrics * Added successhandling on other HTTP methods and some refactor * Added feature toggle- metricsLogRestSuccess * APPEALS-23475. Test case for success handling * reverting changes --------- Co-authored-by: SHarshain --- app/models/metric.rb | 2 +- app/views/reader/appeal/index.html.erb | 3 +- client/app/util/ApiUtil.js | 112 ++++++++++++++++++++++--- client/app/util/Metrics.js | 2 +- client/test/app/util/ApiUtil.test.js | 58 +++++++++++++ 5 files changed, 162 insertions(+), 15 deletions(-) diff --git a/app/models/metric.rb b/app/models/metric.rb index ebb0a543935..9022b493b6e 100644 --- a/app/models/metric.rb +++ b/app/models/metric.rb @@ -3,7 +3,7 @@ class Metric < CaseflowRecord belongs_to :user - METRIC_TYPES = { error: "error", log: "log", performance: "performance" }.freeze + METRIC_TYPES = { error: "error", log: "log", performance: "performance", info: "info" }.freeze LOG_SYSTEMS = { datadog: "datadog", rails_console: "rails_console", javascript_console: "javascript_console" } PRODUCT_TYPES = { queue: "queue", diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index 9be99326886..d9a3086fbf2 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -14,7 +14,8 @@ 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), - metricsReaderRenderText: FeatureToggle.enabled?(:metrics_reader_render_text, user: current_user) + metricsReaderRenderText: FeatureToggle.enabled?(:metrics_reader_render_text, user: current_user), + metricsLogRestSuccess: FeatureToggle.enabled?(:metrics_log_rest_success, user: current_user), }, buildDate: build_date }) %> diff --git a/client/app/util/ApiUtil.js b/client/app/util/ApiUtil.js index 2610ccefc6b..28debf6f2f5 100644 --- a/client/app/util/ApiUtil.js +++ b/client/app/util/ApiUtil.js @@ -5,6 +5,7 @@ import StringUtil from './StringUtil'; import uuid from 'uuid'; import _ from 'lodash'; import { timeFunctionPromise } from '../util/PerfDebug'; +import moment from 'moment'; export const STANDARD_API_TIMEOUT_MILLISECONDS = 60 * 1000; export const RESPONSE_COMPLETE_LIMIT_MILLISECONDS = 5 * 60 * 1000; @@ -47,6 +48,9 @@ const errorHandling = (url, error, method, options = {}) => { const message = `UUID: ${id}.\nProblem with ${method} ${url}.\n${error}`; console.error(new Error(message)); + options.t1 = performance.now(); + options.end = moment().format(); + options.duration = options.t1 - options.t0; // Need to renable this check before going to master // if (options?.metricsLogRestError) { @@ -63,31 +67,80 @@ const errorHandling = (url, error, method, options = {}) => { error }), sent_to: 'javascript_console', + start: options.start, + end: options.end, + duration: options.duration, } }; - request. - post('/metrics/v2/logs'). - set(getHeadersObject()). - send(data). - use(nocache). - on('error', (err) => console.error(`Metric not recorded\nUUID: ${uuid.v4()}.\n: ${err}`)). - end(); + ApiUtil.postMetricLogs('/metrics/v2/logs', { data: data }); // } }; +const successHandling = (url, res, method, options = {}) => { + const id = uuid.v4(); + const message = `UUID: ${id}.\nSuccess with ${method} ${url}.\n${res.status}`; + + // Need to renable this check before going to master + options.t1 = performance.now(); + options.end = moment().format(); + options.duration = options.t1 - options.t0; + + // if (options?.metricsLogRestSuccess) { + const data = { + metric: { + uuid: id, + name: `caseflow.client.rest.${method.toLowerCase()}.info`, + message, + type: 'info', + product: 'caseflow', + metric_attributes: JSON.stringify({ + method, + url + }), + sent_to: 'javascript_console', + sent_to_info: JSON.stringify({ + metric_group: "Rest call", + metric_name: "Javascript request", + metric_value: options.duration, + app_name: "JS reader", + attrs: { + service: "rest service", + endpoint: url, + uuid: id + } + }), + + start: options.start, + end: options.end, + duration: options.duration, + } + }; + + ApiUtil.postMetricLogs('/metrics/v2/logs', { data: data }); +}; + const httpMethods = { delete(url, options = {}) { + options.t0 = performance.now(); + options.start = moment().format(); + return request. delete(url). set(getHeadersObject(options.headers)). send(options.data). use(nocache). - on('error', (err) => errorHandling(url, err, 'DELETE', options)); + on('error', (err) => errorHandling(url, err, 'DELETE', options)). + then(res => { + successHandling(url, res, 'DELETE', options); + return res; + }); }, get(url, options = {}) { const timeoutSettings = Object.assign({}, defaultTimeoutSettings, _.get(options, 'timeout', {})); + options.t0 = performance.now(); + options.start = moment().format(); let promise = request. get(url). @@ -109,34 +162,69 @@ const httpMethods = { } return promise. - use(nocache); + use(nocache). + then(res => { + successHandling(url, res, 'GET', options); + return res; + }); }, patch(url, options = {}) { + options.t0 = performance.now(); + options.start = moment().format(); + return request. post(url). set(getHeadersObject({ 'X-HTTP-METHOD-OVERRIDE': 'patch' })). send(options.data). use(nocache). - on('error', (err) => errorHandling(url, err, 'PATCH', options)); + on('error', (err) => errorHandling(url, err, 'PATCH', options)). + then(res => { + successHandling(url, res, 'PATCH', options); + return res; + }); }, post(url, options = {}) { + options.t0 = performance.now(); + options.start = moment().format(); + return request. post(url). set(getHeadersObject(options.headers)). send(options.data). use(nocache). - on('error', (err) => errorHandling(url, err, 'POST', options)); + on('error', (err) => errorHandling(url, err, 'POST', options)). + then(res => { + successHandling(url, res, 'POST', options); + return res; + }); }, put(url, options = {}) { + options.t0 = performance.now(); + options.start = moment().format(); + return request. put(url). set(getHeadersObject(options.headers)). send(options.data). use(nocache). - on('error', (err) => errorHandling(url, err, 'PUT', options)); + on('error', (err) => errorHandling(url, err, 'PUT', options)). + then(res => { + successHandling(url, res, 'PUT', options); + return res; + }); + }, + + postMetricLogs(url, options = {}) { + return request. + post('/metrics/v2/logs'). + set(getHeadersObject()). + send(options.data). + use(nocache). + on('error', (err) => console.error(`Metric not recorded\nUUID: ${uuid.v4()}.\n: ${err}`)). + end(); } }; diff --git a/client/app/util/Metrics.js b/client/app/util/Metrics.js index 18e15f28446..8626a523980 100644 --- a/client/app/util/Metrics.js +++ b/client/app/util/Metrics.js @@ -94,7 +94,7 @@ export const storeMetrics = (uniqueId, data, { message, type = 'log', product, s } }; - ApiUtil.post('/metrics/v2/logs', { data: postData }); + ApiUtil.postMetricLogs('/metrics/v2/logs', { data: postData }); }; export const recordMetrics = (targetFunction, { uniqueId, data, message, type = 'log', product }, diff --git a/client/test/app/util/ApiUtil.test.js b/client/test/app/util/ApiUtil.test.js index 26e426b36a5..78924a0beee 100644 --- a/client/test/app/util/ApiUtil.test.js +++ b/client/test/app/util/ApiUtil.test.js @@ -16,6 +16,7 @@ jest.mock('superagent', () => ({ timeout: jest.fn().mockReturnThis(), use: jest.fn().mockReturnThis(), on: jest.fn().mockReturnThis(), + then: jest.fn().mockReturnThis() })); const defaultHeaders = { @@ -54,6 +55,25 @@ describe('ApiUtil', () => { expect(request.use).toHaveBeenCalledWith(nocache); expect(req).toMatchObject(request); }); + + test('calls success handling method when calls the api request', () => { + const successHandling = jest.fn(); + + const res = {}; + + // Setup the test + const options = { data: { sample: 'data' } }; + + // Run the test + const req = ApiUtil.patch('/foo', options); + + // Expectations + req.then(() => { + // Assert that successHandling method is called + expect(request.then).toHaveBeenCalled(res); + expect(successHandling).toHaveBeenCalled(); + }) + }); }); describe('.post', () => { @@ -72,6 +92,25 @@ describe('ApiUtil', () => { expect(req).toMatchObject(request); }); + test('calls success handling method when calls the api request', () => { + const successHandling = jest.fn(); + + const res = {}; + + // Setup the test + const options = { data: { sample: 'data' } }; + + // Run the test + const req = ApiUtil.post('/bar', options); + + // Expectations + req.then(() => { + // Assert that successHandling method is called + expect(request.then).toHaveBeenCalled(res); + expect(successHandling).toHaveBeenCalled(); + }) + }); + test('attaches custom headers when provided', () => { // Setup the test const options = { headers: { sample: 'header' } }; @@ -128,5 +167,24 @@ describe('ApiUtil', () => { expect(request.use).toHaveBeenCalledWith(nocache); expect(req).toMatchObject(request); }); + + test('calls success handling method when calls the api request', () => { + const successHandling = jest.fn(); + + const res = {}; + + // Setup the test + const options = { query: { bar: 'baz' } }; + + // Run the test + const req = ApiUtil.get('/foo', options); + + // Expectations + req.then(() => { + // Assert that successHandling method is called + expect(request.then).toHaveBeenCalled(res); + expect(successHandling).toHaveBeenCalled(); + }) + }); }); }); From c036bbcaef77d03e1d880781e16656b2bd0df78e Mon Sep 17 00:00:00 2001 From: AnandEdara <131183324+AnandEdara@users.noreply.github.com> Date: Wed, 21 Jun 2023 14:39:19 -0500 Subject: [PATCH 21/39] Aedara/appeals 23485 (#18809) * Update PdfPage.jsx * Update Metrics.js * Update PdfPage.jsx --- client/app/reader/PdfPage.jsx | 3 ++- client/app/util/Metrics.js | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/client/app/reader/PdfPage.jsx b/client/app/reader/PdfPage.jsx index 9dc202fb3db..c368be82818 100644 --- a/client/app/reader/PdfPage.jsx +++ b/client/app/reader/PdfPage.jsx @@ -244,9 +244,10 @@ export class PdfPage extends React.PureComponent { value: this.measureTimeStartMs ? performance.now() - this.measureTimeStartMs : 0, appName: 'Reader', attrs: { + documentId: this.props.documentId, overscan: this.props.windowingOverscan, documentType: this.props.documentType, - pageCount: this.props.pdfDocument.pdfInfo?.numPages + pageCount: this.props.pdfDocument.numPages } }); }); diff --git a/client/app/util/Metrics.js b/client/app/util/Metrics.js index 8626a523980..3eb6cf0f798 100644 --- a/client/app/util/Metrics.js +++ b/client/app/util/Metrics.js @@ -35,6 +35,25 @@ export const collectHistogram = (data) => { initialize(); histograms.push(ApiUtil.convertToSnakeCase(data)); + + const id = uuid.v4(); + const metricsData = data; + const time = Date(Date.now()).toString(); + const readerData = { + message: 'Render document content for "' + data.attrs.documentType + '"', + type: 'performance', + product: 'pdfjs.document.render', + start:time, + end: Date(Date.now()).toString(), + duration: data.value, + } + + if(data.value > 0){ + storeMetrics(id,metricsData,readerData); + } + else if(data.attrs.pageCount < 2){ + storeMetrics(id,metricsData,readerData); + } }; // ------------------------------------------------------------------------------------------ From a835dcb656456a02452d92e9a465ac4f1dac4f13 Mon Sep 17 00:00:00 2001 From: Chris-Martine <135330019+Chris-Martine@users.noreply.github.com> Date: Thu, 22 Jun 2023 08:30:21 -0400 Subject: [PATCH 22/39] Cmartine/appeals 23480 (#18812) * Added PDFJS page and text getting/storing metrics * Add toggle feature for PDFJS page and text metrics * Adjusted metrics and feature toggle naming * Remove unused props pass * Revert "Added PDFJS page and text getting/storing metrics" This reverts commit ac6513ea492f7037874c770946e2b6c538bd035c. --- app/views/reader/appeal/index.html.erb | 1 + client/app/reader/Pdf.jsx | 1 + client/app/reader/PdfPage.jsx | 80 +++++++++++++++++--------- 3 files changed, 56 insertions(+), 26 deletions(-) diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index d9a3086fbf2..de6fe107016 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -16,6 +16,7 @@ metricsLoadScreen: FeatureToggle.enabled?(:metrics_load_screen, 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/client/app/reader/Pdf.jsx b/client/app/reader/Pdf.jsx index dab645992af..8e78299a731 100644 --- a/client/app/reader/Pdf.jsx +++ b/client/app/reader/Pdf.jsx @@ -72,6 +72,7 @@ export class Pdf extends React.PureComponent { render() { const pages = [...this.props.prefetchFiles, this.props.file].map((file) => { return { // eslint-disable-next-line no-underscore-dangle if (this.props.pdfDocument && !this.props.pdfDocument._transport.destroyed) { - this.props.pdfDocument. - getPage(pageNumberOfPageIndex(this.props.pageIndex)). - then((page) => { - this.page = page; - - const uuid = uuidv4(); - - const readerRenderText = { - uuid, - message: 'Searching within Reader document text', - type: 'performance', - product: 'reader', - data: { - documentId: this.props.documentId, - documentType: this.props.documentType, - file: this.props.file - }, - }; - - this.getText(page).then((text) => { - this.drawText(page, text); - // eslint-disable-next-line max-len - recordMetrics(this.drawText(page, text), readerRenderText, this.props.featureToggles.metricsReaderRenderText); - }); + const pageMetricData = { + message: 'Storing PDF page', + product: 'pdfjs.document.pages', + type: 'performance', + data: { + file: this.props.file, + documentId: this.props.documentId, + pageIndex: this.props.pageIndex, + numPagesInDoc: this.props.pdfDocument.numPages, + }, + }; + + const textMetricData = { + message: 'Storing PDF page text', + product: 'pdfjs.document.pages', + type: 'performance', + data: { + file: this.props.file, + documentId: this.props.documentId, + }, + }; + + const pageAndTextFeatureToggle = this.props.featureToggles.metricsPdfStorePages; + const document = this.props.pdfDocument; + const pageIndex = pageNumberOfPageIndex(this.props.pageIndex); + const pageResult = recordAsyncMetrics(document.getPage(pageIndex), pageMetricData, pageAndTextFeatureToggle); + + pageResult.then((page) => { + this.page = page; + + const uuid = uuidv4(); + + const readerRenderText = { + uuid, + message: 'Searching within Reader document text', + type: 'performance', + product: 'reader', + data: { + documentId: this.props.documentId, + documentType: this.props.documentType, + file: this.props.file + }, + }; + + const textResult = recordAsyncMetrics(this.getText(page), textMetricData, pageAndTextFeatureToggle); + + textResult.then((text) => { + this.drawText(page, text); + // eslint-disable-next-line max-len + recordMetrics(this.drawText(page, text), readerRenderText, this.props.featureToggles.metricsReaderRenderText); + }); this.drawPage(page).then(() => { collectHistogram({ @@ -251,7 +278,8 @@ export class PdfPage extends React.PureComponent { } }); }); - }). + }); + }). catch(() => { // We might need to do something else here. }); From 297db5cc24d5d386436e9e196b9ad25c33b52761 Mon Sep 17 00:00:00 2001 From: Kodi Shiflett Date: Thu, 22 Jun 2023 08:56:35 -0400 Subject: [PATCH 23/39] APPEALS-23487 - Add Browser Error Metrics (#18824) * Added browser error feature toggle to all pages * Revert changes I did not make * Revert change made in metric model --------- Co-authored-by: Matt Roth --- app/controllers/help_controller.rb | 3 ++- app/controllers/intakes_controller.rb | 3 ++- app/views/certifications/v2.html.erb | 5 ++++- app/views/decision_reviews/index.html.erb | 3 ++- app/views/dispatch/establish_claims/index.html.erb | 7 +++++-- app/views/hearings/index.html.erb | 5 ++++- app/views/inbox/index.html.erb | 3 +++ app/views/intake_manager/index.html.erb | 3 +++ app/views/queue/index.html.erb | 3 ++- app/views/test/users/index.html.erb | 6 +++++- 10 files changed, 32 insertions(+), 9 deletions(-) diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index 17d7f610e3f..b03af1f4e3d 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -5,7 +5,8 @@ class HelpController < ApplicationController def feature_toggle_ui_hash(user = current_user) { - programOfficeTeamManagement: FeatureToggle.enabled?(:program_office_team_management, user: user) + programOfficeTeamManagement: FeatureToggle.enabled?(:program_office_team_management, user: 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 1086f199659..4f701fee3d7 100644 --- a/app/controllers/intakes_controller.rb +++ b/app/controllers/intakes_controller.rb @@ -151,7 +151,8 @@ def feature_toggle_ui_hash eduPreDocketAppeals: FeatureToggle.enabled?(:edu_predocket_appeals, user: current_user), 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) + vhaClaimReviewEstablishment: FeatureToggle.enabled?(:vha_claim_review_establishment, 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 6b739da67b0..8634f07ea5d 100644 --- a/app/views/certifications/v2.html.erb +++ b/app/views/certifications/v2.html.erb @@ -4,6 +4,9 @@ dropdownUrls: dropdown_urls, feedbackUrl: feedback_url, buildDate: build_date, - vacolsId: @certification.vacols_id + vacolsId: @certification.vacols_id, + featureToggles: { + 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 6a6b1f10c70..69752ada904 100644 --- a/app/views/decision_reviews/index.html.erb +++ b/app/views/decision_reviews/index.html.erb @@ -9,7 +9,8 @@ businessLine: business_line.name, businessLineUrl: business_line.url, featureToggles: { - decisionReviewQueueSsnColumn: FeatureToggle.enabled?(:decision_review_queue_ssn_column, user: current_user) + decisionReviewQueueSsnColumn: FeatureToggle.enabled?(:decision_review_queue_ssn_column, 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 8a9cc4d7d64..3c8c256783a 100644 --- a/app/views/dispatch/establish_claims/index.html.erb +++ b/app/views/dispatch/establish_claims/index.html.erb @@ -8,6 +8,9 @@ buildDate: build_date, buttonText: start_text, userQuota: user_quota && user_quota.to_hash, - currentUserHistoricalTasks: current_user_historical_tasks.map(&:to_hash) + currentUserHistoricalTasks: current_user_historical_tasks.map(&:to_hash), + featureToggles: { + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) + } }) %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/hearings/index.html.erb b/app/views/hearings/index.html.erb index db78283f635..55241926043 100644 --- a/app/views/hearings/index.html.erb +++ b/app/views/hearings/index.html.erb @@ -29,6 +29,9 @@ userIsDvc: current_user.can_view_judge_team_management?, userIsHearingManagement: current_user.in_hearing_management_team?, userIsBoardAttorney: current_user.attorney?, - userIsHearingAdmin: current_user.in_hearing_admin_team? + userIsHearingAdmin: current_user.in_hearing_admin_team?, + featureToggles: { + 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 65ba31a93e8..dba5d4f67ae 100644 --- a/app/views/inbox/index.html.erb +++ b/app/views/inbox/index.html.erb @@ -8,6 +8,9 @@ inbox: { messages: messages, pagination: pagination + }, + featureToggles: { + 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 09ed9a2c07e..9659d728be5 100644 --- a/app/views/intake_manager/index.html.erb +++ b/app/views/intake_manager/index.html.erb @@ -5,5 +5,8 @@ dropdownUrls: dropdown_urls, feedbackUrl: feedback_url, buildDate: build_date + featureToggles: { + 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 88b5f34ca42..67c30eb0101 100644 --- a/app/views/queue/index.html.erb +++ b/app/views/queue/index.html.erb @@ -52,7 +52,8 @@ split_appeal_workflow: FeatureToggle.enabled?(:split_appeal_workflow, user: current_user), 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) + cc_appeal_workflow: FeatureToggle.enabled?(:cc_appeal_workflow, user: current_user), + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/test/users/index.html.erb b/app/views/test/users/index.html.erb index f8a29402c45..3bb0dff6ff5 100644 --- a/app/views/test/users/index.html.erb +++ b/app/views/test/users/index.html.erb @@ -14,6 +14,10 @@ appSelectList: Test::UsersController::APPS, userSession: user_session, timezone: { getlocal: Time.now.getlocal.zone, zone: Time.zone.name }, - epTypes: ep_types + epTypes: ep_types, + featureToggles: { + interfaceVersion2: FeatureToggle.enabled?(:interface_version_2, user: current_user), + metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) + } }) %> <% end %> From 2d902f64cd52db022c52a71e724c230f7c5a918a Mon Sep 17 00:00:00 2001 From: cdetlefva <133903625+cdetlefva@users.noreply.github.com> Date: Thu, 22 Jun 2023 17:06:44 -0400 Subject: [PATCH 24/39] Chrisbdetlef/appeals 23479 (#18842) * First draft * ready * Add meterics capture for PDFJS document loading - Remove dev debugging code * Remove test code for v2.0 (not in use) * Revert test code for v2.0 * Restore v2.0 docviewer * Fix variable issue * Fix variable type issue --------- Co-authored-by: Christopher Detlef --- app/views/reader/appeal/index.html.erb | 1 + client/app/reader/Pdf.jsx | 1 - client/app/reader/PdfFile.jsx | 14 +++++++++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index de6fe107016..514b8454f65 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -14,6 +14,7 @@ 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), diff --git a/client/app/reader/Pdf.jsx b/client/app/reader/Pdf.jsx index 8e78299a731..dab645992af 100644 --- a/client/app/reader/Pdf.jsx +++ b/client/app/reader/Pdf.jsx @@ -72,7 +72,6 @@ export class Pdf extends React.PureComponent { render() { const pages = [...this.props.prefetchFiles, this.props.file].map((file) => { return { + const metricData = { + message: `Getting PDF document id: "${this.props.documentId}"`, + type: 'performance', + product: 'reader', + data: { + file: this.props.file, + } + }; + this.loadingTask = PDFJS.getDocument({ data: resp.body }); + const promise = this.loadingTask.promise; - return this.loadingTask.promise; + return recordAsyncMetrics(promise, metricData, + this.props.featureToggles.metricsRecordPDFJSGetDocument); }). then((pdfDocument) => { From 581e70b046583dee9009c5e81bda7ec2926ff28f Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Fri, 23 Jun 2023 14:52:28 -0400 Subject: [PATCH 25/39] Update index.html.erb --- app/views/reader/appeal/index.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index 514b8454f65..3bb0fa33c11 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -14,7 +14,7 @@ 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) + 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), From 8e87adb3e04e1120db41f7425dd745ac69ed01ef Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Thu, 29 Jun 2023 14:51:31 -0400 Subject: [PATCH 26/39] Add feature toggle to enable all metrics at once (#18902) * enabled_metric? * update caseflow-commons * update caseflow-commons * fix jslint, issues, add feature toggle to retrieving PDF, wrap error and success metrics with feature toggle --- Gemfile | 2 +- Gemfile.lock | 8 +- 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 +- .../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 +- client/app/reader/Pdf.jsx | 3 +- client/app/reader/PdfFile.jsx | 9 +- client/app/util/ApiUtil.js | 142 +++++++++--------- client/app/util/Metrics.js | 110 +++++++------- 17 files changed, 159 insertions(+), 149 deletions(-) diff --git a/Gemfile b/Gemfile index 04bf9e255d5..5b16d90f11d 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: "6377b46c2639248574673adc6a708d2568c6958c" +gem "caseflow", git: "https://github.com/department-of-veterans-affairs/caseflow-commons", ref: "91019427bbeac8f0210f23af3e4f104be85ebdf2" 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/Gemfile.lock b/Gemfile.lock index 710c4378d8d..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) @@ -209,7 +209,7 @@ GEM crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.6) - d3-rails (7.0.0) + d3-rails (7.8.5) railties (>= 3.1) danger (6.2.2) claide (~> 1.0) @@ -349,7 +349,7 @@ GEM activerecord (>= 3.0) jaro_winkler (1.5.4) jmespath (1.3.1) - jquery-rails (4.5.1) + jquery-rails (4.6.0) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index b03af1f4e3d..bcd4b1f84d8 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) } end diff --git a/app/controllers/intakes_controller.rb b/app/controllers/intakes_controller.rb index 4f701fee3d7..033bd3a6c10 100644 --- a/app/controllers/intakes_controller.rb +++ b/app/controllers/intakes_controller.rb @@ -152,7 +152,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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) } end diff --git a/app/views/certifications/v2.html.erb b/app/views/certifications/v2.html.erb index 8634f07ea5d..86abe688bf7 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(: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 69752ada904..a61b46295d4 100644 --- a/app/views/decision_reviews/index.html.erb +++ b/app/views/decision_reviews/index.html.erb @@ -10,7 +10,7 @@ businessLineUrl: business_line.url, featureToggles: { decisionReviewQueueSsnColumn: FeatureToggle.enabled?(:decision_review_queue_ssn_column, user: current_user), - metricsBrowserError: FeatureToggle.enabled?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(: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 3c8c256783a..1084ca8126e 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/hearings/index.html.erb b/app/views/hearings/index.html.erb index 55241926043..a86792326ac 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/inbox/index.html.erb b/app/views/inbox/index.html.erb index dba5d4f67ae..0e551431277 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(: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 9659d728be5..bb52177d28b 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/queue/index.html.erb b/app/views/queue/index.html.erb index 67c30eb0101..7d357ca9017 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/app/views/reader/appeal/index.html.erb b/app/views/reader/appeal/index.html.erb index 3bb0fa33c11..229c0afe607 100644 --- a/app/views/reader/appeal/index.html.erb +++ b/app/views/reader/appeal/index.html.erb @@ -11,13 +11,13 @@ interfaceVersion2: FeatureToggle.enabled?(:interface_version_2, user: current_user), windowSlider: FeatureToggle.enabled?(:window_slider, user: current_user), readerSelectorsMemoized: FeatureToggle.enabled?(:bulk_upload_documents, 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), + 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), }, buildDate: build_date }) %> diff --git a/app/views/test/users/index.html.erb b/app/views/test/users/index.html.erb index 3bb0dff6ff5..0ac3fbdee9c 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?(:metrics_browser_error, user: current_user) + metricsBrowserError: FeatureToggle.enabled_metric?(:metrics_browser_error, user: current_user) } }) %> <% end %> diff --git a/client/app/reader/Pdf.jsx b/client/app/reader/Pdf.jsx index dab645992af..ff92cfdb37a 100644 --- a/client/app/reader/Pdf.jsx +++ b/client/app/reader/Pdf.jsx @@ -148,5 +148,6 @@ Pdf.propTypes = { scale: PropTypes.number, selectedAnnotationId: PropTypes.number, stopPlacingAnnotation: PropTypes.func, - togglePdfSidebar: PropTypes.func + togglePdfSidebar: PropTypes.func, + featureToggles: PropTypes.object }; diff --git a/client/app/reader/PdfFile.jsx b/client/app/reader/PdfFile.jsx index 50edd49f187..ef54f533627 100644 --- a/client/app/reader/PdfFile.jsx +++ b/client/app/reader/PdfFile.jsx @@ -24,7 +24,7 @@ import { INTERACTION_TYPES } from '../reader/analytics'; import { getCurrentMatchIndex, getMatchesPerPageInFile, getSearchTerm } from './selectors'; import pdfjsWorker from 'pdfjs-dist/build/pdf.worker.entry'; import uuid from 'uuid'; -import { recordMetrics, recordAsyncMetrics } from '../util/Metrics'; +import { recordAsyncMetrics } from '../util/Metrics'; PDFJS.GlobalWorkerOptions.workerSrc = pdfjsWorker; @@ -50,7 +50,9 @@ export class PdfFile extends React.PureComponent { cache: true, withCredentials: true, timeout: true, - responseType: 'arraybuffer' + responseType: 'arraybuffer', + metricsLogRestError: this.props.featureToggles.metricsLogRestError, + metricsLogRestSuccess: this.props.featureToggles.metricsLogRestSuccess }; window.addEventListener('keydown', this.keyListener); @@ -544,7 +546,8 @@ PdfFile.propTypes = { togglePdfSidebar: PropTypes.func, updateSearchIndexPage: PropTypes.func, updateSearchRelativeIndex: PropTypes.func, - windowingOverscan: PropTypes.number + windowingOverscan: PropTypes.number, + featureToggles: PropTypes.object }; const mapDispatchToProps = (dispatch) => ({ diff --git a/client/app/util/ApiUtil.js b/client/app/util/ApiUtil.js index 28debf6f2f5..c9a71615350 100644 --- a/client/app/util/ApiUtil.js +++ b/client/app/util/ApiUtil.js @@ -42,6 +42,16 @@ export const getHeadersObject = (options = {}) => { return headers; }; +export const postMetricLogs = (data) => { + return request. + post('/metrics/v2/logs'). + set(getHeadersObject()). + send(data). + use(nocache). + on('error', (err) => console.error(`Metric not recorded\nUUID: ${uuid.v4()}.\n: ${err}`)). + end(); +}; + // eslint-disable-next-line no-unused-vars const errorHandling = (url, error, method, options = {}) => { const id = uuid.v4(); @@ -53,28 +63,28 @@ const errorHandling = (url, error, method, options = {}) => { options.duration = options.t1 - options.t0; // Need to renable this check before going to master - // if (options?.metricsLogRestError) { - const data = { - metric: { - uuid: id, - name: `caseflow.client.rest.${method.toLowerCase()}.error`, - message, - type: 'error', - product: 'caseflow', - metric_attributes: JSON.stringify({ - method, - url, - error - }), - sent_to: 'javascript_console', - start: options.start, - end: options.end, - duration: options.duration, - } - }; + if (options?.metricsLogRestError) { + const data = { + metric: { + uuid: id, + name: `caseflow.client.rest.${method.toLowerCase()}.error`, + message, + type: 'error', + product: 'caseflow', + metric_attributes: JSON.stringify({ + method, + url, + error + }), + sent_to: 'javascript_console', + start: options.start, + end: options.end, + duration: options.duration, + } + }; - ApiUtil.postMetricLogs('/metrics/v2/logs', { data: data }); - // } + postMetricLogs(data); + } }; const successHandling = (url, res, method, options = {}) => { @@ -86,38 +96,39 @@ const successHandling = (url, res, method, options = {}) => { options.end = moment().format(); options.duration = options.t1 - options.t0; - // if (options?.metricsLogRestSuccess) { - const data = { - metric: { - uuid: id, - name: `caseflow.client.rest.${method.toLowerCase()}.info`, - message, - type: 'info', - product: 'caseflow', - metric_attributes: JSON.stringify({ - method, - url - }), - sent_to: 'javascript_console', - sent_to_info: JSON.stringify({ - metric_group: "Rest call", - metric_name: "Javascript request", - metric_value: options.duration, - app_name: "JS reader", - attrs: { - service: "rest service", - endpoint: url, - uuid: id - } - }), - - start: options.start, - end: options.end, - duration: options.duration, - } - }; + if (options?.metricsLogRestSuccess) { + const data = { + metric: { + uuid: id, + name: `caseflow.client.rest.${method.toLowerCase()}.info`, + message, + type: 'info', + product: 'caseflow', + metric_attributes: JSON.stringify({ + method, + url + }), + sent_to: 'javascript_console', + sent_to_info: JSON.stringify({ + metric_group: 'Rest call', + metric_name: 'Javascript request', + metric_value: options.duration, + app_name: 'JS reader', + attrs: { + service: 'rest service', + endpoint: url, + uuid: id + } + }), + + start: options.start, + end: options.end, + duration: options.duration, + } + }; - ApiUtil.postMetricLogs('/metrics/v2/logs', { data: data }); + postMetricLogs(data); + } }; const httpMethods = { @@ -131,14 +142,16 @@ const httpMethods = { send(options.data). use(nocache). on('error', (err) => errorHandling(url, err, 'DELETE', options)). - then(res => { + then((res) => { successHandling(url, res, 'DELETE', options); + return res; }); }, get(url, options = {}) { const timeoutSettings = Object.assign({}, defaultTimeoutSettings, _.get(options, 'timeout', {})); + options.t0 = performance.now(); options.start = moment().format(); @@ -163,8 +176,9 @@ const httpMethods = { return promise. use(nocache). - then(res => { + then((res) => { successHandling(url, res, 'GET', options); + return res; }); }, @@ -179,8 +193,9 @@ const httpMethods = { send(options.data). use(nocache). on('error', (err) => errorHandling(url, err, 'PATCH', options)). - then(res => { + then((res) => { successHandling(url, res, 'PATCH', options); + return res; }); }, @@ -195,8 +210,9 @@ const httpMethods = { send(options.data). use(nocache). on('error', (err) => errorHandling(url, err, 'POST', options)). - then(res => { + then((res) => { successHandling(url, res, 'POST', options); + return res; }); }, @@ -211,21 +227,13 @@ const httpMethods = { send(options.data). use(nocache). on('error', (err) => errorHandling(url, err, 'PUT', options)). - then(res => { + then((res) => { successHandling(url, res, 'PUT', options); + return res; }); - }, - - postMetricLogs(url, options = {}) { - return request. - post('/metrics/v2/logs'). - set(getHeadersObject()). - send(options.data). - use(nocache). - on('error', (err) => console.error(`Metric not recorded\nUUID: ${uuid.v4()}.\n: ${err}`)). - end(); } + }; // TODO(jd): Fill in other HTTP methods as needed diff --git a/client/app/util/Metrics.js b/client/app/util/Metrics.js index 3eb6cf0f798..b04408c115b 100644 --- a/client/app/util/Metrics.js +++ b/client/app/util/Metrics.js @@ -1,61 +1,8 @@ -import ApiUtil from './ApiUtil'; +import ApiUtil, { postMetricLogs } from './ApiUtil'; import _ from 'lodash'; import moment from 'moment'; import uuid from 'uuid'; -// ------------------------------------------------------------------------------------------ -// Histograms -// ------------------------------------------------------------------------------------------ - -const INTERVAL_TO_SEND_METRICS_MS = moment.duration(60, 'seconds'); - -let histograms = []; - -const sendHistogram = () => { - if (histograms.length === 0) { - return; - } - - ApiUtil.post('/metrics/v1/histogram', { data: { histograms } }). - catch((error) => { - console.error(error); - }); - histograms = []; -}; - -const initialize = _.once(() => { - // Only record values for a sample of our users. - if (_.random(2) === 0) { - // Add jitter to requests - setInterval(sendHistogram, INTERVAL_TO_SEND_METRICS_MS + _.random(moment.duration(5, 'seconds'))); - } -}); - -export const collectHistogram = (data) => { - initialize(); - - histograms.push(ApiUtil.convertToSnakeCase(data)); - - const id = uuid.v4(); - const metricsData = data; - const time = Date(Date.now()).toString(); - const readerData = { - message: 'Render document content for "' + data.attrs.documentType + '"', - type: 'performance', - product: 'pdfjs.document.render', - start:time, - end: Date(Date.now()).toString(), - duration: data.value, - } - - if(data.value > 0){ - storeMetrics(id,metricsData,readerData); - } - else if(data.attrs.pageCount < 2){ - storeMetrics(id,metricsData,readerData); - } -}; - // ------------------------------------------------------------------------------------------ // Metric Storage and recording // ------------------------------------------------------------------------------------------ @@ -113,7 +60,7 @@ export const storeMetrics = (uniqueId, data, { message, type = 'log', product, s } }; - ApiUtil.postMetricLogs('/metrics/v2/logs', { data: postData }); + postMetricLogs(postData); }; export const recordMetrics = (targetFunction, { uniqueId, data, message, type = 'log', product }, @@ -183,7 +130,58 @@ export const recordAsyncMetrics = async (asyncFunction, { uniqueId, data, messag storeMetrics(uniqueId, metricData, { message, type, product, start, end, duration }); } - return result; }; +// ------------------------------------------------------------------------------------------ +// Histograms +// ------------------------------------------------------------------------------------------ + +const INTERVAL_TO_SEND_METRICS_MS = moment.duration(60, 'seconds'); + +let histograms = []; + +const sendHistogram = () => { + if (histograms.length === 0) { + return; + } + + ApiUtil.post('/metrics/v1/histogram', { data: { histograms } }). + catch((error) => { + console.error(error); + }); + histograms = []; +}; + +const initialize = _.once(() => { + // Only record values for a sample of our users. + if (_.random(2) === 0) { + // Add jitter to requests + setInterval(sendHistogram, INTERVAL_TO_SEND_METRICS_MS + _.random(moment.duration(5, 'seconds'))); + } +}); + +export const collectHistogram = (data) => { + initialize(); + + histograms.push(ApiUtil.convertToSnakeCase(data)); + + const id = uuid.v4(); + const metricsData = data; + const time = Date(Date.now()).toString(); + const readerData = { + message: `Render document content for "${ data.attrs.documentType }"`, + type: 'performance', + product: 'pdfjs.document.render', + start: time, + end: Date(Date.now()).toString(), + duration: data.value, + }; + + if (data.value > 0) { + storeMetrics(id, metricsData, readerData); + } else if (data.attrs.pageCount < 2) { + storeMetrics(id, metricsData, readerData); + } +}; + From 3d594a1095dc2ec5bf19f40d5e4686f2fe19a413 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Fri, 30 Jun 2023 10:45:01 -0400 Subject: [PATCH 27/39] add additional storeMetrics for error catches and cleanup code --- client/app/2.0/store/reader/documentViewer.js | 29 +++++++++---------- client/app/index.js | 10 ++++--- client/app/reader/PdfFile.jsx | 18 ++++++++++-- client/app/reader/PdfPage.jsx | 21 ++++++++++++-- client/app/util/Metrics.js | 6 ++-- 5 files changed, 56 insertions(+), 28 deletions(-) diff --git a/client/app/2.0/store/reader/documentViewer.js b/client/app/2.0/store/reader/documentViewer.js index 2d33c7ffe02..605ffc0a713 100644 --- a/client/app/2.0/store/reader/documentViewer.js +++ b/client/app/2.0/store/reader/documentViewer.js @@ -233,7 +233,7 @@ export const showPage = async (params) => { export const showPdf = createAsyncThunk( 'documentViewer/show', async ( - { rotation = null, pageNumber, currentDocument, scale, featureToggles = {} }, + { rotation = null, pageNumber, currentDocument, scale }, { dispatch } ) => { // Update the Document as read if not already @@ -248,23 +248,20 @@ export const showPdf = createAsyncThunk( withCredentials: true, timeout: true, responseType: 'arraybuffer', - metricsLogRestError: featureToggles.metricsLogRestError }); - if (body) { - // Store the Document in-memory so that we do not serialize through Redux, but still persist - pdfDocuments[currentDocument.id] = { - pdf: await PDF.getDocument({ data: body }).promise, - }; - - // Store the pages for the PDF - pdfDocuments[currentDocument.id].pages = await Promise.all( - range(0, pdfDocuments[currentDocument.id].pdf.numPages).map( - (pageIndex) => - pdfDocuments[currentDocument.id].pdf.getPage(pageIndex + 1) - ) - ); - } + // Store the Document in-memory so that we do not serialize through Redux, but still persist + pdfDocuments[currentDocument.id] = { + pdf: await PDF.getDocument({ data: body }).promise, + }; + + // Store the pages for the PDF + pdfDocuments[currentDocument.id].pages = await Promise.all( + range(0, pdfDocuments[currentDocument.id].pdf.numPages).map( + (pageIndex) => + pdfDocuments[currentDocument.id].pdf.getPage(pageIndex + 1) + ) + ); } // Store the Viewport diff --git a/client/app/index.js b/client/app/index.js index b8fc8de52ea..94d8d5cd734 100644 --- a/client/app/index.js +++ b/client/app/index.js @@ -112,16 +112,18 @@ const componentWrapper = (component) => (props, railsContext, domNodeId) => { const t1 = performance.now(); const end = Date.now(); const duration = t1 - t0; + storeMetrics( id, data, { type: 'error', - product: 'browser', - start: start, - end: end, - duration: duration } + product: 'browser', + start, + end, + duration } ); } + return true; }; diff --git a/client/app/reader/PdfFile.jsx b/client/app/reader/PdfFile.jsx index ef54f533627..d66c170e32d 100644 --- a/client/app/reader/PdfFile.jsx +++ b/client/app/reader/PdfFile.jsx @@ -24,7 +24,7 @@ import { INTERACTION_TYPES } from '../reader/analytics'; import { getCurrentMatchIndex, getMatchesPerPageInFile, getSearchTerm } from './selectors'; import pdfjsWorker from 'pdfjs-dist/build/pdf.worker.entry'; import uuid from 'uuid'; -import { recordAsyncMetrics } from '../util/Metrics'; +import { storeMetrics, recordAsyncMetrics } from '../util/Metrics'; PDFJS.GlobalWorkerOptions.workerSrc = pdfjsWorker; @@ -94,7 +94,21 @@ export class PdfFile extends React.PureComponent { return this.props.setPdfDocument(this.props.file, this.pdfDocument); }, (reason) => this.onRejected(reason, 'setPdfDocument')). catch((error) => { - console.error(`${uuid.v4()} : GET ${this.props.file} : ${error}`); + const id = uuid.v4(); + const data = { + file: this.props.file + }; + const message = `${id} : GET ${this.props.file} : ${error}`; + + console.error(message); + storeMetrics( + id, + data, + { message, + type: 'error', + product: 'browser', + } + ); this.loadingTask = null; this.props.setDocumentLoadError(this.props.file); }); diff --git a/client/app/reader/PdfPage.jsx b/client/app/reader/PdfPage.jsx index 914fe9724f1..599f030d385 100644 --- a/client/app/reader/PdfPage.jsx +++ b/client/app/reader/PdfPage.jsx @@ -13,7 +13,7 @@ import { bindActionCreators } from 'redux'; import { PDF_PAGE_HEIGHT, PDF_PAGE_WIDTH, SEARCH_BAR_HEIGHT, PAGE_DIMENSION_SCALE, PAGE_MARGIN } from './constants'; import { pageNumberOfPageIndex } from './utils'; import * as PDFJS from 'pdfjs-dist'; -import { collectHistogram, recordMetrics, recordAsyncMetrics } from '../util/Metrics'; +import { collectHistogram, recordMetrics, recordAsyncMetrics, storeMetrics } from '../util/Metrics'; import { css } from 'glamor'; import classNames from 'classnames'; @@ -258,7 +258,6 @@ export class PdfPage extends React.PureComponent { const textResult = recordAsyncMetrics(this.getText(page), textMetricData, pageAndTextFeatureToggle); textResult.then((text) => { - this.drawText(page, text); recordMetrics(this.drawText(page, text), readerRenderText, this.props.featureToggles.metricsReaderRenderText); }); @@ -278,7 +277,23 @@ export class PdfPage extends React.PureComponent { }); }); }).catch((error) => { - console.error(`${uuid.v4()} : setUpPage ${this.props.file} : ${error}`); + const id = uuid.v4(); + const data = { + documentId: this.props.documentId, + documentType: this.props.documentType, + file: this.props.file + }; + const message = `${id} : setUpPage ${this.props.file} : ${error}`; + + console.error(message); + storeMetrics( + id, + data, + { message, + type: 'error', + product: 'browser', + } + ); }); } }; diff --git a/client/app/util/Metrics.js b/client/app/util/Metrics.js index b04408c115b..77d365a3f82 100644 --- a/client/app/util/Metrics.js +++ b/client/app/util/Metrics.js @@ -100,18 +100,18 @@ export const recordMetrics = (targetFunction, { uniqueId, data, message, type = * * Might need to split into async and promise versions if issues */ -export const recordAsyncMetrics = async (asyncFunction, { uniqueId, data, message, type = 'log', product }, +export const recordAsyncMetrics = async (promise, { uniqueId, data, message, type = 'log', product }, saveMetrics = true) => { let id = checkUuid(uniqueId, data, message, type); const t0 = performance.now(); const start = Date.now(); - const name = message || asyncFunction; + const name = message || promise; // eslint-disable-next-line no-console console.info(`STARTED: ${id} ${name}`); - const prom = () => asyncFunction; + const prom = () => promise; const result = await prom(); const t1 = performance.now(); const end = Date.now(); From 127cb4087ea97823aac8d16553476cec54ee5aae Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Fri, 30 Jun 2023 15:42:52 -0400 Subject: [PATCH 28/39] Update schema.rb --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index f77c20dbee9..723d42fad34 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_05_08_202742) do +ActiveRecord::Schema.define(version: 2023_05_23_174750) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" From a2682743c8b3b4be101c7f509f13d93938a5b972 Mon Sep 17 00:00:00 2001 From: Matt Roth Date: Fri, 30 Jun 2023 16:01:34 -0400 Subject: [PATCH 29/39] revert changes to client/app/2.0/screens/reader/DocumentViewer.jsx --- .../app/2.0/screens/reader/DocumentViewer.jsx | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/client/app/2.0/screens/reader/DocumentViewer.jsx b/client/app/2.0/screens/reader/DocumentViewer.jsx index 3923d45a079..f396c9db552 100644 --- a/client/app/2.0/screens/reader/DocumentViewer.jsx +++ b/client/app/2.0/screens/reader/DocumentViewer.jsx @@ -173,8 +173,7 @@ const DocumentViewer = (props) => { showPdf: (currentPage, currentDocument, scale) => dispatch(showPdf({ currentDocument, pageNumber: currentPage, - scale, - featureToggles: props.featureToggles, + scale })), toggleKeyboardInfo: (val) => dispatch(toggleKeyboardInfo(val)), startMove: (commentId) => dispatch(startMove(commentId)), @@ -293,16 +292,14 @@ const DocumentViewer = (props) => { dispatch(showPdf({ currentDocument: state.currentDocument, - scale: 1, - featureToggles: props.featureToggles, + scale: 1 })); }, rotateDocument: () => { dispatch(showPdf({ currentDocument: state.currentDocument, rotation: state.currentDocument.rotation, - scale: state.scale, - featureToggles: props.featureToggles, + scale: state.scale })); }, zoom: (direction) => { @@ -314,11 +311,7 @@ const DocumentViewer = (props) => { window.analyticsEvent(CATEGORIES.VIEW_DOCUMENT_PAGE, `zoom ${direction}`, scale); - dispatch(showPdf({ - currentDocument: state.currentDocument, - scale, - featureToggles: props.featureToggles, - })); + dispatch(showPdf({ currentDocument: state.currentDocument, scale })); }, setPageNumber: (pageNumber) => { // Add the analytics event @@ -364,8 +357,7 @@ const DocumentViewer = (props) => { if (currentDocument?.id) { dispatch(showPdf({ currentDocument, - scale: state.scale, - featureToggles: props.featureToggles, + scale: state.scale })); } else { // Load the Documents From 5ed9e0da8add635c9cbce7d690576996b8c0fabf Mon Sep 17 00:00:00 2001 From: SHarshain <133917878+SHarshain@users.noreply.github.com> Date: Mon, 10 Jul 2023 10:19:28 -0400 Subject: [PATCH 30/39] Updated the cache to return (#18971) Co-authored-by: SHarshain --- client/app/util/ApiUtil.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/client/app/util/ApiUtil.js b/client/app/util/ApiUtil.js index c9a71615350..cd36638d572 100644 --- a/client/app/util/ApiUtil.js +++ b/client/app/util/ApiUtil.js @@ -171,14 +171,17 @@ const httpMethods = { } if (options.cache) { - return promise; + return promise. + then((res) => { + successHandling(url, res, 'GET', options); + return res; + }); } return promise. use(nocache). then((res) => { successHandling(url, res, 'GET', options); - return res; }); }, From 1c22bcdb984df3170b0912719bcb28054df5d31f Mon Sep 17 00:00:00 2001 From: SHarshain <133917878+SHarshain@users.noreply.github.com> Date: Thu, 10 Aug 2023 11:29:11 -0400 Subject: [PATCH 31/39] added newrelic ignore helper (#19027) * added newrelic ignore helper * minor tweak --------- Co-authored-by: SHarshain --- app/controllers/idt/api/v2/appeals_controller.rb | 1 + app/controllers/reader/application_controller.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/app/controllers/idt/api/v2/appeals_controller.rb b/app/controllers/idt/api/v2/appeals_controller.rb index ce1d9526698..4b1c3bd936e 100644 --- a/app/controllers/idt/api/v2/appeals_controller.rb +++ b/app/controllers/idt/api/v2/appeals_controller.rb @@ -5,6 +5,7 @@ class Idt::Api::V2::AppealsController < Idt::Api::V1::BaseController protect_from_forgery with: :exception before_action :verify_access + newrelic_ignore skip_before_action :verify_authenticity_token, only: [:outcode] diff --git a/app/controllers/reader/application_controller.rb b/app/controllers/reader/application_controller.rb index 604e54522fc..0682ae07852 100644 --- a/app/controllers/reader/application_controller.rb +++ b/app/controllers/reader/application_controller.rb @@ -2,6 +2,7 @@ class Reader::ApplicationController < ApplicationController before_action :verify_access, :react_routed, :check_reader_out_of_service + newrelic_ignore def set_application RequestStore.store[:application] = "reader" From 5ca8f84f33877e4940eb73299778bf1c2849707a Mon Sep 17 00:00:00 2001 From: SHarshain <133917878+SHarshain@users.noreply.github.com> Date: Mon, 14 Aug 2023 14:10:41 -0400 Subject: [PATCH 32/39] Merge conflict fix 08 14 2023 (#19185) * remove all slack channels other than appeals-job-alerts * Update slack_service.rb * Add 508 ARIA label for screen readers in CF Help * Schema check in * Update enable_features_dev.rb * APPEALS-17497: MST & PACT Special issues identification * optimized participant contentions method to return contentions to cut down on BGS calls * added feature toggle guard clause for bgs calls for ratings * hid pact and mst check behind toggles * added caching to BGS contention call * updated feature toggles to include current user * adding logging for BGS contention calls * added 24 hr caching and commented out toggle for bgs call * APPEALS-17497: MST & PACT Special issues identification * optimized participant contentions method to return contentions to cut down on BGS calls * added feature toggle guard clause for bgs calls for ratings * hid pact and mst check behind toggles * added caching to BGS contention call * updated feature toggles to include current user * adding logging for BGS contention calls * added 24 hr caching and commented out toggle for bgs call * hid blue water and burn pit behind mst/pact toggle * added toggle for mst on special issues list * routed judge/attorney decisions to special issues pending on mst pact toggle * updated back button routing and special issues showing on decisions for ama appeals * hid mst pact special issues banner behind feature toggles * HunJerBAH APPEALS-26881: Reject Unknown Attributes in RatingDecisions Hash (#19072) * added handling for unknown attributes being added into rating decision hash * updated paren grouping * Update open_hearing_tasks_without_active_descendants_checker_spec.rb * allow slack notifications in uat * Add ro-colocated examples * Fix ro-colocated request body * add exception handling to audit remove script * Change address_line_1 to nullable: true * Add migration * Allow audit table entry for address_line_1 to have null val * test update to yarn dependency cache keys * Take setup-node for a spin * Revert "Take setup-node for a spin" This reverts commit 337ea0a23b553d4f4835acabcfa1610b3a55df66. * Add a spec * Fix whitespace * Remove flipper tables * unskip test (#19076) Co-authored-by: Craig Reese <109101548+craigrva@users.noreply.github.com> * hotfix/APPEALS-27124 Updated MailRequestValidator and VbmsDistributionDestination spec for derived destinations * updated deserialize method to check on rating decision model for attributes in hash * Add class checks for ro-colocated? * Craig/test yarn cache (#19096) modify .github/workflows/workflow.yml * exclude db/scripts in codeclimate (#19103) * Revert "APPEALS-17497: MST & PACT Special issues identification" * Use case * remove z-index * add z-index * Update webdrivers * Revert "Update webdrivers" This reverts commit 7cd71cd54709f6d15e2ca6b5c8eb7734c2fe9e5f. * Test wait * Reverse test update * test adding assertion (#19127) * test adding assertion * added assertion to more tests * add concurrency to workflow.yml (#19136) * add concurrency to workflow.yml --------- Co-authored-by: Matt Roth Co-authored-by: Matt Roth Co-authored-by: AimanK Co-authored-by: Aiman Kayad Co-authored-by: raymond-hughes Co-authored-by: youfoundmanesh Co-authored-by: HunJerBAH Co-authored-by: HunJerBAH <99915461+HunJerBAH@users.noreply.github.com> Co-authored-by: Matthew Thornton Co-authored-by: kristeja <112115264+kristeja@users.noreply.github.com> Co-authored-by: kristeja Co-authored-by: Craig Reese Co-authored-by: Matthew Thornton <99351305+ThorntonMatthew@users.noreply.github.com> Co-authored-by: Clay Sheppard Co-authored-by: Craig Reese <109101548+craigrva@users.noreply.github.com> Co-authored-by: Jeff Marks Co-authored-by: Raymond Hughes <131811099+raymond-hughes@users.noreply.github.com> Co-authored-by: SHarshain --- .codeclimate.yml | 1 + .github/workflows/workflow.yml | 36 +++++--- README.md | 2 +- .../api/docs/pacman/idt-pacman-spec.yml | 92 +++++++++++++++++++ app/jobs/amo_metrics_report_job.rb | 4 +- app/jobs/data_integrity_checks_job.rb | 4 +- app/jobs/etl_builder_job.rb | 2 - ...foreign_key_polymorphic_association_job.rb | 3 +- .../push_priority_appeals_to_judges_job.rb | 4 +- ...more_than_one_open_hearing_task_checker.rb | 4 - app/services/data_integrity_checker.rb | 5 - app/services/decision_date_checker.rb | 4 - app/services/etl/syncer.rb | 2 +- app/services/expired_async_jobs_checker.rb | 4 - .../multiple_open_root_child_task_checker.rb | 4 - .../open_tasks_with_closed_at_checker.rb | 4 - .../open_tasks_with_parent_not_on_hold.rb | 4 - ...reviews_with_duplicate_ep_error_checker.rb | 4 - app/services/slack_service.rb | 4 +- app/services/stuck_appeals_checker.rb | 4 - .../stuck_virtual_hearings_checker.rb | 5 - ...asks_assigned_to_inactive_users_checker.rb | 4 - app/validators/mail_request_validator.rb | 34 +++++++ client/app/help/HelpRootView.jsx | 27 ++++-- client/app/styles/_noncomp.scss | 2 +- ...0731194341_make_address_line_1_nullable.rb | 5 + db/schema.rb | 8 +- .../audit/remove_caseflow_audit_schema.rb | 18 +++- ...te_vbms_distribution_destinations_audit.rb | 2 +- ...e_vbms_distribution_destinations_audit.sql | 2 +- docs/tech-specs/README.md | 4 +- scripts/enable_features_dev.rb | 5 +- spec/factories/mail_request.rb | 18 ++++ spec/feature/hearings/add_hearing_day_spec.rb | 6 +- .../hearings/assign_hearings_table_spec.rb | 3 + .../edit_hearsched_spec.rb | 3 + .../feature/hearings/edit_hearing_day_spec.rb | 4 + .../hearing_worksheet/hearing_prep_spec.rb | 4 +- .../schedule_veteran/build_hearsched_spec.rb | 3 + spec/jobs/data_integrity_checks_job_spec.rb | 3 +- spec/jobs/etl_builder_job_spec.rb | 3 +- .../vbms_distribution_destination_spec.rb | 62 +++++++++---- ...than_one_open_hearing_task_checker_spec.rb | 6 -- spec/services/etl/syncer_spec.rb | 2 +- ...tiple_open_root_child_task_checker_spec.rb | 5 - ...without_active_descendants_checker_spec.rb | 6 -- ...nd_uncancelled_task_timers_checker_spec.rb | 1 - spec/services/slack_service_spec.rb | 6 +- .../stuck_virtual_hearings_checker_spec.rb | 6 -- spec/workflows/mail_request_spec.rb | 36 ++++++-- 50 files changed, 327 insertions(+), 157 deletions(-) create mode 100644 db/migrate/20230731194341_make_address_line_1_nullable.rb diff --git a/.codeclimate.yml b/.codeclimate.yml index a8e4f40fa31..bc142b4e9d5 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -116,6 +116,7 @@ plugins: exclude_patterns: - 'db/schema.rb' - 'db/seeds.rb' + - 'db/scripts/*' - 'node_modules/**/*' - 'app/mappers/zip_code_to_lat_lng_mapper.rb' - 'tmp/**/*' diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 622314d76f0..2a1f53c9cca 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -10,6 +10,10 @@ env: rspec_active: true FORCE_COLOR: "1" #Forces color within GHA - Note RSPEC still won't use color see line 199 --tty for rspec color +concurrency: + group: ${{ github.ref }} + cancel-in-progress: true + jobs: # This job runs the main deployment of caseflow caseflow_rspec_job: @@ -76,30 +80,38 @@ jobs: steps: - uses: actions/checkout@v3 + # If we don't explicitly set this, the runner doesn't find the path when trying to save the cache + - name: Set yarn cache directory + id: set-yarn-cache-dir + run: mkdir -p ~/.cache/yarn && yarn config set cache-folder ~/.cache/yarn + - name: restore yarn cache id: cache-yarn-cache uses: actions/cache/restore@v3 with: - key: dot-cache-yarn-v2-{{ arch }}-{{ checksum "client/yarn.lock" }} + # hashFiles('client/yarn.lock') will use a unique cache based on dependencies so that we don't + # create a cache for each target branch + key: yarn-cache-${{ hashFiles('client/yarn.lock') }} + # We are including node_modules because most of the time is used to build the dependencies path: | + node_modules + client/node_modules ~/.cache/yarn - public/assets - tmp/cache/assets/sprockets - restore-keys: - dot-cache-yarn-v2-{{ arch }}-{{ checksum "client/yarn.lock" }} + restore-keys: yarn-cache-${{ hashFiles('client/yarn.lock') }} + # We run yarn install after loading the cache to update any dependencies if their version is different - name: Install Node Dependencies - run: ./ci-bin/capture-log "cd client && yarn install --frozen-lockfile" + run: ./ci-bin/capture-log "cd client && yarn install --frozen-lockfile --prefer-offline" - name: Save Yarn Cache if: steps.cache-yarn-cache.outputs.cache-hit != 'true' uses: actions/cache/save@v3 with: - key: dot-cache-yarn-v2-{{ arch }}-{{ checksum "client/yarn.lock" }} + key: yarn-cache-${{ hashFiles('client/yarn.lock') }} path: | + node_modules + client/node_modules ~/.cache/yarn - public/assets - tmp/cache/assets/sprockets - name: setup testfiles directory run: ./ci-bin/capture-log "mkdir -p tmp/testfiles" @@ -178,9 +190,11 @@ jobs: ./ci-bin/capture-log "DB=etl bundle exec rake db:create db:schema:load db:migrate" ./ci-bin/capture-log "bundle exec rake db:create db:schema:load db:migrate" - - name: make seed-dbs + # We don't want to seed DBs here because DatabaseCleaner just truncates it anyway. The setup_vacols + # rake task needs to be run because it adds data to two tables that are ignored by DBCleaner + - name: Seed databases run: | - ./ci-bin/capture-log "make -f Makefile.example seed-dbs" + ./ci-bin/capture-log "bundle exec rake spec:setup_vacols" - name: Assets Precompile run: | diff --git a/README.md b/README.md index 3e1e7322ddc..a9f82e9f92d 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ Missing test coverage will be reported automatically at the end of the test run. --- ## Debugging FACOLS setup ####################################################### -See debugging steps as well as more information about FACOLS in our [wiki](https://github.com/department-of-veterans-affairs/caseflow/wiki/FACOLS#debugging-facols) or join the DSVA slack channel #appeals-facols-issues. +See debugging steps as well as more information about FACOLS in our [wiki](https://github.com/department-of-veterans-affairs/caseflow/wiki/FACOLS#debugging-facols) or join the DSVA slack channel #appeals-development. Review the [FACOLS documentation](docs/FACOLS.md) for details. diff --git a/app/controllers/api/docs/pacman/idt-pacman-spec.yml b/app/controllers/api/docs/pacman/idt-pacman-spec.yml index 0d094879c00..88d49d05851 100644 --- a/app/controllers/api/docs/pacman/idt-pacman-spec.yml +++ b/app/controllers/api/docs/pacman/idt-pacman-spec.yml @@ -187,10 +187,27 @@ paths: schema: oneOf: - $ref: '#/components/schemas/UploadDocumentRequestWithRecipientInformation' + - $ref: '#/components/schemas/UploadDocumentRequestWithRoColocatedRecipientInformation' - $ref: '#/components/schemas/UploadDocumentRequest' examples: "With recipient information for Package Manager": $ref: '#/components/schemas/UploadDocumentRequestWithRecipientInformation' + "With ro-colocated recipient information for Package Manager": + value: + veteran_identifier: '555555555' + document_type: 'BVA Decision' + document_subject: 'Test' + document_name: 'Test Doc' + file: 'VGhpcyBpcyBhIHRlc3QuIERvIG5vdCBiZSBhbGFybWVkLgo=' + recipient_info: + - { + recipient_type: "ro-colocated", + name: "POA Name", + claimant_station_of_jurisdiction: "123", + poa_code: "02A", + destination_type: "derived", + copies: 1 + } "Without recipient information for Package Manager": value: veteran_identifier: '555555555' @@ -389,10 +406,27 @@ paths: schema: oneOf: - $ref: '#/components/schemas/UploadDocumentRequestWithRecipientInformation' + - $ref: '#/components/schemas/UploadDocumentRequestWithRoColocatedRecipientInformation' - $ref: '#/components/schemas/UploadDocumentRequest' examples: "With recipient information for Package Manager": $ref: '#/components/schemas/UploadDocumentRequestWithRecipientInformation' + "With ro-colocated recipient information for Package Manager": + value: + veteran_identifier: '555555555' + document_type: 'BVA Decision' + document_subject: 'Test' + document_name: 'Test Doc' + file: 'VGhpcyBpcyBhIHRlc3QuIERvIG5vdCBiZSBhbGFybWVkLgo=' + recipient_info: + - { + recipient_type: "ro-colocated", + name: "POA Name", + claimant_station_of_jurisdiction: "123", + poa_code: "02A", + destination_type: "derived", + copies: 1 + } "Without recipient information for Package Manager": value: veteran_identifier: '555555555' @@ -600,10 +634,26 @@ paths: schema: oneOf: - $ref: '#/components/schemas/OutcodeRequestWithRecipientInformation' + - $ref: '#/components/schemas/OutcodeRequestWithRoColocatedRecipientInformation' - $ref: '#/components/schemas/OutcodeRequest' examples: "With recipient information for Package Manager": $ref: '#/components/schemas/OutcodeRequestWithRecipientInformation' + "With ro-colocated recipient information for Package Manager": + value: + citation_number: "A19062122" + decision_date: "December 21, 2022" + redacted_document_location: "\\path.va.gov\\archdata$\\some-file.pdf" + file: "VGVzdGluZyAxMjMK" + recipient_info: + - { + recipient_type: "ro-colocated", + name: "POA Name", + claimant_station_of_jurisdiction: "123", + poa_code: "02A", + destination_type: "derived", + copies: 1 + } "Without recipient information for Package Manager": value: citation_number: "A19062122" @@ -960,6 +1010,16 @@ components: items: type: object $ref: '#/components/schemas/RecipientRequestInformation' + UploadDocumentRequestWithRoColocatedRecipientInformation: + allOf: + - $ref: '#/components/schemas/UploadDocumentRequest' + - type: object + properties: + recipient_info: + type: array + items: + type: object + $ref: '#/components/schemas/RoColocatedRecipientRequestInformation' UploadDocumentRequest: type: object properties: @@ -992,6 +1052,38 @@ components: items: type: object $ref: '#/components/schemas/RecipientRequestInformation' + OutcodeRequestWithRoColocatedRecipientInformation: + allOf: + - $ref: '#/components/schemas/OutcodeRequest' + - type: object + properties: + recipient_info: + type: array + items: + type: object + $ref: '#/components/schemas/RoColocatedRecipientRequestInformation' + RoColocatedRecipientRequestInformation: + type: object + properties: + recipient_type: + type: string + example: 'ro-colocated' + name: + description: 'Required if recipient_type is organization, system, or ro-colocated. Unused for people.' + type: string + example: "POA Name" + claimant_station_of_jurisdiction: + type: string + description: 'Required if recipient_type is ro-colocated.' + example: "123" + postal_code: + type: string + destination_type: + type: string + example: "derived" + copies: + type: integer + example: 1 RecipientRequestInformation: type: object properties: diff --git a/app/jobs/amo_metrics_report_job.rb b/app/jobs/amo_metrics_report_job.rb index ec34181464c..d7433598f78 100644 --- a/app/jobs/amo_metrics_report_job.rb +++ b/app/jobs/amo_metrics_report_job.rb @@ -7,8 +7,6 @@ class AMOMetricsReportJob < CaseflowJob queue_with_priority :low_priority application_attr :intake - SLACK_CHANNEL = "#caseflow-vbms-intake" - def perform setup_dates async_stats = ClaimReviewAsyncStatsReporter.new(start_date: start_date, end_date: end_date) @@ -33,7 +31,7 @@ def setup_dates def send_report(async_stats:) msg = build_report(async_stats) - slack_service.send_notification(msg, self.class.to_s, SLACK_CHANNEL) + slack_service.send_notification(msg, self.class.to_s) end # rubocop:disable Metrics/AbcSize diff --git a/app/jobs/data_integrity_checks_job.rb b/app/jobs/data_integrity_checks_job.rb index f15ee259840..cbd9886e9b9 100644 --- a/app/jobs/data_integrity_checks_job.rb +++ b/app/jobs/data_integrity_checks_job.rb @@ -39,7 +39,7 @@ def perform log_error(error, extra: { checker: klass }) slack_msg = "Error running #{klass}." slack_msg += " See Sentry event #{Raven.last_event_id}" if Raven.last_event_id.present? - slack_service.send_notification(slack_msg, klass, checker.slack_channel) + slack_service.send_notification(slack_msg, klass) end end @@ -52,6 +52,6 @@ def report_msg(msg) end def send_to_slack(checker) - slack_service.send_notification(report_msg(checker.report), checker.class.name, checker.slack_channel) + slack_service.send_notification(report_msg(checker.report), checker.class.name) end end diff --git a/app/jobs/etl_builder_job.rb b/app/jobs/etl_builder_job.rb index afc0f944bed..a43bf6e7f35 100644 --- a/app/jobs/etl_builder_job.rb +++ b/app/jobs/etl_builder_job.rb @@ -19,8 +19,6 @@ def perform slack_msg = "Error running #{klass}." slack_msg += " See Sentry event #{Raven.last_event_id}" if Raven.last_event_id.present? slack_service.send_notification(slack_msg, klass) - # Also send a message to #appeals-data-workgroup - slack_service.send_notification(slack_msg, klass, "#appeals-data-workgroup") end private diff --git a/app/jobs/foreign_key_polymorphic_association_job.rb b/app/jobs/foreign_key_polymorphic_association_job.rb index 36f40bcdd19..f6ce3af6376 100644 --- a/app/jobs/foreign_key_polymorphic_association_job.rb +++ b/app/jobs/foreign_key_polymorphic_association_job.rb @@ -76,8 +76,7 @@ def send_alert(heading, klass, config, record_ids) (id, #{config[:type_column]}, #{config[:id_column]}) #{record_ids.map(&:to_s).join("\n")} MSG - slack_service.send_notification(message, "#{klass.name} orphaned records via #{config[:id_column]}", - "#appeals-data-workgroup") + slack_service.send_notification(message, "#{klass.name} orphaned records via #{config[:id_column]}") end # Maps the includes_method to a hash containing all the possible types. Each hash entry is: diff --git a/app/jobs/push_priority_appeals_to_judges_job.rb b/app/jobs/push_priority_appeals_to_judges_job.rb index bdb2ccc2363..c5685f5ae40 100644 --- a/app/jobs/push_priority_appeals_to_judges_job.rb +++ b/app/jobs/push_priority_appeals_to_judges_job.rb @@ -23,14 +23,14 @@ def perform start_time ||= Time.zone.now # temporary fix to get this job to succeed duration = time_ago_in_words(start_time) slack_msg = "\n [ERROR] after running for #{duration}: #{error.message}" - slack_service.send_notification(slack_msg, self.class.name, "#appeals-job-alerts") + slack_service.send_notification(slack_msg, self.class.name) log_error(error) ensure datadog_report_runtime(metric_group_name: "priority_appeal_push_job") end def send_job_report - slack_service.send_notification(slack_report.join("\n"), self.class.name, "#appeals-job-alerts") + slack_service.send_notification(slack_report.join("\n"), self.class.name) end def slack_report diff --git a/app/services/appeals_with_more_than_one_open_hearing_task_checker.rb b/app/services/appeals_with_more_than_one_open_hearing_task_checker.rb index 914d7d2aa67..c3036aaf30e 100644 --- a/app/services/appeals_with_more_than_one_open_hearing_task_checker.rb +++ b/app/services/appeals_with_more_than_one_open_hearing_task_checker.rb @@ -6,10 +6,6 @@ def call build_report(appeals_with_more_than_one_open_hearing_task) end - def slack_channel - "#appeals-tango" - end - private HELP_DOCUMENT_LINK = "https://github.com/department-of-veterans-affairs/appeals-deployment/" \ diff --git a/app/services/data_integrity_checker.rb b/app/services/data_integrity_checker.rb index cab80455ed3..84cd27c87d9 100644 --- a/app/services/data_integrity_checker.rb +++ b/app/services/data_integrity_checker.rb @@ -30,9 +30,4 @@ def add_to_report(msg) def add_to_buffer(thing) @buffer << thing end - - def slack_channel - "#appeals-job-alerts" - # override this to specify a different channel - end end diff --git a/app/services/decision_date_checker.rb b/app/services/decision_date_checker.rb index e431dbe23b7..94e114dd55b 100644 --- a/app/services/decision_date_checker.rb +++ b/app/services/decision_date_checker.rb @@ -5,10 +5,6 @@ def call build_report end - def slack_channel - "#appeals-foxtrot" - end - private def request_issues_without_decision_date diff --git a/app/services/etl/syncer.rb b/app/services/etl/syncer.rb index a5db37a8f20..93b15e67411 100644 --- a/app/services/etl/syncer.rb +++ b/app/services/etl/syncer.rb @@ -38,7 +38,7 @@ def dump_messages_to_slack(target_class) return unless target_class.messages slack_msg = target_class.messages.join("\n") - slack_service.send_notification(slack_msg, target_class.name, "#appeals-data-workgroup") + slack_service.send_notification(slack_msg, target_class.name) target_class.clear_messages end diff --git a/app/services/expired_async_jobs_checker.rb b/app/services/expired_async_jobs_checker.rb index 9b5d1b987e3..7d014ebeb57 100644 --- a/app/services/expired_async_jobs_checker.rb +++ b/app/services/expired_async_jobs_checker.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true class ExpiredAsyncJobsChecker < DataIntegrityChecker - def slack_channel - "#appeals-foxtrot" - end - def call jobs = AsyncableJobs.new(page_size: -1).jobs.select(&:expired_without_processing?) job_reporter = AsyncableJobsReporter.new(jobs: jobs) diff --git a/app/services/multiple_open_root_child_task_checker.rb b/app/services/multiple_open_root_child_task_checker.rb index f0fa0501f52..17ae4bec089 100644 --- a/app/services/multiple_open_root_child_task_checker.rb +++ b/app/services/multiple_open_root_child_task_checker.rb @@ -13,10 +13,6 @@ def call build_report(appeals_with_multiple_open_root_child_task) end - def slack_channel - "#appeals-echo" - end - def self.open_exclusive_root_children_tasks(appeal) appeal.tasks.open.of_type(EXCLUSIVE_OPEN_TASKS).where(parent: appeal.root_task) end diff --git a/app/services/open_tasks_with_closed_at_checker.rb b/app/services/open_tasks_with_closed_at_checker.rb index a598ae341d2..4eff5241786 100644 --- a/app/services/open_tasks_with_closed_at_checker.rb +++ b/app/services/open_tasks_with_closed_at_checker.rb @@ -10,10 +10,6 @@ def call add_to_report "These tasks likely were manually re-opened and should have closed_at set to NULL" end - def slack_channel - "#appeals-echo" - end - private def open_tasks_with_closed_at_defined diff --git a/app/services/open_tasks_with_parent_not_on_hold.rb b/app/services/open_tasks_with_parent_not_on_hold.rb index 94f97f85171..46cd31eea88 100644 --- a/app/services/open_tasks_with_parent_not_on_hold.rb +++ b/app/services/open_tasks_with_parent_not_on_hold.rb @@ -15,10 +15,6 @@ def call end end - def slack_channel - "#appeals-echo" - end - private def open_tasks_with_parent_not_on_hold diff --git a/app/services/reviews_with_duplicate_ep_error_checker.rb b/app/services/reviews_with_duplicate_ep_error_checker.rb index 9e4ed391770..83b4fd3f57a 100644 --- a/app/services/reviews_with_duplicate_ep_error_checker.rb +++ b/app/services/reviews_with_duplicate_ep_error_checker.rb @@ -7,10 +7,6 @@ def call build_report(higher_level_review_ids, supplemental_claim_ids) end - def slack_channel - "#appeals-foxtrot" - end - private ERROR_SELECTOR = "establishment_error ILIKE '%duplicateep%'" diff --git a/app/services/slack_service.rb b/app/services/slack_service.rb index c84a4fb570e..78f896a284d 100644 --- a/app/services/slack_service.rb +++ b/app/services/slack_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class SlackService - DEFAULT_CHANNEL = "#appeals-job-alerts" + DEFAULT_CHANNEL = Rails.deploy_env?(:prod) ? "#appeals-job-alerts" : "#appeals-uat-alerts" COLORS = { error: "#ff0000", info: "#cccccc", @@ -15,7 +15,7 @@ def initialize(url:) attr_reader :url def send_notification(msg, title = "", channel = DEFAULT_CHANNEL) - return unless url && (aws_env != "uat") + return unless url && (aws_env == "uat" || aws_env == "prod") slack_msg = format_slack_msg(msg, title, channel) diff --git a/app/services/stuck_appeals_checker.rb b/app/services/stuck_appeals_checker.rb index b0181ac3a60..1e2a2f66ad4 100644 --- a/app/services/stuck_appeals_checker.rb +++ b/app/services/stuck_appeals_checker.rb @@ -1,10 +1,6 @@ # frozen_string_literal: true class StuckAppealsChecker < DataIntegrityChecker - def slack_channel - "#appeals-echo" - end - def call return if stuck_appeals.count == 0 && appeals_maybe_not_closed.count == 0 diff --git a/app/services/stuck_virtual_hearings_checker.rb b/app/services/stuck_virtual_hearings_checker.rb index cd5a3c9fcda..dc1f548cada 100644 --- a/app/services/stuck_virtual_hearings_checker.rb +++ b/app/services/stuck_virtual_hearings_checker.rb @@ -10,11 +10,6 @@ def call build_report(stuck_virtual_hearings) end - # sending to appeals-tango for now, might later change to #appeals-hearings - def slack_channel - "#appeals-tango" - end - private TRACKING_DOCUMENT_LINK = "https://hackmd.io/DKPyLFB7QHuw6JuuTfc_8A" diff --git a/app/services/tasks_assigned_to_inactive_users_checker.rb b/app/services/tasks_assigned_to_inactive_users_checker.rb index c86c7ecd9e0..bfe13019803 100644 --- a/app/services/tasks_assigned_to_inactive_users_checker.rb +++ b/app/services/tasks_assigned_to_inactive_users_checker.rb @@ -4,10 +4,6 @@ # Checks for all open tasks assigned to inactive users. class TasksAssignedToInactiveUsersChecker < DataIntegrityChecker - def slack_channel - "#appeals-echo" - end - def call return if tasks_for_inactive_users.count == 0 diff --git a/app/validators/mail_request_validator.rb b/app/validators/mail_request_validator.rb index 3f3acbcc273..5a64c2036d7 100644 --- a/app/validators/mail_request_validator.rb +++ b/app/validators/mail_request_validator.rb @@ -35,6 +35,18 @@ module DistributionDestination validates :country_name, if: -> { destination_type == "internationalAddress" } end + # If destination type is derived: + # - The recipient type of associated distribution must be ro-colocated + # - All physical address fields must be null/blank + with_options if: -> { destination_type == "derived" } do + validate :recipient_type_ro_colocated? + validates(*PHYSICAL_ADDRESS_FIELDS, absence: true) + end + # And vice versa: if recipient type is ro-colocated, destination type must be derived + validates :destination_type, + inclusion: { in: ["derived"], message: "must be derived if recipient type is ro-colocated" }, + if: -> { ro_colocated? } + validates :treat_line_2_as_addressee, inclusion: { in: [true], message: "cannot be false if line 3 is treated as addressee" }, if: -> { treat_line_3_as_addressee == true } @@ -49,10 +61,32 @@ def physical_mail? %w[domesticAddress internationalAddress militaryAddress].include?(destination_type) end + PHYSICAL_ADDRESS_FIELDS = [ + :address_line_1, :address_line_2, :address_line_3, :address_line_4, :address_line_5, :address_line_6, + :treat_line_2_as_addressee, :treat_line_3_as_addressee, :city, :state, :postal_code, :country_name, + :country_code + ].freeze + def us_address? %w[domesticAddress militaryAddress].include?(destination_type) end + def ro_colocated? + case self + when MailRequest then recipient_type == "ro-colocated" + when VbmsDistributionDestination then vbms_distribution&.recipient_type == "ro-colocated" + else + false + end + end + + def recipient_type_ro_colocated? + unless ro_colocated? + errors.add(:destination_type, + "cannot be derived unless recipient type of associated distribution is ro-colocated") + end + end + def valid_country_code? unless iso_country_codes.include?(country_code) errors.add(:country_code, "is not a valid ISO 3166-2 code") diff --git a/client/app/help/HelpRootView.jsx b/client/app/help/HelpRootView.jsx index 115620d135d..264a0001444 100644 --- a/client/app/help/HelpRootView.jsx +++ b/client/app/help/HelpRootView.jsx @@ -6,29 +6,36 @@ const HelpRootView = () => { const pages = [ { name: 'Certification Help', - url: '/certification/help' }, + url: '/certification/help', + ariaLabel: 'Click for help resources/frequently asked questions on using Caseflow Certification' }, { name: 'Dispatch Help', - url: '/dispatch/help' }, + url: '/dispatch/help', + ariaLabel: 'Click for help resources/frequently asked questions on using Caseflow Dispatch' }, { name: 'Reader Help', - url: '/reader/help' }, + url: '/reader/help', + ariaLabel: 'Click for help resources/frequently asked questions on using Caseflow Reader' }, { name: 'Hearings Help', - url: '/hearing_prep/help' }, + url: '/hearing_prep/help', + ariaLabel: 'Click for help resources/frequently asked questions on using Caseflow Hearings' }, { name: 'Intake Help', - url: '/intake/help' }, + url: '/intake/help', + ariaLabel: 'Click for help resources/frequently asked questions on using Caseflow Intake' }, { name: 'Queue Help', - url: '/queue/help' }, + url: '/queue/help', + ariaLabel: 'Click for help resources/frequently asked questions on using Caseflow Queue' }, { name: 'VHA Help', - url: '/vha/help' }, + url: '/vha/help', + ariaLabel: 'Click for help resources/frequently asked questions on using Caseflow VHA' }, ]; return

Go Back

-

Caseflow Help

+

Caseflow Help

    - {pages.map(({ name, url }) => -
  • {name}
  • + {pages.map(({ name, url, ariaLabel }) => +
  • {name}
  • )}
; diff --git a/client/app/styles/_noncomp.scss b/client/app/styles/_noncomp.scss index 85ddeef3ee5..e8b0ab1e622 100644 --- a/client/app/styles/_noncomp.scss +++ b/client/app/styles/_noncomp.scss @@ -138,7 +138,7 @@ .cf-noncomp-search { position: absolute; right: 0; - z-index: 99999; + z-index: 1; } .cf-pagination-pages { diff --git a/db/migrate/20230731194341_make_address_line_1_nullable.rb b/db/migrate/20230731194341_make_address_line_1_nullable.rb new file mode 100644 index 00000000000..c96c3815ebd --- /dev/null +++ b/db/migrate/20230731194341_make_address_line_1_nullable.rb @@ -0,0 +1,5 @@ +class MakeAddressLine1Nullable < Caseflow::Migration + def change + change_column_null(:vbms_distribution_destinations, :address_line_1, true) + end +end diff --git a/db/schema.rb b/db/schema.rb index c2b16e78ef4..f815801609f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_06_29_184615) do +ActiveRecord::Schema.define(version: 2023_07_31_194341) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1293,7 +1293,7 @@ t.string "appeals_type", null: false, comment: "Type of Appeal" t.datetime "created_at", comment: "Timestamp of when Noticiation was Created" t.boolean "email_enabled", default: true, null: false - t.text "email_notification_content", comment: "Full Email Text Content of Notification" + t.string "email_notification_content", comment: "Full Email Text Content of Notification" t.string "email_notification_external_id", comment: "VA Notify Notification Id for the email notification send through their API " t.string "email_notification_status", comment: "Status of the Email Notification" t.date "event_date", null: false, comment: "Date of Event" @@ -1304,7 +1304,7 @@ t.string "participant_id", comment: "ID of Participant" t.string "recipient_email", comment: "Participant's Email Address" t.string "recipient_phone_number", comment: "Participants Phone Number" - t.text "sms_notification_content", comment: "Full SMS Text Content of Notification" + t.string "sms_notification_content", comment: "Full SMS Text Content of Notification" t.string "sms_notification_external_id", comment: "VA Notify Notification Id for the sms notification send through their API " t.string "sms_notification_status", comment: "Status of SMS/Text Notification" t.datetime "updated_at", comment: "TImestamp of when Notification was Updated" @@ -1836,7 +1836,7 @@ end create_table "vbms_distribution_destinations", force: :cascade do |t| - t.string "address_line_1", null: false, comment: "PII. If destination_type is domestic, international, or military then Must not be null." + t.string "address_line_1", comment: "PII. If destination_type is domestic, international, or military then Must not be null." t.string "address_line_2", comment: "PII. If treatLine2AsAddressee is [true] then must not be null" t.string "address_line_3", comment: "PII. If treatLine3AsAddressee is [true] then must not be null" t.string "address_line_4", comment: "PII." diff --git a/db/scripts/audit/remove_caseflow_audit_schema.rb b/db/scripts/audit/remove_caseflow_audit_schema.rb index 344617e8aa8..b1a0cb7ea5e 100644 --- a/db/scripts/audit/remove_caseflow_audit_schema.rb +++ b/db/scripts/audit/remove_caseflow_audit_schema.rb @@ -2,8 +2,16 @@ require "pg" -conn = CaseflowRecord.connection -conn.execute( - "drop schema IF EXISTS caseflow_audit CASCADE;" -) -conn.close +begin + conn = CaseflowRecord.connection + conn.execute( + "drop schema IF EXISTS caseflow_audit CASCADE;" + ) + conn.close +rescue ActiveRecord::NoDatabaseError => error + if error.message.include?('database "caseflow_certification_development" does not exist') + puts "Database caseflow_certification_development does not exist; skipping make audit-remove" + else + raise error + end +end diff --git a/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.rb b/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.rb index ec1e1e3973b..13dad17b764 100644 --- a/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.rb +++ b/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.rb @@ -8,7 +8,7 @@ type_of_change CHAR(1) not null, vbms_distribution_destinations_id bigint not null, destination_type varchar NOT NULL, - address_line_1 varchar NOT NULL, + address_line_1 varchar NULL, address_line_2 varchar NULL, address_line_3 varchar NULL, address_line_4 varchar NULL, diff --git a/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.sql b/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.sql index bc27fea7588..8ad4525488a 100644 --- a/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.sql +++ b/db/scripts/audit/tables/create_vbms_distribution_destinations_audit.sql @@ -3,7 +3,7 @@ create table caseflow_audit.vbms_distribution_destinations_audit ( type_of_change CHAR(1) not null, vbms_distribution_destinations_id bigint not null, destination_type varchar NOT NULL, - address_line_1 varchar NOT NULL, + address_line_1 varchar NULL, address_line_2 varchar NULL, address_line_3 varchar NULL, address_line_4 varchar NULL, diff --git a/docs/tech-specs/README.md b/docs/tech-specs/README.md index b12111259aa..3d9efc8e903 100644 --- a/docs/tech-specs/README.md +++ b/docs/tech-specs/README.md @@ -18,7 +18,7 @@ This folder contains tech specs for the Caseflow team. ## In order to write a tech spec and get it reviewed do the following: 1. Clone this repo 2. Start drafting a tech spec by opening a PR creating a file titled YYYY-MM-DD-(tech-spec-name).md in this folder with your tech spec. You can use the [tech spec template here](https://github.com/department-of-veterans-affairs/caseflow/blob/master/.github/ISSUE_TEMPLATE/tech-spec.md) -3. Post in #appeals-engineering and request for others to review the tech spec +3. Post in #appeals-development and request for others to review the tech spec 4. Schedule time on the VA Appeals calendar with your scrum team or the whole Caseflow engineering team as appropriate to discuss the tech spec 5. Once comments are addressed, the tech spec is finalized and your PR is approved, merge your commit to master so that the tech spec is preserved for future team mates -6. Link your tech spec in future PRs that implement the changes \ No newline at end of file +6. Link your tech spec in future PRs that implement the changes diff --git a/scripts/enable_features_dev.rb b/scripts/enable_features_dev.rb index f6053eedb20..f15d0ec5e18 100644 --- a/scripts/enable_features_dev.rb +++ b/scripts/enable_features_dev.rb @@ -59,7 +59,10 @@ def call cavc_dashboard_workflow poa_auto_refresh interface_version_2 - cc_vacatur_visibility + cc_vacatur_visibility, + acd_disable_legacy_distributions, + acd_disable_nonpriority_distributions, + acd_disable_legacy_lock_ready_appeals ] all_features = AllFeatureToggles.new.call.flatten.uniq diff --git a/spec/factories/mail_request.rb b/spec/factories/mail_request.rb index 256c80ed196..036d40aee74 100644 --- a/spec/factories/mail_request.rb +++ b/spec/factories/mail_request.rb @@ -19,6 +19,24 @@ recipient_type { nil } end + trait :ro_colocated_recipient do + recipient_type { "ro-colocated" } + first_name { nil } + last_name { nil } + name { "WYOMING VETERANS COMMISSION" } + poa_code { "869" } + claimant_station_of_jurisdiction { "329" } + participant_id { nil } + destination_type { "derived" } + address_line_1 { nil } + city { nil } + country_code { nil } + postal_code { nil } + state { nil } + treat_line_2_as_addressee { nil } + treat_line_3_as_addressee { nil } + end + initialize_with { new(attributes) } end end diff --git a/spec/feature/hearings/add_hearing_day_spec.rb b/spec/feature/hearings/add_hearing_day_spec.rb index 78f753365bb..88397537a9e 100644 --- a/spec/feature/hearings/add_hearing_day_spec.rb +++ b/spec/feature/hearings/add_hearing_day_spec.rb @@ -359,12 +359,14 @@ end scenario "select a vlj from the dropdown works" do - click_dropdown(name: "vlj", text: judge.full_name, wait: 30) + expect(page).to have_content("VLJ") + click_dropdown(name: "vlj", text: judge.full_name) expect(page).to have_content(judge.full_name) end scenario "select a coordinator from the dropdown works" do - click_dropdown(name: "coordinator", text: coordinator.full_name, wait: 30) + expect(page).to have_content("Hearing Coordinator") + click_dropdown(name: "coordinator", text: coordinator.full_name) expect(page).to have_content(coordinator.full_name) end end diff --git a/spec/feature/hearings/assign_hearings_table_spec.rb b/spec/feature/hearings/assign_hearings_table_spec.rb index a6aad0be0b2..542497ea9d6 100644 --- a/spec/feature/hearings/assign_hearings_table_spec.rb +++ b/spec/feature/hearings/assign_hearings_table_spec.rb @@ -19,6 +19,7 @@ context "No upcoming hearing days" do scenario "Show status message for empty upcoming hearing days" do visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") click_dropdown(text: "Winston-Salem, NC") expect(page).to have_content("No upcoming hearing days") end @@ -449,6 +450,8 @@ def navigate_to_ama_tab it "filters are correct, and filter as expected" do step "navigate to St. Petersburg legacy veterans tab" do visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") + click_dropdown(text: "St. Petersburg") click_button("Legacy Veterans Waiting", exact: true) end diff --git a/spec/feature/hearings/convert_hearing_request_type/edit_hearsched_spec.rb b/spec/feature/hearings/convert_hearing_request_type/edit_hearsched_spec.rb index cca6c943244..5e7bfee7b9e 100644 --- a/spec/feature/hearings/convert_hearing_request_type/edit_hearsched_spec.rb +++ b/spec/feature/hearings/convert_hearing_request_type/edit_hearsched_spec.rb @@ -218,6 +218,7 @@ def change_request_type(appeal, request_type, ro_message) step "go to schedule veterans page" do visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") click_dropdown(text: "Virtual Hearings") end @@ -272,6 +273,7 @@ def change_request_type(appeal, request_type, ro_message) # Check the schedule veterans tab to ensure the hearing is present visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") click_dropdown(text: "Central") click_button("AMA Veterans Waiting") @@ -320,6 +322,7 @@ def change_request_type(appeal, request_type, ro_message) # Check the schedule veterans tab to ensure the hearing is present visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") click_dropdown(text: "St. Petersburg, FL") click_button("AMA Veterans Waiting") diff --git a/spec/feature/hearings/edit_hearing_day_spec.rb b/spec/feature/hearings/edit_hearing_day_spec.rb index b1ad7a915df..2675640d659 100644 --- a/spec/feature/hearings/edit_hearing_day_spec.rb +++ b/spec/feature/hearings/edit_hearing_day_spec.rb @@ -39,6 +39,7 @@ def navigate_to_docket(hearings = false) visit "hearings/schedule" + expect(page).to have_content("Add Hearing Day") find_link(hearing_day.scheduled_for.strftime("%a %-m/%d/%Y")).click if hearings == false @@ -63,6 +64,7 @@ def navigate_to_docket(hearings = false) end it "can make changes to the VLJ on the docket" do + expect(page).to have_content("Select VLJ") click_dropdown(name: "vlj", index: 1, wait: 30) find("button", text: "Save Changes").click @@ -71,6 +73,7 @@ def navigate_to_docket(hearings = false) end it "can make changes to the Coordinator on the docket" do + expect(page).to have_content("Select Hearing Coordinator") click_dropdown(name: "coordinator", text: "#{coordinator.snamef} #{coordinator.snamel}") find("button", text: "Save Changes").click @@ -80,6 +83,7 @@ def navigate_to_docket(hearings = false) end it "can make changes to the Notes on the docket" do + expect(page).to have_content("Notes") find("textarea", id: "Notes").fill_in(with: sample_notes) find("button", text: "Save Changes").click diff --git a/spec/feature/hearings/hearing_worksheet/hearing_prep_spec.rb b/spec/feature/hearings/hearing_worksheet/hearing_prep_spec.rb index 5af9e504f87..b5936926f3e 100644 --- a/spec/feature/hearings/hearing_worksheet/hearing_prep_spec.rb +++ b/spec/feature/hearings/hearing_worksheet/hearing_prep_spec.rb @@ -107,9 +107,7 @@ end end - # skipping due to RSpec::Core::MultipleExceptionError - # https://circleci.com/gh/department-of-veterans-affairs/caseflow/53676 - xscenario "Can click from hearing worksheet to reader" do + scenario "Can click from hearing worksheet to reader" do visit "/hearings/" + legacy_hearing.external_id.to_s + "/worksheet" link = find("#review-claims-folder") link_href = link[:href] diff --git a/spec/feature/hearings/schedule_veteran/build_hearsched_spec.rb b/spec/feature/hearings/schedule_veteran/build_hearsched_spec.rb index 69a05dcdb64..dafa1965633 100644 --- a/spec/feature/hearings/schedule_veteran/build_hearsched_spec.rb +++ b/spec/feature/hearings/schedule_veteran/build_hearsched_spec.rb @@ -667,6 +667,7 @@ def format_hearing_day(hearing_day, detail_label = nil, total_slots = 0) scenario "can schedule a veteran without an error" do visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") click_dropdown(text: "Denver") click_button("AMA Veterans Waiting", exact: true) @@ -691,6 +692,7 @@ def format_hearing_day(hearing_day, detail_label = nil, total_slots = 0) scenario "should not see room displayed under Available Hearing Days and Assign Hearing Tabs" do visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") click_dropdown(text: "Denver") click_button("AMA Veterans Waiting", exact: true) @@ -736,6 +738,7 @@ def format_hearing_day(hearing_day, detail_label = nil, total_slots = 0) scenario "can schedule a veteran without an error" do visit "hearings/schedule/assign" + expect(page).to have_content("Regional Office") click_dropdown(text: "Denver") click_button("Legacy Veterans Waiting", exact: true) diff --git a/spec/jobs/data_integrity_checks_job_spec.rb b/spec/jobs/data_integrity_checks_job_spec.rb index 73e9f7faae7..890e841275a 100644 --- a/spec/jobs/data_integrity_checks_job_spec.rb +++ b/spec/jobs/data_integrity_checks_job_spec.rb @@ -130,8 +130,7 @@ expect(slack_service).to have_received(:send_notification).with( "Error running ExpiredAsyncJobsChecker. See Sentry event sentry_12345", - "ExpiredAsyncJobsChecker", - "#appeals-foxtrot" + "ExpiredAsyncJobsChecker" ) expect(@raven_called).to eq(true) end diff --git a/spec/jobs/etl_builder_job_spec.rb b/spec/jobs/etl_builder_job_spec.rb index 3aa64b11f7f..05a72e7670e 100644 --- a/spec/jobs/etl_builder_job_spec.rb +++ b/spec/jobs/etl_builder_job_spec.rb @@ -25,8 +25,7 @@ expect(slack_service).to have_received(:send_notification).with( "Error running ETLBuilderJob. See Sentry event sentry_12345", - "ETLBuilderJob", - "#appeals-data-workgroup" + "ETLBuilderJob" ) expect(@raven_called).to eq(true) end diff --git a/spec/models/vbms_distribution_destination_spec.rb b/spec/models/vbms_distribution_destination_spec.rb index e23f2f1c18f..98e9ecc54ae 100644 --- a/spec/models/vbms_distribution_destination_spec.rb +++ b/spec/models/vbms_distribution_destination_spec.rb @@ -116,17 +116,7 @@ end context "destination type is militaryAddress" do - let(:destination) do - VbmsDistributionDestination.new( - destination_type: "militaryAddress", - vbms_distribution: distribution, - address_line_1: "address line 1", - city: "city", - state: "NY", - postal_code: "11385", - country_code: "US" - ) - end + before { destination.destination_type = "militaryAddress" } include_examples "destination has valid attributes" include_examples "destination is a physical mailing address" @@ -134,12 +124,9 @@ end context "destination type is internationalAddress" do - let(:destination) do - VbmsDistributionDestination.new( + before do + destination.update( destination_type: "internationalAddress", - vbms_distribution: distribution, - address_line_1: "address line 1", - city: "city", country_name: "France", country_code: "FR" ) @@ -154,4 +141,47 @@ expect(destination.errors[:country_name]).to eq(["can't be blank"]) end end + + context "destination type is derived" do + let(:destination) do + VbmsDistributionDestination.new( + destination_type: "derived", + vbms_distribution: distribution + ) + end + + before { distribution.recipient_type = "ro-colocated" } + + it "the recipient_type of associated vbms_distribution must be ro-colocated" do + expect(destination).to be_valid + + distribution.recipient_type = "person" + expect(destination).to_not be_valid + + error_msg = destination.errors.messages[:destination_type] + expect(error_msg).to eq(["cannot be derived unless recipient type of associated distribution is ro-colocated"]) + end + + PHYSICAL_ADDRESS_FIELDS = [ + :address_line_1, :address_line_2, :address_line_3, :address_line_4, :address_line_5, :address_line_6, + :treat_line_2_as_addressee, :treat_line_3_as_addressee, :city, :state, :postal_code, :country_name, + :country_code + ].freeze + + it "physical mailing address fields must be blank" do + PHYSICAL_ADDRESS_FIELDS.each do |field| + destination[field] = "address info" + expect(destination).to_not be_valid + expect(destination.errors[field]).to eq(["must be blank"]) + end + end + + context "recipient_type of associated vbms_distribution is ro-colocated" do + it "must have a destination_type of derived" do + destination.destination_type = "domesticAddress" + expect(destination).to_not be_valid + expect(destination.errors[:destination_type]).to eq(["must be derived if recipient type is ro-colocated"]) + end + end + end end diff --git a/spec/services/appeals_with_more_than_one_open_hearing_task_checker_spec.rb b/spec/services/appeals_with_more_than_one_open_hearing_task_checker_spec.rb index 47390bc3f79..7cc5af11098 100644 --- a/spec/services/appeals_with_more_than_one_open_hearing_task_checker_spec.rb +++ b/spec/services/appeals_with_more_than_one_open_hearing_task_checker_spec.rb @@ -5,12 +5,6 @@ let(:appeal2) { create(:appeal, :with_schedule_hearing_tasks) } let(:legacy_appeal) { create(:legacy_appeal, :with_schedule_hearing_tasks) } - it "reports to correct slack channel" do - subject.call - - expect(subject.slack_channel).to eq("#appeals-tango") - end - context "there are no appeals with more than one open hearing task" do it "does not generate a report" do subject.call diff --git a/spec/services/etl/syncer_spec.rb b/spec/services/etl/syncer_spec.rb index cc48cf585e5..41ea50630a8 100644 --- a/spec/services/etl/syncer_spec.rb +++ b/spec/services/etl/syncer_spec.rb @@ -65,7 +65,7 @@ def target_class syncer.dump_messages_to_slack(ETL::Record) expect(slack_service).to have_received(:send_notification) .with("100: Expected some_attribute to equal 20 but got 4", - "ETL::Record", "#appeals-data-workgroup") + "ETL::Record") expect(ETL::Record.messages).to eq nil end end diff --git a/spec/services/multiple_open_root_child_task_checker_spec.rb b/spec/services/multiple_open_root_child_task_checker_spec.rb index daf9e67cf36..e40c77c01fc 100644 --- a/spec/services/multiple_open_root_child_task_checker_spec.rb +++ b/spec/services/multiple_open_root_child_task_checker_spec.rb @@ -5,11 +5,6 @@ let!(:appeal2) { create(:appeal, :with_schedule_hearing_tasks) } let!(:appeal3) { create(:appeal, :at_attorney_drafting) } - it "reports to correct slack channel" do - subject.call - expect(subject.slack_channel).to eq("#appeals-echo") - end - context "there are no appeals with more than one open root-child task" do it "does not generate a report" do subject.call diff --git a/spec/services/open_hearing_tasks_without_active_descendants_checker_spec.rb b/spec/services/open_hearing_tasks_without_active_descendants_checker_spec.rb index c6d2df8a030..0ade777ed25 100644 --- a/spec/services/open_hearing_tasks_without_active_descendants_checker_spec.rb +++ b/spec/services/open_hearing_tasks_without_active_descendants_checker_spec.rb @@ -70,10 +70,4 @@ end end end - - describe ".slack_channel" do - it "is available as a public method" do - expect { subject.slack_channel }.to_not raise_error - end - end end diff --git a/spec/services/pending_incomplete_and_uncancelled_task_timers_checker_spec.rb b/spec/services/pending_incomplete_and_uncancelled_task_timers_checker_spec.rb index 73ddc43661d..c4bed64057b 100644 --- a/spec/services/pending_incomplete_and_uncancelled_task_timers_checker_spec.rb +++ b/spec/services/pending_incomplete_and_uncancelled_task_timers_checker_spec.rb @@ -13,7 +13,6 @@ expect(subject.report?).to eq(true) expect(subject.report).to match("1 pending and incomplete") - expect(subject.slack_channel).to eq("#appeals-job-alerts") end end end diff --git a/spec/services/slack_service_spec.rb b/spec/services/slack_service_spec.rb index bdcc6af10ad..4e0b262f4bb 100644 --- a/spec/services/slack_service_spec.rb +++ b/spec/services/slack_service_spec.rb @@ -6,6 +6,8 @@ let(:ssl_config) { double("ssl") } before do + stub_const("ENV", "DEPLOY_ENV" => "uat") + @http_params = nil allow(HTTPClient).to receive(:new) { http_agent } allow(http_agent).to receive(:ssl_config) { ssl_config } @@ -22,9 +24,9 @@ expect(response).to eq("response") end - context "when it is run in the uat environment" do + context "when it is not run in the uat or prod environment" do it "does not make post request" do - stub_const("ENV", "DEPLOY_ENV" => "uat") + stub_const("ENV", "DEPLOY_ENV" => "dev") slack_service.send_notification("filler message contents") expect(@http_params).to be_nil end diff --git a/spec/services/stuck_virtual_hearings_checker_spec.rb b/spec/services/stuck_virtual_hearings_checker_spec.rb index f88fb74f8ad..15ada3afc21 100644 --- a/spec/services/stuck_virtual_hearings_checker_spec.rb +++ b/spec/services/stuck_virtual_hearings_checker_spec.rb @@ -3,12 +3,6 @@ describe StuckVirtualHearingsChecker, :postgres do let(:hearing_day) { create(:hearing_day, scheduled_for: Time.zone.today + 2.weeks) } - it "reports to correct slack channel" do - subject.call - - expect(subject.slack_channel).to eq("#appeals-tango") - end - context "there are no stuck virtual hearings" do let!(:virtual_hearing) do create( diff --git a/spec/workflows/mail_request_spec.rb b/spec/workflows/mail_request_spec.rb index ef43959f4e7..c1f7be5f731 100644 --- a/spec/workflows/mail_request_spec.rb +++ b/spec/workflows/mail_request_spec.rb @@ -41,6 +41,20 @@ end end + shared_examples "Valid mail request called upon creates desired artifacts" do + before do + RequestStore.store[:current_user] = User.system_user + end + + it "creates a vbms_distribution" do + expect { subject }.to change(VbmsDistribution, :count).by(1) + end + + it "creates a vbms_distribution_destination" do + expect { subject }.to change(VbmsDistributionDestination, :count).by(1) + end + end + let(:mail_request_spec_object_1) { build(:mail_request, :nil_recipient_type) } include_examples "mail request has valid attributes" it "is not valid without a recipient type" do @@ -51,17 +65,23 @@ context "when valid parameters are passed into the mail requests initialize method." do subject { described_class.new(mail_request_params).call } - before do - RequestStore.store[:current_user] = User.system_user - end + include_examples "Valid mail request called upon creates desired artifacts" + end - it "creates a vbms_distribution" do - expect { subject }.to change(VbmsDistribution, :count).by(1) + context "When the recipient_type param is 'ro-colocated" do + let(:ro_colocated_mail_request_params) do + ActionController::Parameters.new( + recipient_type: "ro-colocated", + claimant_station_of_jurisdiction: "123", + poa_code: "02A", + name: "POA Name", + destination_type: "derived" + ) end - it "creates a vbms_distribution_destination" do - expect { subject }.to change(VbmsDistributionDestination, :count).by(1) - end + subject { described_class.new(ro_colocated_mail_request_params).call } + + include_examples "Valid mail request called upon creates desired artifacts" end context "when invalid parameters are passed into the mail requests initialize method." do From cb9dc6d472e7c54fb144598adcb542ae0294caac Mon Sep 17 00:00:00 2001 From: Chris-Martine <135330019+Chris-Martine@users.noreply.github.com> Date: Fri, 25 Aug 2023 08:33:35 -0400 Subject: [PATCH 33/39] Add Sentry error capturing for logs controller (#19034) Co-authored-by: kshiflett88 --- app/controllers/metrics/v2/logs_controller.rb | 13 ++++- .../metrics/v2/logs_controller_spec.rb | 48 ++++++++++--------- spec/models/metric_spec.rb | 42 ++++++++-------- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/app/controllers/metrics/v2/logs_controller.rb b/app/controllers/metrics/v2/logs_controller.rb index bb0db7bfdd7..7ccdc1ec306 100644 --- a/app/controllers/metrics/v2/logs_controller.rb +++ b/app/controllers/metrics/v2/logs_controller.rb @@ -5,10 +5,21 @@ class Metrics::V2::LogsController < ApplicationController def create metric = Metric.create_metric_from_rest(self, allowed_params, current_user) - 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/spec/controllers/metrics/v2/logs_controller_spec.rb b/spec/controllers/metrics/v2/logs_controller_spec.rb index 5918a9c64aa..23a824d7a94 100644 --- a/spec/controllers/metrics/v2/logs_controller_spec.rb +++ b/spec/controllers/metrics/v2/logs_controller_spec.rb @@ -1,40 +1,44 @@ # frozen_string_literal: true describe Metrics::V2::LogsController, type: :controller do - - let(:request_params_javascript) do + let(:current_user) { create(:user) } + let(:request_params) do { metric: { + uuid: SecureRandom.uuid, method: "123456789", - uuid: "PAT123456^CFL200^A", - url: '', - message: '', - isError: false, - isPerformance: false, - source: 'javascript' - } + name: 'log', + group: 'service', + message: 'This is a test', + type: 'performance', + product: 'reader', + } } end - let(:request_params_min) do - { - metric: { - message: 'min' - } - } + before do + @raven_called = false end - + before { User.authenticate!(user: current_user) } context "with good request" do - it "returns 200 for javascript source" do - expect(Metric).to receive(:create_javascript_metric).and_return(nil) - post :create, params: request_params_javascript + it "returns 200 for request params" do + post :create, params: request_params + expect(@raven_called).to eq(false) expect(response.status).to eq(200) end + end - it "returns 200 for min params" do - post :create, params: request_params_min - expect(response.status).to eq(200) + context "With error type record to sentry" do + it "Records to Sentry" do + capture_raven_log + request_params[:metric][:type] = 'error' + post :create, params: request_params + expect(@raven_called).to eq(true) end end + + def capture_raven_log + allow(Raven).to receive(:capture_exception) { @raven_called = true } + end end diff --git a/spec/models/metric_spec.rb b/spec/models/metric_spec.rb index f46c5163942..e18bc1a076c 100644 --- a/spec/models/metric_spec.rb +++ b/spec/models/metric_spec.rb @@ -3,46 +3,46 @@ describe Metric do let(:user) { create(:user) } - describe "create_javascript_metric" do + before { User.authenticate!(user: user) } + + describe "create_metric" do let!(:params) do { + uuid: SecureRandom.uuid, method: "123456789", - uuid: "PAT123456^CFL200^A", - url: '', - message: '', - isError: false, - isPerformance: false, - source: 'javascript' + name: 'log', + group: 'service', + message: 'This is a test', + type: 'performance', + product: 'reader', } end - it "creates a javascript metric for log" do - options = {is_error: false, performance: false} - metric = Metric.create_javascript_metric(params, user, options) + it "creates a javascript metric for performance" do + metric = Metric.create_metric(self, params, user) expect(metric.valid?).to be true - expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:log]) + expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:performance]) end - it "creates a javascript metric for error" do - options = {is_error: true, performance: false} - metric = Metric.create_javascript_metric(params, user, options) + it "creates a javascript metric for log" do + params[:type] = 'log' + metric = Metric.create_metric(self, params, user) expect(metric.valid?).to be true - expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:error]) + expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:log]) end - it "creates a javascript metric for performance" do - options = {is_error: false, performance: true} - metric = Metric.create_javascript_metric(params, user, options) + it "creates a javascript metric for error" do + params[:type] = 'error' + metric = Metric.create_metric(self, params, user) expect(metric.valid?).to be true - expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:performance]) + expect(metric.metric_type).to eq(Metric::METRIC_TYPES[:error]) end it "creates a javascript metric with invalid sent_to" do - options = {is_error: false, performance: false} - metric = Metric.create_javascript_metric(params.merge({sent_to: 'fake'}), user, options) + metric = Metric.create_metric(self, params.merge({sent_to: 'fake'}), user) expect(metric.valid?).to be false end From d0bbb676c5f515f769d3641b4f95aa38d4f63052 Mon Sep 17 00:00:00 2001 From: mikefinneran <110622959+mikefinneran@users.noreply.github.com> Date: Fri, 25 Aug 2023 16:27:44 -0400 Subject: [PATCH 34/39] Add Sentry error capturing for logs controller (#19271) Co-authored-by: Chris-Martine Co-authored-by: kshiflett88 From bb205728d1fae878410e201afcb3322ebc492585 Mon Sep 17 00:00:00 2001 From: mikefinneran <110622959+mikefinneran@users.noreply.github.com> Date: Mon, 28 Aug 2023 22:01:25 -0400 Subject: [PATCH 35/39] update caseflow commons gem to original version used on master --- Gemfile | 2 +- Gemfile.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 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/Gemfile.lock b/Gemfile.lock index 8f18a4cb67c..710c4378d8d 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) @@ -209,7 +209,7 @@ GEM crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.6) - d3-rails (7.8.5) + d3-rails (7.0.0) railties (>= 3.1) danger (6.2.2) claide (~> 1.0) @@ -349,7 +349,7 @@ GEM activerecord (>= 3.0) jaro_winkler (1.5.4) jmespath (1.3.1) - jquery-rails (4.6.0) + jquery-rails (4.5.1) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) From c7cc0a965a3f0799f2ef0c37e38a2e1b49b7a165 Mon Sep 17 00:00:00 2001 From: mikefinneran <110622959+mikefinneran@users.noreply.github.com> Date: Wed, 30 Aug 2023 11:38:57 -0400 Subject: [PATCH 36/39] APPEALS-26109: Metric Service Sentry Updates (#19316) Co-authored-by: kshiflett88 Co-authored-by: Chris-Martine --- app/controllers/help_controller.rb | 2 +- app/controllers/intakes_controller.rb | 2 +- app/jobs/update_appellant_representation_job.rb | 1 - app/models/metric.rb | 2 +- app/services/metrics_service.rb | 17 ++++++++++++++++- app/views/certifications/v2.html.erb | 2 +- app/views/decision_reviews/index.html.erb | 2 +- .../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 +- .../update_appellant_representation_job_spec.rb | 2 +- 15 files changed, 35 insertions(+), 21 deletions(-) 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/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/models/metric.rb b/app/models/metric.rb index 9022b493b6e..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: 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], diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index cf00098a244..ee36efb98aa 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 = { + 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. 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 %> 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 From 56324eea1f3b32673d1b18ad6f55c987ec568d83 Mon Sep 17 00:00:00 2001 From: mikefinneran <110622959+mikefinneran@users.noreply.github.com> Date: Wed, 6 Sep 2023 10:31:45 -0400 Subject: [PATCH 37/39] remove new relic mention --- app/controllers/idt/api/v2/appeals_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/idt/api/v2/appeals_controller.rb b/app/controllers/idt/api/v2/appeals_controller.rb index 4b1c3bd936e..ce1d9526698 100644 --- a/app/controllers/idt/api/v2/appeals_controller.rb +++ b/app/controllers/idt/api/v2/appeals_controller.rb @@ -5,7 +5,6 @@ class Idt::Api::V2::AppealsController < Idt::Api::V1::BaseController protect_from_forgery with: :exception before_action :verify_access - newrelic_ignore skip_before_action :verify_authenticity_token, only: [:outcode] From 91a52d7863429f4c853aa6095b264f59d7e019ca Mon Sep 17 00:00:00 2001 From: mikefinneran <110622959+mikefinneran@users.noreply.github.com> Date: Wed, 6 Sep 2023 10:34:31 -0400 Subject: [PATCH 38/39] remove new relic mention --- app/controllers/reader/application_controller.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/controllers/reader/application_controller.rb b/app/controllers/reader/application_controller.rb index 0682ae07852..604e54522fc 100644 --- a/app/controllers/reader/application_controller.rb +++ b/app/controllers/reader/application_controller.rb @@ -2,7 +2,6 @@ class Reader::ApplicationController < ApplicationController before_action :verify_access, :react_routed, :check_reader_out_of_service - newrelic_ignore def set_application RequestStore.store[:application] = "reader" From dc9c17eab490332929ef08b94b4385ff8a645ec0 Mon Sep 17 00:00:00 2001 From: mikefinneran <110622959+mikefinneran@users.noreply.github.com> Date: Wed, 6 Sep 2023 16:12:42 -0400 Subject: [PATCH 39/39] add feature flag for user metrics being enabled --- app/services/metrics_service.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/services/metrics_service.rb b/app/services/metrics_service.rb index ee36efb98aa..aa74ab6f7ed 100644 --- a/app/services/metrics_service.rb +++ b/app/services/metrics_service.rb @@ -5,6 +5,8 @@ # see https://dropwizard.github.io/metrics/3.1.0/getting-started/ for abstractions on metric types class MetricsService def self.record(description, service: nil, name: "unknown", caller: nil) + return nil unless FeatureToggle.enabled?(:metrics_monitoring, user: current_user) + return_value = nil app = RequestStore[:application] || "other" service ||= app