Emit telemetry events on success and failure of the Create Job, Creat…

…e Job Definition, Create Job from Job Definition hooks (jupyter-server#472) * add job/job definition creation event success or failure logging * fix create-job logging format * log error message and httpStatusCode when error occurs * Update src/index.tsx Co-authored-by: Piyush Jain <[email protected]> * Update src/context.ts Co-authored-by: Piyush Jain <[email protected]> * Update src/context.ts Co-authored-by: Piyush Jain <[email protected]> * log string for logDetails, not whole error * catch errors as unknown, not Error * Make generic error message more user-frinedly and actionable per @JasonWeill --------- Co-authored-by: Piyush Jain <[email protected]>
andrii-i · Jan 22, 2024 · eeb5de2 · eeb5de2
1 parent f3518e1
commit eeb5de2
Show file tree

Hide file tree

Showing 12 changed files with 106 additions and 38 deletions.
diff --git a/src/components/advanced-table/advanced-table.tsx b/src/components/advanced-table/advanced-table.tsx
@@ -13,6 +13,7 @@ import { Scheduler } from '../../handler';
 import { AdvancedTableHeader } from './advanced-table-header';
 import { useTranslator } from '../../hooks';
 import { Alert } from '@mui/material';
+import { getErrorMessage } from '../../util/errors';
 
 const PAGE_SIZE = 25;
 
@@ -102,8 +103,9 @@ export function AdvancedTable<
         setNextToken(payload?.next_token);
         setTotalCount(payload?.total_count);
       })
-      .catch((e: Error) => {
-        setDisplayError(e.message);
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
       });
   };
 
@@ -152,8 +154,9 @@ export function AdvancedTable<
         setPage(newPage);
         setMaxPage(newPage);
       })
-      .catch((e: Error) => {
-        setDisplayError(e.message);
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
       });
   };
 

diff --git a/src/components/job-definition-row.tsx b/src/components/job-definition-row.tsx
@@ -16,6 +16,7 @@ import { Scheduler, SchedulerService } from '../handler';
 import { useEventLogger, useTranslator } from '../hooks';
 import { TranslationBundle } from '@jupyterlab/translation';
 import { ConfirmDeleteButton } from './confirm-buttons';
