Closed
Bug 1340542
Opened 7 years ago
Closed 7 years ago
Move the scalar ID checks to the public API
Categories
(Toolkit :: Telemetry, defect, P1)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: Dexter, Assigned: Dexter)
References
(Blocks 1 open bug)
Details
(Whiteboard: [measurement:client])
Attachments
(1 file)
As done in bug 1339365, we need to move the scalar id checks consistently to the public Scalars API. This means we should move the calls at [1] to the public callable APIs, e.g. [2]. [1] - http://searchfox.org/mozilla-central/rev/ea31c4b64f34a29415a69fb768f8051495547315/toolkit/components/telemetry/TelemetryScalar.cpp#1011,1147 [2] - http://searchfox.org/mozilla-central/rev/ea31c4b64f34a29415a69fb768f8051495547315/toolkit/components/telemetry/TelemetryScalar.cpp#1336
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → alessio.placitelli
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8839868 [details] Bug 1340542 - Move the scalar ID checks to the public API. https://reviewboard.mozilla.org/r/114474/#review117504 ::: toolkit/components/telemetry/tests/gtest/TestScalars.cpp:331 (Diff revision 1) > + // Make sure we don't get scalars from other tests. > + Unused << mTelemetry->ClearScalars(); > + > +// Don't run this part in debug builds as that intentionally asserts. > +#ifndef DEBUG > + Telemetry::ScalarSet(Telemetry::ScalarID::ScalarCount, static_cast<uint32_t>(1)); Don't use only `ScalarCount`, use some other `ScalarCount + highNumber` and `std::numeric_limits<uint32_t>::max()` as well. ::: toolkit/components/telemetry/tests/gtest/TestScalars.cpp:342 (Diff revision 1) > + Telemetry::ScalarSetMaximum(Telemetry::ScalarID::ScalarCount, NS_LITERAL_STRING("key1"), 1); > +#endif You still need to check that we didn't record anything into the scalars.
Attachment #8839868 -
Flags: review?(gfritzsche)
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8839868 [details] Bug 1340542 - Move the scalar ID checks to the public API. https://reviewboard.mozilla.org/r/114474/#review117806 r=me with the below fixed. ::: toolkit/components/telemetry/tests/gtest/TestScalars.cpp:331 (Diff revision 2) > + Telemetry::ScalarID scalarId = static_cast<Telemetry::ScalarID>(std::limits<uint32_t>::max()); > + Telemetry::ScalarSet(scalarId, static_cast<uint32_t>(1)); > + Telemetry::ScalarSet(scalarId, true); As mentioned, use more than one id. At least: `ScalarCount`, `ScalarCount + N`, `max()`. Dito for the keyed ones below.
Attachment #8839868 -
Flags: review?(gfritzsche) → review+
Comment hidden (mozreview-request) |
Pushed by alessio.placitelli@gmail.com: https://hg.mozilla.org/integration/autoland/rev/578d891011c3 Move the scalar ID checks to the public API. r=gfritzsche
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/578d891011c3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in
before you can comment on or make changes to this bug.
Description
•