Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Stop reporting db file sizes during init phase ([#3331](https://github.com/mozilla/glean/pull/3331))
* Tiny performance improvement for putting tasks on the dispatcher ([#3318](https://github.com/mozilla/glean/pull/3318))
* Instrument the case when the `client_id.txt` file does not exist yet ([#3339](https://github.com/mozilla/glean/pull/3339))
* 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))

# v66.1.2 (2025-11-25)

Expand Down
3 changes: 1 addition & 2 deletions docs/dev/core/internal/client_id_recovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ Any inconsistencies in that data compared to the database will be reported
and, if applicable, the client ID restored.

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

The exact flow of decisions is depicted in the chart below.
The implementation is in [`glean-core/src/core/mod.rs`](https://github.com/mozilla/glean/blob/HEAD/glean-core/src/core/mod.rs#L264)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,21 @@ class DeletionPingTest {
val pendingPingDir = File(Glean.getDataDir(), PENDING_PING_DIR)
pendingPingDir.mkdirs()

// We manually disable upload and we don't want the ID to be restored, or this will trigger another
// deletion-request ping.
val clientIdTxt = File(Glean.getDataDir(), "client_id.txt")
assertTrue(clientIdTxt.delete())

// Write a deletion-request ping file
var pingId = "775b6590-7f21-11ea-92e3-479998edf98c"
var submitPath = "/submit/org-mozilla-samples-gleancore/deletion-request/1/$pingId"
var deletionPingId = "775b6590-7f21-11ea-92e3-479998edf98c"
var submitPath = "/submit/org-mozilla-samples-gleancore/deletion-request/1/$deletionPingId"
var content = "$submitPath\n{}"
var pingFile = File(pendingDeletionRequestDir, pingId)
var pingFile = File(pendingDeletionRequestDir, deletionPingId)
assertTrue(pingFile.createNewFile())
pingFile.writeText(content)

// Write a baseline ping file
pingId = "899b0ab8-7f20-11ea-ac03-ff76f2a19f1c"
var pingId = "899b0ab8-7f20-11ea-ac03-ff76f2a19f1c"
submitPath = "/submit/org-mozilla-samples-gleancore/baseline/1/$pingId"
content = "$submitPath\n{}"
pingFile = File(pendingPingDir, pingId)
Expand All @@ -153,7 +158,9 @@ class DeletionPingTest {

var request = server.takeRequest(20L, TimeUnit.SECONDS)!!
var docType = request.path!!.split("/")[3]
var docId = request.path!!.split("/")[5]
assertEquals("Should have received a deletion-request ping", "deletion-request", docType)
assertEquals("Should be the manually constructed ping", deletionPingId, docId)

// deletion-request ping is gone
assertEquals(0, pendingDeletionRequestDir.listFiles()?.size)
Expand Down
5 changes: 2 additions & 3 deletions glean-core/rlb/tests/health_ping_file_overwrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ fn test_pre_post_init_health_pings_exist() {
let exception_uuid = &payload["metrics"]["uuid"]["glean.health_recovered_client_id"];
assert_eq!(&JsonValue::Null, exception_uuid);

// TODO(bug 1996862): We don't run the mitigation yet.
//let ping_client_id = &payload["client_info"]["client_id"];
//assert_eq!(client_id, ping_client_id);
let ping_client_id = &payload["client_info"]["client_id"];
assert_eq!(client_id, ping_client_id);
}
12 changes: 12 additions & 0 deletions glean-core/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,10 @@ impl Glean {
.set_sync(&glean, ExceptionState::EmptyDb);

// state (e) -- mitigation: store recovered client ID in DB
glean
.core_metrics
.client_id
.set_from_uuid_sync(&glean, stored_client_id);
} else {
let db_client_id = glean
.core_metrics
Expand All @@ -375,6 +379,10 @@ impl Glean {
.set_sync(&glean, ExceptionState::RegenDb);

// state (e) -- mitigation: store recovered client ID in DB
glean
.core_metrics
.client_id
.set_from_uuid_sync(&glean, stored_client_id);
}
Some(db_client_id) if db_client_id == *KNOWN_CLIENT_ID => {
// state (i)
Expand All @@ -392,6 +400,10 @@ impl Glean {

// If we have a recovered client ID we also overwrite the database.
// state (e)
glean
.core_metrics
.client_id
.set_from_uuid_sync(&glean, stored_client_id);
}
Some(db_client_id) if db_client_id == stored_client_id => {
// all valid. nothing to do
Expand Down
24 changes: 9 additions & 15 deletions glean-core/tests/clientid_textfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,8 @@ fn reused_clientid_from_file() {
write_clientid_to_file(temp.path(), &new_uuid).unwrap();

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

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

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

// TODO(bug 1996862): We don't run the mitigation yet
_ = file_client_id;
//let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
//assert_eq!(file_client_id, db_client_id);
let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
assert_eq!(file_client_id, db_client_id);

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

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

// TODO(bug 1996862): We don't run the mitigation yet
_ = file_client_id;
//let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
//assert_eq!(file_client_id, db_client_id);
let db_client_id = clientid_metric().get_value(&glean, None).unwrap();
assert_eq!(file_client_id, db_client_id);

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

// TODO(bug 1996862): We don't run the mitigation yet
//let exception_uuid = &payload["client_info"]["client_id"].as_str();
//assert_eq!(&Some(file_client_id), exception_uuid);
// instead we ensure we don't see the c0ffee ID either.
let exception_uuid = &payload["client_info"]["client_id"].as_str();
assert_eq!(&Some(file_client_id), exception_uuid);
// We ensure we don't see the c0ffee ID either.
let client_id = payload["client_info"]["client_id"].as_str().unwrap();
assert_ne!("c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0", client_id);
}
Expand Down