Address review comments from @henrymercer

src/analyze-action.ts

@@ -13,7 +13,7 @@ import { getCodeQL } from "./codeql";
 import { Config, getConfig } from "./config-utils";
 import { uploadDatabases } from "./database-upload";
 import { GitHubFeatureFlags } from "./feature-flags";
-import { getActionsLogger } from "./logging";
+import { getActionsLogger, Logger } from "./logging";
 import { parseRepositoryNwo } from "./repository";
 import { getTotalCacheSize, uploadTrapCaches } from "./trap-caching";
 import * as upload_lib from "./upload-lib";
@@ -29,8 +29,12 @@ interface AnalysisStatusReport
 
 interface FinishStatusReport
   extends actionsUtil.StatusReportBase,
-    AnalysisStatusReport {
+    AnalysisStatusReport {}
+
+interface FinishWithTrapUploadStatusReport extends FinishStatusReport {
+  /** Size of TRAP caches that we uploaded, in bytes. */
   trap_cache_upload_size_bytes: number;
+  /** Time taken to upload TRAP caches, in milliseconds. */
   trap_cache_upload_duration_ms: number;
 }
 
@@ -38,9 +42,10 @@ export async function sendStatusReport(
   startedAt: Date,
   config: Config | undefined,
   stats: AnalysisStatusReport | undefined,
-  error?: Error,
-  trapCacheUploadTime?: number,
-  didUploadTrapCaches?: boolean
+  error: Error | undefined,
+  trapCacheUploadTime: number | undefined,
+  didUploadTrapCaches: boolean,
+  logger: Logger
 ) {
   const status = actionsUtil.getActionsStatus(
     error,
@@ -62,13 +67,20 @@ export async function sendStatusReport(
         }
       : {}),
     ...(stats || {}),
-    trap_cache_upload_duration_ms: trapCacheUploadTime || 0,
-    trap_cache_upload_size_bytes:
-      config && didUploadTrapCaches
-        ? await getTotalCacheSize(config.trapCaches)
-        : 0,
   };
-  await actionsUtil.sendStatusReport(statusReport);
+  if (config && didUploadTrapCaches) {
+    const trapCacheUploadStatusReport: FinishWithTrapUploadStatusReport = {
+      ...statusReport,
+      trap_cache_upload_duration_ms: trapCacheUploadTime || 0,
+      trap_cache_upload_size_bytes: await getTotalCacheSize(
+        config.trapCaches,
+        logger
+      ),
+    };
+    await actionsUtil.sendStatusReport(trapCacheUploadStatusReport);
+  } else {
+    await actionsUtil.sendStatusReport(statusReport);
+  }
 }
 
 async function run() {
@@ -81,6 +93,7 @@ async function run() {
   util.initializeEnvironment(util.Mode.actions, pkg.version);
   await util.checkActionVersion(pkg.version);
 
+  const logger = getActionsLogger();
   try {
     if (
       !(await actionsUtil.sendStatusReport(
@@ -93,7 +106,6 @@ async function run() {
     ) {
       return;
     }
-    const logger = getActionsLogger();
     config = await getConfig(actionsUtil.getTemporaryDirectory(), logger);
     if (config === undefined) {
       throw new Error(
@@ -175,9 +187,9 @@ async function run() {
     await uploadDatabases(repositoryNwo, config, apiDetails, logger);
 
     // Possibly upload the TRAP caches for later re-use
-    const trapCacheUploadStartTime = Date.now();
+    const trapCacheUploadStartTime = performance.now();
     const codeql = await getCodeQL(config.codeQLCmd);
-    trapCacheUploadTime = Date.now() - trapCacheUploadStartTime;
+    trapCacheUploadTime = performance.now() - trapCacheUploadStartTime;
     didUploadTrapCaches = await uploadTrapCaches(codeql, config, logger);
 
     // We don't upload results in test mode, so don't wait for processing
@@ -208,7 +220,8 @@ async function run() {
         stats,
         error,
         trapCacheUploadTime,
-        didUploadTrapCaches
+        didUploadTrapCaches,
+        logger
       );
     } else {
       await sendStatusReport(
@@ -217,7 +230,8 @@ async function run() {
         undefined,
         error,
         trapCacheUploadTime,
-        didUploadTrapCaches
+        didUploadTrapCaches,
+        logger
       );
     }
 
@@ -234,7 +248,8 @@ async function run() {
       },
       undefined,
       trapCacheUploadTime,
-      didUploadTrapCaches
+      didUploadTrapCaches,
+      logger
     );
   } else if (runStats) {
     await sendStatusReport(
@@ -243,7 +258,8 @@ async function run() {
       { ...runStats },
       undefined,
       trapCacheUploadTime,
-      didUploadTrapCaches
+      didUploadTrapCaches,
+      logger
     );
   } else {
     await sendStatusReport(
@@ -252,7 +268,8 @@ async function run() {
       undefined,
       undefined,
       trapCacheUploadTime,
-      didUploadTrapCaches
+      didUploadTrapCaches,
+      logger
     );
   }
 }

src/config-utils.ts

@@ -1052,13 +1052,16 @@ async function downloadCacheWithTime(
   codeQL: CodeQL,
   languages: Language[],
   logger: Logger
-) {
+): Promise<{
+  trapCaches: Partial<Record<Language, string>>;
+  trapCacheDownloadTime: number;
+}> {
   let trapCaches = {};
   let trapCacheDownloadTime = 0;
   if (trapCachingEnabled) {
-    const start = Date.now();
+    const start = performance.now();
     trapCaches = await downloadTrapCaches(codeQL, languages, logger);
-    trapCacheDownloadTime = Date.now() - start;
+    trapCacheDownloadTime = performance.now() - start;
   }
   return { trapCaches, trapCacheDownloadTime };
 }

src/init-action.ts

@@ -24,7 +24,7 @@ import {
   runInit,
 } from "./init";
 import { Language } from "./languages";
-import { getActionsLogger } from "./logging";
+import { getActionsLogger, Logger } from "./logging";
 import { parseRepositoryNwo } from "./repository";
 import { getTotalCacheSize } from "./trap-caching";
 import {
@@ -68,7 +68,7 @@ interface InitSuccessStatusReport extends StatusReportBase {
   workflow_languages: string;
   /** Comma-separated list of languages for which we are using TRAP caching. */
   trap_cache_languages: string;
-  /** Size of TRAP caches that we downloaded, in megabytes. */
+  /** Size of TRAP caches that we downloaded, in bytes. */
   trap_cache_download_size_bytes: number;
   /** Time taken to download TRAP caches, in milliseconds. */
   trap_cache_download_duration_ms: number;
@@ -77,7 +77,8 @@ interface InitSuccessStatusReport extends StatusReportBase {
 async function sendSuccessStatusReport(
   startedAt: Date,
   config: configUtils.Config,
-  toolsVersion: string
+  toolsVersion: string,
+  logger: Logger
 ) {
   const statusReportBase = await createStatusReportBase(
     "init",
@@ -123,7 +124,10 @@ async function sendSuccessStatusReport(
     tools_resolved_version: toolsVersion,
     workflow_languages: workflowLanguages || "",
     trap_cache_languages: Object.keys(config.trapCaches).join(","),
-    trap_cache_download_size_bytes: await getTotalCacheSize(config.trapCaches),
+    trap_cache_download_size_bytes: await getTotalCacheSize(
+      config.trapCaches,
+      logger
+    ),
     trap_cache_download_duration_ms: config.trapCacheDownloadTime,
   };
 
@@ -308,7 +312,7 @@ async function run() {
     );
     return;
   }
-  await sendSuccessStatusReport(startedAt, config, toolsVersion);
+  await sendSuccessStatusReport(startedAt, config, toolsVersion, logger);
 }
 
 async function getTrapCachingEnabled(

src/trap-caching.ts

@@ -185,15 +185,19 @@ export async function getLanguagesSupportingCaching(
 }
 
 export async function getTotalCacheSize(
-  trapCaches: Partial<Record<Language, string>>
+  trapCaches: Partial<Record<Language, string>>,
+  logger: Logger
 ): Promise<number> {
   const sizes = await Promise.all(
     Object.values(trapCaches).map(async (cacheDir) => {
       return new Promise<number>((resolve) => {
         getFolderSize(cacheDir, (err, size) => {
-          // Ignore file system errors when getting the size. It's only used for telemetry anyway.
-          if (err) resolve(0);
-          resolve(size);
+          if (err) {
+            logger.warning(`Error getting size of ${cacheDir}: ${err}`);
+            resolve(0);
+          } else {
+            resolve(size);
+          }
         });
       });
     })