Merge pull request #1267 from brizental/1690253-events-ping

Bug 1690253 - Implement the 'events' ping

Author: Beatriz Rizental

CHANGELOG.md

@@ -5,6 +5,7 @@
 * [#1233](https://github.com/mozilla/glean.js/pull/1233): Add optional `buildDate` argument to `initialize` configuration. The build date can be generated by glean_parser.
 * [#1233](https://github.com/mozilla/glean.js/pull/1233): Update glean_parser to version 5.1.0.
 * [#1217](https://github.com/mozilla/glean.js/pull/1217): Record `InvalidType` error when incorrectly type values are passed to metric recording functions.
+* [#1267](https://github.com/mozilla/glean.js/pull/1267): Implement the 'events' ping.
 
 # v0.32.0 (2022-03-01)
 

glean/src/core/config.ts

@@ -11,6 +11,10 @@ import { Context } from "./context.js";
 
 const LOG_TAG = "core.Config";
 
+// The default maximum number of events Glean will store before submitting the events ping.
+// If the maximum is hit, the events ping is sent immediately.
+const DEFAULT_MAX_EVENTS = 500;
+
 /**
  * Lists Glean's debug options.
  */
@@ -35,6 +39,8 @@ export interface ConfigurationInterface {
   readonly appDisplayVersion?: string,
   // The server pings are sent to.
   readonly serverEndpoint?: string,
+  // The maximum number of events to store before submitting the events ping.
+  readonly maxEvents?: number,
   // Optional list of plugins to include in current Glean instance.
   plugins?: Plugin[],
   // The HTTP client implementation to use for uploading pings.
@@ -61,6 +67,7 @@ export class Configuration implements ConfigurationInterface {
   readonly architecture?: string;
   readonly osVersion?: string;
   readonly buildDate?: Date;
+  readonly maxEvents: number;
 
   // Debug configuration.
   debug: DebugOptions;
@@ -74,6 +81,7 @@ export class Configuration implements ConfigurationInterface {
     this.architecture = config?.architecture;
     this.osVersion = config?.osVersion;
     this.buildDate = config?.buildDate;
+    this.maxEvents = config?.maxEvents || DEFAULT_MAX_EVENTS;
 
     this.debug = {};
 

glean/src/core/constants.ts

@@ -37,6 +37,9 @@ export const DEFAULT_TELEMETRY_ENDPOINT = "https://incoming.telemetry.mozilla.or
 // The name of the deletion-request ping.
 export const DELETION_REQUEST_PING_NAME = "deletion-request";
 
+// The name of the events ping.
+export const EVENTS_PING_NAME = "events";
+
 // The maximum amount of source tags a user can set.
 export const GLEAN_MAX_SOURCE_TAGS = 5;
 

glean/src/core/context.ts

@@ -12,6 +12,8 @@ import type { JSONValue } from "./utils.js";
 import Dispatcher from "./dispatcher.js";
 import log, { LoggingLevel } from "./log.js";
 import type { Configuration } from "./config.js";
+import type CorePings from "./internal_pings.js";
+import type { CoreMetrics } from "./internal_metrics.js";
 
 const LOG_TAG = "core.Context";
 
@@ -32,6 +34,8 @@ export class Context {
 
   private _dispatcher: Dispatcher;
   private _platform!: Platform;
+  private _corePings!: CorePings;
+  private _coreMetrics!: CoreMetrics;
 
   // The following group of properties are all set on Glean.initialize
   // Attempting to get them before they are set will log an error.
@@ -230,6 +234,22 @@ export class Context {
     Context.instance._testing = flag;
   }
 
+  static get corePings(): CorePings {
+    return Context.instance._corePings;
+  }
+
+  static set corePings(pings: CorePings) {
+    Context.instance._corePings = pings;
+  }
+
+  static get coreMetrics(): CoreMetrics {
+    return Context.instance._coreMetrics;
+  }
+
+  static set coreMetrics(metrics: CoreMetrics) {
+    Context.instance._coreMetrics = metrics;
+  }
+
   static set platform(platform: Platform) {
     Context.instance._platform = platform;
   }

glean/src/core/glean.ts

@@ -23,15 +23,6 @@ import log, { LoggingLevel } from "./log.js";
 const LOG_TAG = "core.Glean";
 
 namespace Glean {
-  // The below properties are exported for testing purposes.
-  //
-  // Instances of Glean's core metrics.
-  //
-  // Disabling the lint, because we will actually re-assign this variable in the testInitializeGlean API.
-  // eslint-disable-next-line prefer-const
-  export let coreMetrics = new CoreMetrics();
-  // Instances of Glean's core pings.
-  export const corePings = new CorePings();
   // An instance of the ping uploader.
   export let pingUploader: PingUploadManager;
 
@@ -51,7 +42,7 @@ namespace Glean {
    */
   async function onUploadEnabled(): Promise<void> {
     Context.uploadEnabled = true;
-    await coreMetrics.initialize();
+    await Context.coreMetrics.initialize();
   }
 
   /**
@@ -70,7 +61,7 @@ namespace Glean {
     // We need to use an undispatched submission to guarantee that the
     // ping is collected before metric are cleared, otherwise we end up
     // with malformed pings.
-    await corePings.deletionRequest.submitUndispatched();
+    await Context.corePings.deletionRequest.submitUndispatched();
     await clearMetrics();
   }
 
@@ -97,7 +88,7 @@ namespace Glean {
       firstRunDate = new DatetimeMetric(
         await Context.metricsDatabase.getMetric(
           CLIENT_INFO_STORAGE,
-          coreMetrics.firstRunDate
+          Context.coreMetrics.firstRunDate
         )
       ).date;
     } catch {
@@ -124,10 +115,10 @@ namespace Glean {
     // Store a "dummy" KNOWN_CLIENT_ID in the client_id metric. This will
     // make it easier to detect if pings were unintentionally sent after
     // uploading is disabled.
-    await coreMetrics.clientId.setUndispatched(KNOWN_CLIENT_ID);
+    await Context.coreMetrics.clientId.setUndispatched(KNOWN_CLIENT_ID);
 
     // Restore the first_run_date.
-    await coreMetrics.firstRunDate.setUndispatched(firstRunDate);
+    await Context.coreMetrics.firstRunDate.setUndispatched(firstRunDate);
 
     Context.uploadEnabled = false;
   }
@@ -194,6 +185,9 @@ namespace Glean {
       return;
     }
 
+    Context.coreMetrics = new CoreMetrics();
+    Context.corePings = new CorePings();
+
     Context.applicationId = sanitizeApplicationId(applicationId);
 
     // The configuration constructor will throw in case config has any incorrect prop.
@@ -224,14 +218,20 @@ namespace Glean {
     //
     // The dispatcher will catch and log any exceptions.
     Context.dispatcher.flushInit(async () => {
-      // We need to mark Glean as initialized before dealing with the upload status,
-      // otherwise we will not be able to submit deletion-request pings if necessary.
-      //
-      // This is fine, we are inside a dispatched task that is guaranteed to run before any
-      // other task. No external API call will be executed before we leave this task.
       Context.initialized = true;
 
       Context.uploadEnabled = uploadEnabled;
+
+      // Initialize the events database.
+      //
+      // It's important this happens _after_ the upload state is set,
+      // because initializing the events database may record the execution_counter and
+      // glean.restarted metrics. If the upload state is not defined these metrics cannot be recorded.
+      //
+      // This may also submit an 'events' ping,
+      // so it also needs to happen before application lifetime metrics are cleared.
+      await Context.eventsDatabase.initialize();
+
       // The upload enabled flag may have changed since the last run, for
       // example by the changing of a config file.
       if (uploadEnabled) {
@@ -259,7 +259,7 @@ namespace Glean {
         // deletion request ping.
         const clientId = await Context.metricsDatabase.getMetric(
           CLIENT_INFO_STORAGE,
-          coreMetrics.clientId
+          Context.coreMetrics.clientId
         );
 
         if (clientId) {
@@ -273,13 +273,6 @@ namespace Glean {
         }
       }
 
-      // Initialize the events database.
-      //
-      // It's important this happens _after_ the upload state is dealt with,
-      // because initializing the events database may record the execution_counter and
-      // glean.restarted metrics. If the upload state is not defined these metrics can't be recorded.
-      await Context.eventsDatabase.initialize();
-
       // We only scan the pendings pings **after** dealing with the upload state.
       // If upload is disabled, pending pings files are deleted
       // so we need to know that state **before** scanning the pending pings

glean/src/core/internal_pings.ts

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-import { DELETION_REQUEST_PING_NAME } from "./constants.js";
+import { DELETION_REQUEST_PING_NAME, EVENTS_PING_NAME } from "./constants.js";
 import { InternalPingType as PingType} from "./pings/ping_type.js";
 
 /**
@@ -16,13 +16,22 @@ class CorePings {
   // that the user wishes to have their reported Telemetry data deleted.
   // As such it attempts to send itself at the moment the user opts out of data collection.
   readonly deletionRequest: PingType;
+  // The events ping's purpose is to transport event metric information.
+  readonly events: PingType;
 
   constructor() {
     this.deletionRequest = new PingType({
       name: DELETION_REQUEST_PING_NAME,
       includeClientId: true,
       sendIfEmpty: true,
     });
+
+    this.events = new PingType({
+      name: EVENTS_PING_NAME,
+      includeClientId: true,
+      sendIfEmpty: false,
+      reasonCodes: ["startup", "max_capacity"]
+    });
   }
 }
 

glean/src/core/metrics/database.ts

@@ -262,7 +262,7 @@ class MetricsDatabase {
         if (!validateMetricInternalRepresentation(metricType, metrics[metricIdentifier])) {
           log(
             LOG_TAG,
-            `Invalid value found in storage for metric "${metricIdentifier}". Deleting.`,
+            `Invalid value "${JSON.stringify(metrics[metricIdentifier])}" found in storage for metric "${metricIdentifier}". Deleting.`,
             LoggingLevel.Debug
           );
 

glean/src/core/metrics/events_database/index.ts

@@ -15,6 +15,7 @@ import { generateReservedMetricIdentifiers } from "../database.js";
 import type { ExtraValues , Event } from "./recorded_event.js";
 import { RecordedEvent } from "./recorded_event.js";
 import {
+  EVENTS_PING_NAME,
   GLEAN_EXECUTION_COUNTER_EXTRA_KEY,
   GLEAN_REFERENCE_TIME_EXTRA_KEY
 } from "../../constants.js";
@@ -47,12 +48,14 @@ function createDateObject(str?: ExtraValues): Date {
  * Creates an execution counter metric.
  *
  * @param sendInPings The list of pings this metric is sent in.
+ *        Note: The 'events' ping should not contain glean.restarted events,
+ *        so this ping will be filtered out from the 'sendInPings' array.
  * @returns A metric type instance.
  */
 function getExecutionCounterMetric(sendInPings: string[]): CounterMetricType {
   return new CounterMetricType({
     ...generateReservedMetricIdentifiers("execution_counter"),
-    sendInPings: sendInPings,
+    sendInPings: sendInPings.filter(name => name !== EVENTS_PING_NAME),
     lifetime: Lifetime.Ping,
     disabled: false
   });
@@ -62,13 +65,15 @@ function getExecutionCounterMetric(sendInPings: string[]): CounterMetricType {
  * Creates an `glean.restarted` event metric.
  *
  * @param sendInPings The list of pings this metric is sent in.
+ *        Note: The 'events' ping should not contain glean.restarted events,
+ *        so this ping will be filtered out from the 'sendInPings' array.
  * @returns A metric type instance.
  */
 export function getGleanRestartedEventMetric(sendInPings: string[]): EventMetricType {
   return new EventMetricType({
     category: "glean",
     name: "restarted",
-    sendInPings: sendInPings,
+    sendInPings: sendInPings.filter(name => name !== EVENTS_PING_NAME),
     lifetime: Lifetime.Ping,
     disabled: false
   }, [ GLEAN_REFERENCE_TIME_EXTRA_KEY ]);
@@ -140,6 +145,14 @@ class EventsDatabase {
     }
 
     const storeNames = await this.getAvailableStoreNames();
+    // Submit the events ping in case there are _any_ events unsubmitted from the previous run
+    if (storeNames.includes(EVENTS_PING_NAME)) {
+      const storedEvents = (await this.eventsStore.get([EVENTS_PING_NAME]) as JSONArray) ?? [];
+      if (storedEvents.length > 0) {
+        await Context.corePings.events.submitUndispatched("startup");
+      }
+    }
+
     // Increment the execution counter for known stores.
     // !IMPORTANT! This must happen before any event is recorded for this run.
     await getExecutionCounterMetric(storeNames).addUndispatched(1);
@@ -177,12 +190,18 @@ class EventsDatabase {
       }
       value.addExtra(GLEAN_EXECUTION_COUNTER_EXTRA_KEY, currentExecutionCount);
 
+      let numEvents = 0;
       const transformFn = (v?: JSONValue): JSONArray => {
-        const existing: JSONArray = (v as JSONArray) ?? [];
-        existing.push(value.get());
-        return existing;
+        const events: JSONArray = (v as JSONArray) ?? [];
+        events.push(value.get());
+        numEvents = events.length;
+        return events;
       };
+
       await this.eventsStore.update([ping], transformFn);
+      if (ping === EVENTS_PING_NAME && numEvents >= Context.config.maxEvents) {
+        await Context.corePings.events.submitUndispatched("max_capacity");
+      }
     }
   }
 

glean/src/core/metrics/utils.ts

@@ -11,8 +11,9 @@ import { isInteger, isString } from "../utils.js";
 import { LabeledMetric } from "./types/labeled.js";
 import { Context } from "../context.js";
 import { ErrorType } from "../error/error_type.js";
+import log, { LoggingLevel } from "../log.js";
 
-
+const LOG_TAG = "Glean.core.Metrics.utils";
 
 /**
  * A metric factory function.
@@ -31,7 +32,7 @@ export function createMetric(type: string, v: unknown): Metric<JSONValue, JSONVa
 
   const ctor = Context.getSupportedMetric(type);
   if (!ctor) {
-    throw new Error(`Unable to create metric of unknown type ${type}`);
+    throw new Error(`Unable to create metric of unknown type "${type}".`);
   }
 
   return new ctor(v);
@@ -52,7 +53,8 @@ export function validateMetricInternalRepresentation<T extends JSONValue>(
   try {
     createMetric(type, v);
     return true;
-  } catch {
+  } catch(e) {
+    log(LOG_TAG, (e as Error).message, LoggingLevel.Error);
     return false;
   }
 }

glean/src/core/testing/utils.ts

@@ -7,7 +7,6 @@ import type { ConfigurationInterface } from "../config.js";
 import { Context } from "../context.js";
 import { testResetEvents } from "../events/utils.js";
 import Glean from "../glean.js";
-import { CoreMetrics } from "../internal_metrics.js";
 
 /**
  * Test-only API
@@ -27,10 +26,6 @@ export async function testInitializeGlean(
   uploadEnabled = true,
   config?: ConfigurationInterface
 ): Promise<void> {
-  // Core metrics need to be re-initialized so that
-  // the supportedMetrics map is re-created.
-  Glean.coreMetrics = new CoreMetrics();
-
   Context.testing = true;
 
   Glean.setPlatform(TestPlatform);