зеркало из https://github.com/mozilla/gecko-dev.git
Bug 809930 - Make metrics provider collection API more robust; r=rnewman
This commit is contained in:
Родитель
f419461ab6
Коммит
48d4b18c33
|
@ -10,6 +10,7 @@ const {utils: Cu} = Components;
|
|||
|
||||
Cu.import("resource://gre/modules/commonjs/promise/core.js");
|
||||
Cu.import("resource://services-common/log4moz.js");
|
||||
Cu.import("resource://services-common/utils.js");
|
||||
Cu.import("resource://gre/modules/services/metrics/dataprovider.jsm");
|
||||
|
||||
|
||||
|
@ -24,6 +25,7 @@ this.MetricsCollector = function MetricsCollector() {
|
|||
|
||||
this._providers = [];
|
||||
this.collectionResults = new Map();
|
||||
this.providerErrors = new Map();
|
||||
}
|
||||
|
||||
MetricsCollector.prototype = {
|
||||
|
@ -51,6 +53,8 @@ MetricsCollector.prototype = {
|
|||
provider: provider,
|
||||
constantsCollected: false,
|
||||
});
|
||||
|
||||
this.providerErrors.set(provider.name, []);
|
||||
},
|
||||
|
||||
/**
|
||||
|
@ -65,16 +69,38 @@ MetricsCollector.prototype = {
|
|||
let promises = [];
|
||||
|
||||
for (let provider of this._providers) {
|
||||
let name = provider.provider.name;
|
||||
|
||||
if (provider.constantsCollected) {
|
||||
this._log.trace("Provider has already provided constant data: " +
|
||||
provider.name);
|
||||
name);
|
||||
continue;
|
||||
}
|
||||
|
||||
let result;
|
||||
try {
|
||||
result = provider.provider.collectConstantMeasurements();
|
||||
} catch (ex) {
|
||||
this._log.warn("Exception when calling " + name +
|
||||
".collectConstantMeasurements: " +
|
||||
CommonUtils.exceptionStr(ex));
|
||||
this.providerErrors.get(name).push(ex);
|
||||
continue;
|
||||
}
|
||||
|
||||
let result = provider.provider.collectConstantMeasurements();
|
||||
if (!result) {
|
||||
this._log.trace("Provider does not provide constant data: " +
|
||||
provider.name);
|
||||
this._log.trace("Provider does not provide constant data: " + name);
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
this._log.debug("Populating constant measurements: " + name);
|
||||
result.populate(result);
|
||||
} catch (ex) {
|
||||
this._log.warn("Exception when calling " + name + ".populate(): " +
|
||||
CommonUtils.exceptionStr(ex));
|
||||
result.addError(ex);
|
||||
promises.push(Promise.resolve(result));
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
|
@ -195,9 +195,18 @@ Object.freeze(MetricsMeasurement.prototype);
|
|||
* deferred events until after the result is populated.
|
||||
*
|
||||
* Implementations of collect* functions should call `createResult()` to create
|
||||
* a new `MetricsCollectionResult` instance. When called, they should
|
||||
* initiate population of this instance. Once population has finished (perhaps
|
||||
* asynchronously), they should call `finish()` on the instance.
|
||||
* a new `MetricsCollectionResult` instance. They should then register
|
||||
* expected measurements with this instance, define a `populate` function on
|
||||
* it, then return the instance.
|
||||
*
|
||||
* It is important for the collect* functions to just create the empty
|
||||
* `MetricsCollectionResult` and nothing more. This is to enable the callee
|
||||
* to handle errors gracefully. If the collect* function were to raise, the
|
||||
* callee may not receive a `MetricsCollectionResult` instance and it would not
|
||||
* know what data is missing.
|
||||
*
|
||||
* See the documentation for `MetricsCollectionResult` for details on how
|
||||
* to perform population.
|
||||
*
|
||||
* Receivers of created `MetricsCollectionResult` instances should wait
|
||||
* until population has finished. They can do this by chaining on to the
|
||||
|
@ -264,9 +273,12 @@ Object.freeze(MetricsProvider.prototype);
|
|||
* population of this instance is aborted or times out, downstream consumers
|
||||
* will know there is missing data.
|
||||
*
|
||||
* Next, they should add empty `MetricsMeasurement` instances to it via
|
||||
* `addMeasurement`. Finally, they should populate these measurements with
|
||||
* `setValue`.
|
||||
* Next, they should define the `populate` property to a function that
|
||||
* populates the instance.
|
||||
*
|
||||
* The `populate` function implementation should add empty `MetricsMeasurement`
|
||||
* instances to the result via `addMeasurement`. Then, it should populate these
|
||||
* measurements via `setValue`.
|
||||
*
|
||||
* It is preferred to populate via this type instead of directly on
|
||||
* `MetricsMeasurement` instances so errors with data population can be
|
||||
|
@ -290,6 +302,11 @@ this.MetricsCollectionResult = function MetricsCollectionResult(name) {
|
|||
this.expectedMeasurements = new Set();
|
||||
this.errors = [];
|
||||
|
||||
this.populate = function populate() {
|
||||
throw new Error("populate() must be defined on MetricsCollectionResult " +
|
||||
"instance.");
|
||||
};
|
||||
|
||||
this._deferred = Promise.defer();
|
||||
}
|
||||
|
||||
|
|
|
@ -37,6 +37,8 @@ this.DummyProvider = function DummyProvider(name="DummyProvider") {
|
|||
|
||||
this.constantMeasurementName = "DummyMeasurement";
|
||||
this.collectConstantCount = 0;
|
||||
this.throwDuringCollectConstantMeasurements = null;
|
||||
this.throwDuringConstantPopulate = null;
|
||||
}
|
||||
DummyProvider.prototype = {
|
||||
__proto__: MetricsProvider.prototype,
|
||||
|
@ -46,13 +48,27 @@ DummyProvider.prototype = {
|
|||
|
||||
let result = this.createResult();
|
||||
result.expectMeasurement(this.constantMeasurementName);
|
||||
|
||||
result.populate = this._populateConstantResult.bind(this);
|
||||
|
||||
if (this.throwDuringCollectConstantMeasurements) {
|
||||
throw new Error(this.throwDuringCollectConstantMeasurements);
|
||||
}
|
||||
|
||||
return result;
|
||||
},
|
||||
|
||||
_populateConstantResult: function _populateConstantResult(result) {
|
||||
if (this.throwDuringConstantPopulate) {
|
||||
throw new Error(this.throwDuringConstantPopulate);
|
||||
}
|
||||
|
||||
this._log.debug("Populating constant measurement in DummyProvider.");
|
||||
result.addMeasurement(new DummyMeasurement(this.constantMeasurementName));
|
||||
|
||||
result.setValue(this.constantMeasurementName, "string", "foo");
|
||||
result.setValue(this.constantMeasurementName, "uint32", 24);
|
||||
|
||||
result.finish();
|
||||
|
||||
return result;
|
||||
},
|
||||
};
|
||||
|
|
|
@ -20,11 +20,22 @@ add_test(function test_constructor() {
|
|||
let failed = false;
|
||||
try {
|
||||
new MetricsCollectionResult();
|
||||
} catch(ex) {
|
||||
} catch (ex) {
|
||||
do_check_true(ex.message.startsWith("Must provide name argument to Metrics"));
|
||||
failed = true;
|
||||
} finally {
|
||||
do_check_true(failed);
|
||||
failed = false;
|
||||
}
|
||||
|
||||
try {
|
||||
result.populate();
|
||||
} catch (ex) {
|
||||
do_check_true(ex.message.startsWith("populate() must be defined"));
|
||||
failed = true;
|
||||
} finally {
|
||||
do_check_true(failed);
|
||||
failed = false;
|
||||
}
|
||||
|
||||
run_next_test();
|
||||
|
@ -229,3 +240,4 @@ add_test(function test_finish() {
|
|||
|
||||
result.finish();
|
||||
});
|
||||
|
||||
|
|
|
@ -28,6 +28,7 @@ add_test(function test_register_provider() {
|
|||
do_check_eq(collector._providers.length, 1);
|
||||
collector.registerProvider(dummy);
|
||||
do_check_eq(collector._providers.length, 1);
|
||||
do_check_eq(collector.providerErrors.size, 1);
|
||||
|
||||
let failed = false;
|
||||
try {
|
||||
|
@ -59,6 +60,43 @@ add_test(function test_collect_constant_measurements() {
|
|||
do_check_true(result instanceof MetricsCollectionResult);
|
||||
|
||||
do_check_true(collector._providers[0].constantsCollected);
|
||||
do_check_eq(collector.providerErrors.get("DummyProvider").length, 0);
|
||||
|
||||
run_next_test();
|
||||
});
|
||||
});
|
||||
|
||||
add_test(function test_collect_constant_throws() {
|
||||
let collector = new MetricsCollector();
|
||||
let provider = new DummyProvider();
|
||||
provider.throwDuringCollectConstantMeasurements = "Fake error during collect";
|
||||
collector.registerProvider(provider);
|
||||
|
||||
collector.collectConstantMeasurements().then(function onResult() {
|
||||
do_check_eq(collector.providerErrors.get("DummyProvider").length, 1);
|
||||
do_check_eq(collector.providerErrors.get("DummyProvider")[0].message,
|
||||
provider.throwDuringCollectConstantMeasurements);
|
||||
|
||||
run_next_test();
|
||||
});
|
||||
});
|
||||
|
||||
add_test(function test_collect_constant_populate_throws() {
|
||||
let collector = new MetricsCollector();
|
||||
let provider = new DummyProvider();
|
||||
provider.throwDuringConstantPopulate = "Fake error during constant populate";
|
||||
collector.registerProvider(provider);
|
||||
|
||||
collector.collectConstantMeasurements().then(function onResult() {
|
||||
do_check_eq(collector.collectionResults.size, 1);
|
||||
do_check_true(collector.collectionResults.has("DummyProvider"));
|
||||
|
||||
let result = collector.collectionResults.get("DummyProvider");
|
||||
do_check_eq(result.errors.length, 1);
|
||||
do_check_eq(result.errors[0].message, provider.throwDuringConstantPopulate);
|
||||
|
||||
do_check_false(collector._providers[0].constantsCollected);
|
||||
do_check_eq(collector.providerErrors.get("DummyProvider").length, 0);
|
||||
|
||||
run_next_test();
|
||||
});
|
||||
|
|
|
@ -45,11 +45,12 @@ add_test(function test_default_collectors() {
|
|||
run_next_test();
|
||||
});
|
||||
|
||||
add_test(function test_collect_synchronous() {
|
||||
add_test(function test_collect_constant_synchronous() {
|
||||
let provider = new DummyProvider();
|
||||
|
||||
let result = provider.collectConstantMeasurements();
|
||||
do_check_true(result instanceof MetricsCollectionResult);
|
||||
result.populate(result);
|
||||
|
||||
result.onFinished(function onResult(res2) {
|
||||
do_check_eq(result, res2);
|
||||
|
|
Загрузка…
Ссылка в новой задаче