1945157 - Intermittent comm/mail/test/browser/folder-display/browser_mailTelemetry.js | Count of S/MIME encrypted mails read should be correct in run 1 - 1 == 2 -

Status: RESOLVED FIXED

(Thunderbird :: General, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: mkmelin [at] iki.fi
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=492610277&repo=comm-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/DPcpPhbNSaytG2VKvyOW1w/runs/0/artifacts/public/logs/live_backing.log

---

Comment 1

[Magnus Melin [:mkmelin]]

Using real data since bug 1864205 which would have at least made some timing difference

Comment 2

[Intermittent Failures Robot]

Summary

2 failures in 4566 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 2

Platform and build breakdown:

• linux1804-64-qr: 2
  • opt: 2

Table

	no_variant
linux1804-64-qr--/opt	2

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1945157&startday=2025-01-27&endday=2025-02-02&tree=all

Comment 3

[Intermittent Failures Robot]

Summary

6 failures in 4309 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 6

Platform and build breakdown:

• linux1804-64-ccov-qr: 1
  • opt: 1
• linux1804-64-qr: 5
  • opt: 5

Table

	no_variant
linux1804-64-ccov-qr--/opt	1
linux1804-64-qr--/opt	5

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1945157&startday=2025-02-03&endday=2025-02-09&tree=all

Comment 4

[Intermittent Failures Robot]

Summary

7 failures in 5022 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 7

Platform and build breakdown:

• linux1804-64-ccov-qr: 2
  • opt: 2
• linux1804-64-qr: 5
  • opt: 5

Table

	no_variant
linux1804-64-ccov-qr--/opt	2
linux1804-64-qr--/opt	5

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1945157&startday=2025-02-10&endday=2025-02-16&tree=all

Comment 5

[Intermittent Failures Robot]

Summary

7 failures in 4340 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 7

Platform and build breakdown:

• linux1804-64-ccov-qr: 1
  • opt: 1
• linux1804-64-qr: 6
  • opt: 6

Table

	no_variant
linux1804-64-ccov-qr--/opt	1
linux1804-64-qr--/opt	6

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1945157&startday=2025-02-17&endday=2025-02-23&tree=all

Comment 6

[John Bieling]

Attachment: Bug 1945157 - Attempt to fix intermittent fail in browser_mailTelemetry.js. r=kaie

While trying to understand the intermittent fail, it was observed that
the test is not testing what it should and that the implementation
to report telemetry data is broken.

This patch aims to fix the test and the implementation without changing
the intention of the original implementation.

1. What is wrong with the implementation?

Logs (see [1]) from a test with more debug output shows, that we record
two records for the first encrypted S/MIME message, but none for the
second. The OpenPGP message is recorded as is_encrypted: false.

This is caused by the load process of encrypted messages, which
get reloaded multiple times when they are displayed. This clears the
read flag and the so far collected telemetry data. Any incoming data
after a reload which cleared the read flag is ignored.

2. Why is the test still passing, if there is no telemetry for the
   second message and the OpenPGP message is marked as "not encrypted"?

The test is using a faulty filter to reduce the seen events to encrypted
messages only. The statement assumes that the glean entry for
is_encrypted is a boolean, but is in fact a string (either "true" or
"false"). The filter condition therefore returns true for
is_encrypted being set to "false".

3. What does this patch do?

The telemetry data is no longer cleared on reload, but only if a
different message is loaded (idea from Geoff, see [2]). When data is
collected, we initiate a timer to process the collected data, but the
timer gets reset each time a new telemetry information comes in. This
will give us a single telemetry set per message with the required
information (security, is_encrypted and is_signed).

The intermittent is fixed by introducing a new CustomEvent, which
tells the test and other consumers, when telemetry data has been
processed.

[1] : https://phabricator.services.mozilla.com/D239902#8284314
[2] : https://phabricator.services.mozilla.com/D197795#7213166

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2970431527d2
Attempt to fix intermittent fail in browser_mailTelemetry.js. r=kaie

Comment 8

[Intermittent Failures Robot]

Summary

7 failures in 4564 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• comm-central: 7

Platform and build breakdown:

• linux1804-64-ccov-qr: 3
  • opt: 3
• linux1804-64-qr: 4
  • opt: 4

Table

	no_variant
linux1804-64-ccov-qr--/opt	3
linux1804-64-qr--/opt	4

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1945157&startday=2025-02-24&endday=2025-03-02&tree=all

Comment 9

[Kai Engert [:KaiE:]]

I can try to create a backport patch for esr128.

Comment 10

[Kai Engert [:KaiE:]]

Attachment: 1945157-esr128.patch

This is a backport to esr128.

It applies all the logic changes. The only difference is, it keeps using the old Telemetry reporting (not glean).

Comment 11

[Kai Engert [:KaiE:]]

Attachment: 1945157-esr128-v2.patch

Backport v2, now with liniting fixed...

Comment 12

[Corey Bryant]

Comment on attachment 9469482 [details] [diff] [review]
1945157-esr128-v2.patch

[Triage Comment]
Approved for esr128

Comment 13

[Kai Engert [:KaiE:]]

Comment on attachment 9469482 [details] [diff] [review]
1945157-esr128-v2.patch

[User impact if declined]
No user impact

[Is this code covered by automated tests?]
yes

[Has the fix been verified in Daily? (or Beta for an ESR uplift?)]
no, because it isn't a functional change.

[Needs manual test from QA?]
no

[List of other uplifts needed]
none

[Risk to taking this patch]
low

[Why is the change risky/not risky? (and alternatives if risky)]
The code doesn't change user functionality.
All it changes is how we record telemetry numbers.

[String changes made/needed]
none

Comment 14

[Corey Bryant]

Comment on attachment 9469482 [details] [diff] [review]
1945157-esr128-v2.patch

[Triage Comment]
Approved for esr128

Comment 15

[Corey Bryant]

Thunderbird 128.8.0esr:
https://hg.mozilla.org/releases/comm-esr128/rev/439c9f6276a4

Comment 16

[Corey Bryant]

Thunderbird 128.9.0esr:
https://hg.mozilla.org/releases/comm-esr128/rev/1580a709e828fb86a24265616dce5b21e7ca43ba (see bug 1954442)
https://hg.mozilla.org/releases/comm-esr128/rev/fa4bb08ca2ff92feae671a9e954aad6734caaa29 (see bug 1954442 - reverted)