Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 5836a06

Browse files
fquezefqueze
committed
Bug 1960567 - remove the last C++ and scriptable APIs to accumulate data to legacy telemetry histograms, r=chutten.
Differential Revision: https://phabricator.services.mozilla.com/D255582
1 parent f2d6e73 commit 5836a06

File tree

7 files changed

+22
-966
lines changed

7 files changed

+22
-966
lines changed

‎toolkit/components/crashes/tests/xpcshell/test_crash_manager.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,7 @@ add_task(async function test_telemetryHistogram() {
12141214
let h = Telemetry.getKeyedHistogramById("PROCESS_CRASH_SUBMIT_ATTEMPT");
12151215
h.clear();
12161216
Telemetry.clearScalars();
1217+
Services.fog.testResetFOG();
12171218

12181219
let m = await getManager();
12191220
let processTypes = [];
@@ -1241,7 +1242,7 @@ add_task(async function test_telemetryHistogram() {
12411242
let key = processType + "-" + crashType;
12421243

12431244
keys.push(key);
1244-
h.add(key, 1);
1245+
Glean.crash.submitAttempt[key].add(1);
12451246
keysCount++;
12461247
}
12471248
}

‎toolkit/components/telemetry/core/Telemetry.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1521,10 +1521,6 @@ void RecordShutdownEndTimeStamp() {
15211521

15221522
namespace mozilla::Telemetry {
15231523

1524-
void Accumulate(HistogramID aID, const nsCString& aKey, uint32_t aSample) {
1525-
TelemetryHistogram::Accumulate(aID, aKey, aSample);
1526-
}
1527-
15281524
const char* GetHistogramName(HistogramID id) {
15291525
return TelemetryHistogram::GetHistogramName(id);
15301526
}

‎toolkit/components/telemetry/core/Telemetry.h

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -51,36 +51,6 @@ void Init();
5151
*/
5252
void ShutdownTelemetry();
5353

54-
/**
55-
* DEPRECATED:
56-
* Adds sample to a keyed histogram defined in TelemetryHistogramEnums.h
57-
* The only remaining callers should use keyed boolean or keyed categorical
58-
* histograms, that do not have glean equivalents yet (see bug 1657470).
59-
*
60-
* @param id - keyed histogram id
61-
* @param key - the string key
62-
* @param sample - value to record.
63-
*/
64-
void Accumulate(HistogramID id, const nsCString& key, uint32_t sample);
65-
66-
/**
67-
* DEPRECATED:
68-
* Adds sample to a keyed categorical histogram defined in
69-
* TelemetryHistogramEnums.h This is the typesafe - and preferred - way to use
70-
* the keyed categorical histograms by passing values from the corresponding
71-
* Telemetry::LABELS_* enum.
72-
*
73-
* @param key - the string key
74-
* @param enumValue - Label value from one of the Telemetry::LABELS_* enums.
75-
*/
76-
template <class E>
77-
void AccumulateCategoricalKeyed(const nsCString& key, E enumValue) {
78-
static_assert(IsCategoricalLabelEnum<E>::value,
79-
"Only categorical label enum types are supported.");
80-
Accumulate(static_cast<HistogramID>(CategoricalLabelId<E>::value), key,
81-
static_cast<uint32_t>(enumValue));
82-
};
83-
8454
const char* GetHistogramName(HistogramID id);
8555

8656
/**

‎toolkit/components/telemetry/core/TelemetryHistogram.cpp

Lines changed: 2 additions & 203 deletions
Original file line numberDiff line numberDiff line change
@@ -1714,144 +1714,6 @@ struct JSHistogramData {
17141714
HistogramID histogramId;
17151715
};
17161716

1717-
bool internal_JSHistogram_CoerceValue(JSContext* aCx,
1718-
JS::Handle<JS::Value> aElement,
1719-
HistogramID aId, uint32_t aHistogramType,
1720-
uint32_t& aValue) {
1721-
if (aElement.isString()) {
1722-
// Strings only allowed for categorical histograms
1723-
if (aHistogramType != nsITelemetry::HISTOGRAM_CATEGORICAL) {
1724-
LogToBrowserConsole(
1725-
nsIScriptError::errorFlag,
1726-
nsLiteralString(
1727-
u"String argument only allowed for categorical histogram"));
1728-
return false;
1729-
}
1730-
1731-
// Label is given by the string argument
1732-
nsAutoJSString label;
1733-
if (!label.init(aCx, aElement)) {
1734-
LogToBrowserConsole(nsIScriptError::errorFlag,
1735-
u"Invalid string parameter"_ns);
1736-
return false;
1737-
}
1738-
1739-
// Get the label id for accumulation
1740-
nsresult rv = gHistogramInfos[aId].label_id(
1741-
NS_ConvertUTF16toUTF8(label).get(), &aValue);
1742-
if (NS_FAILED(rv)) {
1743-
nsPrintfCString msg("'%s' is an invalid string label",
1744-
NS_ConvertUTF16toUTF8(label).get());
1745-
LogToBrowserConsole(nsIScriptError::errorFlag,
1746-
NS_ConvertUTF8toUTF16(msg));
1747-
return false;
1748-
}
1749-
} else if (!(aElement.isNumber() || aElement.isBoolean())) {
1750-
LogToBrowserConsole(nsIScriptError::errorFlag, u"Argument not a number"_ns);
1751-
return false;
1752-
} else if (aElement.isNumber() && aElement.toNumber() > UINT32_MAX) {
1753-
// Clamp large numerical arguments to aValue's acceptable values.
1754-
// JS::ToUint32 will take aElement modulo 2^32 before returning it, which
1755-
// may result in a smaller final value.
1756-
aValue = UINT32_MAX;
1757-
#ifdef DEBUG
1758-
LogToBrowserConsole(nsIScriptError::errorFlag,
1759-
u"Clamped large numeric value"_ns);
1760-
#endif
1761-
} else if (!JS::ToUint32(aCx, aElement, &aValue)) {
1762-
LogToBrowserConsole(nsIScriptError::errorFlag,
1763-
u"Failed to convert element to UInt32"_ns);
1764-
return false;
1765-
}
1766-
1767-
// If we're here then all type checks have passed and aValue contains the
1768-
// coerced integer
1769-
return true;
1770-
}
1771-
1772-
bool internal_JSHistogram_GetValueArray(JSContext* aCx, JS::CallArgs& args,
1773-
uint32_t aHistogramType,
1774-
HistogramID aId, bool isKeyed,
1775-
nsTArray<uint32_t>& aArray) {
1776-
// This function populates aArray with the values extracted from args. Handles
1777-
// keyed and non-keyed histograms, and single and array of values. Also
1778-
// performs sanity checks on the arguments. Returns true upon successful
1779-
// population, false otherwise.
1780-
1781-
uint32_t firstArgIndex = 0;
1782-
if (isKeyed) {
1783-
firstArgIndex = 1;
1784-
}
1785-
1786-
// Special case of no argument (or only key) and count histogram
1787-
if (args.length() == firstArgIndex) {
1788-
if (!(aHistogramType == nsITelemetry::HISTOGRAM_COUNT)) {
1789-
LogToBrowserConsole(
1790-
nsIScriptError::errorFlag,
1791-
nsLiteralString(
1792-
u"Need at least one argument for non count type histogram"));
1793-
return false;
1794-
}
1795-
1796-
aArray.AppendElement(1);
1797-
return true;
1798-
}
1799-
1800-
if (args[firstArgIndex].isObject() && !args[firstArgIndex].isString()) {
1801-
JS::Rooted<JSObject*> arrayObj(aCx, &args[firstArgIndex].toObject());
1802-
1803-
bool isArray = false;
1804-
JS::IsArrayObject(aCx, arrayObj, &isArray);
1805-
1806-
if (!isArray) {
1807-
LogToBrowserConsole(
1808-
nsIScriptError::errorFlag,
1809-
nsLiteralString(
1810-
u"The argument to accumulate can't be a non-array object"));
1811-
return false;
1812-
}
1813-
1814-
uint32_t arrayLength = 0;
1815-
if (!JS::GetArrayLength(aCx, arrayObj, &arrayLength)) {
1816-
LogToBrowserConsole(nsIScriptError::errorFlag,
1817-
u"Failed while trying to get array length"_ns);
1818-
return false;
1819-
}
1820-
1821-
for (uint32_t arrayIdx = 0; arrayIdx < arrayLength; arrayIdx++) {
1822-
JS::Rooted<JS::Value> element(aCx);
1823-
1824-
if (!JS_GetElement(aCx, arrayObj, arrayIdx, &element)) {
1825-
nsPrintfCString msg("Failed while trying to get element at index %d",
1826-
arrayIdx);
1827-
LogToBrowserConsole(nsIScriptError::errorFlag,
1828-
NS_ConvertUTF8toUTF16(msg));
1829-
return false;
1830-
}
1831-
1832-
uint32_t value = 0;
1833-
if (!internal_JSHistogram_CoerceValue(aCx, element, aId, aHistogramType,
1834-
value)) {
1835-
nsPrintfCString msg("Element at index %d failed type checks", arrayIdx);
1836-
LogToBrowserConsole(nsIScriptError::errorFlag,
1837-
NS_ConvertUTF8toUTF16(msg));
1838-
return false;
1839-
}
1840-
aArray.AppendElement(value);
1841-
}
1842-
1843-
return true;
1844-
}
1845-
1846-
uint32_t value = 0;
1847-
if (!internal_JSHistogram_CoerceValue(aCx, args[firstArgIndex], aId,
1848-
aHistogramType, value)) {
1849-
return false;
1850-
}
1851-
aArray.AppendElement(value);
1852-
return true;
1853-
}
1854-
18551717
static JSHistogramData* GetJSHistogramData(JSObject* obj) {
18561718
MOZ_ASSERT(JS::GetClass(obj) == &sJSHistogramClass);
18571719
return JS::GetMaybePtrFromReservedSlot<JSHistogramData>(
@@ -2070,7 +1932,6 @@ void internal_JSHistogram_finalize(JS::GCContext* gcx, JSObject* obj) {
20701932

20711933
// NOTE: the functions in this section:
20721934
//
2073-
// internal_JSKeyedHistogram_Add
20741935
// internal_JSKeyedHistogram_Name
20751936
// internal_JSKeyedHistogram_Keys
20761937
// internal_JSKeyedHistogram_Snapshot
@@ -2172,66 +2033,6 @@ bool internal_JSKeyedHistogram_Snapshot(JSContext* cx, unsigned argc,
21722033
return true;
21732034
}
21742035

2175-
bool internal_JSKeyedHistogram_Add(JSContext* cx, unsigned argc,
2176-
JS::Value* vp) {
2177-
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
2178-
2179-
if (!args.thisv().isObject() ||
2180-
JS::GetClass(&args.thisv().toObject()) != &sJSKeyedHistogramClass) {
2181-
JS_ReportErrorASCII(cx, "Wrong JS class, expected JSKeyedHistogram class");
2182-
return false;
2183-
}
2184-
2185-
JSObject* obj = &args.thisv().toObject();
2186-
JSHistogramData* data = GetJSKeyedHistogramData(obj);
2187-
MOZ_ASSERT(data);
2188-
HistogramID id = data->histogramId;
2189-
MOZ_ASSERT(internal_IsHistogramEnumId(id));
2190-
2191-
// This function should always return |undefined| and never fail but
2192-
// rather report failures using the console.
2193-
args.rval().setUndefined();
2194-
if (args.length() < 1) {
2195-
LogToBrowserConsole(nsIScriptError::errorFlag, u"Expected one argument"_ns);
2196-
return true;
2197-
}
2198-
2199-
nsAutoJSString key;
2200-
if (!args[0].isString() || !key.init(cx, args[0])) {
2201-
LogToBrowserConsole(nsIScriptError::errorFlag, u"Not a string"_ns);
2202-
return true;
2203-
}
2204-
2205-
// Check if we're allowed to record in the provided key, for this histogram.
2206-
if (!gHistogramInfos[id].allows_key(NS_ConvertUTF16toUTF8(key))) {
2207-
nsPrintfCString msg("%s - key '%s' not allowed for this keyed histogram",
2208-
gHistogramInfos[id].name(),
2209-
NS_ConvertUTF16toUTF8(key).get());
2210-
LogToBrowserConsole(nsIScriptError::errorFlag, NS_ConvertUTF8toUTF16(msg));
2211-
TelemetryScalar::Add(mozilla::Telemetry::ScalarID::
2212-
TELEMETRY_ACCUMULATE_UNKNOWN_HISTOGRAM_KEYS,
2213-
NS_ConvertASCIItoUTF16(gHistogramInfos[id].name()), 1);
2214-
return true;
2215-
}
2216-
2217-
const uint32_t type = gHistogramInfos[id].histogramType;
2218-
2219-
nsTArray<uint32_t> values;
2220-
if (!internal_JSHistogram_GetValueArray(cx, args, type, id, true, values)) {
2221-
// Either GetValueArray or CoerceValue utility function will have printed a
2222-
// meaningful error message so we simple return true
2223-
return true;
2224-
}
2225-
2226-
{
2227-
StaticMutexAutoLock locker(gTelemetryHistogramMutex);
2228-
for (uint32_t aValue : values) {
2229-
internal_Accumulate(locker, id, NS_ConvertUTF16toUTF8(key), aValue);
2230-
}
2231-
}
2232-
return true;
2233-
}
2234-
22352036
bool internal_JSKeyedHistogram_Name(JSContext* cx, unsigned argc,
22362037
JS::Value* vp) {
22372038
JS::CallArgs args = CallArgsFromVp(argc, vp);
@@ -2379,11 +2180,9 @@ nsresult internal_WrapAndReturnKeyedHistogram(
23792180
HistogramID id, JSContext* cx, JS::MutableHandle<JS::Value> ret) {
23802181
JS::Rooted<JSObject*> obj(cx, JS_NewObject(cx, &sJSKeyedHistogramClass));
23812182
if (!obj) return NS_ERROR_FAILURE;
2382-
// The 6 functions that are wrapped up here are eventually called
2183+
// The functions that are wrapped up here are eventually called
23832184
// by the same thread that runs this function.
2384-
if (!(JS_DefineFunction(cx, obj, "add", internal_JSKeyedHistogram_Add, 2,
2385-
0) &&
2386-
JS_DefineFunction(cx, obj, "name", internal_JSKeyedHistogram_Name, 1,
2185+
if (!(JS_DefineFunction(cx, obj, "name", internal_JSKeyedHistogram_Name, 1,
23872186
0) &&
23882187
JS_DefineFunction(cx, obj, "snapshot",
23892188
internal_JSKeyedHistogram_Snapshot, 1, 0) &&

‎toolkit/components/telemetry/core/nsITelemetry.idl

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -287,11 +287,6 @@ interface nsITelemetry : nsISupports
287287
*
288288
* @param id - unique identifier from TelemetryHistograms.h
289289
* The returned object has the following functions:
290-
* add(string key, [optional] value) - Adds a sample of `value` to the histogram for that key.
291-
If no histogram for that key exists yet, it is created.
292-
`value` may be a categorical histogram's label as a string,
293-
a boolean histogram's value as a boolean,
294-
or a number that fits inside a uint32_t.
295290
* snapshot([optional] {store}) - Returns the snapshots of all the registered keys in the form
296291
{key1: snapshot1, key2: snapshot2, ...} in the specified store.
297292
* Defaults to the "main" store.

‎toolkit/components/telemetry/tests/gtest/TestHistograms.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ TEST_F(TelemetryTestFixture, AccumulateKeyedCountHistogram) {
5959
"TELEMETRY_TEST_KEYED_COUNT"_ns, true);
6060

6161
// Accumulate data in the provided key within the histogram
62-
Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_COUNT, "sample"_ns,
63-
kExpectedValue);
62+
TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_COUNT,
63+
"sample"_ns, kExpectedValue);
6464

6565
// Get a snapshot for all the histograms
6666
JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
@@ -98,11 +98,12 @@ TEST_F(TelemetryTestFixture, TestKeyedKeysHistogram) {
9898

9999
// Test the accumulation on both the allowed and unallowed keys, using
100100
// the API that accepts histogram IDs.
101-
Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "not-allowed"_ns,
102-
1);
103-
Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "testkey"_ns, 0);
104-
Telemetry::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS, "CommonKey"_ns,
105-
1);
101+
TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS,
102+
"not-allowed"_ns, 1);
103+
TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS,
104+
"testkey"_ns, 0);
105+
TelemetryHistogram::Accumulate(Telemetry::TELEMETRY_TEST_KEYED_KEYS,
106+
"CommonKey"_ns, 1);
106107

107108
// Get a snapshot for all the histograms
108109
JS::Rooted<JS::Value> snapshot(cx.GetJSContext());
@@ -191,6 +192,13 @@ TEST_F(TelemetryTestFixture, AccumulateCategoricalHistogram) {
191192
<< "The histogram is not returning expected value";
192193
}
193194

195+
template <class E>
196+
void AccumulateCategoricalKeyed(const nsCString& key, E enumValue) {
197+
TelemetryHistogram::Accumulate(static_cast<Telemetry::HistogramID>(
198+
Telemetry::CategoricalLabelId<E>::value),
199+
key, static_cast<uint32_t>(enumValue));
200+
};
201+
194202
TEST_F(TelemetryTestFixture, AccumulateKeyedCategoricalHistogram) {
195203
const uint32_t kSampleExpectedValue = 2;
196204
const uint32_t kOtherSampleExpectedValue = 1;
@@ -202,15 +210,15 @@ TEST_F(TelemetryTestFixture, AccumulateKeyedCategoricalHistogram) {
202210

203211
// Accumulate one unit into the categorical histogram with label
204212
// Telemetry::LABELS_TELEMETRY_TEST_KEYED_CATEGORICAL::CommonLabel
205-
Telemetry::AccumulateCategoricalKeyed(
213+
AccumulateCategoricalKeyed(
206214
"sample"_ns,
207215
Telemetry::LABELS_TELEMETRY_TEST_KEYED_CATEGORICAL::CommonLabel);
208216
// Accumulate another unit into the same categorical histogram
209-
Telemetry::AccumulateCategoricalKeyed(
217+
AccumulateCategoricalKeyed(
210218
"sample"_ns,
211219
Telemetry::LABELS_TELEMETRY_TEST_KEYED_CATEGORICAL::CommonLabel);
212220
// Accumulate another unit into a different categorical histogram
213-
Telemetry::AccumulateCategoricalKeyed(
221+
AccumulateCategoricalKeyed(
214222
"other-sample"_ns,
215223
Telemetry::LABELS_TELEMETRY_TEST_KEYED_CATEGORICAL::CommonLabel);
216224

0 commit comments

Comments
 (0)