From ad0ce3147c8a96c33737b8ce9e5fe44db0b78d93 Mon Sep 17 00:00:00 2001 From: Alessio Placitelli Date: Wed, 27 Sep 2017 10:12:13 +0200 Subject: [PATCH] Bug 1395835 - Rename "group" to "category" in the scalar code and docs. r=chutten MozReview-Commit-ID: HQ3I2cfEaa --HG-- extra : rebase_source : e7639b2b33d6ad1b20d6e20f7e2c84fef68e9096 --- toolkit/components/telemetry/Scalars.yaml | 2 +- toolkit/components/telemetry/Telemetry.cpp | 4 +- .../components/telemetry/TelemetryScalar.cpp | 20 ++++---- .../components/telemetry/TelemetryScalar.h | 2 +- .../telemetry/docs/collection/scalars.rst | 22 ++++----- toolkit/components/telemetry/nsITelemetry.idl | 10 ++-- toolkit/components/telemetry/parse_scalars.py | 47 ++++++++++--------- .../tests/browser/browser_DynamicScalars.js | 2 +- .../tests/unit/test_TelemetryScalars.js | 14 +++--- 9 files changed, 62 insertions(+), 61 deletions(-) diff --git a/toolkit/components/telemetry/Scalars.yaml b/toolkit/components/telemetry/Scalars.yaml index b711caa028f9..2dc2a9f2771b 100644 --- a/toolkit/components/telemetry/Scalars.yaml +++ b/toolkit/components/telemetry/Scalars.yaml @@ -1277,5 +1277,5 @@ telemetry.test: - 'all_childs' # NOTE: Please don't add new definitions below this point. Consider adding -# them earlier in the file and leave the telemetry.test group as the last +# them earlier in the file and leave the telemetry.test category as the last # one for readability. diff --git a/toolkit/components/telemetry/Telemetry.cpp b/toolkit/components/telemetry/Telemetry.cpp index 2fb84bdb970a..5d22479756d8 100644 --- a/toolkit/components/telemetry/Telemetry.cpp +++ b/toolkit/components/telemetry/Telemetry.cpp @@ -1694,11 +1694,11 @@ TelemetryImpl::SnapshotKeyedScalars(unsigned int aDataset, bool aClearScalars, J } NS_IMETHODIMP -TelemetryImpl::RegisterScalars(const nsACString& aGroupName, +TelemetryImpl::RegisterScalars(const nsACString& aCategoryName, JS::Handle aScalarData, JSContext* cx) { - return TelemetryScalar::RegisterScalars(aGroupName, aScalarData, cx); + return TelemetryScalar::RegisterScalars(aCategoryName, aScalarData, cx); } NS_IMETHODIMP diff --git a/toolkit/components/telemetry/TelemetryScalar.cpp b/toolkit/components/telemetry/TelemetryScalar.cpp index 6c782a219c83..aa0cac89dc43 100644 --- a/toolkit/components/telemetry/TelemetryScalar.cpp +++ b/toolkit/components/telemetry/TelemetryScalar.cpp @@ -91,10 +91,10 @@ namespace { const uint32_t kMaximumNumberOfKeys = 100; const uint32_t kMaximumKeyStringLength = 70; const uint32_t kMaximumStringValueLength = 50; -// The group and scalar name maximum lengths are used by the dynamic +// The category and scalar name maximum lengths are used by the dynamic // scalar registration function and must match the constants used by // the 'parse_scalars.py' script for static scalars. -const uint32_t kMaximumGroupNameLength = 40; +const uint32_t kMaximumCategoryNameLength = 40; const uint32_t kMaximumScalarNameLength = 40; const uint32_t kScalarCount = static_cast(mozilla::Telemetry::ScalarID::ScalarCount); @@ -2051,7 +2051,7 @@ TelemetryScalar::SetMaximum(mozilla::Telemetry::ScalarID aId, const nsAString& a /** * Serializes the scalars from the given dataset to a json-style object and resets them. * The returned structure looks like: - * {"process": {"group1.probe":1,"group1.other_probe":false,...}, ... }. + * {"process": {"category1.probe":1,"category1.other_probe":false,...}, ... }. * * @param aDataset DATASET_RELEASE_CHANNEL_OPTOUT or DATASET_RELEASE_CHANNEL_OPTIN. * @param aClear Whether to clear out the scalars after snapshotting. @@ -2158,7 +2158,7 @@ TelemetryScalar::CreateSnapshots(unsigned int aDataset, bool aClearScalars, JSCo /** * Serializes the scalars from the given dataset to a json-style object and resets them. * The returned structure looks like: - * { "process": { "group1.probe": { "key_1": 2, "key_2": 1, ... }, ... }, ... } + * { "process": { "category1.probe": { "key_1": 2, "key_2": 1, ... }, ... }, ... } * * @param aDataset DATASET_RELEASE_CHANNEL_OPTOUT or DATASET_RELEASE_CHANNEL_OPTIN. * @param aClear Whether to clear out the keyed scalars after snapshotting. @@ -2280,16 +2280,16 @@ TelemetryScalar::CreateKeyedSnapshots(unsigned int aDataset, bool aClearScalars, } nsresult -TelemetryScalar::RegisterScalars(const nsACString& aGroupName, +TelemetryScalar::RegisterScalars(const nsACString& aCategoryName, JS::Handle aScalarData, JSContext* cx) { MOZ_ASSERT(XRE_IsParentProcess(), "Dynamic scalars should only be created in the parent process."); - if (!IsValidIdentifierString(aGroupName, kMaximumGroupNameLength, true, false)) { - JS_ReportErrorASCII(cx, "Invalid group name %s.", - PromiseFlatCString(aGroupName).get()); + if (!IsValidIdentifierString(aCategoryName, kMaximumCategoryNameLength, true, false)) { + JS_ReportErrorASCII(cx, "Invalid category name %s.", + PromiseFlatCString(aCategoryName).get()); return NS_ERROR_INVALID_ARG; } @@ -2322,9 +2322,9 @@ TelemetryScalar::RegisterScalars(const nsACString& aGroupName, return NS_ERROR_INVALID_ARG; } - // Join the group and the probe names. + // Join the category and the probe names. nsPrintfCString fullName("%s.%s", - PromiseFlatCString(aGroupName).get(), + PromiseFlatCString(aCategoryName).get(), NS_ConvertUTF16toUTF8(scalarName).get()); JS::RootedValue value(cx); diff --git a/toolkit/components/telemetry/TelemetryScalar.h b/toolkit/components/telemetry/TelemetryScalar.h index ed0a53df0886..79df802686df 100644 --- a/toolkit/components/telemetry/TelemetryScalar.h +++ b/toolkit/components/telemetry/TelemetryScalar.h @@ -55,7 +55,7 @@ void Set(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, uint32_t aValu void Set(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, bool aValue); void SetMaximum(mozilla::Telemetry::ScalarID aId, const nsAString& aKey, uint32_t aValue); -nsresult RegisterScalars(const nsACString& aGroupName, JS::Handle aScalarData, +nsresult RegisterScalars(const nsACString& aCategoryName, JS::Handle aScalarData, JSContext* cx); // Only to be used for testing. diff --git a/toolkit/components/telemetry/docs/collection/scalars.rst b/toolkit/components/telemetry/docs/collection/scalars.rst index 5054e4095cf4..d20f2da358b2 100644 --- a/toolkit/components/telemetry/docs/collection/scalars.rst +++ b/toolkit/components/telemetry/docs/collection/scalars.rst @@ -39,11 +39,11 @@ additional information. .. code-block:: js - Services.telemetry.registerScalars(group, scalarData); + Services.telemetry.registerScalars(category, scalarData); Register new scalars from add-ons. -* ``group`` - *(required, string)* The unique group the scalars are registered in (see :ref:`limitations `). +* ``category`` - *(required, string)* The unique category the scalars are registered in (see :ref:`limitations `). * ``scalarData`` - *(required, object)* An object of the form ``{scalarName1: scalar1Data, ...}`` that contains registration data for multiple scalars; ``scalarName1`` is subject to :ref:`limitations `; each scalar is an object with the following properties: * ``kind`` - *(required, uint)* One of the scalar types (nsITelemetry::SCALAR_TYPE_*). @@ -59,13 +59,13 @@ After registration, the scalars can be recorded through the usual scalar JS API. Accumulating in dynamic scalars only works in content child processes and in the parent process. All the accumulations (parent and content chldren) are aggregated together . -New scalars registered here are subject to the same :ref:`limitations ` as the ones registered through ``Scalars.yaml``, e.g. the length of the group name or the allowed characters. +New scalars registered here are subject to the same :ref:`limitations ` as the ones registered through ``Scalars.yaml``, e.g. the length of the category name or the allowed characters. Example: .. code-block:: js - Services.telemetry.registerScalars("myAddon.group", { + Services.telemetry.registerScalars("myAddon.category", { "counter_scalar": { kind: Ci.nsITelemetry.SCALAR_TYPE_COUNT, keyed: false, @@ -73,7 +73,7 @@ Example: }, }); // Now scalars can be recorded. - Services.telemetry.scalarSet("myAddon.group.counter_scalar", 37); + Services.telemetry.scalarSet("myAddon.category.counter_scalar", 37); C++ API ------- @@ -106,8 +106,8 @@ The probes in the definition file are represented in a fixed-depth, two-level st .. code-block:: yaml - # The following is a group. - a.group.hierarchy: + # The following is a category. + a.category.hierarchy: a_probe_name: kind: uint ... @@ -115,24 +115,24 @@ The probes in the definition file are represented in a fixed-depth, two-level st kind: string ... ... - group2: + category2: probe: kind: int ... .. _scalar-limitations: -Group and probe names need to follow a few rules: +Category and probe names need to follow a few rules: - they cannot exceed 40 characters each; -- group names must be alpha-numeric + ``.``, with no leading/trailing digit or ``.``; +- category names must be alpha-numeric + ``.``, with no leading/trailing digit or ``.``; - probe names must be alpha-numeric + ``_``, with no leading/trailing digit or ``_``. A probe can be defined as follows: .. code-block:: yaml - a.group.hierarchy: + a.category.hierarchy: a_scalar: bug_numbers: - 1276190 diff --git a/toolkit/components/telemetry/nsITelemetry.idl b/toolkit/components/telemetry/nsITelemetry.idl index 68ea599fbafa..dacfa4f61d62 100644 --- a/toolkit/components/telemetry/nsITelemetry.idl +++ b/toolkit/components/telemetry/nsITelemetry.idl @@ -355,7 +355,7 @@ interface nsITelemetry : nsISupports /** * Serializes the scalars from the given dataset to a JSON-style object and resets them. * The returned structure looks like: - * {"process": {"group1.probe":1,"group1.other_probe":false,...}, ... }. + * {"process": {"category1.probe":1,"category1.other_probe":false,...}, ... }. * * @param aDataset DATASET_RELEASE_CHANNEL_OPTOUT or DATASET_RELEASE_CHANNEL_OPTIN. * @param [aClear=false] Whether to clear out the scalars after snapshotting. @@ -398,7 +398,7 @@ interface nsITelemetry : nsISupports * Serializes the keyed scalars from the given dataset to a JSON-style object and * resets them. * The returned structure looks like: - * { "process": { "group1.probe": { "key_1": 2, "key_2": 1, ... }, ... }, ... } + * { "process": { "category1.probe": { "key_1": 2, "key_2": 1, ... }, ... }, ... } * * @param aDataset DATASET_RELEASE_CHANNEL_OPTOUT or DATASET_RELEASE_CHANNEL_OPTIN. * @param [aClear=false] Whether to clear out the scalars after snapshotting. @@ -488,11 +488,11 @@ interface nsITelemetry : nsISupports /** * Parent process only. Register new scalars to record them from addons. This - * allows registering multiple scalars for a group. They will be valid only for + * allows registering multiple scalars for a category. They will be valid only for * the current Firefox session. * Note that scalars shipping in Firefox should be registered in Scalars.yaml. * - * @param aGroupName The unique group the scalars are registered in. + * @param aCategoryName The unique category the scalars are registered in. * @param aScalarData An object that contains registration data for multiple scalars in the form: * { * "sample_scalar": { @@ -511,7 +511,7 @@ interface nsITelemetry : nsISupports * recording it without error, but it will be discarded. Defaults to false. */ [implicit_jscontext] - void registerScalars(in ACString aGroupName, in jsval aScalarData); + void registerScalars(in ACString aCategoryName, in jsval aScalarData); /** * Resets all the stored events. This is intended to be only used in tests. diff --git a/toolkit/components/telemetry/parse_scalars.py b/toolkit/components/telemetry/parse_scalars.py index 80db01fde33a..dc30b77d547b 100755 --- a/toolkit/components/telemetry/parse_scalars.py +++ b/toolkit/components/telemetry/parse_scalars.py @@ -24,13 +24,13 @@ SCALAR_TYPES_MAP = { class ScalarType: """A class for representing a scalar definition.""" - def __init__(self, group_name, probe_name, definition, strict_type_checks): + def __init__(self, category_name, probe_name, definition, strict_type_checks): # Validate and set the name, so we don't need to pass it to the other # validation functions. self._strict_type_checks = strict_type_checks - self.validate_names(group_name, probe_name) + self.validate_names(category_name, probe_name) self._name = probe_name - self._group_name = group_name + self._category_name = category_name # Validating the scalar definition. self.validate_types(definition) @@ -40,20 +40,20 @@ class ScalarType: self._definition = definition self._expires = utils.add_expiration_postfix(definition['expires']) - def validate_names(self, group_name, probe_name): - """Validate the group and probe name: - - Group name must be alpha-numeric + '.', no leading/trailing digit or '.'. + def validate_names(self, category_name, probe_name): + """Validate the category and probe name: + - Category name must be alpha-numeric + '.', no leading/trailing digit or '.'. - Probe name must be alpha-numeric + '_', no leading/trailing digit or '_'. - :param group_name: the name of the group the probe is in. + :param category_name: the name of the category the probe is in. :param probe_name: the name of the scalar probe. :raises ParserError: if the length of the names exceeds the limit or they don't conform our name specification. """ - # Enforce a maximum length on group and probe names. + # Enforce a maximum length on category and probe names. MAX_NAME_LENGTH = 40 - for n in [group_name, probe_name]: + for n in [category_name, probe_name]: if len(n) > MAX_NAME_LENGTH: raise ParserError(("Name '{}' exceeds maximum name length of {} characters.\n" "See: {}#the-yaml-definition-file") @@ -72,7 +72,7 @@ class ScalarType: "digit, a dot or underscore. Got: '{}'.\n" " See: {}#the-yaml-definition-file").format(name, BASE_DOC_URL)) - check_name(group_name, 'Group', r'\.') + check_name(category_name, 'Category', r'\.') check_name(probe_name, 'Probe', r'_') def validate_types(self, definition): @@ -202,12 +202,12 @@ class ScalarType: @property def label(self): - """Get the scalar label generated from the scalar and group names.""" - return self._group_name + '.' + self._name + """Get the scalar label generated from the scalar and category names.""" + return self._category_name + '.' + self._name @property def enum_label(self): - """Get the enum label generated from the scalar and group names. This is used to + """Get the enum label generated from the scalar and category names. This is used to generate the enum tables.""" # The scalar name can contain informations about its hierarchy (e.g. 'a.b.scalar'). @@ -303,19 +303,20 @@ def load_scalars(filename, strict_type_checks=True): scalar_list = [] # Scalars are defined in a fixed two-level hierarchy within the definition file. - # The first level contains the group name, while the second level contains the - # probe name (e.g. "group.name: probe: ..."). - for group_name in scalars: - group = scalars[group_name] + # The first level contains the category name, while the second level contains the + # probe name (e.g. "category.name: probe: ..."). + for category_name in scalars: + category = scalars[category_name] - # Make sure that the group has at least one probe in it. - if not group or len(group) == 0: + # Make sure that the category has at least one probe in it. + if not category or len(category) == 0: raise ParserError('Category "{}" must have at least one probe in it' + - '.\nSee: {}'.format(group_name, BASE_DOC_URL)) + '.\nSee: {}'.format(category_name, BASE_DOC_URL)) - for probe_name in group: + for probe_name in category: # We found a scalar type. Go ahead and parse it. - scalar_info = group[probe_name] - scalar_list.append(ScalarType(group_name, probe_name, scalar_info, strict_type_checks)) + scalar_info = category[probe_name] + scalar_list.append( + ScalarType(category_name, probe_name, scalar_info, strict_type_checks)) return scalar_list diff --git a/toolkit/components/telemetry/tests/browser/browser_DynamicScalars.js b/toolkit/components/telemetry/tests/browser/browser_DynamicScalars.js index ed94a660289d..9ce8e523a579 100644 --- a/toolkit/components/telemetry/tests/browser/browser_DynamicScalars.js +++ b/toolkit/components/telemetry/tests/browser/browser_DynamicScalars.js @@ -107,7 +107,7 @@ add_task(async function test_aggregation() { // Register test scalars before spawning the content process: the scalar // definitions will propagate to it. Also cheat TelemetrySession to put // the test scalar in the payload by using "cheattest" instead of "test" in - // the scalar group name. + // the scalar category name. Services.telemetry.registerScalars("telemetry.cheattest.dynamic", { "test_aggregation": { kind: Ci.nsITelemetry.SCALAR_TYPE_COUNT, diff --git a/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js b/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js index 3a9873702c04..4550533f752d 100644 --- a/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js +++ b/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js @@ -541,7 +541,7 @@ add_task(function* test_dynamicScalars_registration() { const TEST_CASES = [ { - "group": "telemetry.test", + "category": "telemetry.test", "data": { "missing_kind": { keyed: false, @@ -552,7 +552,7 @@ add_task(function* test_dynamicScalars_registration() { "description": "Registration must fail if required fields are missing" }, { - "group": "telemetry.test", + "category": "telemetry.test", "data": { "invalid_collection": { kind: Ci.nsITelemetry.SCALAR_TYPE_COUNT, @@ -563,7 +563,7 @@ add_task(function* test_dynamicScalars_registration() { "description": "Registration must fail if 'record_on_release' is of the wrong type" }, { - "group": "telemetry.test", + "category": "telemetry.test", "data": { "invalid_kind": { kind: "12", @@ -573,7 +573,7 @@ add_task(function* test_dynamicScalars_registration() { "description": "Registration must fail if 'kind' is of the wrong type" }, { - "group": "telemetry.test", + "category": "telemetry.test", "data": { "invalid_expired": { kind: Ci.nsITelemetry.SCALAR_TYPE_COUNT, @@ -584,7 +584,7 @@ add_task(function* test_dynamicScalars_registration() { "description": "Registration must fail if 'expired' is of the wrong type" }, { - "group": "telemetry.test", + "category": "telemetry.test", "data": { "valid_scalar": { kind: Ci.nsITelemetry.SCALAR_TYPE_COUNT, @@ -599,7 +599,7 @@ add_task(function* test_dynamicScalars_registration() { "description": "No scalar must be registered if the batch contains an invalid one" }, { - "group": "telemetry.test", + "category": "telemetry.test", "data": { "unsigned_int_kind": { kind: Ci.nsITelemetry.SCALAR_TYPE_COUNT, @@ -613,7 +613,7 @@ add_task(function* test_dynamicScalars_registration() { ]; for (let testCase of TEST_CASES) { - Assert.throws(() => Telemetry.registerScalars(testCase.group, testCase.data), + Assert.throws(() => Telemetry.registerScalars(testCase.category, testCase.data), testCase.evaluation, testCase.description); } });