Skip to content

Commit 9a2d988

Browse files
author
Beatriz Rizental
authored
Merge pull request #415 from brizental/1712080-gzip
Bug 1712080 - Gzip ping paylod before upload
2 parents 2ed9fc7 + 8a5dba8 commit 9a2d988

File tree

12 files changed

+106
-23
lines changed

12 files changed

+106
-23
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
* [#411](https://github.com/mozilla/glean.js/pull/411): Tag all messages logged by Glean with the component they are coming from.
66
* [#399](https://github.com/mozilla/glean.js/pull/399): Check if there are ping data before attempting to delete it.
77
* This change lowers the amount of log messages related to attempting to delete inexistent data.
8+
* [#415](https://github.com/mozilla/glean.js/pull/415): Gzip ping paylod before upload
9+
* This changes the signature of `Uploader.post` to accept `string | Uint8Array` for the `body` parameter, instead of only `string`.
10+
* The gizpping functionality is disabled in Qt/QML environments. Follow [Bug 1716322](https://bugzilla.mozilla.org/show_bug.cgi?id=1716322) for updates on the status of this feature on that platform.
811

912
# v0.15.0 (2021-06-03)
1013

glean/package-lock.json

Lines changed: 11 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

glean/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"version": "0.15.0",
44
"description": "An implementation of the Glean SDK, a modern cross-platform telemetry client, for JavaScript environments.",
55
"type": "module",
6+
"sideEffects": "false",
67
"exports": {
78
"./package.json": "./package.json",
89
"./webext": "./dist/webext/index/webext.js",
@@ -51,7 +52,7 @@
5152
"build:webext:lib": "tsc -p ./tsconfig/webext/index.json",
5253
"build:webext:types": "tsc -p ./tsconfig/webext/types.json",
5354
"build:webext": "rm -rf dist/webext && run-s build:webext:lib build:webext:types",
54-
"build:qt": "rm -rf dist/qt && webpack --config webpack.config.qt.js --mode production && ../bin/prepare-qml-module.sh",
55+
"build:qt": "rm -rf dist/qt && webpack --config webpack.config.qt.js && ../bin/prepare-qml-module.sh",
5556
"build:docs": "rm -rf dist/docs && typedoc src/ --out dist/docs --tsconfig tsconfig/docs.json --theme minimal",
5657
"publish:docs": "NODE_DEBUG=gh-pages gh-pages --dotfiles --message \"[skip ci] Updates\" --dist dist/docs",
5758
"prepublishOnly": "cp ../README.md ./README.md && run-s build:cli build:webext",
@@ -106,6 +107,7 @@
106107
"webpack-cli": "^4.5.0"
107108
},
108109
"dependencies": {
110+
"fflate": "^0.7.0",
109111
"jose": "^3.7.0",
110112
"uuid": "^8.3.2"
111113
},

glean/src/core/upload/index.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5+
import { gzipSync } from "fflate";
6+
57
import type Platform from "../../platform/index.js";
68
import type { Configuration } from "../config.js";
79
import { GLEAN_VERSION } from "../constants.js";
@@ -124,25 +126,35 @@ class PingUploader implements PingsDatabaseObserver {
124126
*/
125127
private async preparePingForUpload(ping: QueuedPing): Promise<{
126128
headers: Record<string, string>,
127-
payload: string
129+
payload: string | Uint8Array
128130
}> {
129-
const stringifiedBody = JSON.stringify(ping.payload);
130-
131131
let headers = ping.headers || {};
132132
headers = {
133133
...ping.headers,
134134
"Content-Type": "application/json; charset=utf-8",
135-
"Content-Length": stringifiedBody.length.toString(),
136135
"Date": (new Date()).toISOString(),
137136
"X-Client-Type": "Glean.js",
138137
"X-Client-Version": GLEAN_VERSION,
139138
"X-Telemetry-Agent": `Glean/${GLEAN_VERSION} (JS on ${await this.platformInfo.os()})`
140139
};
141140

142-
return {
143-
headers,
144-
payload: stringifiedBody
145-
};
141+
const stringifiedBody = JSON.stringify(ping.payload);
142+
try {
143+
const encoder = new TextEncoder();
144+
const compressedBody = gzipSync(encoder.encode(stringifiedBody));
145+
headers["Content-Encoding"] = "gzip";
146+
headers["Content-Length"] = compressedBody.length.toString();
147+
return {
148+
headers,
149+
payload: compressedBody
150+
};
151+
} catch {
152+
headers["Content-Length"] = stringifiedBody.length.toString();
153+
return {
154+
headers,
155+
payload: stringifiedBody
156+
};
157+
}
146158
}
147159

148160
/**

glean/src/core/upload/uploader.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,13 @@ export interface Uploader {
4343
* Makes a POST request to a given url, with the given headers and body.
4444
*
4545
* @param url The URL to make the POST request
46-
* @param body The stringified body of this post request
46+
* @param body The body of this post request. The body may be a stringified JSON or, most likely,
47+
* a Uint8Array containing the gzipped version of said stringified JSON. We need to accept
48+
* both in case the compression fails.
4749
* @param headers Optional header to include in the request
4850
* @returns The status code of the response.
4951
*/
50-
post(url: string, body: string, headers?: Record<string, string>): Promise<UploadResult>;
52+
post(url: string, body: string | Uint8Array, headers?: Record<string, string>): Promise<UploadResult>;
5153
}
5254

5355
export default Uploader;
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
/**
6+
* This file is a drop in replacement for `fflate` in QML.
7+
* This library raises errors in QML. https://github.com/101arrowz/fflate/issues/71
8+
* could be the the culprit, but it is unclear at this point.
9+
*/
10+
11+
// eslint-disable-next-line jsdoc/require-jsdoc
12+
export function gzipSync() {
13+
// We throw here because when the gzipping action throws the ping upload manager will
14+
// catch and send the uncompressed ping, which is what we want on QML for the time being.
15+
// We are trying to figure out how to actually add the gzipping step to QML on Bug 1716322.
16+
throw new Error("Attempted to use `gzipSync` from QML, but that is not supported.");
17+
}

glean/src/platform/test/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import type Platform from "../index.js";
1212

1313
class MockUploader implements Uploader {
1414
// eslint-disable-next-line @typescript-eslint/no-unused-vars
15-
post(_url: string, _body: string, _headers?: Record<string, string>): Promise<UploadResult> {
15+
post(_url: string, _body: string | Uint8Array, _headers?: Record<string, string>): Promise<UploadResult> {
1616
const result: UploadResult = {
1717
result: UploadResultStatus.Success,
1818
status: 200

glean/src/platform/webext/uploader.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { DEFAULT_UPLOAD_TIMEOUT_MS, UploadResultStatus } from "../../core/upload
1010
const LOG_TAG = "platform.webext.Uploader";
1111

1212
class BrowserUploader implements Uploader {
13-
async post(url: string, body: string, headers: Record<string, string> = {}): Promise<UploadResult> {
13+
async post(url: string, body: string | Uint8Array, headers: Record<string, string> = {}): Promise<UploadResult> {
1414
const controller = new AbortController();
1515
const timeout = setTimeout(() => controller.abort(), DEFAULT_UPLOAD_TIMEOUT_MS);
1616

glean/tests/integration/schema/schema.spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { UploadResultStatus } from "../../../src/core/upload/uploader";
1414
// Generated files.
1515
import * as metrics from "./generated/forTesting";
1616
import * as pings from "./generated/pings";
17+
import { unzipPingPayload } from "../../utils";
1718

1819
const GLEAN_SCHEMA_URL = "https://raw.githubusercontent.com/mozilla-services/mozilla-pipeline-schemas/main/schemas/glean/glean/glean.1.schema.json";
1920

@@ -45,7 +46,7 @@ function fetchSchema(): Promise<JSONObject> {
4546
*/
4647
class WaitableHttpClient implements Uploader {
4748
private waitingFor?: string;
48-
private waitResolver?: (pingBody: string) => void;
49+
private waitResolver?: (pingBody: JSONObject) => void;
4950

5051
/**
5152
* Returns a promise that resolves once a ping is submitted or times out after a 2s wait.
@@ -56,12 +57,11 @@ class WaitableHttpClient implements Uploader {
5657
waitForPingSubmission(name: string): Promise<JSONObject> {
5758
this.waitingFor = name;
5859
return new Promise<JSONObject>((resolve, reject) => {
59-
this.waitResolver = (pingBody: string) => {
60+
this.waitResolver = (pingBody: JSONObject) => {
6061
this.waitingFor = undefined;
61-
const parsedBody = JSON.parse(pingBody) as JSONObject;
6262
// Uncomment for debugging the ping payload.
63-
// console.log(JSON.stringify(parsedBody, null, 2));
64-
resolve(parsedBody);
63+
// console.log(JSON.stringify(pingBody, null, 2));
64+
resolve(pingBody);
6565
};
6666

6767
setTimeout(() => reject(), 2000);
@@ -70,7 +70,7 @@ class WaitableHttpClient implements Uploader {
7070

7171
post(url: string, body: string): Promise<UploadResult> {
7272
if (this.waitingFor && url.includes(this.waitingFor)) {
73-
this.waitResolver?.(body);
73+
this.waitResolver?.(unzipPingPayload(body));
7474
}
7575

7676
return Promise.resolve({

glean/tests/unit/plugins/encryption.spec.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { UploadResultStatus } from "../../../src/core/upload/uploader";
2020
import CounterMetricType from "../../../src/core/metrics/types/counter";
2121
import { Lifetime } from "../../../src/core/metrics/lifetime";
2222
import { Context } from "../../../src/core/context";
23+
import { unzipPingPayload } from "../../utils";
2324

2425
const sandbox = sinon.createSandbox();
2526

@@ -60,13 +61,13 @@ describe("PingEncryptionPlugin", function() {
6061

6162
const postSpy = sandbox.spy(TestPlatform.uploader, "post").withArgs(
6263
sinon.match(makePath(Context.applicationId, pingId, ping)),
63-
sinon.match.string
64+
sinon.match.any
6465
);
6566

6667
await collectAndStorePing(pingId, ping);
6768
assert.ok(postSpy.calledOnce);
6869

69-
const payload = JSON.parse(postSpy.args[0][1]) as JSONObject;
70+
const payload = unzipPingPayload(postSpy.args[0][1]);
7071
assert.ok("payload" in payload);
7172
assert.ok(typeof payload.payload === "string");
7273
});
@@ -127,7 +128,7 @@ describe("PingEncryptionPlugin", function() {
127128

128129
const { url, body } = await uploadPromise;
129130

130-
const parsedBody = JSON.parse(body) as JSONObject;
131+
const parsedBody = unzipPingPayload(body);
131132
const { plaintext, protectedHeader } = await compactDecrypt(
132133
parsedBody?.payload as string,
133134
privateKey
@@ -152,3 +153,4 @@ describe("PingEncryptionPlugin", function() {
152153
assert.ok("typ" in protectedHeader);
153154
});
154155
});
156+

0 commit comments

Comments
 (0)