Skip to content

Commit d2f7348

Browse files
author
brizental
committed
Trigger the afterPingCollection event at the expected time
1 parent 2886705 commit d2f7348

File tree

3 files changed

+61
-7
lines changed

3 files changed

+61
-7
lines changed

glean/src/core/pings/database.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,9 @@ export interface PingPayload extends JSONObject {
4242
metrics?: MetricsPayload,
4343
events?: JSONArray,
4444
}
45-
4645
export interface PingInternalRepresentation extends JSONObject {
4746
path: string,
48-
payload: PingPayload,
47+
payload: JSONObject,
4948
headers?: Record<string, string>
5049
}
5150

@@ -117,7 +116,7 @@ class PingsDatabase {
117116
async recordPing(
118117
path: string,
119118
identifier: string,
120-
payload: PingPayload,
119+
payload: JSONObject,
121120
headers?: Record<string, string>
122121
): Promise<void> {
123122
const ping: PingInternalRepresentation = {

glean/src/core/pings/maker.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import TimeUnit from "../metrics/time_unit";
1010
import { ClientInfo, PingInfo, PingPayload } from "../pings/database";
1111
import PingType from "../pings";
1212
import Glean from "../glean";
13+
import CoreEvents from "../events";
14+
import { JSONObject } from "../utils";
1315

1416
// The moment the current Glean.js session started.
1517
const GLEAN_START_TIME = new Date();
@@ -206,27 +208,35 @@ function makePath(identifier: string, ping: PingType): string {
206208
/**
207209
* Collects and stores a ping on the pings database.
208210
*
211+
* This function will trigger the `AfterPingCollection` event.
212+
* This event is triggered **after** logging the ping, which happens if `logPings` is set.
213+
* We will log the payload before it suffers any change by plugins instrumenting this event.
214+
*
209215
* @param identifier The pings UUID identifier.
210216
* @param ping The ping to submit.
211217
* @param reason An optional reason code to include in the ping.
212218
*
213219
* @returns A promise that is resolved once collection and storing is done.
214220
*/
215221
export async function collectAndStorePing(identifier: string, ping: PingType, reason?: string): Promise<void> {
216-
const payload = await collectPing(ping, reason);
217-
if (!payload) {
222+
const collectedPayload = await collectPing(ping, reason);
223+
if (!collectedPayload) {
218224
return;
219225
}
220226

221227
if (Glean.logPings) {
222-
console.info(JSON.stringify(payload, null, 2));
228+
console.info(JSON.stringify(collectedPayload, null, 2));
223229
}
224230

225231
const headers = getPingHeaders();
232+
233+
const modifiedPayload = await CoreEvents.afterPingCollection.trigger(collectedPayload);
234+
const finalPayload = modifiedPayload ? modifiedPayload : collectedPayload;
235+
226236
return Glean.pingsDatabase.recordPing(
227237
makePath(identifier, ping),
228238
identifier,
229-
payload,
239+
finalPayload,
230240
headers
231241
);
232242
}

glean/tests/core/pings/maker.spec.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,27 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
import assert from "assert";
6+
import sinon from "sinon";
67

78
import PingType from "../../../src/core/pings";
89
import * as PingMaker from "../../../src/core/pings/maker";
910
import Glean from "../../../src/core/glean";
11+
import CoreEvents from "../../../src/core/events";
12+
import Plugin from "../../../src/plugins";
13+
import { PingPayload } from "../../../src/core/pings/database";
14+
import { JSONObject } from "../../../src/core/utils";
15+
16+
const sandbox = sinon.createSandbox();
1017

1118
describe("PingMaker", function() {
1219
beforeEach(async function() {
1320
await Glean.testResetGlean("something something");
1421
});
1522

23+
afterEach(function () {
24+
sandbox.restore();
25+
});
26+
1627
it("ping info must contain a non-empty start and end time", async function() {
1728
const ping = new PingType({
1829
name: "custom",
@@ -100,4 +111,38 @@ describe("PingMaker", function() {
100111
assert.strictEqual(await PingMaker.getSequenceNumber(ping2), 11);
101112
assert.strictEqual(await PingMaker.getSequenceNumber(ping1), 13);
102113
});
114+
115+
it("collect and store triggers the AfterPingCollection and deals with possible result correctly", async function () {
116+
// Disable ping uploading for it not to interfere with this tests.
117+
sandbox.stub(Glean["pingUploader"], "triggerUpload").callsFake(() => Promise.resolve());
118+
119+
class MockPlugin extends Plugin<typeof CoreEvents["afterPingCollection"]> {
120+
constructor() {
121+
super(CoreEvents["afterPingCollection"].name, "mockPlugin");
122+
}
123+
124+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
125+
action(_payload: PingPayload): Promise<JSONObject> {
126+
return Promise.resolve({ "you": "got mocked!" });
127+
}
128+
}
129+
130+
await Glean.testUninitialize();
131+
await Glean.testInitialize("something something", true, { plugins: [ new MockPlugin() ]});
132+
const ping = new PingType({
133+
name: "ping",
134+
includeClientId: true,
135+
sendIfEmpty: true,
136+
});
137+
138+
await PingMaker.collectAndStorePing("ident", ping);
139+
const recordedPing = (await Glean.pingsDatabase.getAllPings())["ident"];
140+
assert.deepStrictEqual(recordedPing.payload, { "you": "got mocked!" });
141+
142+
await Glean.testUninitialize();
143+
await Glean.testInitialize("something something", true);
144+
await PingMaker.collectAndStorePing("ident", ping);
145+
const recordedPingNoPlugin = (await Glean.pingsDatabase.getAllPings())["ident"];
146+
assert.notDeepStrictEqual(recordedPingNoPlugin.payload, { "you": "got mocked!" });
147+
});
103148
});

0 commit comments

Comments
 (0)