+import { getErrorMessage } from '../util/errors';
 
 function CreatedAt(props: {
   job: Scheduler.IDescribeJobDefinition;
@@ -120,8 +121,9 @@ export function buildJobDefinitionRow(
             .then(_ => {
               forceReload();
             })
-            .catch((error: Error) => {
-              handleApiError(error.message);
+            .catch((e: unknown) => {
+              const message = getErrorMessage(e);
+              handleApiError(message);
             });
         }}
       />
@@ -134,8 +136,9 @@ export function buildJobDefinitionRow(
             .then(_ => {
               forceReload();
             })
-            .catch((error: Error) => {
-              handleApiError(error.message);
+            .catch((e: unknown) => {
+              const message = getErrorMessage(e);
+              handleApiError(message);
             });
         }}
       />
@@ -148,8 +151,9 @@ export function buildJobDefinitionRow(
             .then(_ => {
               deleteRow(jobDef.job_definition_id);
             })
-            .catch((error: Error) => {
-              handleApiError(error.message);
+            .catch((e: unknown) => {
+              const message = getErrorMessage(e);
+              handleApiError(message);
             });
         }}
       />

diff --git a/src/components/job-row.tsx b/src/components/job-row.tsx
@@ -10,6 +10,7 @@ import { ICreateJobModel } from '../model';
 import DownloadIcon from '@mui/icons-material/Download';
 import StopIcon from '@mui/icons-material/Stop';
 import { IconButton, Stack, Link, TableCell, TableRow } from '@mui/material';
+import { getErrorMessage } from '../util/errors';
 
 function StopButton(props: {
   job: Scheduler.IDescribeJob;
@@ -106,8 +107,9 @@ function DownloadFilesButton(props: DownloadFilesButtonProps) {
               props.reload();
             })
           )
-          .catch((e: Error) => {
-            props.setDisplayError(e.message);
+          .catch((e: unknown) => {
+            const message = getErrorMessage(e);
+            props.setDisplayError(message);
           });
       }}
     >
@@ -178,7 +180,10 @@ export function buildJobRow(
               id: job.job_id
             })
             .then(_ => deleteRow(job.job_id))
-            .catch((e: Error) => setDisplayError(e.message));
+            .catch((e: unknown) => {
+              const message = getErrorMessage(e);
+              setDisplayError(message);
+            });
         }}
       />
       <StopButton
@@ -189,7 +194,10 @@ export function buildJobRow(
             .execute(CommandIDs.stopJob, {
               id: job.job_id
             })
-            .catch((e: Error) => setDisplayError(e.message));
+            .catch((e: unknown) => {
+              const message = getErrorMessage(e);
+              setDisplayError(message);
+            });
         }}
       />
     </Stack>

diff --git a/src/context.ts b/src/context.ts
@@ -1,10 +1,12 @@
 import { ITranslator, nullTranslator } from '@jupyterlab/translation';
 import React from 'react';
 
-export type Logger = (eventName: string) => void;
-export const LogContext = React.createContext<Logger>((eventName: string) => {
-  /*noop*/
-});
+export type Logger = (eventName: string, eventDetail?: string) => void;
+export const LogContext = React.createContext<Logger>(
+  (eventName: string, eventDetail?: string) => {
+    /*noop*/
+  }
+);
 
 // Context to be overridden with JupyterLab context
 const TranslatorContext = React.createContext<ITranslator>(nullTranslator);

diff --git a/src/index.tsx b/src/index.tsx
@@ -41,6 +41,11 @@ export namespace CommandIDs {
 export const NotebookJobsPanelId = 'notebook-jobs-panel';
 export { Scheduler } from './tokens';
 
+type EventLog = {
+  body: { name: string; detail?: string };
+  timestamp: Date;
+};
+
 /**
  * Initialization data for the jupyterlab-scheduler extension.
  */
@@ -178,16 +183,21 @@ async function activatePlugin(
   let mainAreaWidget: MainAreaWidget<NotebookJobsPanel> | undefined;
   let jobsPanel: NotebookJobsPanel | undefined;
 
-  const eventLogger: Scheduler.EventLogger = eventName => {
+  const eventLogger: Scheduler.EventLogger = (eventName, eventDetail) => {
     if (!eventName) {
       return;
     }
-    const eventLog = {
+    const eventLog: EventLog = {
       body: {
         name: `org.jupyter.jupyter-scheduler.${eventName}`
       },
       timestamp: new Date()
     };
+
+    if (eventDetail) {
+      eventLog.body.detail = eventDetail;
+    }
+
     telemetryHandler(eventLog).then();
   };
 

diff --git a/src/mainviews/create-job-from-definition.tsx b/src/mainviews/create-job-from-definition.tsx
@@ -13,6 +13,7 @@ import { Alert, Button, CircularProgress } from '@mui/material';
 import { Box, Stack } from '@mui/system';
 
 import { LabeledValue } from '../components/labeled-value';
+import { getErrorMessage } from '../util/errors';
 
 export interface ICreateJobFromDefinitionProps {
   model: ICreateJobModel;
@@ -139,17 +140,20 @@ export function CreateJobFromDefinition(
         createJobFromDefinitionModel
       )
       .then(response => {
+        log('create-job-from-definition.create-job.success');
         // Switch to the list view with "Job List" active
         props.showListView(
           JobsView.ListJobs,
           response.job_id,
           props.model.jobName
         );
       })
-      .catch((error: Error) => {
+      .catch((e: unknown) => {
+        const detail = getErrorMessage(e);
+        log('create-job-from-definition.create-job.failure', detail);
         props.handleModelChange({
           ...props.model,
-          createError: error.message,
+          createError: detail,
           createInProgress: false
         });
       });
@@ -240,7 +244,7 @@ export function CreateJobFromDefinition(
                 <Button
                   variant="contained"
                   onClick={(e: React.MouseEvent) => {
-                    log('create-job-from-definition.create');
+                    log('create-job-from-definition.create-job');
                     submitCreateJobRequest(e);
                     return false;
                   }}

diff --git a/src/mainviews/create-job.tsx b/src/mainviews/create-job.tsx
@@ -41,6 +41,7 @@ import {
 } from '@mui/material';
 
 import { Box, Stack } from '@mui/system';
+import { getErrorMessage } from '../util/errors';
 
 export interface ICreateJobProps {
   model: ICreateJobModel;
@@ -335,13 +336,16 @@ export function CreateJob(props: ICreateJobProps): JSX.Element {
     api
       .createJob(jobOptions)
       .then(response => {
+        log('create-job.create-job.success');
         // Switch to the list view with "Job List" active
         props.showListView(JobsView.ListJobs, response.job_id, jobOptions.name);
       })
-      .catch((error: Error) => {
+      .catch((e: unknown) => {
+        const detail = getErrorMessage(e);
+        log('create-job.create-job.failure', detail);
         props.handleModelChange({
           ...props.model,
-          createError: error.message,
+          createError: detail,
           createInProgress: false
         });
       });
@@ -382,17 +386,20 @@ export function CreateJob(props: ICreateJobProps): JSX.Element {
     api
       .createJobDefinition(jobDefinitionOptions)
       .then(response => {
+        log('create-job.create-job-definition.success');
         // Switch to the list view with "Job Definition List" active
         props.showListView(
           JobsView.ListJobDefinitions,
           response.job_definition_id,
           jobDefinitionOptions.name
         );
       })
-      .catch((error: Error) => {
+      .catch((e: unknown) => {
+        const detail = getErrorMessage(e);
+        log('create-job.create-job-definition.failure', detail);
         props.handleModelChange({
           ...props.model,
-          createError: error.message,
+          createError: detail,
           createInProgress: false
         });
       });
@@ -591,7 +598,11 @@ export function CreateJob(props: ICreateJobProps): JSX.Element {
                 <Button
                   variant="contained"
                   onClick={(e: React.MouseEvent) => {
-                    log('create-job.create');
+                    const eventType =
+                      props.model.createType === 'Job'
+                        ? 'create-job'
+                        : 'create-job-definition';
+                    log(`create-job.${eventType}`);
                     submitForm(e);
                     return false;
                   }}

diff --git a/src/mainviews/detail-view/job-definition.tsx b/src/mainviews/detail-view/job-definition.tsx
@@ -31,6 +31,7 @@ import {
 import { Scheduler as SchedulerTokens } from '../../tokens';
 
 import { timestampLocalize } from './job-detail';
+import { getErrorMessage } from '../../util/errors';
 
 export interface IJobDefinitionProps {
   app: JupyterFrontEnd;
@@ -79,21 +80,30 @@ export function JobDefinition(props: IJobDefinitionProps): JSX.Element {
   const handleDeleteJobDefinition = async () => {
     ss.deleteJobDefinition(model.definitionId ?? '')
       .then(_ => props.setJobsView(JobsView.ListJobDefinitions))
-      .catch((e: Error) => setDisplayError(e.message));
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
+      });
   };
 
   const pauseJobDefinition = async () => {
     setDisplayError(null);
     ss.pauseJobDefinition(model.definitionId)
       .then(_ => props.refresh())
-      .catch((e: Error) => setDisplayError(e.message));
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
+      });
   };
 
   const resumeJobDefinition = async () => {
     setDisplayError(null);
     ss.resumeJobDefinition(model.definitionId)
       .then(_ => props.refresh())
-      .catch((e: Error) => setDisplayError(e.message));
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
+      });
   };
 
   const runJobDefinition = () => {

diff --git a/src/mainviews/detail-view/job-detail.tsx b/src/mainviews/detail-view/job-detail.tsx
@@ -38,6 +38,7 @@ import {
   ILabeledValueProps,
   LabeledValue
 } from '../../components/labeled-value';
+import { getErrorMessage } from '../../util/errors';
 
 export interface IJobDetailProps {
   app: JupyterFrontEnd;
@@ -103,7 +104,10 @@ export function JobDetail(props: IJobDetailProps): JSX.Element {
     setDisplayError(null);
     ss.deleteJob(props.model?.jobId ?? '')
       .then(_ => props.setJobsView(JobsView.ListJobs))
-      .catch((e: Error) => setDisplayError(e.message));
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
+      });
   };
 
   const handleStopJob = async () => {
@@ -114,7 +118,10 @@ export function JobDetail(props: IJobDetailProps): JSX.Element {
         id: props.model?.jobId
       })
       .then(_ => props.handleModelChange())
-      .catch((e: Error) => setDisplayError(e.message));
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
+      });
   };
 
   const downloadFiles = async () => {
@@ -130,8 +137,9 @@ export function JobDetail(props: IJobDetailProps): JSX.Element {
           props.handleModelChange().then(_ => setDownloading(false))
         );
       })
-      .catch((e: Error) => {
-        setDisplayError(e.message);
+      .catch((e: unknown) => {
+        const message = getErrorMessage(e);
+        setDisplayError(message);
         setDownloading(false);
       });
   };

diff --git a/src/mainviews/edit-job-definition.tsx b/src/mainviews/edit-job-definition.tsx
@@ -21,6 +21,7 @@ import { Scheduler } from '../tokens';
 import { InputFileSnapshot } from '../components/input-file-snapshot';
 import { LabeledValue } from '../components/labeled-value';
 import { timestampLocalize } from './detail-view/job-detail';
+import { getErrorMessage } from '../util/errors';
 
 export type EditJobDefinitionProps = {
   model: IUpdateJobDefinitionModel;
@@ -77,9 +78,10 @@ function EditJobDefinitionBody(props: EditJobDefinitionProps): JSX.Element {
       .then(() => {
         props.showJobDefinitionDetail(props.model.definitionId);
       })
-      .catch((e: Error) => {
+      .catch((e: unknown) => {
         setSaving(false);
-        setDisplayError(e.message);
+        const message = getErrorMessage(e);
+        setDisplayError(message);
       });
   };
 

diff --git a/src/tokens.ts b/src/tokens.ts
@@ -60,5 +60,5 @@ export namespace Scheduler {
     '@jupyterlab/scheduler:ITelemetryHandler'
   );
 
-  export type EventLogger = (eventName: string) => void;
+  export type EventLogger = (eventName: string, eventDetail?: string) => void;
 }
diff --git a/src/util/errors.tsx b/src/util/errors.tsx
@@ -26,3 +26,9 @@ export const SERVER_EXTENSION_404_JSX = (
     </p>
   </div>
 );
+
+export function getErrorMessage(e: unknown): string {
+  return e instanceof Error
+    ? e.message
+    : 'An error occurred. Please try again.';
+}