Closed Bug 1600352 Opened 3 years ago Closed 1 year ago

Renew the QM_INIT_TELEMETRY_ERROR telemetry

Categories

(Core :: Storage: Quota Manager, task, P3)

task

Tracking

()

RESOLVED INVALID

People

(Reporter: tt, Assigned: tt)

References

(Blocks 1 open bug)

Details

Attachments

(6 obsolete files)

We might want:

  • Continue traverse directories even an error is found on all channels, but only for the first run.
  • Report only for the first attempt
  • All Channels?
  • At least record for the new 6 releases
Status: NEW → ASSIGNED
Priority: -- → P2

Not sure if want to do this on Release (we will have the new telemetry for that). However, we may start discussing if this should be permanent on Nightly at least (maybe even beta).

Regarding reporting it for the first attempt only, yes, I agree. However, if the goal is to use the measurement dashboard directly (without running own queries for adjusting data), we might consider creating own collector class for collecting errors during initialization and reporting telemetry just before exiting EnsureTemporaryStorageIsInitialized method.

Another thing is that we don't cover EnsureStorageIsInitialized at all (especially the upgrade methods).

And, we need to allocate enough "n_values", so we will be able to add new labels easily later.

Maybe we should introduce a telemetry for EnsureStorageIsInitialized, especially for upgrade methods first. I don't plan big changes there, so it makes sense to do it before QM v3.

(In reply to Jan Varga [:janv] from comment #1)

Not sure if want to do this on Release (we will have the new telemetry for that). However, we may start discussing if this should be permanent on Nightly at least (maybe even beta).

Regarding reporting it for the first attempt only, yes, I agree. However, if the goal is to use the measurement dashboard directly (without running own queries for adjusting data), we might consider creating own collector class for collecting errors during initialization and reporting telemetry just before exiting EnsureTemporaryStorageIsInitialized method.

Another thing is that we don't cover EnsureStorageIsInitialized at all (especially the upgrade methods).

And, we need to allocate enough "n_values", so we will be able to add new labels easily later.

Thanks for the suggestion! Will work on a patch for:

  • Reporting it for the first attempt only
  • Implement a class to report init error
  • Cover the EnsureStorageIsInitialized in another new telemetry
Attachment #9112907 - Attachment description: Bug 1600352 - P1 - Return the result if there is an error on the non-first run for InitializeReepository on Nightly; → Bug 1600352 - P1 - Return the result if there is an error on the non-first run for InitializeRepository on Nightly;
Attachment #9115757 - Attachment description: Bug 1600352 - P3 - Rename QM_INIT_TELEMETRY_ERROR to QM_INIT_REPOSITORY_ERROR and add QM_INIT_STORAGE_ERROR; → Bug 1600352 - P3 - Rename QM_INIT_TELEMETRY_ERROR to QM_INIT_REPOSITORY_ERROR;

Depends on D57102

Unassign me since I will be on vacation for several weeks. People in the worker-and-storage team are welcome to take it.

Assignee: ttung → nobody
Status: ASSIGNED → NEW
Assignee: nobody → ttung
Status: NEW → ASSIGNED
Attachment #9115783 - Attachment description: Bug 1600352 - P4 - Introduce QM_INIT_STORAGE_ERROR; → Bug 1600352 - P6 - Introduce QM_INIT_STORAGE_ERROR;
Priority: P2 → P3

I hate to say it, but it seems this bug and patches are obsoleted by QM_TRY and the new event based telemetry.

Yes, for the most part this seems true. Collecting the errors and reporting all of them seems to have had similar goals as reporting QM_TRY errors via telemetry.

I wonder if we should still do what D55469 introduced, i.e. doing the Nightly-specific behaviour of continuing on errors only on the first attempt. The patch will need to be heavily rebased though :(

Yeah, feel free to file a new bug for that.
And we should allow telemetry reporting of QM_TRY failures only on the first attempt too (another new bug should be filed).

Blocks: 1689349
No longer blocks: 1689349
See Also: → 1689349

(In reply to Jan Varga [:janv] from comment #13)

Yeah, feel free to file a new bug for that.
And we should allow telemetry reporting of QM_TRY failures only on the first attempt too (another new bug should be filed).

I filed bug 1689349 and I am going to close this one.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → INVALID
Attachment #9112907 - Attachment is obsolete: true
Attachment #9112908 - Attachment is obsolete: true
Attachment #9115757 - Attachment is obsolete: true
Attachment #9117199 - Attachment is obsolete: true
Attachment #9117200 - Attachment is obsolete: true
Attachment #9115783 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.