1600352 - Renew the QM_INIT_TELEMETRY_ERROR telemetry

Status: RESOLVED INVALID

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

Description

[Tom Tung [:tt, :ttung]]

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

Comment 1

[Jan Varga [:janv]]

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.

Comment 2

[Jan Varga [:janv]]

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.

Comment 3

[Tom Tung [:tt, :ttung]]

(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

Comment 4

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1600352 - P1 - Return the result if there is an error on the non-first run for InitializeRepository on Nightly;

Comment 5

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1600352 - P2 - Collect init errors while initializing the repositories and report all errors at once;

Depends on D55469

Comment 6

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1600352 - P3 - Rename QM_INIT_TELEMETRY_ERROR to QM_INIT_REPOSITORY_ERROR;

Comment 7

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1600352 - P6 - Introduce QM_INIT_STORAGE_ERROR;

Depends on D57102

Comment 8

[Tom Tung [:tt, :ttung]]

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

Comment 9

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1600352 - P4 - Make the InitializationTypes be able to reuse the mInitializationErrors for different telemetry probes;

Depends on D57102

Comment 10

[Tom Tung [:tt, :ttung]]

Attachment: Bug 1600352 - P5 - Make the test_qm_first_repository_init_errs more generic to test the another telemetry probe in the future;

Depends on D57950

Comment 11

[Jan Varga [:janv]]

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

Comment 12

[Simon Giesecke [:sg] [he/him]]

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 :(

Comment 13

[Jan Varga [:janv]]

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).

Comment 14

[Tom Tung [:tt, :ttung]]

(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.