Skip to content

Commit deab250

Browse files
committed
Bug 1874944: send distinct telemetry data for empty attribution strings r=nalexander
An empty attribution string is a different case from one that exists that cannot be read (they generally point towards different potential causes in our infrastructure). We should send them as distinct errors. Differential Revision: https://phabricator.services.mozilla.com/D199558
1 parent e9fb897 commit deab250

File tree

4 files changed

+75
-4
lines changed

4 files changed

+75
-4
lines changed

browser/components/attribution/AttributionCode.sys.mjs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,23 @@ export var AttributionCode = {
192192
`_getMacAttrDataAsync: getAttributionString: "${attrStr}"`
193193
);
194194

195-
gCachedAttrData = this.parseAttributionCode(attrStr);
195+
if (attrStr === null) {
196+
gCachedAttrData = {};
197+
198+
lazy.log.debug(`_getMacAttrDataAsync: null attribution string`);
199+
Services.telemetry
200+
.getHistogramById("BROWSER_ATTRIBUTION_ERRORS")
201+
.add("null_error");
202+
} else if (attrStr == "") {
203+
gCachedAttrData = {};
204+
205+
lazy.log.debug(`_getMacAttrDataAsync: empty attribution string`);
206+
Services.telemetry
207+
.getHistogramById("BROWSER_ATTRIBUTION_ERRORS")
208+
.add("empty_error");
209+
} else {
210+
gCachedAttrData = this.parseAttributionCode(attrStr);
211+
}
196212
} catch (ex) {
197213
// Avoid partial attribution data.
198214
gCachedAttrData = {};

browser/components/attribution/test/browser/browser_AttributionCode_Mac_telemetry.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,49 @@ add_task(async function test_no_attribution() {
7373
sandbox.restore();
7474
}
7575
});
76+
77+
add_task(async function test_empty_attribution() {
78+
const sandbox = sinon.createSandbox();
79+
await MacAttribution.setAttributionString("");
80+
81+
AttributionCode._clearCache();
82+
83+
const histogram = Services.telemetry.getHistogramById(
84+
"BROWSER_ATTRIBUTION_ERRORS"
85+
);
86+
try {
87+
// Clear any existing telemetry
88+
histogram.clear();
89+
90+
await AttributionCode.getAttrDataAsync();
91+
92+
TelemetryTestUtils.assertHistogram(histogram, INDEX_EMPTY_ERROR, 1);
93+
} finally {
94+
AttributionCode._clearCache();
95+
histogram.clear();
96+
sandbox.restore();
97+
}
98+
});
99+
100+
add_task(async function test_null_attribution() {
101+
const sandbox = sinon.createSandbox();
102+
sandbox.stub(MacAttribution, "getAttributionString").resolves(null);
103+
104+
AttributionCode._clearCache();
105+
106+
const histogram = Services.telemetry.getHistogramById(
107+
"BROWSER_ATTRIBUTION_ERRORS"
108+
);
109+
try {
110+
// Clear any existing telemetry
111+
histogram.clear();
112+
113+
await AttributionCode.getAttrDataAsync();
114+
115+
TelemetryTestUtils.assertHistogram(histogram, INDEX_NULL_ERROR, 1);
116+
} finally {
117+
AttributionCode._clearCache();
118+
histogram.clear();
119+
sandbox.restore();
120+
}
121+
});

browser/components/attribution/test/browser/head.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const INDEX_READ_ERROR = 0;
1212
const INDEX_DECODE_ERROR = 1;
1313
const INDEX_WRITE_ERROR = 2;
1414
const INDEX_QUARANTINE_ERROR = 3;
15+
const INDEX_EMPTY_ERROR = 4;
16+
const INDEX_NULL_ERROR = 5;
1517

1618
add_setup(function () {
1719
// AttributionCode._clearCache is only possible in a testing environment

toolkit/components/telemetry/Histograms.json

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12832,10 +12832,17 @@
1283212832
"products": ["firefox"],
1283312833
"expires_in_version": "never",
1283412834
"kind": "categorical",
12835-
"labels": ["read_error", "decode_error", "write_error", "quarantine_error"],
12836-
"description": "Count for the number of errors encountered trying to determine attribution data: on Windows, from the installers (stub and full); on macOS, from the quarantine database.",
12835+
"labels": [
12836+
"read_error",
12837+
"decode_error",
12838+
"write_error",
12839+
"quarantine_error",
12840+
"empty_error",
12841+
"null_error"
12842+
],
12843+
"description": "Count for the number of errors encountered trying to determine attribution data: on Windows, from the installers (stub and full); on macOS, from an extended attributed on the .app bundle.",
1283712844
"releaseChannelCollection": "opt-out",
12838-
"bug_numbers": [1621402, 1525076],
12845+
"bug_numbers": [1621402, 1525076, 1874944],
1283912846
"alert_emails": ["aoprea@mozilla.com"],
1284012847
"operating_systems": ["mac", "windows"]
1284112848
},

0 commit comments

Comments
 (0)