Skip to content

Commit 9e12c1b

Browse files
committed
Apply the client ID regen mitigation now
1 parent f60e3b8 commit 9e12c1b

File tree

5 files changed

+25
-20
lines changed

5 files changed

+25
-20
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* Stop reporting db file sizes during init phase ([#3331](https://github.com/mozilla/glean/pull/3331))
77
* Tiny performance improvement for putting tasks on the dispatcher ([#3318](https://github.com/mozilla/glean/pull/3318))
88
* Instrument the case when the `client_id.txt` file does not exist yet ([#3339](https://github.com/mozilla/glean/pull/3339))
9+
* When a missing client ID in the database is detected, Glean now restores the backup client ID ([#3334](https://github.com/mozilla/glean/pull/3334))
910

1011
# v66.1.2 (2025-11-25)
1112

docs/dev/core/internal/client_id_recovery.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ Any inconsistencies in that data compared to the database will be reported
1414
and, if applicable, the client ID restored.
1515

1616
**Note:** Glean v66.1.0 will only report the inconsistency, but will not restore a recovered client ID.
17-
This allows us to measure the impact.
18-
The mitigation will be enabled in a later release ([bug 1996862](https://bugzilla.mozilla.org/show_bug.cgi?id=1996862)).
17+
From Glean v66.2.0 on we apply the mitigation and restore the client ID.
1918

2019
The exact flow of decisions is depicted in the chart below.
2120
The implementation is in [`glean-core/src/core/mod.rs`](https://github.com/mozilla/glean/blob/HEAD/glean-core/src/core/mod.rs#L264)

glean-core/rlb/tests/health_ping_file_overwrite.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ fn test_pre_post_init_health_pings_exist() {
7979
let exception_uuid = &payload["metrics"]["uuid"]["glean.health_recovered_client_id"];
8080
assert_eq!(&JsonValue::Null, exception_uuid);
8181

82-
// TODO(bug 1996862): We don't run the mitigation yet.
83-
//let ping_client_id = &payload["client_info"]["client_id"];
84-
//assert_eq!(client_id, ping_client_id);
82+
let ping_client_id = &payload["client_info"]["client_id"];
83+
assert_eq!(client_id, ping_client_id);
8584
}

glean-core/src/core/mod.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,10 @@ impl Glean {
359359
.set_sync(&glean, ExceptionState::EmptyDb);
360360

361361
// state (e) -- mitigation: store recovered client ID in DB
362+
glean
363+
.core_metrics
364+
.client_id
365+
.set_from_uuid_sync(&glean, stored_client_id);
362366
} else {
363367
let db_client_id = glean
364368
.core_metrics
@@ -375,6 +379,10 @@ impl Glean {
375379
.set_sync(&glean, ExceptionState::RegenDb);
376380

377381
// state (e) -- mitigation: store recovered client ID in DB
382+
glean
383+
.core_metrics
384+
.client_id
385+
.set_from_uuid_sync(&glean, stored_client_id);
378386
}
379387
Some(db_client_id) if db_client_id == *KNOWN_CLIENT_ID => {
380388
// state (i)
@@ -392,6 +400,10 @@ impl Glean {
392400

393401
// If we have a recovered client ID we also overwrite the database.
394402
// state (e)
403+
glean
404+
.core_metrics
405+
.client_id
406+
.set_from_uuid_sync(&glean, stored_client_id);
395407
}
396408
Some(db_client_id) if db_client_id == stored_client_id => {
397409
// all valid. nothing to do

glean-core/tests/clientid_textfile.rs

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,8 @@ fn reused_clientid_from_file() {
7171
write_clientid_to_file(temp.path(), &new_uuid).unwrap();
7272

7373
let (glean, temp) = new_glean(Some(temp));
74-
// TODO(bug 1996862): We don't run the mitigation yet
75-
//let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
76-
//assert_eq!(new_uuid, db_client_id);
74+
let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
75+
assert_eq!(new_uuid, db_client_id);
7776

7877
glean.submit_ping_by_name("health", Some("pre_init"));
7978
let mut pending = get_queued_pings(temp.path()).unwrap();
@@ -141,10 +140,8 @@ fn clientid_regen_issue_with_existing_db() {
141140

142141
let (glean, temp) = new_glean(Some(temp));
143142

144-
// TODO(bug 1996862): We don't run the mitigation yet
145-
_ = file_client_id;
146-
//let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
147-
//assert_eq!(file_client_id, db_client_id);
143+
let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
144+
assert_eq!(file_client_id, db_client_id);
148145

149146
glean.submit_ping_by_name("health", Some("pre_init"));
150147
let mut pending = get_queued_pings(temp.path()).unwrap();
@@ -225,10 +222,8 @@ fn c0ffee_in_db_gets_overwritten_by_stored_client_id() {
225222

226223
let (glean, temp) = new_glean(Some(temp));
227224

228-
// TODO(bug 1996862): We don't run the mitigation yet
229-
_ = file_client_id;
230-
//let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
231-
//assert_eq!(file_client_id, db_client_id);
225+
let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
226+
assert_eq!(file_client_id, db_client_id);
232227

233228
glean.submit_ping_by_name("health", Some("pre_init"));
234229
let mut pending = get_queued_pings(temp.path()).unwrap();
@@ -244,10 +239,9 @@ fn c0ffee_in_db_gets_overwritten_by_stored_client_id() {
244239
let exception_uuid = &payload["metrics"]["uuid"]["glean.health.recovered_client_id"].as_str();
245240
assert_eq!(&Some(file_client_id), exception_uuid);
246241

247-
// TODO(bug 1996862): We don't run the mitigation yet
248-
//let exception_uuid = &payload["client_info"]["client_id"].as_str();
249-
//assert_eq!(&Some(file_client_id), exception_uuid);
250-
// instead we ensure we don't see the c0ffee ID either.
242+
let exception_uuid = &payload["client_info"]["client_id"].as_str();
243+
assert_eq!(&Some(file_client_id), exception_uuid);
244+
// We ensure we don't see the c0ffee ID either.
251245
let client_id = payload["client_info"]["client_id"].as_str().unwrap();
252246
assert_ne!("c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0", client_id);
253247
}

0 commit comments

Comments
 (0)