Closed Bug 1513496 Opened 2 years ago Closed 2 years ago
Stop logging warning when scalar is expired
47 bytes, text/x-phabricator-request
|Details | Review|
STR: 1) Run a test in debug mode that includes getting keyed scalars, e.g. browser/modules/test/browser/browser_BrowserErrorReporter.js or browser/components/search/test/browser/browser_searchTelemetry.js 2) Look through the output Actual Results Lots of messages like: [Parent 13597, Main Thread] WARNING: NS_FAILED internal_GetScalarByEnum for CHILD: file /Users/mark/dev/gecko/toolkit/components/telemetry/core/TelemetryScalar.cpp, line 2161 Expected Results No messages I found this originally with browser_searchTelemetry.js but seeing as this happens in other tests, I'm assuming it isn't something I'm doing wrong.
(( First off, the keyed scalar failure message is wrong and ambiguous. I've filed bug 1513546 to fix that )) This comes from the parent side of the IPC channel where we try to take all the accumulations that happened in the child and apply them. Given that we're not asserting, the ID is valid, we aren't asking for a keyed string scalar (unsupported)... we're unlikely to have failed in the KeyedScalar ctor, so that's not likely. That leaves the case of accumulations in the child-process to expired scalars . Which isn't really that big of a deal. Maybe we shouldn't warn on that? : https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/toolkit/components/telemetry/core/TelemetryScalar.cpp#1712 : https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/toolkit/components/telemetry/core/TelemetryScalar.cpp#1756 : https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/toolkit/components/telemetry/core/TelemetryScalar.cpp#1762 : https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/toolkit/components/telemetry/core/TelemetryScalar.cpp#1751
Hey Chris, do you think we should remove the waning there? If so, what do you think about making this mentored?
Even better would be to both do that _and_ remove the expired scalars. But we can tackle the first part first: To help Mozilla out with this bug, here's the steps: 0) Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you. 1) Download and build the Firefox source code: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_build - If you have any problems, please ask on IRC (https://wiki.mozilla.org/Irc) in the #introduction channel. They're there to help you get started. - You can also read the Developer Guide, which has answers to most development questions: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction 2) Start working on this bug. You'll be working in the TelemetrScalar.cpp file, making sure the warning here  doesn't fire when the nsresult is NS_ERROR_NOT_AVAILABLE (due to the scalar being expired). - If you have any problems with this bug, please comment on this bug and set the needinfo flag for me. Also, you can find me and my teammates on the #telemetry channel on IRC (https://wiki.mozilla.org/Irc) most hours of most days. Be sure to mention this bug number so we can see the context. 3) Build your change with `mach build` and test your change with `mach test toolkit/components/telemetry/tests/`. Also check your changes for adherence to our style guidelines by using `mach lint` 4) Submit the patch for review. Mark me as a reviewer so I'll get an email to come look at your code. - Here's the guide: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch 5) After a series of reviews and changes to your patch, I'll ask Phabricator to push it to autoland. Your code will soon be shipping to Firefox users worldwide! 6) ...now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution. : https://searchfox.org/mozilla-central/rev/adcc169dcf58c2e45ba65c4ed5661d666fc3ac74/toolkit/components/telemetry/core/TelemetryScalar.cpp#2274
Priority: -- → P3
Whiteboard: [good first bug][lang=c++]
Hi! I'd love to help with this bug if that's alright!
Assignee: nobody → kaioaugusto.8
Status: NEW → ASSIGNED
Summary: WARNING: NS_FAILED internal_GetScalarByEnum for CHILD: file /Users/mark/dev/gecko/toolkit/components/telemetry/core/TelemetryScalar.cpp → Stop logging warning when scalar is expired.
Currently, when operating with scalars, if a call to internal_GetScalarByEnum (or its keyed variant) return an error, then a warning will be logged. If one of the requested scalars is expired, this could lead to an unwated flood of logs. With this change, the return of the function is checked, and if it is NS_ERROR_NOT_AVAILABLE (i.e. expired scalar), then no warning is issued.
Attachment #9036136 - Attachment description: Stop logging failure warnings if the scalar is expired. → Bug 1513496 - Stop logging failure warnings if the scalar is expired. r=chutten
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/d352b4fe9d68 Stop logging failure warnings if the scalar is expired. r=chutten
You need to log in before you can comment on or make changes to this bug.