From 6f11b06b6ac4afe67bb443539a7a5e93d75690fc Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Mon, 22 Jun 2026 17:32:14 +0200 Subject: [PATCH 1/4] Revert "Merge pull request #3504 from mozilla/main-sqlite" This reverts commit e8e0b55b3869c7bc931627f8cdb0bf67d2708458, reversing changes made to 2a2e41730876efc35f7b927054d5a7fa9cd1d6e2. --- CHANGELOG.md | 10 - Cargo.lock | 112 +- glean-core/Cargo.toml | 2 - glean-core/benchmark/Cargo.lock | 96 +- glean-core/glean-sym/src/types.rs | 11 +- glean-core/metrics.yaml | 111 -- glean-core/python/glean/_loader.py | 4 +- .../python/tests/metrics/test_boolean.py | 8 +- .../python/tests/metrics/test_counter.py | 10 +- .../python/tests/metrics/test_datetime.py | 4 +- glean-core/python/tests/metrics/test_event.py | 18 +- .../python/tests/metrics/test_labeled.py | 20 +- .../tests/metrics/test_memory_distribution.py | 10 +- .../python/tests/metrics/test_object.py | 10 +- .../python/tests/metrics/test_quantity.py | 10 +- glean-core/python/tests/metrics/test_rate.py | 10 +- .../python/tests/metrics/test_string.py | 10 +- .../python/tests/metrics/test_string_list.py | 12 +- glean-core/python/tests/metrics/test_text.py | 4 +- .../python/tests/metrics/test_timespan.py | 26 +- .../tests/metrics/test_timing_distribution.py | 16 +- glean-core/python/tests/metrics/test_url.py | 10 +- glean-core/python/tests/metrics/test_uuid.py | 18 +- .../python/tests/test_collection_enabled.py | 4 +- glean-core/python/tests/test_glean.py | 28 +- glean-core/python/tests/test_network.py | 4 +- glean-core/rlb-tests/Cargo.toml | 3 - glean-core/rlb-tests/build.rs | 9 - glean-core/rlb-tests/metrics.yaml | 253 --- glean-core/rlb-tests/pings.yaml | 18 - glean-core/rlb-tests/src/bin/verify-data.rs | 255 --- glean-core/rlb-tests/tests/it.rs | 16 - .../rlb-tests/tests/rkv-database.safe.bin | Bin 4065 -> 0 bytes glean-core/rlb/src/private/event.rs | 2 +- glean-core/rlb/src/private/object.rs | 2 +- glean-core/rlb/src/test.rs | 12 +- glean-core/rlb/tests/metric_metadata.rs | 10 +- glean-core/rlb/tests/upload_timing.rs | 6 +- glean-core/src/common_metric_data.rs | 153 +- glean-core/src/core/mod.rs | 87 +- glean-core/src/database/migration.rs | 362 ---- glean-core/src/database/mod.rs | 1621 ++++++++++++++++- glean-core/src/database/sqlite.rs | 672 ------- glean-core/src/database/sqlite/connection.rs | 111 -- glean-core/src/database/sqlite/schema.rs | 76 - glean-core/src/error.rs | 31 - glean-core/src/error_recording.rs | 63 +- glean-core/src/glean.udl | 10 +- glean-core/src/internal_metrics.rs | 70 +- glean-core/src/lib.rs | 2 +- glean-core/src/lib_unit_tests.rs | 6 +- glean-core/src/metrics/boolean.rs | 14 +- glean-core/src/metrics/counter.rs | 14 +- glean-core/src/metrics/custom_distribution.rs | 15 +- glean-core/src/metrics/datetime.rs | 8 +- glean-core/src/metrics/denominator.rs | 8 +- .../src/metrics/dual_labeled_counter.rs | 261 ++- glean-core/src/metrics/experiment.rs | 9 +- glean-core/src/metrics/labeled.rs | 121 +- glean-core/src/metrics/memory_distribution.rs | 14 +- glean-core/src/metrics/mod.rs | 24 +- glean-core/src/metrics/object.rs | 8 +- glean-core/src/metrics/quantity.rs | 14 +- glean-core/src/metrics/rate.rs | 8 +- glean-core/src/metrics/string.rs | 18 +- glean-core/src/metrics/string_list.rs | 8 +- glean-core/src/metrics/text.rs | 16 +- glean-core/src/metrics/timespan.rs | 8 +- glean-core/src/metrics/timing_distribution.rs | 15 +- glean-core/src/metrics/url.rs | 22 +- glean-core/src/metrics/uuid.rs | 8 +- glean-core/src/ping/mod.rs | 17 +- glean-core/src/storage/mod.rs | 86 +- ...a0472-5124-4f6b-971d-4a2a928fb158.safe.bin | Bin 1155 -> 0 bytes glean-core/tests/clientid_textfile.rs | 39 +- glean-core/tests/collection_enabled.rs | 37 - glean-core/tests/metrics_sync.rs | 2 - glean-core/tests/ping.rs | 2 +- glean-core/tests/sqlite.rs | 232 --- glean-core/tests/sqlite_migration.rs | 220 --- samples/glean-sym-test/Cargo.lock | 79 - samples/glean-sym-test/Makefile | 2 +- supply-chain/audits.toml | 47 - supply-chain/config.toml | 10 +- supply-chain/imports.lock | 243 +-- 85 files changed, 2381 insertions(+), 3676 deletions(-) delete mode 100644 glean-core/rlb-tests/build.rs delete mode 100644 glean-core/rlb-tests/metrics.yaml delete mode 100644 glean-core/rlb-tests/pings.yaml delete mode 100644 glean-core/rlb-tests/src/bin/verify-data.rs delete mode 100644 glean-core/rlb-tests/tests/rkv-database.safe.bin delete mode 100644 glean-core/src/database/migration.rs delete mode 100644 glean-core/src/database/sqlite.rs delete mode 100644 glean-core/src/database/sqlite/connection.rs delete mode 100644 glean-core/src/database/sqlite/schema.rs delete mode 100644 glean-core/tests/77ca0472-5124-4f6b-971d-4a2a928fb158.safe.bin delete mode 100644 glean-core/tests/sqlite.rs delete mode 100644 glean-core/tests/sqlite_migration.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index f8bf33ff66..28f96ba6ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,16 +6,6 @@ [Full changelog](https://github.com/mozilla/glean/compare/v68.0.0...v69.0.0) -* General - * BREAKING CHANGE: Replaced the client-side storage backend with a SQLite-powered one. - This is major change and large refactoring of the Glean SDK internals, but has very limited breaking changes in the - public API (breaking changes in the Rust API only). - Data is migrated from the old rkv-powered storage to the SQLite database on first start with this Glean SDK version. - The Rkv data is retained for now. Only the SQLite-backed data is written to for new metric recordings. - New `glean.migration.*` metrics were added to monitor and validate the migration process. - Other monitoring metrics are preserved. - The `glean.database.rkv_load_error` has been replaced by a more generic `glean.database.load_error` with similar semantics. - # v68.0.0 (2026-06-19) [Full changelog](https://github.com/mozilla/glean/compare/v67.5.0...v68.0.0) diff --git a/Cargo.lock b/Cargo.lock index 9d1a88b043..b36b4b43cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -154,11 +154,11 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.11.1" +version = "2.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" +checksum = "327762f6e5a765692301e5bb513e0d9fef63be86bbc14528052b1cd3e6f03e07" dependencies = [ - "serde_core", + "serde", ] [[package]] @@ -224,12 +224,11 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" [[package]] name = "cc" -version = "1.2.53" +version = "1.0.83" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "755d2fce177175ffca841e9a06afdb2c4ab0f593d53b4dee48147dfaade85932" +checksum = "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0" dependencies = [ - "find-msvc-tools", - "shlex", + "libc", ] [[package]] @@ -471,30 +470,12 @@ dependencies = [ "libc", ] -[[package]] -name = "fallible-iterator" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" - -[[package]] -name = "fallible-streaming-iterator" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" - [[package]] name = "fastrand" version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" -[[package]] -name = "find-msvc-tools" -version = "0.1.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8591b0bcc8a98a64310a2fae1bb3e9b8564dd10e381e6e28010fde8e8e8568db" - [[package]] name = "flate2" version = "1.0.35" @@ -600,8 +581,6 @@ dependencies = [ "pulldown-cmark", "pulldown-cmark-to-cmark", "rkv", - "rmp-serde", - "rusqlite", "serde", "serde_json", "tempfile", @@ -646,7 +625,6 @@ dependencies = [ "env_logger", "flate2", "glean", - "glean-build", "libc", "log", "once_cell", @@ -871,17 +849,6 @@ dependencies = [ "windows-targets", ] -[[package]] -name = "libsqlite3-sys" -version = "0.35.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "133c182a6a2c87864fe97778797e46c7e999672690dc9fa3ee8e241aa4a9c13f" -dependencies = [ - "cc", - "pkg-config", - "vcpkg", -] - [[package]] name = "linux-raw-sys" version = "0.4.10" @@ -938,9 +905,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.19" +version = "0.2.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" dependencies = [ "autocfg", ] @@ -993,12 +960,6 @@ version = "2.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "478c572c3d73181ff3c2539045f6eb99e5491218eae919370993b890cdbdd98e" -[[package]] -name = "pkg-config" -version = "0.3.32" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" - [[package]] name = "plain" version = "0.2.3" @@ -1085,7 +1046,7 @@ version = "0.13.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e9f068eba8e7071c5f9511831b44f32c740d5adf574e990f946ddb53db2f314e" dependencies = [ - "bitflags 2.11.1", + "bitflags 2.4.1", "memchr", "unicase", ] @@ -1180,7 +1141,7 @@ checksum = "0f67a9dbc634fcd36a2d1d800ca818065dcf71a1d907dc35130c2d1552c6e1dc" dependencies = [ "arrayref", "bincode", - "bitflags 2.11.1", + "bitflags 2.4.1", "id-arena", "lazy_static", "log", @@ -1194,39 +1155,6 @@ dependencies = [ "wr_malloc_size_of", ] -[[package]] -name = "rmp" -version = "0.8.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ba8be72d372b2c9b35542551678538b562e7cf86c3315773cae48dfbfe7790c" -dependencies = [ - "num-traits", -] - -[[package]] -name = "rmp-serde" -version = "1.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72f81bee8c8ef9b577d1681a70ebbc962c232461e397b22c208c43c04b67a155" -dependencies = [ - "rmp", - "serde", -] - -[[package]] -name = "rusqlite" -version = "0.37.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "165ca6e57b20e1351573e3729b958bc62f0e48025386970b6e4d29e7a7e71f3f" -dependencies = [ - "bitflags 2.11.1", - "fallible-iterator", - "fallible-streaming-iterator", - "hashlink", - "libsqlite3-sys", - "smallvec", -] - [[package]] name = "rustc-hash" version = "2.1.1" @@ -1239,7 +1167,7 @@ version = "0.38.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67ce50cb2e16c2903e30d1cbccfd8387a74b9d4c938b6a4c5ec6cc7556f7a8a0" dependencies = [ - "bitflags 2.11.1", + "bitflags 2.4.1", "errno", "libc", "linux-raw-sys", @@ -1359,24 +1287,12 @@ dependencies = [ "serde_core", ] -[[package]] -name = "shlex" -version = "1.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" - [[package]] name = "siphasher" version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56199f7ddabf13fe5074ce809e7d3f42b42ae711800501b5b16ea82ad029c39d" -[[package]] -name = "smallvec" -version = "1.15.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ed6a63f02c8539c91a8685a86f4099661ba3da017932f6ebbea6de3f0fa7c90" - [[package]] name = "smawk" version = "0.3.2" @@ -1711,12 +1627,6 @@ dependencies = [ "getrandom", ] -[[package]] -name = "vcpkg" -version = "0.2.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" - [[package]] name = "wait-timeout" version = "0.2.1" diff --git a/glean-core/Cargo.toml b/glean-core/Cargo.toml index 912d5760bb..d86af4720a 100644 --- a/glean-core/Cargo.toml +++ b/glean-core/Cargo.toml @@ -44,8 +44,6 @@ uniffi = { version = "0.31.0", default-features = false } env_logger = { version = "0.10.0", default-features = false, optional = true } malloc_size_of_derive = "0.1.3" malloc_size_of = { version = "0.2.2", package = "wr_malloc_size_of", default-features = false, features = ["once_cell"] } -rusqlite = { version = "0.37.0", features = ["bundled"] } -rmp-serde = "1.3.1" [target.'cfg(target_os = "android")'.dependencies] android_logger = { version = "0.12.0", default-features = false } diff --git a/glean-core/benchmark/Cargo.lock b/glean-core/benchmark/Cargo.lock index ffaff22103..83737fe785 100644 --- a/glean-core/benchmark/Cargo.lock +++ b/glean-core/benchmark/Cargo.lock @@ -477,18 +477,6 @@ dependencies = [ "windows-sys", ] -[[package]] -name = "fallible-iterator" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" - -[[package]] -name = "fallible-streaming-iterator" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" - [[package]] name = "fastrand" version = "2.3.0" @@ -511,12 +499,6 @@ dependencies = [ "miniz_oxide", ] -[[package]] -name = "foldhash" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" - [[package]] name = "form_urlencoded" version = "1.2.2" @@ -561,8 +543,6 @@ dependencies = [ "once_cell", "oslog", "rkv", - "rmp-serde", - "rusqlite", "serde", "serde_json", "thiserror", @@ -644,30 +624,12 @@ dependencies = [ "zerocopy", ] -[[package]] -name = "hashbrown" -version = "0.15.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" -dependencies = [ - "foldhash", -] - [[package]] name = "hashbrown" version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" -[[package]] -name = "hashlink" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7382cf6263419f2d8df38c55d7da83da5c18aef87fc7a7fc1fb1e344edfe14c1" -dependencies = [ - "hashbrown 0.15.5", -] - [[package]] name = "heck" version = "0.5.0" @@ -819,7 +781,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7714e70437a7dc3ac8eb7e6f8df75fd8eb422675fc7678aff7364301092b1017" dependencies = [ "equivalent", - "hashbrown 0.16.1", + "hashbrown", "serde", "serde_core", ] @@ -871,17 +833,6 @@ dependencies = [ "windows-link", ] -[[package]] -name = "libsqlite3-sys" -version = "0.35.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "133c182a6a2c87864fe97778797e46c7e999672690dc9fa3ee8e241aa4a9c13f" -dependencies = [ - "cc", - "pkg-config", - "vcpkg", -] - [[package]] name = "linux-raw-sys" version = "0.11.0" @@ -1010,12 +961,6 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" -[[package]] -name = "pkg-config" -version = "0.3.32" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7edddbd0b52d732b21ad9a5fab5c704c14cd949e5e9a1ec5929a24fded1b904c" - [[package]] name = "plain" version = "0.2.3" @@ -1186,39 +1131,6 @@ dependencies = [ "wr_malloc_size_of", ] -[[package]] -name = "rmp" -version = "0.8.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ba8be72d372b2c9b35542551678538b562e7cf86c3315773cae48dfbfe7790c" -dependencies = [ - "num-traits", -] - -[[package]] -name = "rmp-serde" -version = "1.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72f81bee8c8ef9b577d1681a70ebbc962c232461e397b22c208c43c04b67a155" -dependencies = [ - "rmp", - "serde", -] - -[[package]] -name = "rusqlite" -version = "0.37.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "165ca6e57b20e1351573e3729b958bc62f0e48025386970b6e4d29e7a7e71f3f" -dependencies = [ - "bitflags", - "fallible-iterator", - "fallible-streaming-iterator", - "hashlink", - "libsqlite3-sys", - "smallvec", -] - [[package]] name = "rustc-hash" version = "2.1.1" @@ -1689,12 +1601,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "vcpkg" -version = "0.2.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" - [[package]] name = "walkdir" version = "2.5.0" diff --git a/glean-core/glean-sym/src/types.rs b/glean-core/glean-sym/src/types.rs index 783e431de8..48dbd33283 100644 --- a/glean-core/glean-sym/src/types.rs +++ b/glean-core/glean-sym/src/types.rs @@ -18,12 +18,11 @@ pub enum Lifetime { } #[derive(uniffi::Enum)] -pub enum MetricLabel { - Static(String), +pub enum DynamicLabelType { Label(String), - KeyOnly(String, String), - CategoryOnly(String, String), - KeyAndCategory(String, String), + KeyOnly(String), + CategoryOnly(String), + KeyAndCategory(String), } #[derive(uniffi::Record, Default)] @@ -33,7 +32,7 @@ pub struct CommonMetricData { pub send_in_pings: Vec, pub lifetime: Lifetime, pub disabled: bool, - pub label: Option, + pub dynamic_label: Option, pub in_session: bool, } diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml index 7b5dc10f6a..3a6fb3fc79 100644 --- a/glean-core/metrics.yaml +++ b/glean-core/metrics.yaml @@ -972,23 +972,6 @@ glean.database: - glean-team@mozilla.com expires: never - load_error: - type: string - description: | - If there was an error loading the sqlite database, record it. - send_in_pings: - - metrics - - health - bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_sensitivity: - - technical - notification_emails: - - glean-team@mozilla.com - expires: never - write_time: type: timing_distribution time_unit: microsecond @@ -1425,97 +1408,3 @@ glean.health: - permission-denied - io - not-found - -glean.migration: - failed_metrics: - type: counter - lifetime: ping - description: | - Number of metrics that failed to deserialize from storage - while iterating the Rkv database for migration. - notification_emails: - - glean-team@mozilla.com - - jrediger@mozilla.com - bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_sensitivity: - - technical - expires: never - send_in_pings: - - metrics - - health - metrics_in_sqlite: - type: counter - lifetime: ping - description: | - The number of metrics stored in SQLite after a migration run. - notification_emails: - - glean-team@mozilla.com - - jrediger@mozilla.com - bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_sensitivity: - - technical - expires: never - send_in_pings: - - metrics - - health - migrated_metrics: - type: counter - lifetime: ping - description: | - The number of metrics migrated from Rkv storage to SQLite storage. - notification_emails: - - glean-team@mozilla.com - - jrediger@mozilla.com - bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_sensitivity: - - technical - expires: never - send_in_pings: - - metrics - - health - migration_duration: - type: timing_distribution - lifetime: ping - time_unit: millisecond - description: | - The duration for one full migration run at startup. - notification_emails: - - glean-team@mozilla.com - - jrediger@mozilla.com - bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_sensitivity: - - technical - expires: never - send_in_pings: - - metrics - - health - error: - type: counter - lifetime: ping - description: | - The number of times a migration was attempted and failed. - notification_emails: - - glean-team@mozilla.com - - jrediger@mozilla.com - bugs: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 - data_sensitivity: - - technical - expires: never - send_in_pings: - - metrics - - health diff --git a/glean-core/python/glean/_loader.py b/glean-core/python/glean/_loader.py index ca50f388cb..00cb48c9ec 100644 --- a/glean-core/python/glean/_loader.py +++ b/glean-core/python/glean/_loader.py @@ -284,8 +284,8 @@ def _get_metric_objects( glean_metric = metrics.ObjectMetricType(metrics.CommonMetricData(**args), obj_cls) # type: ignore else: # Hack for the time being. - if "label" not in args: - args["label"] = None + if "dynamic_label" not in args: + args["dynamic_label"] = None meta_args, rest = _split_ctor_args(args) if getattr(metric, "labeled", False): glean_metric = metric_type( diff --git a/glean-core/python/tests/metrics/test_boolean.py b/glean-core/python/tests/metrics/test_boolean.py index ba0733e024..9e3ed68f8e 100644 --- a/glean-core/python/tests/metrics/test_boolean.py +++ b/glean-core/python/tests/metrics/test_boolean.py @@ -14,7 +14,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="boolean_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -35,7 +35,7 @@ def test_disabled_booleans_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="boolean_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -52,7 +52,7 @@ def test_get_value_throws_if_nothing_is_stored(): lifetime=Lifetime.APPLICATION, name="boolean_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -67,7 +67,7 @@ def test_the_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="boolean_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_counter.py b/glean-core/python/tests/metrics/test_counter.py index fd6a2f9d24..a8231bd26e 100644 --- a/glean-core/python/tests/metrics/test_counter.py +++ b/glean-core/python/tests/metrics/test_counter.py @@ -16,7 +16,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -43,7 +43,7 @@ def test_disabled_counters_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -62,7 +62,7 @@ def test_get_value_throws_value_error_if_nothing_is_stored(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -78,7 +78,7 @@ def test_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -104,7 +104,7 @@ def test_negative_values_are_not_counted(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_datetime.py b/glean-core/python/tests/metrics/test_datetime.py index d8f0d72694..b58d6ccf87 100644 --- a/glean-core/python/tests/metrics/test_datetime.py +++ b/glean-core/python/tests/metrics/test_datetime.py @@ -17,7 +17,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="datetime_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=metrics.TimeUnit.MINUTE, ) @@ -69,7 +69,7 @@ def test_disabled_datetimes_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="datetime_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=metrics.TimeUnit.MINUTE, ) diff --git a/glean-core/python/tests/metrics/test_event.py b/glean-core/python/tests/metrics/test_event.py index f6b5c35564..398861e362 100644 --- a/glean-core/python/tests/metrics/test_event.py +++ b/glean-core/python/tests/metrics/test_event.py @@ -46,7 +46,7 @@ def test_the_api_records_to_its_storage_engine(): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["object_id", "other"], ) @@ -84,7 +84,7 @@ def test_the_api_records_to_its_storage_engine_when_category_is_empty(): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["object_id"], ) @@ -117,7 +117,7 @@ def test_disabled_events_must_not_record_data(): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["object_id", "other"], ) @@ -137,7 +137,7 @@ def test_test_get_value_throws_valueerror_if_nothing_is_stored(): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["object_id", "other"], ) @@ -153,7 +153,7 @@ def test_the_api_records_to_secondary_pings(): lifetime=Lifetime.PING, name="click", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["object_id"], ) @@ -191,7 +191,7 @@ class EventKeys(enum.Enum): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["test_name"], ) @@ -226,7 +226,7 @@ class EventKeys(enum.Enum): lifetime=Lifetime.PING, name="test_event", send_in_pings=["events"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["some_extra"], ) @@ -255,7 +255,7 @@ def test_long_extra_values_record_an_error(): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["object_id", "other"], ) @@ -304,7 +304,7 @@ def test_the_convenient_extrakeys_api(): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), allowed_extra_keys=["object_id", "other"], ) diff --git a/glean-core/python/tests/metrics/test_labeled.py b/glean-core/python/tests/metrics/test_labeled.py index 3ba30baec3..eac757826f 100644 --- a/glean-core/python/tests/metrics/test_labeled.py +++ b/glean-core/python/tests/metrics/test_labeled.py @@ -18,7 +18,7 @@ def test_labeled_counter_type(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -40,7 +40,7 @@ def test_labeled_counter_type_test_get_metrics(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -64,7 +64,7 @@ def test_labeled_boolean_type(): lifetime=Lifetime.APPLICATION, name="labeled_boolean_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -86,7 +86,7 @@ def test_labeled_string_type(): lifetime=Lifetime.APPLICATION, name="labeled_string_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -108,7 +108,7 @@ def test_labeled_quantity_type(): lifetime=Lifetime.APPLICATION, name="labeled_quantity_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -129,7 +129,7 @@ def test_other_label_with_predefined_labels(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ), labels=["foo", "bar", "baz"], @@ -157,7 +157,7 @@ def test_other_label_without_predefined_labels(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -182,7 +182,7 @@ def test_other_label_without_predefined_labels_before_glean_init(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -214,7 +214,7 @@ def test_invalid_labels_go_to_other(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) ) @@ -251,7 +251,7 @@ def test_rapidly_recreating_labeled_metrics_does_not_crash(): send_in_pings=["metrics"], lifetime=Lifetime.APPLICATION, disabled=False, - label=None, + dynamic_label=None, ) ), labels=["foo"], diff --git a/glean-core/python/tests/metrics/test_memory_distribution.py b/glean-core/python/tests/metrics/test_memory_distribution.py index b346027039..a4b4d4dc52 100644 --- a/glean-core/python/tests/metrics/test_memory_distribution.py +++ b/glean-core/python/tests/metrics/test_memory_distribution.py @@ -15,7 +15,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="memory_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), memory_unit=MemoryUnit.KILOBYTE, ) @@ -40,7 +40,7 @@ def test_values_are_truncated_to_1tb(): lifetime=Lifetime.APPLICATION, name="memory_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), memory_unit=MemoryUnit.GIGABYTE, ) @@ -61,7 +61,7 @@ def test_disabled_memory_distributions_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="memory_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), memory_unit=MemoryUnit.KILOBYTE, ) @@ -79,7 +79,7 @@ def test_get_value_throws_if_nothing_is_stored(): lifetime=Lifetime.APPLICATION, name="memory_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), memory_unit=MemoryUnit.KILOBYTE, ) @@ -95,7 +95,7 @@ def test_the_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="memory_distribution", send_in_pings=["store1", "store2", "store3"], - label=None, + dynamic_label=None, ), memory_unit=MemoryUnit.KILOBYTE, ) diff --git a/glean-core/python/tests/metrics/test_object.py b/glean-core/python/tests/metrics/test_object.py index b83432ee45..15ee6a8556 100644 --- a/glean-core/python/tests/metrics/test_object.py +++ b/glean-core/python/tests/metrics/test_object.py @@ -37,7 +37,7 @@ def test_the_api_records_to_its_storage_engine(): name="baloon", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ), BalloonsObject, @@ -62,7 +62,7 @@ def test_object_must_not_record_if_disabled(): name="baloon", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=True, ), BalloonsObject, @@ -82,7 +82,7 @@ def test_object_get_value_returns_nil_if_nothing_is_stored(): name="baloon", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=True, ), BalloonsObject, @@ -98,7 +98,7 @@ def test_object_saves_to_secondary_pings(): name="baloon", lifetime=Lifetime.PING, send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, disabled=False, ), BalloonsObject, @@ -125,7 +125,7 @@ def test_wrong_object_records_an_error(): name="baloon", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ), BalloonsObject, diff --git a/glean-core/python/tests/metrics/test_quantity.py b/glean-core/python/tests/metrics/test_quantity.py index 84ba6b54c9..80a49841b5 100644 --- a/glean-core/python/tests/metrics/test_quantity.py +++ b/glean-core/python/tests/metrics/test_quantity.py @@ -16,7 +16,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -42,7 +42,7 @@ def test_disabled_quantities_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -61,7 +61,7 @@ def test_get_value_throws_value_error_if_nothing_is_stored(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -77,7 +77,7 @@ def test_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -101,7 +101,7 @@ def test_negative_values_are_not_counted(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_rate.py b/glean-core/python/tests/metrics/test_rate.py index 3650d5ed9e..770cddcbe8 100644 --- a/glean-core/python/tests/metrics/test_rate.py +++ b/glean-core/python/tests/metrics/test_rate.py @@ -19,7 +19,7 @@ def test_rate_smoke(): name="rate", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ), ) @@ -55,7 +55,7 @@ def test_numerator_smoke(): name="numerator", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ), ) @@ -87,7 +87,7 @@ def test_denominator_smoke(): name="rate1", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ) @@ -96,7 +96,7 @@ def test_denominator_smoke(): name="rate2", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ) @@ -107,7 +107,7 @@ def test_denominator_smoke(): name="counter", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ), [meta1, meta2], diff --git a/glean-core/python/tests/metrics/test_string.py b/glean-core/python/tests/metrics/test_string.py index d985e0c438..c5bc118454 100644 --- a/glean-core/python/tests/metrics/test_string.py +++ b/glean-core/python/tests/metrics/test_string.py @@ -16,7 +16,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="string_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -37,7 +37,7 @@ def test_disabled_strings_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="string_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -54,7 +54,7 @@ def test_the_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="string_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -75,7 +75,7 @@ def test_setting_a_long_string_records_an_error(): lifetime=Lifetime.APPLICATION, name="string_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -92,7 +92,7 @@ def test_setting_a_string_as_none(): lifetime=Lifetime.APPLICATION, name="string_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_string_list.py b/glean-core/python/tests/metrics/test_string_list.py index 102bab37ae..1309100cc0 100644 --- a/glean-core/python/tests/metrics/test_string_list.py +++ b/glean-core/python/tests/metrics/test_string_list.py @@ -15,7 +15,7 @@ def test_the_api_saves_to_its_storage_engine_by_first_adding_then_setting(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -40,7 +40,7 @@ def test_the_api_saves_to_its_storage_engine_by_first_setting_then_adding(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -63,7 +63,7 @@ def test_disabled_lists_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -84,7 +84,7 @@ def test_test_get_value_throws(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -99,7 +99,7 @@ def test_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -124,7 +124,7 @@ def test_long_string_lists_are_truncated(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_text.py b/glean-core/python/tests/metrics/test_text.py index 41b1fd0a6c..8eb52f07e7 100644 --- a/glean-core/python/tests/metrics/test_text.py +++ b/glean-core/python/tests/metrics/test_text.py @@ -19,7 +19,7 @@ def test_text_smoke(): name="text", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ), ) @@ -37,7 +37,7 @@ def test_text_truncation(): name="text", lifetime=Lifetime.PING, send_in_pings=["store1"], - label=None, + dynamic_label=None, disabled=False, ), ) diff --git a/glean-core/python/tests/metrics/test_timespan.py b/glean-core/python/tests/metrics/test_timespan.py index aa0d4e4e7c..59f8b9a196 100644 --- a/glean-core/python/tests/metrics/test_timespan.py +++ b/glean-core/python/tests/metrics/test_timespan.py @@ -22,7 +22,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.MILLISECOND, ) @@ -41,7 +41,7 @@ def test_disabled_timespans_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.MILLISECOND, ) @@ -60,7 +60,7 @@ def test_the_api_must_correctly_cancel(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.MILLISECOND, ) @@ -81,7 +81,7 @@ def test_get_value_throws_if_nothing_is_stored(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.MILLISECOND, ) @@ -97,7 +97,7 @@ def test_the_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.MILLISECOND, ) @@ -116,7 +116,7 @@ def test_records_an_error_if_started_twice(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.MILLISECOND, ) @@ -137,7 +137,7 @@ def test_value_unchanged_if_stopped_twice(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.MILLISECOND, ) @@ -163,7 +163,7 @@ def test_set_raw_nanos(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.SECOND, ) @@ -182,7 +182,7 @@ def test_set_raw_nanos_followed_by_other_api(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.SECOND, ) @@ -205,7 +205,7 @@ def test_set_raw_nanos_does_not_overwrite_value(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.SECOND, ) @@ -228,7 +228,7 @@ def test_set_raw_nanos_does_nothing_when_timer_is_running(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.SECOND, ) @@ -248,7 +248,7 @@ def test_measure(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -267,7 +267,7 @@ def test_measure_exception(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) diff --git a/glean-core/python/tests/metrics/test_timing_distribution.py b/glean-core/python/tests/metrics/test_timing_distribution.py index 74a05b5afe..7ce9f37993 100644 --- a/glean-core/python/tests/metrics/test_timing_distribution.py +++ b/glean-core/python/tests/metrics/test_timing_distribution.py @@ -20,7 +20,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -42,7 +42,7 @@ def test_disabled_timing_distributions_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -61,7 +61,7 @@ def test_get_value_throws(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -77,7 +77,7 @@ def test_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["store1", "store2", "store3"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -100,7 +100,7 @@ def test_stopping_a_non_existent_timer_records_an_error(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["store1", "store2", "store3"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -120,7 +120,7 @@ def test_measure(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["baseline"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -143,7 +143,7 @@ def test_measure_exception(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["baseline"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -163,7 +163,7 @@ def test_the_accumulate_apis_record_data(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["store1"], - label=None, + dynamic_label=None, ), time_unit=TimeUnit.NANOSECOND, ) diff --git a/glean-core/python/tests/metrics/test_url.py b/glean-core/python/tests/metrics/test_url.py index fe2d87bc47..c9cf82210a 100644 --- a/glean-core/python/tests/metrics/test_url.py +++ b/glean-core/python/tests/metrics/test_url.py @@ -16,7 +16,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="url_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -37,7 +37,7 @@ def test_disabled_urls_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="url_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -54,7 +54,7 @@ def test_the_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="url_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -75,7 +75,7 @@ def test_setting_a_long_url_records_an_error(): lifetime=Lifetime.APPLICATION, name="url_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -108,7 +108,7 @@ def test_setting_a_url_as_none(): lifetime=Lifetime.APPLICATION, name="url_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_uuid.py b/glean-core/python/tests/metrics/test_uuid.py index 9402264869..071c16e9de 100644 --- a/glean-core/python/tests/metrics/test_uuid.py +++ b/glean-core/python/tests/metrics/test_uuid.py @@ -24,7 +24,7 @@ def test_the_api_saves_to_its_storage_engine(): lifetime=Lifetime.APPLICATION, name="uuid_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -52,7 +52,7 @@ def test_disabled_uuids_must_not_record_data(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -68,7 +68,7 @@ def test_test_get_value_throws_exception_if_nothing_is_stored(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -83,7 +83,7 @@ def test_the_api_saves_to_secondary_pings(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1", "store2"], - label=None, + dynamic_label=None, ) ) @@ -107,7 +107,7 @@ def test_invalid_uuid_must_not_crash(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -126,7 +126,7 @@ def test_invalid_uuid_string(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -163,7 +163,7 @@ def test_what_looks_like_it_might_be_uuid(tmpdir, helpers): lifetime=Lifetime.PING, name="chksum", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) @@ -174,7 +174,7 @@ def test_what_looks_like_it_might_be_uuid(tmpdir, helpers): lifetime=Lifetime.PING, name="random", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) @@ -185,7 +185,7 @@ def test_what_looks_like_it_might_be_uuid(tmpdir, helpers): lifetime=Lifetime.PING, name="valid", send_in_pings=["metrics"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/test_collection_enabled.py b/glean-core/python/tests/test_collection_enabled.py index 5925290597..a1dbb0470e 100644 --- a/glean-core/python/tests/test_collection_enabled.py +++ b/glean-core/python/tests/test_collection_enabled.py @@ -56,7 +56,7 @@ def test_pings_with_follows_false_follow_their_own_setting(tmpdir, helpers): lifetime=Lifetime.PING, name="counter", send_in_pings=["nofollows"], - label=None, + dynamic_label=None, ) ) @@ -93,7 +93,7 @@ def test_loader_sets_flags(tmpdir, helpers): lifetime=Lifetime.PING, name="counter", send_in_pings=["nofollows-defined"], - label=None, + dynamic_label=None, ) ) diff --git a/glean-core/python/tests/test_glean.py b/glean-core/python/tests/test_glean.py index 002ddea8ea..d391e0da1d 100644 --- a/glean-core/python/tests/test_glean.py +++ b/glean-core/python/tests/test_glean.py @@ -76,7 +76,7 @@ def test_submit_a_ping(safe_httpserver): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["baseline"], - label=None, + dynamic_label=None, ) ) @@ -105,7 +105,7 @@ def test_disabling_upload_should_disable_metrics_recording(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -198,7 +198,7 @@ def test_queued_recorded_metrics_correctly_during_init(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -236,7 +236,7 @@ def test_dont_schedule_pings_if_metrics_disabled(safe_httpserver): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -331,7 +331,7 @@ def test_ping_collection_must_happen_after_currently_scheduled_metrics_recording lifetime=Lifetime.PING, name="string_metric", send_in_pings=[ping_name], - label=None, + dynamic_label=None, ) ) @@ -368,7 +368,7 @@ def test_basic_metrics_should_be_cleared_when_disabling_uploading(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -503,7 +503,7 @@ def test_configuration_property(safe_httpserver): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["baseline"], - label=None, + dynamic_label=None, ) ) @@ -676,7 +676,7 @@ def test_clear_application_lifetime_metrics(tmpdir): lifetime=Lifetime.APPLICATION, name="lifetime_reset", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -884,7 +884,7 @@ def test_sending_of_custom_pings(safe_httpserver): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -955,7 +955,7 @@ def test_max_events_overflow(tmpdir, helpers): lifetime=Lifetime.APPLICATION, name="event", send_in_pings=["events"], - label=None, + dynamic_label=None, ), allowed_extra_keys=[], ) @@ -1004,7 +1004,7 @@ def test_glean_shutdown(safe_httpserver): send_in_pings=["custom"], lifetime=Lifetime.APPLICATION, disabled=False, - label=None, + dynamic_label=None, ) ) @@ -1059,7 +1059,7 @@ def test_uploader_capabilities_reported(tmpdir, helpers): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -1110,7 +1110,7 @@ def test_uploader_capabilities_empty_not_reported(tmpdir, helpers): lifetime=Lifetime.PING, name="stringlist_metric", send_in_pings=["store1"], - label=None, + dynamic_label=None, ) ) @@ -1246,7 +1246,7 @@ def test_uploader_capabilities_in_events_ping(tmpdir, helpers): name="custom", lifetime=Lifetime.PING, send_in_pings=["custom-events"], - label=None, + dynamic_label=None, ), allowed_extra_keys=[], ) diff --git a/glean-core/python/tests/test_network.py b/glean-core/python/tests/test_network.py index d84d7fea0d..89bd5d750c 100644 --- a/glean-core/python/tests/test_network.py +++ b/glean-core/python/tests/test_network.py @@ -35,7 +35,7 @@ def get_upload_failure_metric(): name="ping_upload_failure", category="glean.upload", lifetime=metrics.Lifetime.PING, - label=None, + dynamic_label=None, ) ), labels=[ @@ -90,7 +90,7 @@ def test_recording_upload_errors_doesnt_clobber_database(tmpdir, safe_httpserver lifetime=Lifetime.PING, name="counter_metric", send_in_pings=["baseline"], - label=None, + dynamic_label=None, ) ) counter_metric.add(10) diff --git a/glean-core/rlb-tests/Cargo.toml b/glean-core/rlb-tests/Cargo.toml index 77be9081fb..4a879203af 100644 --- a/glean-core/rlb-tests/Cargo.toml +++ b/glean-core/rlb-tests/Cargo.toml @@ -17,6 +17,3 @@ serde_json = "1.0.44" [dev-dependencies] assert_cmd = "2.1.2" tempfile = "3.1.0" - -[build-dependencies] -glean-build = { path = "../build" } diff --git a/glean-core/rlb-tests/build.rs b/glean-core/rlb-tests/build.rs deleted file mode 100644 index 8a17552ca4..0000000000 --- a/glean-core/rlb-tests/build.rs +++ /dev/null @@ -1,9 +0,0 @@ -use glean_build::Builder; - -fn main() { - Builder::default() - .file("metrics.yaml") - .file("pings.yaml") - .generate() - .expect("Error generating Glean Rust bindings"); -} diff --git a/glean-core/rlb-tests/metrics.yaml b/glean-core/rlb-tests/metrics.yaml deleted file mode 100644 index e7edd08203..0000000000 --- a/glean-core/rlb-tests/metrics.yaml +++ /dev/null @@ -1,253 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - ---- - -$schema: moz://mozilla.org/schemas/glean/metrics/2-0-0 - -test.metrics: - sample_counter: - type: counter - description: | - Just testing booleans - bugs: - - https://bugzilla.mozilla.org/123456789 - data_reviews: - - N/A - notification_emails: - - CHANGE-ME@example.com - expires: never - send_in_pings: - - prototype - - usage-reporting - - sample_url: - type: url - description: | - Just testing booleans - bugs: - - https://bugzilla.mozilla.org/123456789 - data_reviews: - - N/A - notification_emails: - - CHANGE-ME@example.com - expires: never - send_in_pings: - - prototype - - usage-reporting - - sample_boolean: - type: boolean - description: | - Just testing booleans - bugs: - - https://bugzilla.mozilla.org/123456789 - data_reviews: - - N/A - notification_emails: - - CHANGE-ME@example.com - expires: never - send_in_pings: - - prototype - - usage-reporting - - sample_labeled_counter: &defaults - type: labeled_counter - description: | - Just testing labeled_counter. - bugs: - - https://bugzilla.mozilla.org/1907991 - data_reviews: - - N/A - notification_emails: - - nobody@example.com - expires: never - send_in_pings: - - prototype - no_lint: - - COMMON_PREFIX - - sample_labeled_string: - <<: *defaults - type: labeled_string - description: | - Just testing labeled_string - bugs: - - https://bugzilla.mozilla.org/1907991 - data_reviews: - - N/A - notification_emails: - - nobody@example.com - expires: never - send_in_pings: - - prototype - no_lint: - - COMMON_PREFIX - - timings: - <<: *defaults - type: timing_distribution - time_unit: millisecond - -party: - balloons: - type: object - description: | - Just testing objects - bugs: - - https://bugzilla.mozilla.org/1839640 - data_reviews: - - N/A - notification_emails: - - CHANGE-ME@example.com - expires: never - send_in_pings: - - prototype - structure: - type: array - items: - type: object - properties: - colour: - type: string - diameter: - type: number - - drinks: - type: object - description: | - Just testing objects - bugs: - - https://bugzilla.mozilla.org/1910809 - data_reviews: - - N/A - notification_emails: - - CHANGE-ME@example.com - expires: never - send_in_pings: - - prototype - structure: - type: array - items: - type: object - properties: - name: - type: string - ingredients: - type: array - items: - type: string - - chooser: - type: object - description: | - Array of key-value elements - bugs: - - https://bugzilla.mozilla.org/123456789 - data_reviews: - - http://example.com/reviews - notification_emails: - - CHANGE-ME@example.com - expires: never - structure: - type: array - items: - type: object - properties: - key: - type: string - value: - oneOf: - - type: string - - type: number - - type: boolean - -test.dual_labeled: - static_static: - type: dual_labeled_counter - description: > - A dual labeled counter with static keys and categories - dual_labels: - key: - description: > - The key for the dual labeled counter - labels: - - key1 - - key2 - category: - description: > - The category for the dual labeled counter - labels: - - category1 - - category2 - bugs: - - https://bugzilla.mozilla.org/11137353 - data_reviews: - - http://example.com/reviews - notification_emails: - - CHANGE-ME@example.com - expires: never - - dynamic_static: - type: dual_labeled_counter - description: > - A dual labeled counter with static keys and dynamic categories - dual_labels: - key: - description: > - The key for the dual labeled counter - labels: - - key1 - - key2 - category: - description: > - The category for the dual labeled counter - bugs: - - https://bugzilla.mozilla.org/11137353 - data_reviews: - - http://example.com/reviews - notification_emails: - - CHANGE-ME@example.com - expires: never - - static_dynamic: - type: dual_labeled_counter - description: > - A dual labeled counter with dynamic keys and static categories - dual_labels: - key: - description: > - The key for the dual labeled counter - category: - description: > - The category for the dual labeled counter - labels: - - category1 - - category2 - bugs: - - https://bugzilla.mozilla.org/11137353 - data_reviews: - - http://example.com/reviews - notification_emails: - - CHANGE-ME@example.com - expires: never - - dynamic_dynamic: - type: dual_labeled_counter - description: > - A dual labeled counter with dynamic keys and dynamic categories - dual_labels: - key: - description: > - The key for the dual labeled counter - category: - description: > - The category for the dual labeled counter - bugs: - - https://bugzilla.mozilla.org/11137353 - data_reviews: - - http://example.com/reviews - notification_emails: - - CHANGE-ME@example.com - expires: never diff --git a/glean-core/rlb-tests/pings.yaml b/glean-core/rlb-tests/pings.yaml deleted file mode 100644 index 106b3b7f8d..0000000000 --- a/glean-core/rlb-tests/pings.yaml +++ /dev/null @@ -1,18 +0,0 @@ -# This Source Code Form is subject to the terms of the Mozilla Public -# License, v. 2.0. If a copy of the MPL was not distributed with this -# file, You can obtain one at http://mozilla.org/MPL/2.0/. - ---- - -$schema: moz://mozilla.org/schemas/glean/pings/2-0-0 - -prototype: - description: | - A sample custom ping. - include_client_id: true - bugs: - - https://bugzilla.mozilla.org/123456789 - data_reviews: - - N/A - notification_emails: - - CHANGE-ME@example.com diff --git a/glean-core/rlb-tests/src/bin/verify-data.rs b/glean-core/rlb-tests/src/bin/verify-data.rs deleted file mode 100644 index 908d3a1ec3..0000000000 --- a/glean-core/rlb-tests/src/bin/verify-data.rs +++ /dev/null @@ -1,255 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -use std::time::Duration; -use std::{env, fs}; - -use glean::{ClientInfoMetrics, ConfigurationBuilder, ErrorType, TestGetValue, net}; - -#[allow(clippy::all)] // Don't lint generated code. -pub mod glean_metrics { - include!(concat!(env!("OUT_DIR"), "/glean_metrics.rs")); -} - -#[derive(Debug)] -struct Uploader; - -impl net::PingUploader for Uploader { - fn upload(&self, _upload_request: net::CapablePingUploadRequest) -> net::UploadResult { - net::UploadResult::recoverable_failure() - } -} - -fn main() { - env_logger::init(); - - let mut args = env::args().skip(1); - - let data_path = args.next().expect("data path required"); - let verify = args.next().map(|arg| arg == "verify").unwrap_or(false); - if !verify { - println!("Removing {data_path}"); - _ = fs::remove_dir_all(&data_path); - } - - let cfg = ConfigurationBuilder::new(true, data_path, "org.mozilla.glean_core.example") - .with_server_endpoint("invalid-test-host") - .with_use_core_mps(false) - .with_uploader(Uploader) - .build(); - - let client_info = ClientInfoMetrics { - app_build: env!("CARGO_PKG_VERSION").to_string(), - app_display_version: env!("CARGO_PKG_VERSION").to_string(), - channel: None, - locale: None, - os_version: None, - }; - - _ = &*glean_metrics::prototype; - glean::initialize(cfg, client_info); - - if verify { - println!("Verifying metric values..."); - assert!( - glean_metrics::test_metrics::sample_boolean - .test_get_value(None) - .unwrap() - ); - assert_eq!( - Some(1), - glean_metrics::test_metrics::sample_counter.test_get_value(None) - ); - assert_eq!( - "https://example.com", - glean_metrics::test_metrics::sample_url - .test_get_value(None) - .unwrap() - ); - assert_eq!( - 1, - glean_metrics::test_metrics::sample_url - .test_get_num_recorded_errors(ErrorType::InvalidValue) - ); - - assert_eq!( - 1, - glean_metrics::test_metrics::sample_labeled_counter - .get("test") - .test_get_value(None) - .unwrap() - ); - assert_eq!( - "foo", - glean_metrics::test_metrics::sample_labeled_string - .get("test") - .test_get_value(None) - .unwrap() - ); - - let exp_balloons = serde_json::json!([ - { "colour": "red", "diameter": 5 }, - { "colour": "blue" }, - ]); - assert_eq!( - Some(exp_balloons), - glean_metrics::party::balloons.test_get_value(None) - ); - - let exp_chooser = serde_json::json!([ - { "key": "fortytwo", "value": 42 }, - { "key": "to-be", "value": false }, - ]); - assert_eq!( - Some(exp_chooser), - glean_metrics::party::chooser.test_get_value(None) - ); - assert_eq!( - 2, - glean_metrics::test_dual_labeled::static_static - .get("key1", "category1") - .test_get_value(None) - .unwrap() - ); - assert_eq!( - 0, - glean_metrics::test_dual_labeled::static_static - .test_get_num_recorded_errors(ErrorType::InvalidLabel) - ); - assert_eq!( - 0, - glean_metrics::test_dual_labeled::dynamic_static - .test_get_num_recorded_errors(ErrorType::InvalidLabel) - ); - assert_eq!( - Some(3), - glean_metrics::test_dual_labeled::dynamic_static - .get("party", "category1") - .test_get_value(None) - ); - assert_eq!( - 0, - glean_metrics::test_dual_labeled::static_dynamic - .test_get_num_recorded_errors(ErrorType::InvalidLabel) - ); - assert_eq!( - Some(4), - glean_metrics::test_dual_labeled::static_dynamic - .get("key1", "balloons") - .test_get_value(None) - ); - assert_eq!( - 0, - glean_metrics::test_dual_labeled::dynamic_dynamic - .test_get_num_recorded_errors(ErrorType::InvalidLabel) - ); - assert_eq!( - Some(5), - glean_metrics::test_dual_labeled::dynamic_dynamic - .get("party", "balloons") - .test_get_value(None) - ); - - assert_eq!( - Some(6), - glean_metrics::test_dual_labeled::static_static - .get("__other__", "__other__") - .test_get_value(None) - ); - - assert_eq!( - 0, - glean_metrics::party::drinks.test_get_num_recorded_errors(ErrorType::InvalidValue) - ); - - let timings = glean_metrics::test_metrics::timings - .test_get_value(None) - .unwrap(); - assert_eq!(100, timings.count); - assert_eq!(1_000_000_000, timings.sum); - assert_eq!(100, timings.values[&9975792]); - - println!("OK."); - } else { - println!("Setting metric values..."); - glean_metrics::test_metrics::sample_boolean.set(true); - glean_metrics::test_metrics::sample_counter.add(1); - glean_metrics::test_metrics::sample_url.set("https://example.com"); - glean_metrics::test_metrics::sample_url.set("data:application/json"); - glean_metrics::test_metrics::sample_labeled_counter - .get("test") - .add(1); - glean_metrics::test_metrics::sample_labeled_string - .get("test") - .set(String::from("foo")); - - use glean_metrics::party::{BalloonsObject, BalloonsObjectItem}; - let balloons = BalloonsObject::from([ - BalloonsObjectItem { - colour: Some("red".to_string()), - diameter: Some(5), - }, - BalloonsObjectItem { - colour: Some("blue".to_string()), - diameter: None, - }, - ]); - glean_metrics::party::balloons.set(balloons); - - use glean_metrics::party::{ChooserObject, ChooserObjectItem, ChooserObjectItemValueEnum}; - let mut ch = ChooserObject::new(); - let it = ChooserObjectItem { - key: Some("fortytwo".to_string()), - value: Some(ChooserObjectItemValueEnum::Number(42)), - }; - ch.push(it); - let it = ChooserObjectItem { - key: Some("to-be".to_string()), - value: Some(ChooserObjectItemValueEnum::Boolean(false)), - }; - ch.push(it); - glean_metrics::party::chooser.set(ch); - - glean_metrics::test_dual_labeled::static_static - .get("key1", "category1") - .add(2); - glean_metrics::test_dual_labeled::dynamic_static - .get("party", "category1") - .add(3); - glean_metrics::test_dual_labeled::static_dynamic - .get("key1", "balloons") - .add(4); - glean_metrics::test_dual_labeled::dynamic_dynamic - .get("party", "balloons") - .add(5); - - // Testing the `__other__` label. - glean_metrics::test_dual_labeled::static_static - .get("party", "balloons") - .add(6); - - // Testing with empty and null values. - let drinks = serde_json::json!([ - { "name": "lemonade", "ingredients": ["lemon", "water", "sugar"] }, - { "name": "sparkling-water", "ingredients": [] }, - { "name": "still-water", "ingredients": null }, - ]); - glean_metrics::party::drinks.set_string(drinks.to_string()); - - { - let mut buffer = glean_metrics::test_metrics::timings.start_buffer(); - - let mock_duration = Duration::from_millis(10); - for _ in 0..100 { - buffer.accumulate(mock_duration.as_millis() as u64); - } - } - - // Ensure Glean actually catches up. - _ = glean_metrics::party::drinks.test_get_value(None); - println!("OK."); - } - - glean::shutdown(); -} diff --git a/glean-core/rlb-tests/tests/it.rs b/glean-core/rlb-tests/tests/it.rs index 71bc65a68a..ddf712b995 100644 --- a/glean-core/rlb-tests/tests/it.rs +++ b/glean-core/rlb-tests/tests/it.rs @@ -295,19 +295,3 @@ fn enabled_pings() { let payload = fs::read_to_string(&entries[0]).unwrap(); assert!(payload.contains("/two/"), "Payload: {payload}"); } - -#[test] -fn rkv_sqlite_migration() { - let tempdir = tempfile::tempdir().unwrap(); - - let db_dir = tempdir.path().join("db"); - fs::create_dir_all(&db_dir).unwrap(); - let rkv_db = db_dir.join("data.safe.bin"); - fs::write(&rkv_db, include_bytes!("rkv-database.safe.bin")).unwrap(); - - cargo_bin_cmd!("verify-data") - .arg(tempdir.path()) - .arg("verify") - .assert() - .success(); -} diff --git a/glean-core/rlb-tests/tests/rkv-database.safe.bin b/glean-core/rlb-tests/tests/rkv-database.safe.bin deleted file mode 100644 index 8858f9339dfdabcad0055201f8d50bdadbab4023..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 4065 zcmcInTW{k;6i(UYVu3(hg;-RIyIdq1$9A*1u?U0^5|5yL+ErzZ?MX7~#A9a0?M79_ z3vaygfcOpk03LbbfnNjvfHUJc>85egY$;aS8GDYubD8hVIlk4Zubta&QYk9U^aXc+ z$Bh$CNs(ln(LyJz7>j+J=R+srg*bunuABFV>5#h5s=#wxSh2e7` z`|gq=R%j{a=ivc*9|lnnez-ND zX0vN-Q*U_Kk8AtQmhL)rE+{r$L46ajg6Sj^RiV*Z?{b_MTrOELF;KYe$j_80%2`48 zYvdEc8PJP~A~0{LL{;gGX-)G^t#qxX!zM$kk_(bssHC1JW5RePX-CpboKQ(p-prC) zfVZ~+6ZR}eu+N;F*L9N@VKg)|*Gs?EG&X?d!tswk+xGtK+w=Rs%YQx(>|Lw16zS{7 z0|UGcPE9MFi&MyErb?o-DV517VR|cH&^H{v`0|w{Up~0Pmxb~h(};;Wqgt{|IXY1( zg#@13FedQ7q8&|@4xCt^$gJcvsU%-ZtdE^9%g8PTfU5)|2xQVi&h6>-vy=CJaub96 zgPT7(^FcbHkD1cSAB_oD^uRtU$c*|UKTrMhgR6w-2^<(|3&9dd3Tc$sT=LJ4ke|!G z{KqVL-}%-EdgE!*Y1DT8Jc+KtTX6ONNEAHHmt*&YHw*1ImML#ua=3&tHxII@5YTRX z=N7$h+{BOOSYRNLYvbr+0CGlnLyq~jo{{> z?q3OBgD#AQ5JVRm8gWfo{d8O06`L2$jd<(u{L_e1vuMA}&qUD;2_g5j1woJKI6nE{@7ZPMIPfJO4Ku;xf7J z38}QDb}b}s-*%7+%6c)eh)5Q%?m1d5(<5V7H5LNv(Pm&M`B>S#_ndbI@O3wzYF(;P zr$Zmx79q&Q413`4dO`Rvv>t+S diff --git a/glean-core/rlb/src/private/event.rs b/glean-core/rlb/src/private/event.rs index 378f7bad66..3ad16c11d5 100644 --- a/glean-core/rlb/src/private/event.rs +++ b/glean-core/rlb/src/private/event.rs @@ -34,7 +34,7 @@ impl MallocSizeOf for EventMetric { } impl<'a, K> MetricIdentifier<'a> for EventMetric { - fn get_identifiers(&'a self) -> (&'a str, &'a str, Option) { + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option<&'a str>) { self.inner.get_identifiers() } } diff --git a/glean-core/rlb/src/private/object.rs b/glean-core/rlb/src/private/object.rs index 5356a4a486..214e4175a8 100644 --- a/glean-core/rlb/src/private/object.rs +++ b/glean-core/rlb/src/private/object.rs @@ -34,7 +34,7 @@ impl MallocSizeOf for ObjectMetric { } impl<'a, K> MetricIdentifier<'a> for ObjectMetric { - fn get_identifiers(&'a self) -> (&'a str, &'a str, Option) { + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option<&'a str>) { self.inner.get_identifiers() } } diff --git a/glean-core/rlb/src/test.rs b/glean-core/rlb/src/test.rs index 16a4432972..cdda69bff0 100644 --- a/glean-core/rlb/src/test.rs +++ b/glean-core/rlb/src/test.rs @@ -8,7 +8,7 @@ use std::time::{Duration, Instant}; use crossbeam_channel::RecvTimeoutError; use flate2::read::GzDecoder; -use glean_core::{glean_test_get_experimentation_id, LabeledCounter, MetricLabel}; +use glean_core::{glean_test_get_experimentation_id, DynamicLabelType, LabeledCounter}; use serde_json::Value as JsonValue; use crate::private::PingType; @@ -142,7 +142,7 @@ fn disabling_upload_disables_metrics_recording() { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - label: None, + dynamic_label: None, ..Default::default() }); @@ -438,7 +438,7 @@ fn queued_recorded_metrics_correctly_record_during_init() { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - label: None, + dynamic_label: None, ..Default::default() }); @@ -1260,7 +1260,7 @@ fn test_a_ping_before_submission() { send_in_pings: vec!["custom1".into()], lifetime: Lifetime::Application, disabled: false, - label: None, + dynamic_label: None, ..Default::default() }); @@ -1291,7 +1291,7 @@ fn test_boolean_get_num_errors() { send_in_pings: vec!["custom1".into()], lifetime: Lifetime::Application, disabled: false, - label: Some(MetricLabel::Label(str::to_string("asdf"))), + dynamic_label: Some(DynamicLabelType::Label(str::to_string("asdf"))), ..Default::default() }); @@ -1378,7 +1378,7 @@ fn test_text_can_hold_long_string() { send_in_pings: vec!["custom1".into()], lifetime: Lifetime::Application, disabled: false, - label: Some(MetricLabel::Label(str::to_string("text"))), + dynamic_label: Some(DynamicLabelType::Label(str::to_string("text"))), ..Default::default() }); diff --git a/glean-core/rlb/tests/metric_metadata.rs b/glean-core/rlb/tests/metric_metadata.rs index c0cba456a8..803290bb03 100644 --- a/glean-core/rlb/tests/metric_metadata.rs +++ b/glean-core/rlb/tests/metric_metadata.rs @@ -15,7 +15,7 @@ mod metrics { use glean::private::*; use glean::traits; use glean::CommonMetricData; - use glean_core::MetricLabel; + use glean_core::DynamicLabelType; use once_cell::sync::Lazy; use std::collections::HashMap; @@ -45,7 +45,7 @@ mod metrics { name: "count_von_count".into(), category: "sesame".into(), send_in_pings: vec!["validation".into()], - label: Some(MetricLabel::Label("ah_ah_ah".into())), + dynamic_label: Some(DynamicLabelType::Label("ah_ah_ah".into())), ..Default::default() }) }); @@ -56,7 +56,7 @@ mod metrics { name: "birthday".into(), category: "shire".into(), send_in_pings: vec!["validation".into()], - label: Some(MetricLabel::Label("111th".into())), + dynamic_label: Some(DynamicLabelType::Label("111th".into())), ..Default::default() }) }); @@ -87,14 +87,14 @@ fn check_metadata() { let (category, name, label) = metrics::countit.get_identifiers(); assert_eq!(category, "sesame"); assert_eq!(name, "count_von_count"); - assert_eq!(label, Some("ah_ah_ah".to_string())); + assert_eq!(label, Some("ah_ah_ah")); // Events and Objects have MetricIdentifier implemented explicitly, as // they wrap the glean-core Event and Object types let (category, name, label) = metrics::event.get_identifiers(); assert_eq!(category, "shire"); assert_eq!(name, "birthday"); - assert_eq!(label, Some("111th".to_string())); + assert_eq!(label, Some("111th")); let (category, name, label) = metrics::object.get_identifiers(); assert_eq!(category, "court"); diff --git a/glean-core/rlb/tests/upload_timing.rs b/glean-core/rlb/tests/upload_timing.rs index 4b55ef82b9..657f87f568 100644 --- a/glean-core/rlb/tests/upload_timing.rs +++ b/glean-core/rlb/tests/upload_timing.rs @@ -55,7 +55,7 @@ pub mod metrics { send_in_pings: vec!["metrics".into()], lifetime: Lifetime::Ping, disabled: false, - label: None, + dynamic_label: None, ..Default::default() }, TimeUnit::Millisecond, @@ -71,7 +71,7 @@ pub mod metrics { send_in_pings: vec!["metrics".into()], lifetime: Lifetime::Ping, disabled: false, - label: None, + dynamic_label: None, ..Default::default() }, TimeUnit::Millisecond, @@ -87,7 +87,7 @@ pub mod metrics { send_in_pings: vec!["metrics".into()], lifetime: Lifetime::Ping, disabled: false, - label: None, + dynamic_label: None, ..Default::default() }, TimeUnit::Millisecond, diff --git a/glean-core/src/common_metric_data.rs b/glean-core/src/common_metric_data.rs index 4d5b2427a3..2c075a4d79 100644 --- a/glean-core/src/common_metric_data.rs +++ b/glean-core/src/common_metric_data.rs @@ -2,17 +2,15 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::fmt::Display; +use std::ops::Deref; use std::sync::atomic::{AtomicU8, Ordering}; use malloc_size_of_derive::MallocSizeOf; -use rusqlite::Transaction; use crate::error::{Error, ErrorKind}; -use crate::error_recording::record_error_sqlite; -use crate::metrics::dual_labeled_counter::validate_dual_label_sqlite; -use crate::metrics::labeled::validate_dynamic_label_sqlite; -use crate::{ErrorType, Glean}; +use crate::metrics::dual_labeled_counter::validate_dynamic_key_and_or_category; +use crate::metrics::labeled::validate_dynamic_label; +use crate::Glean; use serde::{Deserialize, Serialize}; /// The supported metrics' lifetimes. @@ -70,6 +68,13 @@ pub struct CommonMetricData { /// /// Disabled metrics are never recorded. pub disabled: bool, + /// Dynamic label. + /// + /// When a [`LabeledMetric`](crate::metrics::LabeledMetric) factory creates the specific + /// metric to be recorded to, dynamic labels are stored in the specific + /// label so that we can validate them when the Glean singleton is + /// available. + pub dynamic_label: Option, /// Whether this metric is inside of the session scope. /// /// `false` (the default) means this metric bypasses session sampling and @@ -77,49 +82,37 @@ pub struct CommonMetricData { /// for now. Event metrics that participate in session tracking must /// explicitly set this to `true`. pub in_session: bool, - - /// Label for this metric. - /// - /// When a [`LabeledMetric`](crate::metrics::LabeledMetric) factory - /// or [`DualLabeledCounterMetric`](crate::metrics::DualLabeledCounterMetric) - /// creates the specific metric to be recorded to, - /// labels are stored in the metric data - /// so that it can validated against the database later. - pub label: Option, } /// The type of dynamic label applied to a base metric. Used to help identify /// the necessary validation to be performed. #[derive(Debug, Clone, Deserialize, Serialize, MallocSizeOf, uniffi::Enum)] -pub enum MetricLabel { - /// Static Label -- no validation required - Static(String), +pub enum DynamicLabelType { /// A dynamic label applied from a `LabeledMetric` Label(String), - /// A label applied by a `DualLabeledCounter` that contains a dynamic key and static category - KeyOnly(String, String), - /// A label applied by a `DualLabeledCounter` that contains a static key and dynamic category - CategoryOnly(String, String), + /// A label applied by a `DualLabeledCounter` that contains a dynamic key + KeyOnly(String), + /// A label applied by a `DualLabeledCounter` that contains a dynamic category + CategoryOnly(String), /// A label applied by a `DualLabeledCounter` that contains a dynamic key and category - KeyAndCategory(String, String), + KeyAndCategory(String), } -impl Default for MetricLabel { +impl Default for DynamicLabelType { fn default() -> Self { Self::Label(String::new()) } } -impl Display for MetricLabel { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - use crate::metrics::dual_labeled_counter::RECORD_SEPARATOR; +impl Deref for DynamicLabelType { + type Target = str; + + fn deref(&self) -> &Self::Target { match self { - MetricLabel::Static(label) | MetricLabel::Label(label) => write!(f, "{label}"), - MetricLabel::KeyOnly(key, category) - | MetricLabel::CategoryOnly(key, category) - | MetricLabel::KeyAndCategory(key, category) => { - write!(f, "{key}{RECORD_SEPARATOR}{category}") - } + DynamicLabelType::Label(label) => label, + DynamicLabelType::KeyOnly(key) => key, + DynamicLabelType::CategoryOnly(category) => category, + DynamicLabelType::KeyAndCategory(key_and_category) => key_and_category, } } } @@ -149,64 +142,6 @@ impl From for CommonMetricDataInternal { } } -/// A label checked for validity against label rules and against the database (for a specific metric). -pub enum LabelCheck { - /// No label was supplied, no check done. - NoLabel, - /// Label was validated and is usable. - Label(String), - /// Label check errored, the provided replacement label should be used. - /// Errors can be recorded using [`LabelCheck::record_error`]. - Error(String, i32), -} - -impl LabelCheck { - /// Extract the label to be used (or an empty string if no label was checked) - pub fn label(&self) -> &str { - use LabelCheck::*; - match self { - NoLabel => "", - Label(label) | Error(label, _) => label, - } - } - - /// Record the number of errors that were detected during the label check. - pub fn record_error( - &self, - glean: &Glean, - tx: &mut Transaction, - metric_name: &str, - send_in_pings: &[String], - ) { - let LabelCheck::Error(_, count) = self else { - return; - }; - - record_error_sqlite( - glean, - tx, - metric_name, - send_in_pings, - ErrorType::InvalidLabel, - *count, - ); - } - - /// Maps a `LabelCheck` by applying a function to the contained label (if any). - /// - /// This only maps over the already checked label. - /// No further label check is done. - fn map(self, mut f: impl FnMut(String) -> String) -> Self { - use LabelCheck::*; - - match self { - NoLabel => NoLabel, - Label(s) => Label(f(s)), - Error(s, cnt) => Error(f(s), cnt), - } - } -} - impl CommonMetricDataInternal { /// Creates a new metadata object. pub fn new, B: Into, C: Into>( @@ -237,35 +172,27 @@ impl CommonMetricDataInternal { } } - /// Check the label for validity against the database. + /// The metric's unique identifier, including the category, name and label. /// - /// Returns the result of the check. - /// No error is recorded in the database if the check fails. - /// Extract the validated label, if any, using [`LabelCheck::label`]. - /// Record an error, if any, using [`LabelCheck::record_error`]. - pub(crate) fn check_labels(&self, tx: &Transaction<'_>) -> LabelCheck { + /// If `category` is empty, it's ommitted. + /// Otherwise, it's the combination of the metric's `category`, `name` and `label`. + pub(crate) fn identifier(&self, glean: &Glean) -> String { let base_identifier = self.base_identifier(); - if let Some(label) = &self.inner.label { + if let Some(label) = &self.inner.dynamic_label { match label { - MetricLabel::Static(label) => LabelCheck::Label(label.to_string()), - MetricLabel::Label(label) => { - validate_dynamic_label_sqlite(tx, &base_identifier, label) - } - MetricLabel::KeyOnly(key, static_category) => { - validate_dual_label_sqlite(tx, &base_identifier, key, "") - .map(|key| format!("{key}{static_category}")) - } - MetricLabel::CategoryOnly(static_key, category) => { - validate_dual_label_sqlite(tx, &base_identifier, "", category) - .map(|category| format!("{static_key}{category}")) - } - MetricLabel::KeyAndCategory(key, category) => { - validate_dual_label_sqlite(tx, &base_identifier, key, category) + DynamicLabelType::Label(label) => { + validate_dynamic_label(glean, self, &base_identifier, label) } + _ => validate_dynamic_key_and_or_category( + glean, + self, + &base_identifier, + label.clone(), + ), } } else { - LabelCheck::NoLabel + base_identifier } } diff --git a/glean-core/src/core/mod.rs b/glean-core/src/core/mod.rs index 59ee831974..0130646604 100644 --- a/glean-core/src/core/mod.rs +++ b/glean-core/src/core/mod.rs @@ -15,7 +15,7 @@ use malloc_size_of_derive::MallocSizeOf; use once_cell::sync::OnceCell; use uuid::Uuid; -use crate::database::sqlite::{Database, MigrationResult}; +use crate::database::Database; use crate::debug::DebugOptions; use crate::error::ClientIdFileError; use crate::event_database::EventDatabase; @@ -30,7 +30,7 @@ use crate::ping::PingMaker; use crate::session::{self, EventSessionContext, SessionManager, SessionMode, SessionState}; use crate::storage::{StorageManager, INTERNAL_STORAGE}; use crate::upload::{PingUploadManager, PingUploadTask, UploadResult, UploadTaskAction}; -use crate::util::{local_now_with_offset, sanitize_application_id, truncate_string_at_boundary}; +use crate::util::{local_now_with_offset, sanitize_application_id}; use crate::{ scheduler, system, AttributionMetrics, CommonMetricData, DistributionMetrics, ErrorKind, InternalConfiguration, Lifetime, PingRateLimit, Result, DEFAULT_MAX_EVENTS, @@ -307,32 +307,6 @@ impl Glean { ping_lifetime_max_time, )?); - if let Some(state) = glean.data_store.as_mut().unwrap().migration_state.take() { - glean - .database_metrics - .migrated_metrics - .add_sync(&glean, state.migrated_metrics); - glean - .database_metrics - .metrics_in_sqlite - .add_sync(&glean, state.metrics_in_sql); - glean - .database_metrics - .failed_metrics - .add_sync(&glean, state.failed_metrics); - - let duration_ns = state.duration.as_nanos().try_into().unwrap_or(u64::MAX); - log::error!("set duration: {duration_ns:?}"); - glean - .database_metrics - .migration_duration - .accumulate_raw_samples_nanos_sync(&glean, &[duration_ns]); - } - - if glean.data_store.as_mut().unwrap().migration_error == MigrationResult::Error { - glean.database_metrics.migration_error.add_sync(&glean, 1); - } - glean.restore_session_state_from_storage(); // This code references different states from the "Client ID recovery" flowchart. @@ -395,7 +369,7 @@ impl Glean { { let data_store = glean.data_store.as_ref().unwrap(); - let file_size = data_store.file_size().map(|n| n.get()).unwrap_or(0); + let file_size = data_store.file_size.map(|n| n.get()).unwrap_or(0); // If we have a client ID on disk, we check the database if let Some(stored_client_id) = stored_client_id { @@ -739,14 +713,14 @@ impl Glean { .accumulate_sync(self, size.get() as i64) } - if let Some(load_state) = self + if let Some(rkv_load_state) = self .data_store .as_ref() - .and_then(|database| database.load_state()) + .and_then(|database| database.rkv_load_state()) { - use crate::metrics::string::MAX_LENGTH_VALUE; - let load_state = truncate_string_at_boundary(load_state, MAX_LENGTH_VALUE); - self.database_metrics.load_error.set_sync(self, load_state) + self.database_metrics + .rkv_load_error + .set_sync(self, rkv_load_state) } } @@ -937,34 +911,13 @@ impl Glean { // Note that this also includes the ping sequence numbers, so it has // the effect of resetting those to their initial values. if let Some(data) = self.data_store.as_ref() { - let warn_on_error = |result, msg| { - if let Err(e) = result { - log::warn!("{msg}: {e}"); - } - }; - - warn_on_error( - data.clear_lifetime_storage(Lifetime::User, INTERNAL_STORAGE), - "failed to clear internal storage", - ); - warn_on_error( - data.remove_single_metric(Lifetime::User, "glean_client_info", "client_id"), - "failed to clear internal client info storage", - ); + _ = data.clear_lifetime_storage(Lifetime::User, "glean_internal_info"); + _ = data.remove_single_metric(Lifetime::User, "glean_client_info", "client_id"); for (ping_name, ping) in &self.ping_registry { if ping.follows_collection_enabled() { - warn_on_error( - data.clear_ping_lifetime_storage(ping_name), - "failed to clear ping lifetime storage", - ); - warn_on_error( - data.clear_lifetime_storage(Lifetime::User, ping_name), - "failed to clear user lifetime storage", - ); - warn_on_error( - data.clear_lifetime_storage(Lifetime::Application, ping_name), - "failed to clear application lifetime storage", - ); + _ = data.clear_ping_lifetime_storage(ping_name); + _ = data.clear_lifetime_storage(Lifetime::User, ping_name); + _ = data.clear_lifetime_storage(Lifetime::Application, ping_name); } } } @@ -1402,10 +1355,12 @@ impl Glean { /// Checks the stored value of the "dirty flag". pub fn is_dirty_flag_set(&self) -> bool { let dirty_bit_metric = self.get_dirty_bit_metric(); - match self - .storage() - .get_metric(dirty_bit_metric.meta(), INTERNAL_STORAGE) - { + match StorageManager.snapshot_metric( + self.storage(), + INTERNAL_STORAGE, + &dirty_bit_metric.meta().identifier(self), + dirty_bit_metric.meta().inner.lifetime, + ) { Some(Metric::Boolean(b)) => b, _ => false, } @@ -1892,7 +1847,7 @@ impl Glean { _ = data.remove_single_metric( meta.inner.lifetime, &meta.storage_names()[0], - &meta.base_identifier(), + &meta.identifier(self), ); }); } @@ -1958,7 +1913,7 @@ impl Glean { _ = data.remove_single_metric( meta.inner.lifetime, &meta.storage_names()[0], - &meta.base_identifier(), + &meta.identifier(self), ); } } diff --git a/glean-core/src/database/migration.rs b/glean-core/src/database/migration.rs deleted file mode 100644 index 0518c931b8..0000000000 --- a/glean-core/src/database/migration.rs +++ /dev/null @@ -1,362 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -use std::fs; -use std::path::Path; -use std::str; -use std::time::Duration; -use std::time::Instant; - -use crate::metrics::Metric; -use crate::Error; -use crate::Lifetime; -use crate::Result; - -use rkv::StoreOptions; -use rusqlite::Transaction; - -use super::sqlite; - -pub type Rkv = rkv::Rkv; -pub type SingleStore = rkv::SingleStore; - -pub(crate) const RECORD_SEPARATOR: char = '\x1E'; - -pub fn rkv_new(path: &Path) -> std::result::Result { - match Rkv::new::(path) { - // An invalid file can mean: - // 1. An empty file. - // 2. A corrupted file. - // - // In both instances there's not much we can do. - // Drop the data by removing the file. - Err(rkv::StoreError::FileInvalid) => { - log::debug!("rkv failed: invalid file. starting from scratch."); - let safebin = path.join("data.safe.bin"); - fs::remove_file(safebin).map_err(|_| rkv::StoreError::FileInvalid)?; - Err(rkv::StoreError::FileInvalid) - } - Err(rkv::StoreError::DatabaseCorrupted) => { - log::debug!("rkv failed: database corrupted. starting from scratch."); - let safebin = path.join("data.safe.bin"); - fs::remove_file(safebin).map_err(|_| rkv::StoreError::DatabaseCorrupted)?; - Err(rkv::StoreError::DatabaseCorrupted) - } - other => { - let rkv = other?; - Ok(rkv) - } - } -} - -pub struct RkvDatabase { - /// Handle to the database environment. - rkv: Rkv, - - /// Handles to the "lifetime" stores. - /// - /// A "store" is a handle to the underlying database. - /// We keep them open for fast and frequent access. - user_store: SingleStore, - ping_store: SingleStore, - application_store: SingleStore, -} - -impl RkvDatabase { - /// Open the Rkv database and the embbedded stores. - pub fn new(data_path: &Path) -> Result { - log::debug!("Rkv database path: {:?}", data_path.display()); - - let rkv = Self::open_rkv(data_path)?; - let user_store = rkv.open_single(Lifetime::User.as_str(), StoreOptions::create())?; - let ping_store = rkv.open_single(Lifetime::Ping.as_str(), StoreOptions::create())?; - let application_store = - rkv.open_single(Lifetime::Application.as_str(), StoreOptions::create())?; - - let db = Self { - rkv, - user_store, - ping_store, - application_store, - }; - - Ok(db) - } - - fn open_rkv(path: &Path) -> Result { - let rkv = rkv_new(path)?; - Ok(rkv) - } - - fn get_store(&self, lifetime: Lifetime) -> &SingleStore { - match lifetime { - Lifetime::User => &self.user_store, - Lifetime::Ping => &self.ping_store, - Lifetime::Application => &self.application_store, - } - } - - /// Iterates with the provided transaction function - /// over the requested data from the given storage. - /// - /// * If the storage is unavailable, the transaction function is never invoked. - /// * If the read data cannot be deserialized it will be silently skipped. - /// - /// # Arguments - /// - /// * `lifetime` - The metric lifetime to iterate over. - /// * `transaction_fn` - Called for each entry being iterated over. - /// It is passed two arguments: `(key: String, metric: &Metric)`. - /// - /// # Returns - /// - /// The number of metrics that failed to deserialize from the stored data. - pub fn iter_store(&self, lifetime: Lifetime, mut transaction_fn: F) -> i32 - where - F: FnMut(String, &Metric), - { - let Ok(reader) = self.rkv.read() else { - return 0; - }; - let Ok(mut iter) = self.get_store(lifetime).iter_start(&reader) else { - log::debug!("No store for {lifetime:?}"); - return 0; - }; - - let mut failed = 0; - while let Some(Ok((key, value))) = iter.next() { - let Ok(key) = String::from_utf8(key.to_vec()) else { - log::debug!("Key is not valid UTF-8: {key:?}"); - continue; - }; - let metric: Metric = match value { - rkv::Value::Blob(blob) => { - let Ok(value) = bincode::deserialize(blob) else { - log::debug!("Value for key {key:?} could not be deserialized"); - failed += 1; - continue; - }; - value - } - _ => { - failed += 1; - log::debug!("Blob for key {key:?} is not a valid blob"); - continue; - } - }; - transaction_fn(key, &metric); - } - - failed - } -} - -#[derive(Debug, Clone, PartialEq, Eq)] -struct MetricKey<'a> { - ping: &'a str, - id: &'a str, - label: Option, -} - -/// Split a database key into its metric key parts. -fn split_key(key: &str) -> Option> { - let (ping, rest) = key.split_once('#')?; - if ping.is_empty() || rest.is_empty() { - return None; - } - - let (id, labels) = match rest.split_once(|c| ['/', RECORD_SEPARATOR].contains(&c)) { - Some((id, labels)) => { - if labels.is_empty() { - return None; - } - (id, labels) - } - _ => (rest, ""), - }; - if id.is_empty() { - return None; - } - - let label = if labels.is_empty() { - // No label at all - None - } else if labels.contains(RECORD_SEPARATOR) { - // Label separated by - let (key, category) = labels.split_once(RECORD_SEPARATOR)?; - - if key.is_empty() || category.is_empty() { - return None; - } - - Some(String::from(labels)) - } else { - Some(String::from(labels)) - }; - - Some(MetricKey { ping, id, label }) -} - -#[derive(Debug, Default)] -pub struct MigrationState { - /// The number of metrics migrated from Rkv storage to SQLite storage - pub migrated_metrics: i32, - /// Number of metrics that failed to deserialize from storage - /// while iterating the Rkv database for migration. - pub failed_metrics: i32, - /// The number of metrics stored in SQLite after a migration run - pub metrics_in_sql: i32, - /// The duration for one full migration run at startup - pub duration: Duration, -} - -/// Migrate the `rkv` database to SQL. -/// -/// Returns numbers about the full migration. -fn migrate(rkv: &RkvDatabase, sql_db: &sqlite::Database, tx: &mut Transaction) -> MigrationState { - let mut state = MigrationState::default(); - let mut migrate_metric = |lifetime: Lifetime, key: String, metric: &Metric| { - let Some(metric_id) = split_key(&key) else { - log::debug!("Invalid metric key: {key:?}"); - state.failed_metrics += 1; - return; - }; - let label = metric_id.label.as_deref().unwrap_or(""); - _ = sql_db.record_per_lifetime(tx, lifetime, metric_id.ping, metric_id.id, label, metric); - state.migrated_metrics += 1; - }; - - let mut failed_metrics = 0; - - let migration_start = Instant::now(); - let snapshotter_user = - |key: String, metric: &Metric| migrate_metric(Lifetime::User, key, metric); - failed_metrics += rkv.iter_store(Lifetime::User, snapshotter_user); - - let snapshotter_app = - |key: String, metric: &Metric| migrate_metric(Lifetime::Application, key, metric); - failed_metrics += rkv.iter_store(Lifetime::Application, snapshotter_app); - - let snapshotter_ping = - |key: String, metric: &Metric| migrate_metric(Lifetime::Ping, key, metric); - failed_metrics += rkv.iter_store(Lifetime::Ping, snapshotter_ping); - - state.failed_metrics = failed_metrics; - state.metrics_in_sql = tx - .query_one("SELECT COUNT(*) FROM telemetry", [], |row| row.get(0)) - .unwrap_or(0); - state.duration = migration_start.elapsed(); - - state -} - -pub fn try_migrate(data_path: &Path, db: &sqlite::Database) -> Result> { - let rkv_file = data_path.join("data.safe.bin"); - log::debug!( - "Trying to migrate. Data path: {}, expected file: {}", - data_path.display(), - rkv_file.display() - ); - - if !rkv_file.exists() { - log::debug!("No rkv file. No migration."); - return Ok(None); - } - - let rkv = RkvDatabase::new(data_path)?; - - let state = db.conn.write(|tx| { - let state = migrate(&rkv, db, tx); - Ok::<_, Error>(state) - })?; - - log::debug!( - "Data migrated. Would be removing Rkv database at {}", - rkv_file.display() - ); - - Ok(Some(state)) -} - -#[cfg(test)] -mod test { - use super::*; - - impl<'a> MetricKey<'a> { - fn new<'b>(ping: &'a str, id: &'a str, label: impl Into>) -> Self { - Self { - ping, - id, - label: label.into().map(|s| s.to_string()), - } - } - } - - #[test] - fn splitting_key() { - let matches = &[ - (MetricKey::new("metrics", "name", None), "metrics#name"), - ( - MetricKey::new("metrics", "cat.name", None), - "metrics#cat.name", - ), - ( - MetricKey::new("metrics", "cat1.cat2.name", None), - "metrics#cat1.cat2.name", - ), - ( - MetricKey::new("metrics", "cat1.cat2.name", "label"), - "metrics#cat1.cat2.name/label", - ), - ( - // This currently works. We do allow slashes in labels. - // Maybe we shouldn't have. - MetricKey::new("metrics", "cat1.cat2.name", "label1/label2"), - "metrics#cat1.cat2.name/label1/label2", - ), - ( - // This currently works. We do allow slashes in labels. - // Maybe we shouldn't have. - MetricKey::new("metrics", "cat.name", "label//"), - "metrics#cat.name/label//", - ), - ( - MetricKey::new("metrics", "cat1.cat2.name", "label1\x1Elabel2"), - "metrics#cat1.cat2.name\x1elabel1\x1elabel2", - ), - ( - MetricKey::new("glean_internal_info", "baseline#sequence", None), - "glean_internal_info#baseline#sequence", - ), - ]; - - for (exp, key) in matches { - let m = split_key(key).unwrap_or_else(|| panic!("{key:?} should be splittable")); - assert_eq!(*exp, m, "did not split correctly: {key:?}"); - } - } - - #[test] - fn splitting_key_fails() { - let matches = &[ - "", - "metrics", - "metrics#", - "#cat", - "#cat.name", - "metrics#/", - "metrics#//", - "metrics#/label", - "metrics#cat.name/", - "metrics#cat.name\x1e", - "metrics#cat.name\x1e\x1e", - "metrics#cat.name\x1elabel1\x1e", - "metrics#cat.name\x1e\x1elabel2", - ]; - - for key in matches { - assert_eq!(None, split_key(key), "should not split: {key:?}"); - } - } -} diff --git a/glean-core/src/database/mod.rs b/glean-core/src/database/mod.rs index 3c1fee301a..8408aa1f18 100644 --- a/glean-core/src/database/mod.rs +++ b/glean-core/src/database/mod.rs @@ -2,5 +2,1622 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -pub mod migration; -pub mod sqlite; +use std::cell::{Cell, RefCell}; +use std::collections::btree_map::Entry; +use std::collections::BTreeMap; +use std::fs; +use std::io; +use std::num::NonZeroU64; +use std::path::Path; +use std::str; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::sync::RwLock; +use std::time::{Duration, Instant}; + +use crate::ErrorKind; + +use malloc_size_of::MallocSizeOf; +use rkv::{StoreError, StoreOptions}; + +/// Unwrap a `Result`s `Ok` value or do the specified action. +/// +/// This is an alternative to the question-mark operator (`?`), +/// when the other action should not be to return the error. +macro_rules! unwrap_or { + ($expr:expr, $or:expr) => { + match $expr { + Ok(x) => x, + Err(_) => { + $or; + } + } + }; +} + +macro_rules! measure_commit { + ($this:ident, $expr:expr) => {{ + let now = ::std::time::Instant::now(); + let res = $expr; + let elapsed = now.elapsed(); + if let Ok(elapsed) = elapsed.as_micros().try_into() { + let mut samples = $this.write_timings.borrow_mut(); + samples.push(elapsed); + } + res + }}; +} + +/// cbindgen:ignore +pub type Rkv = rkv::Rkv; +/// cbindgen:ignore +pub type SingleStore = rkv::SingleStore; +/// cbindgen:ignore +pub type Writer<'t> = rkv::Writer>; + +#[derive(Debug)] +pub enum RkvLoadState { + Ok, + Err(rkv::StoreError), +} + +pub fn rkv_new(path: &Path) -> std::result::Result<(Rkv, RkvLoadState), rkv::StoreError> { + match Rkv::new::(path) { + // An invalid file can mean: + // 1. An empty file. + // 2. A corrupted file. + // + // In both instances there's not much we can do. + // Drop the data by removing the file, and start over. + Err(rkv::StoreError::FileInvalid) => { + log::debug!("rkv failed: invalid file. starting from scratch."); + let safebin = path.join("data.safe.bin"); + fs::remove_file(safebin).map_err(|_| rkv::StoreError::FileInvalid)?; + // Now try again, we only handle that error once. + let rkv = Rkv::new::(path)?; + Ok((rkv, RkvLoadState::Err(rkv::StoreError::FileInvalid))) + } + Err(rkv::StoreError::DatabaseCorrupted) => { + log::debug!("rkv failed: database corrupted. starting from scratch."); + let safebin = path.join("data.safe.bin"); + fs::remove_file(safebin).map_err(|_| rkv::StoreError::DatabaseCorrupted)?; + // Try again, only allowing the error once. + let rkv = Rkv::new::(path)?; + Ok((rkv, RkvLoadState::Err(rkv::StoreError::DatabaseCorrupted))) + } + other => { + let rkv = other?; + Ok((rkv, RkvLoadState::Ok)) + } + } +} + +use crate::common_metric_data::CommonMetricDataInternal; +use crate::metrics::Metric; +use crate::Glean; +use crate::Lifetime; +use crate::Result; + +pub struct Database { + /// Handle to the database environment. + rkv: Rkv, + + /// Handles to the "lifetime" stores. + /// + /// A "store" is a handle to the underlying database. + /// We keep them open for fast and frequent access. + user_store: SingleStore, + ping_store: SingleStore, + application_store: SingleStore, + + /// If the `delay_ping_lifetime_io` Glean config option is `true`, + /// we will save metrics with 'ping' lifetime data in a map temporarily + /// so as to persist them to disk using rkv in bulk on demand. + ping_lifetime_data: Option>>, + + /// A count of how many database writes have been done since the last ping-lifetime flush. + /// + /// A ping-lifetime flush is automatically done after `ping_lifetime_threshold` writes. + /// + /// Only relevant if `delay_ping_lifetime_io` is set to `true`, + ping_lifetime_count: AtomicUsize, + + /// Write-count threshold when to auto-flush. `0` disables it. + ping_lifetime_threshold: usize, + + /// The last time the `lifetime=ping` data was flushed to disk. + /// + /// Data is flushed to disk automatically when the last flush was more than + /// `ping_lifetime_max_time` ago. + /// + /// Only relevant if `delay_ping_lifetime_io` is set to `true`, + ping_lifetime_store_ts: Cell, + + /// After what time to auto-flush. 0 disables it. + ping_lifetime_max_time: Duration, + + /// Initial file size when opening the database. + pub(crate) file_size: Option, + + /// RKV load state + rkv_load_state: RkvLoadState, + + /// Times an Rkv write-commit took. + /// Re-applied as samples in a timing distribution later. + pub(crate) write_timings: RefCell>, +} + +impl MallocSizeOf for Database { + fn size_of(&self, ops: &mut malloc_size_of::MallocSizeOfOps) -> usize { + // TODO(bug 1960592): Fill in gaps. + + let mut n = 0; + + n += self.rkv.size_of(ops); + n += self.user_store.size_of(ops); + n += self.ping_store.size_of(ops); + n += self.application_store.size_of(ops); + + n += self + .ping_lifetime_data + .as_ref() + .map(|data| { + // TODO(bug 1960592): servo's malloc_size_of implements it for BTreeMap. + let lock = data.read().unwrap(); + (*lock).size_of(ops) + }) + .unwrap_or(0); + + n + } +} + +impl std::fmt::Debug for Database { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + fmt.debug_struct("Database") + .field("rkv", &self.rkv) + .field("user_store", &"SingleStore") + .field("ping_store", &"SingleStore") + .field("application_store", &"SingleStore") + .field("ping_lifetime_data", &self.ping_lifetime_data) + .finish() + } +} + +/// Calculate the database size from all the files in the directory. +/// +/// # Arguments +/// +/// *`path` - The path to the directory +/// +/// # Returns +/// +/// Returns the non-zero combined size of all files in a directory, +/// or `None` on error or if the size is `0`. +fn database_size(dir: &Path) -> Option { + let mut total_size = 0; + if let Ok(entries) = fs::read_dir(dir) { + for entry in entries.flatten() { + if let Ok(file_type) = entry.file_type() { + if file_type.is_file() { + let path = entry.path(); + if let Ok(metadata) = fs::metadata(path) { + total_size += metadata.len(); + } else { + continue; + } + } + } + } + } + + NonZeroU64::new(total_size) +} + +impl Database { + /// Initializes the data store. + /// + /// This opens the underlying rkv store and creates + /// the underlying directory structure. + /// + /// It also loads any Lifetime::Ping data that might be + /// persisted, in case `delay_ping_lifetime_io` is set. + pub fn new( + data_path: &Path, + delay_ping_lifetime_io: bool, + ping_lifetime_threshold: usize, + ping_lifetime_max_time: Duration, + ) -> Result { + let path = data_path.join("db"); + log::debug!("Database path: {:?}", path.display()); + let file_size = database_size(&path); + + let (rkv, rkv_load_state) = Self::open_rkv(&path)?; + let user_store = rkv.open_single(Lifetime::User.as_str(), StoreOptions::create())?; + let ping_store = rkv.open_single(Lifetime::Ping.as_str(), StoreOptions::create())?; + let application_store = + rkv.open_single(Lifetime::Application.as_str(), StoreOptions::create())?; + let ping_lifetime_data = if delay_ping_lifetime_io { + Some(RwLock::new(BTreeMap::new())) + } else { + None + }; + + // We are gonna write, so we allocate some capacity upfront. + // The value was chosen at random. + let write_timings = RefCell::new(Vec::with_capacity(64)); + + let now = Instant::now(); + + let db = Self { + rkv, + user_store, + ping_store, + application_store, + ping_lifetime_data, + ping_lifetime_count: AtomicUsize::new(0), + ping_lifetime_threshold, + ping_lifetime_store_ts: Cell::new(now), + ping_lifetime_max_time, + file_size, + rkv_load_state, + write_timings, + }; + + db.load_ping_lifetime_data(); + + Ok(db) + } + + /// Get the initial database file size. + pub fn file_size(&self) -> Option { + self.file_size + } + + /// Get the rkv load state. + pub fn rkv_load_state(&self) -> Option { + if let RkvLoadState::Err(e) = &self.rkv_load_state { + Some(e.to_string()) + } else { + None + } + } + + fn get_store(&self, lifetime: Lifetime) -> &SingleStore { + match lifetime { + Lifetime::User => &self.user_store, + Lifetime::Ping => &self.ping_store, + Lifetime::Application => &self.application_store, + } + } + + /// Creates the storage directories and inits rkv. + fn open_rkv(path: &Path) -> Result<(Rkv, RkvLoadState)> { + fs::create_dir_all(path)?; + + let (rkv, load_state) = rkv_new(path)?; + + log::info!("Database initialized"); + Ok((rkv, load_state)) + } + + /// Build the key of the final location of the data in the database. + /// Such location is built using the storage name and the metric + /// key/name (if available). + /// + /// # Arguments + /// + /// * `storage_name` - the name of the storage to store/fetch data from. + /// * `metric_key` - the optional metric key/name. + /// + /// # Returns + /// + /// A string representing the location in the database. + fn get_storage_key(storage_name: &str, metric_key: Option<&str>) -> String { + match metric_key { + Some(k) => format!("{}#{}", storage_name, k), + None => format!("{}#", storage_name), + } + } + + /// Loads Lifetime::Ping data from rkv to memory, + /// if `delay_ping_lifetime_io` is set to true. + /// + /// Does nothing if it isn't or if there is not data to load. + fn load_ping_lifetime_data(&self) { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + let mut data = ping_lifetime_data + .write() + .expect("Can't read ping lifetime data"); + + let reader = unwrap_or!(self.rkv.read(), return); + let store = self.get_store(Lifetime::Ping); + let mut iter = unwrap_or!(store.iter_start(&reader), return); + + while let Some(Ok((metric_id, value))) = iter.next() { + let metric_id = match str::from_utf8(metric_id) { + Ok(metric_id) => metric_id.to_string(), + _ => continue, + }; + let metric: Metric = match value { + rkv::Value::Blob(blob) => unwrap_or!(bincode::deserialize(blob), continue), + _ => continue, + }; + + data.insert(metric_id, metric); + } + } + } + + /// Iterates with the provided transaction function + /// over the requested data from the given storage. + /// + /// * If the storage is unavailable, the transaction function is never invoked. + /// * If the read data cannot be deserialized it will be silently skipped. + /// + /// # Arguments + /// + /// * `lifetime` - The metric lifetime to iterate over. + /// * `storage_name` - The storage name to iterate over. + /// * `metric_key` - The metric key to iterate over. All metrics iterated over + /// will have this prefix. For example, if `metric_key` is of the form `{category}.`, + /// it will iterate over all metrics in the given category. If the `metric_key` is of the + /// form `{category}.{name}/`, the iterator will iterate over all specific metrics for + /// a given labeled metric. If not provided, the entire storage for the given lifetime + /// will be iterated over. + /// * `transaction_fn` - Called for each entry being iterated over. It is + /// passed two arguments: `(metric_id: &[u8], metric: &Metric)`. + /// + /// # Panics + /// + /// This function will **not** panic on database errors. + pub fn iter_store_from( + &self, + lifetime: Lifetime, + storage_name: &str, + metric_key: Option<&str>, + mut transaction_fn: F, + ) where + F: FnMut(&[u8], &Metric), + { + let iter_start = Self::get_storage_key(storage_name, metric_key); + let len = iter_start.len(); + + // Lifetime::Ping data is not immediately persisted to disk if + // Glean has `delay_ping_lifetime_io` set to true + if lifetime == Lifetime::Ping { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + let data = ping_lifetime_data + .read() + .expect("Can't read ping lifetime data"); + for (key, value) in data.iter() { + if key.starts_with(&iter_start) { + let key = &key[len..]; + transaction_fn(key.as_bytes(), value); + } + } + return; + } + } + + let reader = unwrap_or!(self.rkv.read(), return); + let mut iter = unwrap_or!( + self.get_store(lifetime).iter_from(&reader, &iter_start), + return + ); + + while let Some(Ok((metric_id, value))) = iter.next() { + if !metric_id.starts_with(iter_start.as_bytes()) { + break; + } + + let metric_id = &metric_id[len..]; + let metric: Metric = match value { + rkv::Value::Blob(blob) => unwrap_or!(bincode::deserialize(blob), continue), + _ => continue, + }; + transaction_fn(metric_id, &metric); + } + } + + /// Determines if the storage has the given metric. + /// + /// If data cannot be read it is assumed that the storage does not have the metric. + /// + /// # Arguments + /// + /// * `lifetime` - The lifetime of the metric. + /// * `storage_name` - The storage name to look in. + /// * `metric_identifier` - The metric identifier. + /// + /// # Panics + /// + /// This function will **not** panic on database errors. + pub fn has_metric( + &self, + lifetime: Lifetime, + storage_name: &str, + metric_identifier: &str, + ) -> bool { + let key = Self::get_storage_key(storage_name, Some(metric_identifier)); + + // Lifetime::Ping data is not persisted to disk if + // Glean has `delay_ping_lifetime_io` set to true + if lifetime == Lifetime::Ping { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + return ping_lifetime_data + .read() + .map(|data| data.contains_key(&key)) + .unwrap_or(false); + } + } + + let reader = unwrap_or!(self.rkv.read(), return false); + self.get_store(lifetime) + .get(&reader, &key) + .unwrap_or(None) + .is_some() + } + + /// Writes to the specified storage with the provided transaction function. + /// + /// If the storage is unavailable, it will return an error. + /// + /// # Panics + /// + /// * This function will **not** panic on database errors. + fn write_with_store(&self, store_name: Lifetime, mut transaction_fn: F) -> Result<()> + where + F: FnMut(Writer, &SingleStore) -> Result<()>, + { + let writer = self.rkv.write().unwrap(); + let store = self.get_store(store_name); + transaction_fn(writer, store) + } + + /// Records a metric in the underlying storage system. + pub fn record(&self, glean: &Glean, data: &CommonMetricDataInternal, value: &Metric) { + let name = data.identifier(glean); + for ping_name in data.storage_names() { + if glean.is_ping_enabled(ping_name) { + if let Err(e) = + self.record_per_lifetime(data.inner.lifetime, ping_name, &name, value) + { + log::info!( + "Failed to record metric '{}' into {}: {:?}", + data.base_identifier(), + ping_name, + e + ); + } + } + } + } + + /// Records a metric in the underlying storage system, for a single lifetime. + /// + /// # Returns + /// + /// If the storage is unavailable or the write fails, no data will be stored and an error will be returned. + /// + /// Otherwise `Ok(())` is returned. + /// + /// # Panics + /// + /// This function will **not** panic on database errors. + fn record_per_lifetime( + &self, + lifetime: Lifetime, + storage_name: &str, + key: &str, + metric: &Metric, + ) -> Result<()> { + let final_key = Self::get_storage_key(storage_name, Some(key)); + + // Lifetime::Ping data is not immediately persisted to disk if + // Glean has `delay_ping_lifetime_io` set to true + if lifetime == Lifetime::Ping { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + let mut data = ping_lifetime_data + .write() + .expect("Can't read ping lifetime data"); + data.insert(final_key, metric.clone()); + + // flush ping lifetime + self.persist_ping_lifetime_data_if_full(&data)?; + return Ok(()); + } + } + + let encoded = bincode::serialize(&metric).expect("IMPOSSIBLE: Serializing metric failed"); + let value = rkv::Value::Blob(&encoded); + + let mut writer = self.rkv.write()?; + self.get_store(lifetime) + .put(&mut writer, final_key, &value)?; + measure_commit!(self, writer.commit())?; + Ok(()) + } + + /// Records the provided value, with the given lifetime, + /// after applying a transformation function. + pub fn record_with(&self, glean: &Glean, data: &CommonMetricDataInternal, mut transform: F) + where + F: FnMut(Option) -> Metric, + { + let name = data.identifier(glean); + for ping_name in data.storage_names() { + if glean.is_ping_enabled(ping_name) { + if let Err(e) = self.record_per_lifetime_with( + data.inner.lifetime, + ping_name, + &name, + &mut transform, + ) { + log::info!( + "Failed to record metric '{}' into {}: {:?}", + data.base_identifier(), + ping_name, + e + ); + } + } + } + } + + /// Records a metric in the underlying storage system, + /// after applying the given transformation function, for a single lifetime. + /// + /// # Returns + /// + /// If the storage is unavailable or the write fails, no data will be stored and an error will be returned. + /// + /// Otherwise `Ok(())` is returned. + /// + /// # Panics + /// + /// This function will **not** panic on database errors. + fn record_per_lifetime_with( + &self, + lifetime: Lifetime, + storage_name: &str, + key: &str, + mut transform: F, + ) -> Result<()> + where + F: FnMut(Option) -> Metric, + { + let final_key = Self::get_storage_key(storage_name, Some(key)); + + // Lifetime::Ping data is not persisted to disk if + // Glean has `delay_ping_lifetime_io` set to true + if lifetime == Lifetime::Ping { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + let mut data = ping_lifetime_data + .write() + .expect("Can't access ping lifetime data as writable"); + let entry = data.entry(final_key); + match entry { + Entry::Vacant(entry) => { + entry.insert(transform(None)); + } + Entry::Occupied(mut entry) => { + let old_value = entry.get().clone(); + entry.insert(transform(Some(old_value))); + } + } + + // flush ping lifetime + self.persist_ping_lifetime_data_if_full(&data)?; + return Ok(()); + } + } + + let mut writer = self.rkv.write()?; + let store = self.get_store(lifetime); + let new_value: Metric = { + let old_value = store.get(&writer, &final_key)?; + + match old_value { + Some(rkv::Value::Blob(blob)) => { + let old_value = bincode::deserialize(blob).ok(); + transform(old_value) + } + _ => transform(None), + } + }; + + let encoded = + bincode::serialize(&new_value).expect("IMPOSSIBLE: Serializing metric failed"); + let value = rkv::Value::Blob(&encoded); + store.put(&mut writer, final_key, &value)?; + measure_commit!(self, writer.commit())?; + Ok(()) + } + + /// Clears a storage (only Ping Lifetime). + /// + /// # Returns + /// + /// * If the storage is unavailable an error is returned. + /// * If any individual delete fails, an error is returned, but other deletions might have + /// happened. + /// + /// Otherwise `Ok(())` is returned. + /// + /// # Panics + /// + /// This function will **not** panic on database errors. + pub fn clear_ping_lifetime_storage(&self, storage_name: &str) -> Result<()> { + // Lifetime::Ping data will be saved to `ping_lifetime_data` + // in case `delay_ping_lifetime_io` is set to true + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + ping_lifetime_data + .write() + .expect("Can't access ping lifetime data as writable") + .retain(|metric_id, _| !metric_id.starts_with(storage_name)); + } + + self.write_with_store(Lifetime::Ping, |mut writer, store| { + let mut metrics = Vec::new(); + { + let mut iter = store.iter_from(&writer, storage_name)?; + while let Some(Ok((metric_id, _))) = iter.next() { + if let Ok(metric_id) = std::str::from_utf8(metric_id) { + if !metric_id.starts_with(storage_name) { + break; + } + metrics.push(metric_id.to_owned()); + } + } + } + + let mut res = Ok(()); + for to_delete in metrics { + if let Err(e) = store.delete(&mut writer, to_delete) { + log::warn!("Can't delete from store: {:?}", e); + res = Err(e); + } + } + + measure_commit!(self, writer.commit())?; + Ok(res?) + }) + } + + pub fn clear_lifetime_storage(&self, lifetime: Lifetime, storage_name: &str) -> Result<()> { + self.write_with_store(lifetime, |mut writer, store| { + let mut metrics = Vec::new(); + { + let mut iter = store.iter_from(&writer, storage_name)?; + while let Some(Ok((metric_id, _))) = iter.next() { + if let Ok(metric_id) = std::str::from_utf8(metric_id) { + if !metric_id.starts_with(storage_name) { + break; + } + metrics.push(metric_id.to_owned()); + } + } + } + + let mut res = Ok(()); + for to_delete in metrics { + if let Err(e) = store.delete(&mut writer, to_delete) { + log::warn!("Can't delete from store: {:?}", e); + res = Err(e); + } + } + + measure_commit!(self, writer.commit())?; + Ok(res?) + }) + } + + /// Removes a single metric from the storage. + /// + /// # Arguments + /// + /// * `lifetime` - the lifetime of the storage in which to look for the metric. + /// * `storage_name` - the name of the storage to store/fetch data from. + /// * `metric_id` - the metric category + name. + /// + /// # Returns + /// + /// * If the storage is unavailable an error is returned. + /// * If the metric could not be deleted, an error is returned. + /// + /// Otherwise `Ok(())` is returned. + /// + /// # Panics + /// + /// This function will **not** panic on database errors. + pub fn remove_single_metric( + &self, + lifetime: Lifetime, + storage_name: &str, + metric_id: &str, + ) -> Result<()> { + let final_key = Self::get_storage_key(storage_name, Some(metric_id)); + + // Lifetime::Ping data is not persisted to disk if + // Glean has `delay_ping_lifetime_io` set to true + if lifetime == Lifetime::Ping { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + let mut data = ping_lifetime_data + .write() + .expect("Can't access app lifetime data as writable"); + data.remove(&final_key); + } + } + + self.write_with_store(lifetime, |mut writer, store| { + if let Err(e) = store.delete(&mut writer, final_key.clone()) { + if self.ping_lifetime_data.is_some() { + // If ping_lifetime_data exists, it might be + // that data is in memory, but not yet in rkv. + return Ok(()); + } + return Err(e.into()); + } + measure_commit!(self, writer.commit())?; + Ok(()) + }) + } + + /// Clears all the metrics in the database, for the provided lifetime. + /// + /// Errors are logged. + /// + /// # Panics + /// + /// * This function will **not** panic on database errors. + pub fn clear_lifetime(&self, lifetime: Lifetime) { + let res = self.write_with_store(lifetime, |mut writer, store| { + store.clear(&mut writer)?; + measure_commit!(self, writer.commit())?; + Ok(()) + }); + + if let Err(e) = res { + // We try to clear everything. + // If there was no data to begin with we encounter a `NotFound` error. + // There's no point in logging that. + if let ErrorKind::Rkv(StoreError::IoError(ioerr)) = e.kind() { + if let io::ErrorKind::NotFound = ioerr.kind() { + log::debug!( + "Could not clear store for lifetime {:?}: {:?}", + lifetime, + ioerr + ); + return; + } + } + + log::warn!("Could not clear store for lifetime {:?}: {:?}", lifetime, e); + } + } + + /// Clears all metrics in the database. + /// + /// Errors are logged. + /// + /// # Panics + /// + /// * This function will **not** panic on database errors. + pub fn clear_all(&self) { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + ping_lifetime_data + .write() + .expect("Can't access ping lifetime data as writable") + .clear(); + } + + for lifetime in [Lifetime::User, Lifetime::Ping, Lifetime::Application].iter() { + self.clear_lifetime(*lifetime); + } + } + + /// Persists ping_lifetime_data to disk. + /// + /// Does nothing in case there is nothing to persist. + /// + /// # Panics + /// + /// * This function will **not** panic on database errors. + pub fn persist_ping_lifetime_data(&self) -> Result<()> { + if let Some(ping_lifetime_data) = &self.ping_lifetime_data { + let data = ping_lifetime_data + .read() + .expect("Can't read ping lifetime data"); + + // We can reset the write-counter. Current data has been persisted. + self.ping_lifetime_count.store(0, Ordering::Release); + self.ping_lifetime_store_ts.replace(Instant::now()); + + self.write_with_store(Lifetime::Ping, |mut writer, store| { + for (key, value) in data.iter() { + let encoded = + bincode::serialize(&value).expect("IMPOSSIBLE: Serializing metric failed"); + // There is no need for `get_storage_key` here because + // the key is already formatted from when it was saved + // to ping_lifetime_data. + store.put(&mut writer, key, &rkv::Value::Blob(&encoded))?; + } + measure_commit!(self, writer.commit())?; + Ok(()) + })?; + } + Ok(()) + } + + pub fn persist_ping_lifetime_data_if_full( + &self, + data: &BTreeMap, + ) -> Result<()> { + if self.ping_lifetime_threshold == 0 && self.ping_lifetime_max_time.is_zero() { + return Ok(()); + } + + let write_count = self.ping_lifetime_count.fetch_add(1, Ordering::Release) + 1; + let last_write = self.ping_lifetime_store_ts.get(); + let elapsed = last_write.elapsed(); + + if (self.ping_lifetime_threshold == 0 || write_count < self.ping_lifetime_threshold) + && (self.ping_lifetime_max_time.is_zero() || elapsed < self.ping_lifetime_max_time) + { + log::trace!( + "Not flushing. write_count={} (threshold={}), elapsed={:?} (max={:?})", + write_count, + self.ping_lifetime_threshold, + elapsed, + self.ping_lifetime_max_time + ); + return Ok(()); + } + + if self.ping_lifetime_threshold > 0 && write_count >= self.ping_lifetime_threshold { + log::debug!( + "Flushing database due to threshold of {} reached.", + self.ping_lifetime_threshold + ) + } else if !self.ping_lifetime_max_time.is_zero() && elapsed >= self.ping_lifetime_max_time { + log::debug!( + "Flushing database due to last write more than {:?} ago", + self.ping_lifetime_max_time + ); + } + + self.ping_lifetime_count.store(0, Ordering::Release); + self.ping_lifetime_store_ts.replace(Instant::now()); + self.write_with_store(Lifetime::Ping, |mut writer, store| { + for (key, value) in data.iter() { + let encoded = + bincode::serialize(&value).expect("IMPOSSIBLE: Serializing metric failed"); + // There is no need for `get_storage_key` here because + // the key is already formatted from when it was saved + // to ping_lifetime_data. + store.put(&mut writer, key, &rkv::Value::Blob(&encoded))?; + } + writer.commit()?; + Ok(()) + }) + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::tests::new_glean; + use std::collections::HashMap; + use tempfile::tempdir; + + #[test] + fn test_panicks_if_fails_dir_creation() { + let path = Path::new("/!#\"'@#°ç"); + assert!(Database::new(path, false, 0, Duration::ZERO).is_err()); + } + + #[test] + #[cfg(windows)] + fn windows_invalid_utf16_panicfree() { + use std::ffi::OsString; + use std::os::windows::prelude::*; + + // Here the values 0x0066 and 0x006f correspond to 'f' and 'o' + // respectively. The value 0xD800 is a lone surrogate half, invalid + // in a UTF-16 sequence. + let source = [0x0066, 0x006f, 0xD800, 0x006f]; + let os_string = OsString::from_wide(&source[..]); + let os_str = os_string.as_os_str(); + let dir = tempdir().unwrap(); + let path = dir.path().join(os_str); + + let res = Database::new(&path, false, 0, Duration::ZERO); + + assert!( + res.is_ok(), + "Database should succeed at {}: {:?}", + path.display(), + res + ); + } + + #[test] + #[cfg(target_os = "linux")] + fn linux_invalid_utf8_panicfree() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + // Here, the values 0x66 and 0x6f correspond to 'f' and 'o' + // respectively. The value 0x80 is a lone continuation byte, invalid + // in a UTF-8 sequence. + let source = [0x66, 0x6f, 0x80, 0x6f]; + let os_str = OsStr::from_bytes(&source[..]); + let dir = tempdir().unwrap(); + let path = dir.path().join(os_str); + + let res = Database::new(&path, false, 0, Duration::ZERO); + assert!( + res.is_ok(), + "Database should not fail at {}: {:?}", + path.display(), + res + ); + } + + #[test] + #[cfg(target_os = "macos")] + fn macos_invalid_utf8_panicfree() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + // Here, the values 0x66 and 0x6f correspond to 'f' and 'o' + // respectively. The value 0x80 is a lone continuation byte, invalid + // in a UTF-8 sequence. + let source = [0x66, 0x6f, 0x80, 0x6f]; + let os_str = OsStr::from_bytes(&source[..]); + let dir = tempdir().unwrap(); + let path = dir.path().join(os_str); + + let res = Database::new(&path, false, 0, Duration::ZERO); + assert!( + res.is_err(), + "Database should not fail at {}: {:?}", + path.display(), + res + ); + } + + #[test] + fn test_data_dir_rkv_inits() { + let dir = tempdir().unwrap(); + Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + assert!(dir.path().exists()); + } + + #[test] + fn test_ping_lifetime_metric_recorded() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + let db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + assert!(db.ping_lifetime_data.is_none()); + + // Attempt to record a known value. + let test_value = "test-value"; + let test_storage = "test-storage"; + let test_metric_id = "telemetry_test.test_name"; + db.record_per_lifetime( + Lifetime::Ping, + test_storage, + test_metric_id, + &Metric::String(test_value.to_string()), + ) + .unwrap(); + + // Verify that the data is correctly recorded. + let mut found_metrics = 0; + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + found_metrics += 1; + let metric_id = String::from_utf8_lossy(metric_id).into_owned(); + assert_eq!(test_metric_id, metric_id); + match metric { + Metric::String(s) => assert_eq!(test_value, s), + _ => panic!("Unexpected data found"), + } + }; + + db.iter_store_from(Lifetime::Ping, test_storage, None, &mut snapshotter); + assert_eq!(1, found_metrics, "We only expect 1 Lifetime.Ping metric."); + } + + #[test] + fn test_application_lifetime_metric_recorded() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + let db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + // Attempt to record a known value. + let test_value = "test-value"; + let test_storage = "test-storage1"; + let test_metric_id = "telemetry_test.test_name"; + db.record_per_lifetime( + Lifetime::Application, + test_storage, + test_metric_id, + &Metric::String(test_value.to_string()), + ) + .unwrap(); + + // Verify that the data is correctly recorded. + let mut found_metrics = 0; + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + found_metrics += 1; + let metric_id = String::from_utf8_lossy(metric_id).into_owned(); + assert_eq!(test_metric_id, metric_id); + match metric { + Metric::String(s) => assert_eq!(test_value, s), + _ => panic!("Unexpected data found"), + } + }; + + db.iter_store_from(Lifetime::Application, test_storage, None, &mut snapshotter); + assert_eq!( + 1, found_metrics, + "We only expect 1 Lifetime.Application metric." + ); + } + + #[test] + fn test_user_lifetime_metric_recorded() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + let db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + // Attempt to record a known value. + let test_value = "test-value"; + let test_storage = "test-storage2"; + let test_metric_id = "telemetry_test.test_name"; + db.record_per_lifetime( + Lifetime::User, + test_storage, + test_metric_id, + &Metric::String(test_value.to_string()), + ) + .unwrap(); + + // Verify that the data is correctly recorded. + let mut found_metrics = 0; + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + found_metrics += 1; + let metric_id = String::from_utf8_lossy(metric_id).into_owned(); + assert_eq!(test_metric_id, metric_id); + match metric { + Metric::String(s) => assert_eq!(test_value, s), + _ => panic!("Unexpected data found"), + } + }; + + db.iter_store_from(Lifetime::User, test_storage, None, &mut snapshotter); + assert_eq!(1, found_metrics, "We only expect 1 Lifetime.User metric."); + } + + #[test] + fn test_clear_ping_storage() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + let db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + // Attempt to record a known value for every single lifetime. + let test_storage = "test-storage"; + db.record_per_lifetime( + Lifetime::User, + test_storage, + "telemetry_test.test_name_user", + &Metric::String("test-value-user".to_string()), + ) + .unwrap(); + db.record_per_lifetime( + Lifetime::Ping, + test_storage, + "telemetry_test.test_name_ping", + &Metric::String("test-value-ping".to_string()), + ) + .unwrap(); + db.record_per_lifetime( + Lifetime::Application, + test_storage, + "telemetry_test.test_name_application", + &Metric::String("test-value-application".to_string()), + ) + .unwrap(); + + // Take a snapshot for the data, all the lifetimes. + { + let mut snapshot: HashMap = HashMap::new(); + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + let metric_id = String::from_utf8_lossy(metric_id).into_owned(); + match metric { + Metric::String(s) => snapshot.insert(metric_id, s.to_string()), + _ => panic!("Unexpected data found"), + }; + }; + + db.iter_store_from(Lifetime::User, test_storage, None, &mut snapshotter); + db.iter_store_from(Lifetime::Ping, test_storage, None, &mut snapshotter); + db.iter_store_from(Lifetime::Application, test_storage, None, &mut snapshotter); + + assert_eq!(3, snapshot.len(), "We expect all lifetimes to be present."); + assert!(snapshot.contains_key("telemetry_test.test_name_user")); + assert!(snapshot.contains_key("telemetry_test.test_name_ping")); + assert!(snapshot.contains_key("telemetry_test.test_name_application")); + } + + // Clear the Ping lifetime. + db.clear_ping_lifetime_storage(test_storage).unwrap(); + + // Take a snapshot again and check that we're only clearing the Ping lifetime. + { + let mut snapshot: HashMap = HashMap::new(); + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + let metric_id = String::from_utf8_lossy(metric_id).into_owned(); + match metric { + Metric::String(s) => snapshot.insert(metric_id, s.to_string()), + _ => panic!("Unexpected data found"), + }; + }; + + db.iter_store_from(Lifetime::User, test_storage, None, &mut snapshotter); + db.iter_store_from(Lifetime::Ping, test_storage, None, &mut snapshotter); + db.iter_store_from(Lifetime::Application, test_storage, None, &mut snapshotter); + + assert_eq!(2, snapshot.len(), "We only expect 2 metrics to be left."); + assert!(snapshot.contains_key("telemetry_test.test_name_user")); + assert!(snapshot.contains_key("telemetry_test.test_name_application")); + } + } + + #[test] + fn test_remove_single_metric() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + let db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + let test_storage = "test-storage-single-lifetime"; + let metric_id_pattern = "telemetry_test.single_metric"; + + // Write sample metrics to the database. + let lifetimes = [Lifetime::User, Lifetime::Ping, Lifetime::Application]; + + for lifetime in lifetimes.iter() { + for value in &["retain", "delete"] { + db.record_per_lifetime( + *lifetime, + test_storage, + &format!("{}_{}", metric_id_pattern, value), + &Metric::String((*value).to_string()), + ) + .unwrap(); + } + } + + // Remove "telemetry_test.single_metric_delete" from each lifetime. + for lifetime in lifetimes.iter() { + db.remove_single_metric( + *lifetime, + test_storage, + &format!("{}_delete", metric_id_pattern), + ) + .unwrap(); + } + + // Verify that "telemetry_test.single_metric_retain" is still around for all lifetimes. + for lifetime in lifetimes.iter() { + let mut found_metrics = 0; + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + found_metrics += 1; + let metric_id = String::from_utf8_lossy(metric_id).into_owned(); + assert_eq!(format!("{}_retain", metric_id_pattern), metric_id); + match metric { + Metric::String(s) => assert_eq!("retain", s), + _ => panic!("Unexpected data found"), + } + }; + + // Check the User lifetime. + db.iter_store_from(*lifetime, test_storage, None, &mut snapshotter); + assert_eq!( + 1, found_metrics, + "We only expect 1 metric for this lifetime." + ); + } + } + + #[test] + fn test_delayed_ping_lifetime_persistence() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + let db = Database::new(dir.path(), true, 0, Duration::ZERO).unwrap(); + let test_storage = "test-storage"; + + assert!(db.ping_lifetime_data.is_some()); + + // Attempt to record a known value. + let test_value1 = "test-value1"; + let test_metric_id1 = "telemetry_test.test_name1"; + db.record_per_lifetime( + Lifetime::Ping, + test_storage, + test_metric_id1, + &Metric::String(test_value1.to_string()), + ) + .unwrap(); + + // Attempt to persist data. + db.persist_ping_lifetime_data().unwrap(); + + // Attempt to record another known value. + let test_value2 = "test-value2"; + let test_metric_id2 = "telemetry_test.test_name2"; + db.record_per_lifetime( + Lifetime::Ping, + test_storage, + test_metric_id2, + &Metric::String(test_value2.to_string()), + ) + .unwrap(); + + { + // At this stage we expect `test_value1` to be persisted and in memory, + // since it was recorded before calling `persist_ping_lifetime_data`, + // and `test_value2` to be only in memory, since it was recorded after. + let store: SingleStore = db + .rkv + .open_single(Lifetime::Ping.as_str(), StoreOptions::create()) + .unwrap(); + let reader = db.rkv.read().unwrap(); + + // Verify that test_value1 is in rkv. + assert!(store + .get(&reader, format!("{}#{}", test_storage, test_metric_id1)) + .unwrap_or(None) + .is_some()); + // Verifiy that test_value2 is **not** in rkv. + assert!(store + .get(&reader, format!("{}#{}", test_storage, test_metric_id2)) + .unwrap_or(None) + .is_none()); + + let data = match &db.ping_lifetime_data { + Some(ping_lifetime_data) => ping_lifetime_data, + None => panic!("Expected `ping_lifetime_data` to exist here!"), + }; + let data = data.read().unwrap(); + // Verify that test_value1 is also in memory. + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id1)) + .is_some()); + // Verify that test_value2 is in memory. + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id2)) + .is_some()); + } + + // Attempt to persist data again. + db.persist_ping_lifetime_data().unwrap(); + + { + // At this stage we expect `test_value1` and `test_value2` to + // be persisted, since both were created before a call to + // `persist_ping_lifetime_data`. + let store: SingleStore = db + .rkv + .open_single(Lifetime::Ping.as_str(), StoreOptions::create()) + .unwrap(); + let reader = db.rkv.read().unwrap(); + + // Verify that test_value1 is in rkv. + assert!(store + .get(&reader, format!("{}#{}", test_storage, test_metric_id1)) + .unwrap_or(None) + .is_some()); + // Verifiy that test_value2 is also in rkv. + assert!(store + .get(&reader, format!("{}#{}", test_storage, test_metric_id2)) + .unwrap_or(None) + .is_some()); + + let data = match &db.ping_lifetime_data { + Some(ping_lifetime_data) => ping_lifetime_data, + None => panic!("Expected `ping_lifetime_data` to exist here!"), + }; + let data = data.read().unwrap(); + // Verify that test_value1 is also in memory. + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id1)) + .is_some()); + // Verify that test_value2 is also in memory. + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id2)) + .is_some()); + } + } + + #[test] + fn test_load_ping_lifetime_data_from_memory() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + + let test_storage = "test-storage"; + let test_value = "test-value"; + let test_metric_id = "telemetry_test.test_name"; + + { + let db = Database::new(dir.path(), true, 0, Duration::ZERO).unwrap(); + + // Attempt to record a known value. + db.record_per_lifetime( + Lifetime::Ping, + test_storage, + test_metric_id, + &Metric::String(test_value.to_string()), + ) + .unwrap(); + + // Verify that test_value is in memory. + let data = match &db.ping_lifetime_data { + Some(ping_lifetime_data) => ping_lifetime_data, + None => panic!("Expected `ping_lifetime_data` to exist here!"), + }; + let data = data.read().unwrap(); + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id)) + .is_some()); + + // Attempt to persist data. + db.persist_ping_lifetime_data().unwrap(); + + // Verify that test_value is now in rkv. + let store: SingleStore = db + .rkv + .open_single(Lifetime::Ping.as_str(), StoreOptions::create()) + .unwrap(); + let reader = db.rkv.read().unwrap(); + assert!(store + .get(&reader, format!("{}#{}", test_storage, test_metric_id)) + .unwrap_or(None) + .is_some()); + } + + // Now create a new instace of the db and check if data was + // correctly loaded from rkv to memory. + { + let db = Database::new(dir.path(), true, 0, Duration::ZERO).unwrap(); + + // Verify that test_value is in memory. + let data = match &db.ping_lifetime_data { + Some(ping_lifetime_data) => ping_lifetime_data, + None => panic!("Expected `ping_lifetime_data` to exist here!"), + }; + let data = data.read().unwrap(); + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id)) + .is_some()); + + // Verify that test_value is also in rkv. + let store: SingleStore = db + .rkv + .open_single(Lifetime::Ping.as_str(), StoreOptions::create()) + .unwrap(); + let reader = db.rkv.read().unwrap(); + assert!(store + .get(&reader, format!("{}#{}", test_storage, test_metric_id)) + .unwrap_or(None) + .is_some()); + } + } + + #[test] + fn test_delayed_ping_lifetime_clear() { + // Init the database in a temporary directory. + let dir = tempdir().unwrap(); + let db = Database::new(dir.path(), true, 0, Duration::ZERO).unwrap(); + let test_storage = "test-storage"; + + assert!(db.ping_lifetime_data.is_some()); + + // Attempt to record a known value. + let test_value1 = "test-value1"; + let test_metric_id1 = "telemetry_test.test_name1"; + db.record_per_lifetime( + Lifetime::Ping, + test_storage, + test_metric_id1, + &Metric::String(test_value1.to_string()), + ) + .unwrap(); + + { + let data = match &db.ping_lifetime_data { + Some(ping_lifetime_data) => ping_lifetime_data, + None => panic!("Expected `ping_lifetime_data` to exist here!"), + }; + let data = data.read().unwrap(); + // Verify that test_value1 is in memory. + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id1)) + .is_some()); + } + + // Clear ping lifetime storage for a storage that isn't test_storage. + // Doesn't matter what it's called, just that it isn't test_storage. + db.clear_ping_lifetime_storage(&(test_storage.to_owned() + "x")) + .unwrap(); + + { + let data = match &db.ping_lifetime_data { + Some(ping_lifetime_data) => ping_lifetime_data, + None => panic!("Expected `ping_lifetime_data` to exist here!"), + }; + let data = data.read().unwrap(); + // Verify that test_value1 is still in memory. + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id1)) + .is_some()); + } + + // Clear test_storage's ping lifetime storage. + db.clear_ping_lifetime_storage(test_storage).unwrap(); + + { + let data = match &db.ping_lifetime_data { + Some(ping_lifetime_data) => ping_lifetime_data, + None => panic!("Expected `ping_lifetime_data` to exist here!"), + }; + let data = data.read().unwrap(); + // Verify that test_value1 is no longer in memory. + assert!(data + .get(&format!("{}#{}", test_storage, test_metric_id1)) + .is_none()); + } + } + + #[test] + fn doesnt_record_when_upload_is_disabled() { + let (mut glean, dir) = new_glean(None); + + // Init the database in a temporary directory. + + let test_storage = "test-storage"; + let test_data = CommonMetricDataInternal::new("category", "name", test_storage); + let test_metric_id = test_data.identifier(&glean); + + // Attempt to record metric with the record and record_with functions, + // this should work since upload is enabled. + let db = Database::new(dir.path(), true, 0, Duration::ZERO).unwrap(); + db.record(&glean, &test_data, &Metric::String("record".to_owned())); + db.iter_store_from( + Lifetime::Ping, + test_storage, + None, + &mut |metric_id: &[u8], metric: &Metric| { + assert_eq!( + String::from_utf8_lossy(metric_id).into_owned(), + test_metric_id + ); + match metric { + Metric::String(v) => assert_eq!("record", *v), + _ => panic!("Unexpected data found"), + } + }, + ); + + db.record_with(&glean, &test_data, |_| { + Metric::String("record_with".to_owned()) + }); + db.iter_store_from( + Lifetime::Ping, + test_storage, + None, + &mut |metric_id: &[u8], metric: &Metric| { + assert_eq!( + String::from_utf8_lossy(metric_id).into_owned(), + test_metric_id + ); + match metric { + Metric::String(v) => assert_eq!("record_with", *v), + _ => panic!("Unexpected data found"), + } + }, + ); + + // Disable upload + glean.set_upload_enabled(false); + + // Attempt to record metric with the record and record_with functions, + // this should work since upload is now **disabled**. + db.record(&glean, &test_data, &Metric::String("record_nop".to_owned())); + db.iter_store_from( + Lifetime::Ping, + test_storage, + None, + &mut |metric_id: &[u8], metric: &Metric| { + assert_eq!( + String::from_utf8_lossy(metric_id).into_owned(), + test_metric_id + ); + match metric { + Metric::String(v) => assert_eq!("record_with", *v), + _ => panic!("Unexpected data found"), + } + }, + ); + db.record_with(&glean, &test_data, |_| { + Metric::String("record_with_nop".to_owned()) + }); + db.iter_store_from( + Lifetime::Ping, + test_storage, + None, + &mut |metric_id: &[u8], metric: &Metric| { + assert_eq!( + String::from_utf8_lossy(metric_id).into_owned(), + test_metric_id + ); + match metric { + Metric::String(v) => assert_eq!("record_with", *v), + _ => panic!("Unexpected data found"), + } + }, + ); + } + + mod safe_mode { + use std::fs::File; + + use super::*; + + #[test] + fn empty_data_file() { + let dir = tempdir().unwrap(); + + // Create database directory structure. + let database_dir = dir.path().join("db"); + fs::create_dir_all(&database_dir).expect("create database dir"); + + // Create empty database file. + let safebin = database_dir.join("data.safe.bin"); + let f = File::create(safebin).expect("create database file"); + drop(f); + + let db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + assert!(dir.path().exists()); + assert!( + matches!(db.rkv_load_state, RkvLoadState::Err(_)), + "Load error recorded" + ); + } + + #[test] + fn corrupted_data_file() { + let dir = tempdir().unwrap(); + + // Create database directory structure. + let database_dir = dir.path().join("db"); + fs::create_dir_all(&database_dir).expect("create database dir"); + + // Create empty database file. + let safebin = database_dir.join("data.safe.bin"); + fs::write(safebin, "").expect("write to database file"); + + let db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + assert!(dir.path().exists()); + assert!( + matches!(db.rkv_load_state, RkvLoadState::Err(_)), + "Load error recorded" + ); + } + } +} diff --git a/glean-core/src/database/sqlite.rs b/glean-core/src/database/sqlite.rs deleted file mode 100644 index 797f2e6114..0000000000 --- a/glean-core/src/database/sqlite.rs +++ /dev/null @@ -1,672 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -use std::fs; -use std::num::NonZeroU64; -use std::path::Path; -use std::str; -use std::time::Duration; - -use malloc_size_of::MallocSizeOf; -use rusqlite::params; -use rusqlite::types::FromSqlError; -use rusqlite::OptionalExtension; -use rusqlite::Transaction; -use rusqlite::{Error as SqlError, ErrorCode}; - -use connection::Connection; -use schema::Schema; -pub use schema::SchemaError; - -use crate::common_metric_data::CommonMetricDataInternal; -use crate::database::migration::{self, MigrationState}; -use crate::metrics::dual_labeled_counter::RECORD_SEPARATOR; -use crate::metrics::Metric; -use crate::Error; -use crate::Glean; -use crate::Lifetime; -use crate::Result; - -mod connection; -mod schema; - -#[derive(Debug)] -pub enum LoadState { - Ok, - Err(Error), -} - -#[derive(Debug, PartialEq, Eq, Copy, Clone)] -pub enum MigrationResult { - /// Migration did not happen yet - Unknown, - /// Migration failed - Error, -} - -#[derive(Debug)] -pub struct Database { - /// The database connection. - pub(crate) conn: connection::Connection, - - /// Initial file size when opening the database. - pub(crate) file_size: Option, - - /// Load state - load_state: LoadState, - - /// Migration state, counts migrated metrics and the time it took. - pub(crate) migration_state: Option, - - /// Set when a database migration attempt failed. - pub(crate) migration_error: MigrationResult, -} - -impl MallocSizeOf for Database { - fn size_of(&self, _ops: &mut malloc_size_of::MallocSizeOfOps) -> usize { - // FIXME: Can we get the allocated size of the connection? - 0 - } -} - -const DEFAULT_DATABASE_FILE_NAME: &str = "glean.sqlite"; - -/// Calculate the database size from all the files in the directory. -/// -/// # Arguments -/// -/// *`path` - The path to the directory -/// -/// # Returns -/// -/// Returns the non-zero combined size in bytes of all files in a directory, -/// or `None` on error or if the size is `0`. -fn database_size(dir: &Path) -> Option { - let mut total_size = 0; - if let Ok(entries) = fs::read_dir(dir) { - for entry in entries.flatten() { - if let Ok(file_type) = entry.file_type() { - if file_type.is_file() { - let path = entry.path(); - if let Ok(metadata) = fs::metadata(path) { - total_size += metadata.len(); - } else { - continue; - } - } - } - } - } - - NonZeroU64::new(total_size) -} - -pub fn sqlite_open(path: &Path) -> std::result::Result<(Connection, LoadState), Error> { - // TODO(bug 2049292): Make this more robust, use the correct errors and see how we can test all the branches - // properly. - match Connection::new::(path) { - Err(e @ SchemaError::UnsupportedSchemaVersion(_)) => Err(e.into()), - Err(e @ SchemaError::Sqlite(SqlError::SqliteFailure(err, _))) => { - match err.code { - ErrorCode::PermissionDenied => Err(e.into()), - ErrorCode::NotADatabase => { - log::debug!("sqlite failed: not a database. starting from scratch."); - fs::remove_file(path).map_err(|_| rkv::StoreError::FileInvalid)?; - // Now try again, we only handle that error once. - let conn = Connection::new::(path)?; - Ok((conn, LoadState::Err(e.into()))) - } - ErrorCode::CannotOpen => { - log::debug!("sqlite failed: cannot open. starting from scratch."); - fs::remove_file(path).map_err(|_| rkv::StoreError::FileInvalid)?; - // Now try again, we only handle that error once. - let conn = Connection::new::(path)?; - Ok((conn, LoadState::Err(e.into()))) - } - _ => Err(e.into()), - } - } - Err(err @ SchemaError::Sqlite(SqlError::SqlInputError { .. })) => { - log::debug!("sqlite failed: schema migration failed. starting from scratch."); - fs::remove_file(path).map_err(|_| rkv::StoreError::FileInvalid)?; - // Now try again, we only handle that error once. - let conn = Connection::new::(path)?; - Ok((conn, LoadState::Err(err.into()))) - } - other => { - let conn = other?; - Ok((conn, LoadState::Ok)) - } - } -} - -impl Database { - /// Initializes the data store. - /// - /// This opens the underlying SQLite store and creates - /// the underlying directory structure. - pub fn new( - data_path: &Path, - _delay_ping_lifetime_io: bool, - _ping_lifetime_threshold: usize, - _ping_lifetime_max_time: Duration, - ) -> Result { - let path = data_path.join("db"); - log::debug!("Database path: {:?}", path.display()); - let file_size = database_size(&path); - - fs::create_dir_all(&path)?; - let store_path = path.join(DEFAULT_DATABASE_FILE_NAME); - let sqlite_exists = store_path.exists(); - let (conn, load_state) = sqlite_open(&store_path)?; - - let mut db = Self { - conn, - file_size, - load_state, - migration_state: None, - migration_error: MigrationResult::Unknown, - }; - - if sqlite_exists { - log::debug!("SQLite database already exists. Not trying to migrate Rkv"); - } else { - match migration::try_migrate(&path, &db) { - Ok(Some(state)) => { - log::debug!("Migration done. state={state:?}"); - db.migration_state = Some(state); - } - Ok(None) => { - log::debug!("No migration."); - } - Err(e) => { - db.migration_error = MigrationResult::Error; - log::warn!("Migration failed! Continuing with SQLite backend without migrated data. Error: {e:?}") - } - } - } - - Ok(db) - } - - /// Get the initial database file size. - pub fn file_size(&self) -> Option { - self.file_size - } - - /// Get the load state. - pub fn load_state(&self) -> Option { - if let LoadState::Err(e) = &self.load_state { - Some(e.to_string()) - } else { - None - } - } - - /// Iterates with the provided transaction function - /// over the requested data from the given storage. - /// - /// * If the storage is unavailable, the transaction function is never invoked. - /// * If the read data cannot be deserialized it will be silently skipped. - /// - /// # Arguments - /// - /// * `lifetime` - The metric lifetime to iterate over. - /// * `storage_name` - The storage name to iterate over. - /// * `transaction_fn` - Called for each entry being iterated over. It is - /// passed two arguments: `(metric_id: &[u8], metric: &Metric)`. - /// - /// # Panics - /// - /// This function will **not** panic on database errors. - pub fn iter_store( - &self, - lifetime: Lifetime, - storage_name: &str, - mut transaction_fn: F, - ) -> Result<()> - where - F: FnMut(&[u8], &[&str], &Metric), - { - let iter_sql = r#" - SELECT - id, - value, - labels - FROM telemetry - WHERE - lifetime = ?1 - AND ping = ?2 - "#; - - self.conn.read(|conn| { - let mut stmt = conn.prepare_cached(iter_sql)?; - let rows = stmt.query_map( - params![lifetime.as_str().to_string(), storage_name], - |row| { - let id: String = row.get(0)?; - let blob: Vec = row.get(1)?; - let labels: String = row.get(2)?; - let blob: Metric = - rmp_serde::from_slice(&blob).map_err(|_| FromSqlError::InvalidType)?; - Ok((id, labels, blob)) - }, - )?; - - for row in rows { - let Ok((metric_id, labels, metric)) = row else { - continue; - }; - let labels = labels.split(RECORD_SEPARATOR).collect::>(); - transaction_fn(metric_id.as_bytes(), &labels, &metric); - } - - Ok(()) - }) - } - - /// Get a single metric by name from storage - pub fn get_metric( - &self, - data: &CommonMetricDataInternal, - storage_name: &str, - ) -> Option { - // TODO(bug 2048194): Remove the `LIMIT 1` and error out when more than 1 row is returned. - let get_metric_sql = r#" - SELECT - value - FROM telemetry - WHERE - id = ?1 - AND ping = ?2 - AND labels = ?3 - LIMIT 1 - "#; - - let metric_identifier = &data.base_identifier(); - - self.conn - .read(|tx| { - let labels = data.check_labels(tx); - - let mut stmt = tx.prepare_cached(get_metric_sql)?; - stmt.query_one([metric_identifier, storage_name, labels.label()], |row| { - let blob: Vec = row.get(0)?; - let blob: Metric = - rmp_serde::from_slice(&blob).map_err(|_| FromSqlError::InvalidType)?; - Ok(blob) - }) - .optional() - }) - .unwrap_or(None) // TODO(bug 2047617): Should we handle the error here properly? - } - - /// Determines if the storage has the given metric. - /// - /// If data cannot be read it is assumed that the storage does not have the metric. - /// - /// # Arguments - /// - /// * `lifetime` - The lifetime of the metric. - /// * `storage_name` - The storage name to look in. - /// * `metric_identifier` - The metric identifier. - /// - /// # Panics - /// - /// This function will **not** panic on database errors. - pub fn has_metric( - &self, - lifetime: Lifetime, - storage_name: &str, - metric_identifier: &str, - ) -> bool { - let has_metric_sql = r#" - SELECT id - FROM telemetry - WHERE - lifetime = ?1 - AND ping = ?2 - AND id = ?3 - "#; - - self.conn - .read(|conn| { - let Ok(mut stmt) = conn.prepare_cached(has_metric_sql) else { - return Ok(false); - }; - let Ok(mut metric_iter) = - stmt.query([lifetime.as_str(), storage_name, metric_identifier]) - else { - return Ok(false); - }; - - Result::::Ok(metric_iter.next().map(|m| m.is_some()).unwrap_or(false)) - }) - .unwrap_or(false) - } - - /// Records a metric in the underlying storage system. - pub fn record(&self, glean: &Glean, data: &CommonMetricDataInternal, value: &Metric) { - let name = data.base_identifier(); - - _ = self.conn.write(|tx| { - let labels = data.check_labels(tx); - labels.record_error(glean, tx, &name, data.storage_names()); - - for ping_name in data.storage_names() { - if glean.is_ping_enabled(ping_name) { - if let Err(e) = self.record_per_lifetime( - tx, - data.inner.lifetime, - ping_name, - &name, - labels.label(), - value, - ) { - log::error!( - "Failed to record metric '{}' into {}: {:?}", - data.base_identifier(), - ping_name, - e - ); - } - } - } - - Ok::<(), rusqlite::Error>(()) - }); - } - - /// Records a metric in the underlying storage system, for a single lifetime. - /// - /// # Returns - /// - /// If the storage is unavailable or the write fails, no data will be stored and an error will be returned. - /// - /// Otherwise `Ok(())` is returned. - /// - /// # Panics - /// - /// This function will **not** panic on database errors. - pub(crate) fn record_per_lifetime( - &self, - tx: &mut Transaction, - lifetime: Lifetime, - storage_name: &str, - key: &str, - labels: &str, - metric: &Metric, - ) -> Result<()> { - let insert_sql = r#" - INSERT INTO - telemetry (id, ping, lifetime, labels, value) - VALUES - (?1, ?2, ?3, ?4, ?5) - ON CONFLICT(id, ping, labels) DO UPDATE SET - lifetime = excluded.lifetime, - value = excluded.value - "#; - - let mut stmt = tx.prepare_cached(insert_sql)?; - let encoded = rmp_serde::to_vec(&metric).expect("IMPOSSIBLE: Serializing metric failed"); - stmt.execute(params![ - key, - storage_name, - lifetime.as_str(), - labels, - encoded - ])?; - - Ok(()) - } - - /// Records the provided value, with the given lifetime, - /// after applying a transformation function. - pub fn record_with(&self, glean: &Glean, data: &CommonMetricDataInternal, transform: F) - where - F: FnMut(Option) -> Metric, - { - _ = self - .conn - .write(|tx| self.record_with_transaction(glean, tx, data, transform)); - } - - pub fn record_with_transaction( - &self, - glean: &Glean, - tx: &mut Transaction, - data: &CommonMetricDataInternal, - mut transform: F, - ) -> Result<()> - where - F: FnMut(Option) -> Metric, - { - let name = data.base_identifier(); - - let labels = data.check_labels(tx); - labels.record_error(glean, tx, &name, data.storage_names()); - - for ping_name in data.storage_names() { - if glean.is_ping_enabled(ping_name) { - if let Err(e) = self.record_per_lifetime_with( - tx, - data.inner.lifetime, - ping_name, - &name, - labels.label(), - &mut transform, - ) { - log::error!( - "Failed to record metric '{}' into {}: {:?}", - data.base_identifier(), - ping_name, - e - ); - } - } - } - - Ok(()) - } - - /// Records a metric in the underlying storage system, - /// after applying the given transformation function, for a single lifetime. - /// - /// # Returns - /// - /// If the storage is unavailable or the write fails, no data will be stored and an error will be returned. - /// - /// Otherwise `Ok(())` is returned. - /// - /// # Panics - /// - /// This function will **not** panic on database errors. - fn record_per_lifetime_with( - &self, - tx: &mut Transaction, - lifetime: Lifetime, - storage_name: &str, - key: &str, - labels: &str, - mut transform: F, - ) -> Result<()> - where - F: FnMut(Option) -> Metric, - { - // TODO(bug 2048194): Remove the `LIMIT 1` and error out when more than 1 row is returned. - let value_sql = r#" - SELECT value - FROM telemetry - WHERE - id = ?1 - AND ping = ?2 - AND lifetime = ?3 - AND labels = ?4 - LIMIT 1 - "#; - - let new_value = { - let mut stmt = tx.prepare_cached(value_sql)?; - let mut rows = stmt.query(params![ - key, - storage_name, - lifetime.as_str().to_string(), - labels - ])?; - - if let Ok(Some(row)) = rows.next() { - let blob: Vec = row.get(0)?; - let old_value = rmp_serde::from_slice(&blob).ok(); - transform(old_value) - } else { - transform(None) - } - }; - - let insert_sql = r#" - INSERT INTO - telemetry (id, ping, lifetime, labels, value) - VALUES - (?1, ?2, ?3, ?4, ?5) - ON CONFLICT(id, ping, labels) DO UPDATE SET - lifetime = excluded.lifetime, - value = excluded.value - "#; - - { - let mut stmt = tx.prepare_cached(insert_sql)?; - let encoded = - rmp_serde::to_vec(&new_value).expect("IMPOSSIBLE: Serializing metric failed"); - stmt.execute(params![ - key, - storage_name, - lifetime.as_str(), - labels, - encoded - ])?; - } - - Ok(()) - } - - /// Clears a storage (only Ping Lifetime). - /// - /// # Returns - /// - /// * If the storage is unavailable an error is returned. - /// * If any individual delete fails, an error is returned, but other deletions might have - /// happened. - /// - /// Otherwise `Ok(())` is returned. - /// - /// # Panics - /// - /// This function will **not** panic on database errors. - pub fn clear_ping_lifetime_storage(&self, storage_name: &str) -> Result<()> { - let clear_sql = "DELETE FROM telemetry WHERE lifetime = 'ping' AND ping = ?1"; - self.conn.write(|tx| { - let mut stmt = tx.prepare_cached(clear_sql)?; - stmt.execute([storage_name])?; - Ok(()) - }) - } - - pub fn clear_lifetime_storage(&self, lifetime: Lifetime, storage_name: &str) -> Result<()> { - let clear_sql = "DELETE FROM telemetry WHERE lifetime = ?1 AND ping = ?2"; - self.conn.write(|tx| { - let mut stmt = tx.prepare_cached(clear_sql)?; - stmt.execute([lifetime.as_str(), storage_name])?; - Ok(()) - }) - } - - /// Removes a single metric from the storage. - /// - /// # Arguments - /// - /// * `lifetime` - the lifetime of the storage in which to look for the metric. - /// * `storage_name` - the name of the storage to store/fetch data from. - /// * `metric_id` - the metric category + name. - /// - /// # Returns - /// - /// * If the storage is unavailable an error is returned. - /// * If the metric could not be deleted, an error is returned. - /// - /// Otherwise `Ok(())` is returned. - /// - /// # Panics - /// - /// This function will **not** panic on database errors. - pub fn remove_single_metric( - &self, - lifetime: Lifetime, - storage_name: &str, - metric_id: &str, - ) -> Result<()> { - let clear_sql = "DELETE FROM telemetry WHERE lifetime = ?1 AND ping = ?2 AND id = ?3"; - self.conn.write(|tx| { - let mut stmt = tx.prepare_cached(clear_sql)?; - stmt.execute([lifetime.as_str(), storage_name, metric_id])?; - Ok(()) - }) - } - - /// Clears all the metrics in the database, for the provided lifetime. - /// - /// Errors are logged. - /// - /// # Panics - /// - /// * This function will **not** panic on database errors. - pub fn clear_lifetime(&self, lifetime: Lifetime) { - let clear_sql = "DELETE FROM telemetry WHERE lifetime = ?1"; - _ = self.conn.write(|tx| { - let mut stmt = tx.prepare_cached(clear_sql)?; - let res = stmt.execute([lifetime.as_str()]); - - if let Err(e) = res { - log::warn!("Could not clear store for lifetime {:?}: {:?}", lifetime, e); - } - Ok::<(), rusqlite::Error>(()) - }); - } - - /// Clears all metrics in the database. - /// - /// Errors are logged. - /// - /// # Panics - /// - /// * This function will **not** panic on database errors. - pub fn clear_all(&self) { - let lifetimes = &[ - Lifetime::User.as_str(), - Lifetime::Ping.as_str(), - Lifetime::Application.as_str(), - ]; - let clear_sql = - "DELETE FROM telemetry WHERE lifetime = ?1 OR lifetime = ?2 OR lifetime = ?3"; - _ = self.conn.write(|tx| { - let mut stmt = tx.prepare_cached(clear_sql)?; - let res = stmt.execute(lifetimes); - - if let Err(e) = res { - log::warn!("Could not clear store for all lifetimes: {:?}", e); - } - Ok::<(), rusqlite::Error>(()) - }); - } - - /// Persists ping_lifetime_data to disk. - /// - /// Does nothing in case there is nothing to persist. - /// - /// # Panics - /// - /// * This function will **not** panic on database errors. - pub fn persist_ping_lifetime_data(&self) -> Result<()> { - Ok(()) - } -} diff --git a/glean-core/src/database/sqlite/connection.rs b/glean-core/src/database/sqlite/connection.rs deleted file mode 100644 index 2e8aa54869..0000000000 --- a/glean-core/src/database/sqlite/connection.rs +++ /dev/null @@ -1,111 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -//! Lower-level, generic SQLite connection management. -//! -//! This module is inspired by, and borrows concepts from, the -//! Application Services `sql-support` crate. - -use std::{fmt::Debug, num::NonZeroU32, path::Path, sync::Mutex}; - -use rusqlite::{OpenFlags, Transaction, TransactionBehavior}; - -/// Sets up an SQLite database connection, and either -/// initializes an empty physical database with the latest schema, or -/// upgrades an existing physical database to the latest schema. -pub trait ConnectionOpener { - /// The highest schema version that we support. - const MAX_SCHEMA_VERSION: u32; - - type Error: From; - - /// Sets up an opened connection for use. This is a good place to - /// set pragmas and configuration options, register functions, and - /// load extensions. - fn setup(_conn: &mut rusqlite::Connection) -> Result<(), Self::Error> { - Ok(()) - } - - /// Initializes an empty physical database with the latest schema. - fn create(tx: &mut Transaction<'_>) -> Result<(), Self::Error>; - - /// Upgrades an existing physical database to the schema with - /// the given version. - fn upgrade(tx: &mut Transaction<'_>, to_version: NonZeroU32) -> Result<(), Self::Error>; -} - -/// A thread-safe wrapper around a connection to a physical SQLite database. -pub struct Connection { - /// The inner connection. - conn: Mutex, -} - -impl Connection { - /// Opens a connection to a physical database at the given path. - pub fn new(path: &Path) -> Result - where - O: ConnectionOpener, - { - let flags = OpenFlags::SQLITE_OPEN_NO_MUTEX // Send/Sync is guaranteed by Rust already - | OpenFlags::SQLITE_OPEN_EXRESCODE // Extended result codes - | OpenFlags::SQLITE_OPEN_CREATE // Create if it doesn't exist - | OpenFlags::SQLITE_OPEN_READ_WRITE; // opened for reading and writing - - let mut conn = rusqlite::Connection::open_with_flags(path, flags)?; - O::setup(&mut conn)?; - - // On open upgrade the schema to the latest version. - let mut tx = conn.transaction_with_behavior(TransactionBehavior::Exclusive)?; - match tx.query_row_and_then("PRAGMA user_version", [], |row| row.get(0)) { - Ok(mut version @ 1..) => { - while version < O::MAX_SCHEMA_VERSION { - O::upgrade(&mut tx, NonZeroU32::new(version + 1).unwrap())?; - version += 1; - } - } - Ok(0) => O::create(&mut tx)?, - Err(err) => Err(err)?, - } - // Set the schema version to the highest that we support. - // If the current version is higher than ours, downgrade it, - // so that upgrading to it again in the future can fix up any - // invariants that our version might not uphold. - tx.execute_batch(&format!("PRAGMA user_version = {}", O::MAX_SCHEMA_VERSION))?; - tx.commit()?; - Ok(Self::with_connection(conn)) - } - - fn with_connection(conn: rusqlite::Connection) -> Self { - Self { - conn: Mutex::new(conn), - } - } - - /// Accesses the database for reading. - pub fn read(&self, f: impl FnOnce(&Transaction<'_>) -> Result) -> Result { - let mut conn = self.conn.lock().unwrap(); - let tx = conn - .transaction_with_behavior(TransactionBehavior::Immediate) - .unwrap(); - f(&tx) - } - - /// Accesses the database in a transaction for reading and writing. - pub fn write(&self, f: impl FnOnce(&mut Transaction<'_>) -> Result) -> Result - where - E: From, - { - let mut conn = self.conn.lock().unwrap(); - let mut tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; - let result = f(&mut tx)?; - tx.commit()?; - Ok(result) - } -} - -impl Debug for Connection { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("Connection { .. }") - } -} diff --git a/glean-core/src/database/sqlite/schema.rs b/glean-core/src/database/sqlite/schema.rs deleted file mode 100644 index 42e0ea8407..0000000000 --- a/glean-core/src/database/sqlite/schema.rs +++ /dev/null @@ -1,76 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at http://mozilla.org/MPL/2.0/. - -//! The SQLite database schema. - -use std::num::NonZeroU32; - -use rusqlite::{config::DbConfig, Transaction}; - -use super::connection::ConnectionOpener; - -/// The schema for a physical SQLite database that contains many -/// named logical databases. -#[derive(Debug)] -pub struct Schema; - -impl ConnectionOpener for Schema { - const MAX_SCHEMA_VERSION: u32 = 1; - - type Error = SchemaError; - - fn setup(conn: &mut rusqlite::Connection) -> Result<(), Self::Error> { - conn.execute_batch( - " - -- we unconditionally want write-ahead-logging mode - PRAGMA journal_mode = WAL; - -- Sync at the most criticial moments, but not with every write - PRAGMA synchronous = NORMAL; - -- limit size of the journal. TODO(bug 2049290): value currently arbitrary. - -- needs refinement. - PRAGMA journal_size_limit = 512000; -- 512 KB. - -- We don't care about temp tables being persisted to disk - PRAGMA temp_store = MEMORY; - -- allows adding incremental cleanup later - PRAGMA auto_vacuum = INCREMENTAL; - ", - )?; - - // Set hardening flags. - conn.set_db_config(DbConfig::SQLITE_DBCONFIG_DEFENSIVE, true)?; - - // Turn off misfeatures: double-quoted strings and untrusted schemas. - conn.set_db_config(DbConfig::SQLITE_DBCONFIG_DQS_DML, false)?; - conn.set_db_config(DbConfig::SQLITE_DBCONFIG_DQS_DDL, false)?; - conn.set_db_config(DbConfig::SQLITE_DBCONFIG_TRUSTED_SCHEMA, true)?; - - Ok(()) - } - - fn create(tx: &mut Transaction<'_>) -> Result<(), Self::Error> { - tx.execute_batch( - "CREATE TABLE telemetry( - id TEXT NOT NULL, - ping TEXT NOT NULL, - lifetime TEXT NOT NULL, - labels TEXT NOT NULL, -- can't be null or ON CONFLICT won't work - value BLOB, - UNIQUE(id, ping, labels) - );", - )?; - Ok(()) - } - - fn upgrade(_: &mut Transaction<'_>, to_version: NonZeroU32) -> Result<(), Self::Error> { - Err(SchemaError::UnsupportedSchemaVersion(to_version.get())) - } -} - -#[derive(thiserror::Error, Debug)] -pub enum SchemaError { - #[error("unsupported schema version: {0}")] - UnsupportedSchemaVersion(u32), - #[error("sqlite: {0}")] - Sqlite(#[from] rusqlite::Error), -} diff --git a/glean-core/src/error.rs b/glean-core/src/error.rs index 58823a93d9..1694cae04e 100644 --- a/glean-core/src/error.rs +++ b/glean-core/src/error.rs @@ -9,8 +9,6 @@ use std::result; use rkv::StoreError; -use crate::database::sqlite::SchemaError; - /// A specialized [`Result`] type for this crate's operations. /// /// This is generally used to avoid writing out [`Error`] directly and @@ -67,12 +65,6 @@ pub enum ErrorKind { /// Parsing a UUID from a string failed UuidError(uuid::Error), - - /// Database/SQLite error - SQLite(rusqlite::Error), - - /// Schema error - Schema(SchemaError), } /// A specialized [`Error`] type for this crate's operations. @@ -129,8 +121,6 @@ impl Display for Error { s / 1024 ), UuidError(e) => write!(f, "Failed to parse UUID: {}", e), - SQLite(e) => write!(f, "SQLite error: {}", e), - Schema(e) => write!(f, "Schema error: {}", e), } } } @@ -165,27 +155,6 @@ impl From for Error { } } -impl From for Error { - fn from(error: rusqlite::Error) -> Error { - Error { - kind: ErrorKind::SQLite(error), - } - } -} - -impl From for Error { - fn from(error: SchemaError) -> Error { - match error { - SchemaError::Sqlite(err) => Error { - kind: ErrorKind::SQLite(err), - }, - err => Error { - kind: ErrorKind::Schema(err), - }, - } - } -} - impl From for Error { fn from(error: OsString) -> Error { Error { diff --git a/glean-core/src/error_recording.rs b/glean-core/src/error_recording.rs index e92b9a6d9b..24dcedd3e8 100644 --- a/glean-core/src/error_recording.rs +++ b/glean-core/src/error_recording.rs @@ -13,16 +13,14 @@ //! not some constant value that we could define in `metrics.yaml`. use std::fmt::Display; -use std::sync::atomic::AtomicU8; - -use rusqlite::Transaction; use crate::common_metric_data::CommonMetricDataInternal; use crate::error::{Error, ErrorKind}; -use crate::metrics::{CounterMetric, Metric}; +use crate::metrics::labeled::{combine_base_identifier_and_label, strip_label}; +use crate::metrics::CounterMetric; +use crate::CommonMetricData; use crate::Glean; use crate::Lifetime; -use crate::{CommonMetricData, MetricLabel}; /// The possible error types for metric recording. /// @@ -94,7 +92,8 @@ impl TryFrom for ErrorType { fn get_error_metric_for_metric(meta: &CommonMetricDataInternal, error: ErrorType) -> CounterMetric { // Can't use meta.identifier here, since that might cause infinite recursion // if the label on this metric needs to report an error. - let name = meta.base_identifier(); + let identifier = meta.base_identifier(); + let name = strip_label(&identifier); // Record errors in the pings the metric is in, as well as the metrics ping. let mut send_in_pings = meta.inner.send_in_pings.clone(); @@ -104,11 +103,10 @@ fn get_error_metric_for_metric(meta: &CommonMetricDataInternal, error: ErrorType } CounterMetric::new(CommonMetricData { - name: error.as_str().to_string(), + name: combine_base_identifier_and_label(error.as_str(), name), category: "glean.error".into(), lifetime: Lifetime::Ping, send_in_pings, - label: Some(MetricLabel::Label(name.to_string())), ..Default::default() }) } @@ -144,55 +142,6 @@ pub fn record_error>>( metric.add_sync(glean, to_report); } -pub fn record_error_sqlite( - glean: &Glean, - tx: &mut Transaction, - metric_name: &str, - send_in_pings: &[String], - error: ErrorType, - num_errors: i32, -) { - debug_assert!(num_errors > 0); - if num_errors <= 0 { - log::warn!("Trying to record {num_errors} errors for {metric_name:?} (<= 0). Bailing out."); - return; - } - - // We explicitly don't use the `Counter` metric directly here. - // - // * This is called from within the recording functions in `sqlite.rs` - // * That means a transaction is already opened. We can't open a new one. - // * We can avoid some allocations by constructing only what we need and what we already have - - let ping_name = String::from("metrics"); - let mut send_in_pings = send_in_pings.to_vec(); - if !send_in_pings.contains(&ping_name) { - send_in_pings.push(ping_name); - } - - let lifetime = Lifetime::Ping; - let transform = |old_value| match old_value { - Some(Metric::Counter(old_value)) => Metric::Counter(old_value.saturating_add(num_errors)), - _ => Metric::Counter(num_errors), - }; - - let inner = CommonMetricData { - category: String::from("glean.error"), - name: String::from(error.as_str()), - send_in_pings, - lifetime, - label: Some(MetricLabel::Static(String::from(metric_name))), - ..Default::default() - }; - let cmd = CommonMetricDataInternal { - inner, - disabled: AtomicU8::new(0), - }; - _ = glean - .storage() - .record_with_transaction(glean, tx, &cmd, transform); -} - /// Gets the number of recorded errors for the given metric and error type. /// /// *Notes: This is a **test-only** API, but we need to expose it to be used in integration tests. diff --git a/glean-core/src/glean.udl b/glean-core/src/glean.udl index 667732202e..65bf5ec729 100644 --- a/glean-core/src/glean.udl +++ b/glean-core/src/glean.udl @@ -379,7 +379,7 @@ interface PingType { void set_enabled(boolean enabled); }; -typedef enum MetricLabel; +typedef enum DynamicLabelType; // The common set of data shared across all different metric types. dictionary CommonMetricData { @@ -398,12 +398,12 @@ dictionary CommonMetricData { // Disabled metrics are never recorded. boolean disabled; - // Label for this metric. + // Dynamic label. // // When a labeled metric factory creates the specific metric to be recorded to, - // labels are stored in the metric data - // so that it can validated against the database later. - MetricLabel? label = null; + // dynamic labels are stored in the specific label so that + // we can validate them when the Glean singleton is available. + DynamicLabelType? dynamic_label = null; // Whether this metric is inside of the session scope. // diff --git a/glean-core/src/internal_metrics.rs b/glean-core/src/internal_metrics.rs index ad40960001..a134666b19 100644 --- a/glean-core/src/internal_metrics.rs +++ b/glean-core/src/internal_metrics.rs @@ -218,6 +218,8 @@ impl AdditionalMetrics { category: "glean.internal.metrics".into(), send_in_pings: vec!["glean_internal_info".into()], lifetime: Lifetime::Application, + disabled: false, + dynamic_label: None, ..Default::default() }), } @@ -299,6 +301,7 @@ impl UploadMetrics { send_in_pings: vec!["health".into()], lifetime: Lifetime::Ping, disabled: false, + dynamic_label: None, ..Default::default() }, }, @@ -358,27 +361,11 @@ impl UploadMetrics { pub struct DatabaseMetrics { pub size: MemoryDistributionMetric, - /// sqlite's load result, indicating success or relaying the detected error. - pub load_error: StringMetric, + /// RKV's load result, indicating success or relaying the detected error. + pub rkv_load_error: StringMetric, /// The time it takes for a write-commit for the Glean database. pub write_time: TimingDistributionMetric, - - /// The number of metrics migrated from Rkv storage to SQLite storage - pub migrated_metrics: CounterMetric, - - /// The number of metrics stored in SQLite after a migration run - pub metrics_in_sqlite: CounterMetric, - - /// Number of metrics that failed to deserialize from storage - /// while iterating the Rkv database for migration. - pub failed_metrics: CounterMetric, - - /// The duration for one full migration run at startup - pub migration_duration: TimingDistributionMetric, - - /// Number of times a migration was attempted and failed - pub migration_error: CounterMetric, } impl DatabaseMetrics { @@ -395,8 +382,8 @@ impl DatabaseMetrics { MemoryUnit::Byte, ), - load_error: StringMetric::new(CommonMetricData { - name: "load_error".into(), + rkv_load_error: StringMetric::new(CommonMetricData { + name: "rkv_load_error".into(), category: "glean.database".into(), send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, @@ -414,49 +401,6 @@ impl DatabaseMetrics { }, TimeUnit::Microsecond, ), - - migrated_metrics: CounterMetric::new(CommonMetricData { - name: "migrated_metrics".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), - - metrics_in_sqlite: CounterMetric::new(CommonMetricData { - name: "metrics_in_sqlite".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), - - failed_metrics: CounterMetric::new(CommonMetricData { - name: "failed_metrics".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), - - migration_duration: TimingDistributionMetric::new( - CommonMetricData { - name: "migration_duration".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }, - TimeUnit::Millisecond, - ), - - migration_error: CounterMetric::new(CommonMetricData { - name: "error".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), } } } diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index e9fab6780a..c401c3dafe 100644 --- a/glean-core/src/lib.rs +++ b/glean-core/src/lib.rs @@ -64,7 +64,7 @@ mod util; #[cfg(all(not(target_os = "android"), not(target_os = "ios")))] mod fd_logger; -pub use crate::common_metric_data::{CommonMetricData, Lifetime, MetricLabel}; +pub use crate::common_metric_data::{CommonMetricData, DynamicLabelType, Lifetime}; pub use crate::core::Glean; pub use crate::core_metrics::{AttributionMetrics, ClientInfoMetrics, DistributionMetrics}; use crate::dispatcher::is_test_mode; diff --git a/glean-core/src/lib_unit_tests.rs b/glean-core/src/lib_unit_tests.rs index 52f7fcf8d4..8d4306fa12 100644 --- a/glean-core/src/lib_unit_tests.rs +++ b/glean-core/src/lib_unit_tests.rs @@ -1209,10 +1209,10 @@ fn records_database_file_size() { // We should see the database containing some data. assert!(data.sum > 0); - let load_state = &glean.database_metrics.load_error; - let load_error = load_state.get_value(&glean, "metrics"); + let rkv_load_state = &glean.database_metrics.rkv_load_error; + let rkv_load_error = rkv_load_state.get_value(&glean, "metrics"); - assert_eq!(load_error, None); + assert_eq!(rkv_load_error, None); } #[cfg(not(target_os = "windows"))] diff --git a/glean-core/src/metrics/boolean.rs b/glean-core/src/metrics/boolean.rs index 2614ba1f3a..504f3347a5 100644 --- a/glean-core/src/metrics/boolean.rs +++ b/glean-core/src/metrics/boolean.rs @@ -4,10 +4,11 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{test_get_num_recorded_errors, ErrorType}; use crate::metrics::MetricType; use crate::metrics::{Metric, TestGetValue}; +use crate::storage::StorageManager; use crate::CommonMetricData; use crate::Glean; @@ -32,9 +33,9 @@ impl MetricType for BooleanMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), } @@ -88,7 +89,12 @@ impl BooleanMetric { pub fn get_value(&self, glean: &Glean, ping_name: Option<&str>) -> Option { let queried_ping_name = ping_name.unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Boolean(b)) => Some(b), _ => None, } diff --git a/glean-core/src/metrics/counter.rs b/glean-core/src/metrics/counter.rs index 407426a66b..300a088472 100644 --- a/glean-core/src/metrics/counter.rs +++ b/glean-core/src/metrics/counter.rs @@ -5,10 +5,11 @@ use std::cmp::Ordering; use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -34,9 +35,9 @@ impl MetricType for CounterMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), } @@ -125,7 +126,12 @@ impl CounterMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Counter(i)) => Some(i), _ => None, } diff --git a/glean-core/src/metrics/custom_distribution.rs b/glean-core/src/metrics/custom_distribution.rs index 418c06b2d7..a94af73a08 100644 --- a/glean-core/src/metrics/custom_distribution.rs +++ b/glean-core/src/metrics/custom_distribution.rs @@ -5,10 +5,11 @@ use std::mem; use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::histogram::{Bucketing, Histogram, HistogramType, LinearOrExponential}; use crate::metrics::{DistributionData, Metric, MetricType}; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -54,9 +55,9 @@ impl MetricType for CustomDistributionMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), range_min: self.range_min, @@ -217,7 +218,13 @@ impl CustomDistributionMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { + // Boxing the value, in order to return either of the possible buckets Some(Metric::CustomDistributionExponential(hist)) => Some(snapshot(&hist)), Some(Metric::CustomDistributionLinear(hist)) => Some(snapshot(&hist)), _ => None, diff --git a/glean-core/src/metrics/datetime.rs b/glean-core/src/metrics/datetime.rs index 82b9642c40..b6b39a4909 100644 --- a/glean-core/src/metrics/datetime.rs +++ b/glean-core/src/metrics/datetime.rs @@ -10,6 +10,7 @@ use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorTy use crate::metrics::time_unit::TimeUnit; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::util::{get_iso_time_string, local_now_with_offset}; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -217,7 +218,12 @@ impl DatetimeMetric { ) -> Option<(ChronoDatetime, TimeUnit)> { let queried_ping_name = ping_name.unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Datetime(d, tu)) => Some((d, tu)), _ => None, } diff --git a/glean-core/src/metrics/denominator.rs b/glean-core/src/metrics/denominator.rs index c25d00d4c4..1d343e7d40 100644 --- a/glean-core/src/metrics/denominator.rs +++ b/glean-core/src/metrics/denominator.rs @@ -8,6 +8,7 @@ use crate::metrics::CounterMetric; use crate::metrics::Metric; use crate::metrics::MetricType; use crate::metrics::RateMetric; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -95,7 +96,12 @@ impl DenominatorMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta().identifier(glean), + self.meta().inner.lifetime, + ) { Some(Metric::Counter(i)) => Some(i), _ => None, } diff --git a/glean-core/src/metrics/dual_labeled_counter.rs b/glean-core/src/metrics/dual_labeled_counter.rs index 9a774403f0..597c129878 100644 --- a/glean-core/src/metrics/dual_labeled_counter.rs +++ b/glean-core/src/metrics/dual_labeled_counter.rs @@ -8,14 +8,10 @@ use std::collections::{HashMap, HashSet}; use std::mem; use std::sync::{Arc, Mutex}; -use rusqlite::{params, Transaction}; - -use crate::common_metric_data::{ - CommonMetricData, CommonMetricDataInternal, LabelCheck, MetricLabel, -}; -use crate::error_recording::{test_get_num_recorded_errors, ErrorType}; -use crate::metrics::{CounterMetric, MetricType}; -use crate::TestGetValue; +use crate::common_metric_data::{CommonMetricData, CommonMetricDataInternal, DynamicLabelType}; +use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; +use crate::metrics::{CounterMetric, Metric, MetricType}; +use crate::{Glean, TestGetValue}; const MAX_LABELS: usize = 16; const OTHER_LABEL: &str = "__other__"; @@ -114,25 +110,32 @@ impl DualLabeledCounterMetric { match (&self.keys, &self.categories) { (None, None) => self .counter - .with_label(MetricLabel::KeyAndCategory(key.into(), category.into())), + .with_dynamic_label(DynamicLabelType::KeyAndCategory( + make_label_from_key_and_category(key, category), + )), (None, _) => { let static_category = self.static_category(category); - self.counter - .with_label(MetricLabel::KeyOnly(key.into(), static_category.into())) + self.counter.with_dynamic_label(DynamicLabelType::KeyOnly( + make_label_from_key_and_category(key, static_category), + )) } (_, None) => { let static_key = self.static_key(key); - self.counter.with_label(MetricLabel::CategoryOnly( - static_key.into(), - category.into(), - )) + self.counter + .with_dynamic_label(DynamicLabelType::CategoryOnly( + make_label_from_key_and_category(static_key, category), + )) } (_, _) => { // Both labels are static and can be validated now let static_key = self.static_key(key); let static_category = self.static_category(category); - let label = format!("{static_key}{RECORD_SEPARATOR}{static_category}"); - self.counter.with_label(MetricLabel::Static(label)) + let name = combine_base_identifier_and_labels( + self.counter.meta().inner.name.as_str(), + static_key, + static_category, + ); + self.counter.with_name(name) } } } @@ -248,91 +251,181 @@ impl TestGetValue for DualLabeledCounterMetric { } } -pub fn validate_dual_label_sqlite( - tx: &Transaction, - base_identifier: &str, +/// Combines a metric's base identifier and label +pub fn combine_base_identifier_and_labels( + base_identifer: &str, key: &str, category: &str, -) -> LabelCheck { - let existing_labels_sql = "SELECT DISTINCT labels FROM telemetry WHERE id = ?1"; - - // TODO(bug 2048193): We can now detect if _either_ key or category contains `RECORD_SEPARATOR` and thus keep - // the other potentially valid label. - // This needs adjustement of the test `labels_containing_a_record_separator_record_an_error`. - if key.contains(RECORD_SEPARATOR) || category.contains(RECORD_SEPARATOR) { - log::warn!("Metric {base_identifier:?}: Label cannot contain the ASCII record separator character (0x1E)"); - return LabelCheck::Error(format!("{OTHER_LABEL}{RECORD_SEPARATOR}{OTHER_LABEL}"), 1); - } +) -> String { + format!( + "{}{}", + base_identifer, + make_label_from_key_and_category(key, category) + ) +} - let mut existing_keys = HashSet::new(); - let mut existing_categories = HashSet::new(); - 'checkdb: { - let Ok(mut stmt) = tx.prepare(existing_labels_sql) else { - // If we can't fetch from the database, assume the label is ok to use - break 'checkdb; - }; +/// Separate label into key and category components. +/// Must validate the label format before calling this to ensure it doesn't contain +/// any ASCII record separator characters aside from the one's we put there. +pub fn separate_label_into_key_and_category(label: &str) -> Option<(&str, &str)> { + label + .strip_prefix(RECORD_SEPARATOR) + .unwrap_or(label) + .split_once(RECORD_SEPARATOR) +} - let Ok(mut rows) = stmt.query(params![base_identifier]) else { - // If we can't fetch from the database, assume the label is ok to use - break 'checkdb; - }; +/// Construct and return a label from a given key and category with the RECORD_SEPARATOR +/// characters in the format: `` +pub fn make_label_from_key_and_category(key: &str, category: &str) -> String { + format!( + "{}{}{}{}", + RECORD_SEPARATOR, key, RECORD_SEPARATOR, category + ) +} - while let Ok(Some(row)) = rows.next() { - let existing_labels: String = row.get(0).unwrap(); - let Some((existing_key, existing_category)) = - existing_labels.split_once(RECORD_SEPARATOR) - else { - // TODO(bug 2048195): Instrument this. - log::debug!("Metric {base_identifier:?}: Database contains invalid dual-label: {existing_labels:?}"); - continue; - }; - - existing_keys.insert(existing_key.to_string()); - existing_categories.insert(existing_category.to_string()); - } +/// Validates a dynamic label, changing it to `OTHER_LABEL` if it's invalid. +/// +/// Checks the requested label against limitations, such as the label length and allowed +/// characters. +/// +/// # Arguments +/// +/// * `label` - The requested label +/// +/// # Returns +/// +/// The entire identifier for the metric, including the base identifier and the corrected label. +/// The errors are logged. +pub fn validate_dynamic_key_and_or_category( + glean: &Glean, + meta: &CommonMetricDataInternal, + base_identifier: &str, + label: DynamicLabelType, +) -> String { + // We should have exactly 3 elements when splitting by `RECORD_SEPARATOR`, since the label should begin with one and + // then the key and category are separated by one. Split should contain an empty string, the key, and the category. + // If we have more than 3 elements, then the consuming app must have used this character as part of a label and we + // cannot determine whether it was the key or the category at this point, so we record an `InvalidLabel` error and + // return `OTHER_LABEL` for both key and category. + if label.split(RECORD_SEPARATOR).count() != 3 { + let msg = "Label cannot contain the ASCII record separator character (0x1E)".to_string(); + record_error(glean, meta, ErrorType::InvalidLabel, msg, None); + return combine_base_identifier_and_labels(base_identifier, OTHER_LABEL, OTHER_LABEL); } - let mut errors = 0; - let new_key = if (existing_keys.contains(key) || existing_keys.len() < MAX_LABELS) - && label_is_valid(key, base_identifier) - { - key - } else { - errors += 1; - OTHER_LABEL - }; - - let new_category = if (existing_categories.contains(category) - || existing_categories.len() < MAX_LABELS) - && label_is_valid(category, base_identifier) - { - category - } else { - errors += 1; - OTHER_LABEL - }; + // Pick out the key and category from the supplied label + if let Some((mut key, mut category)) = separate_label_into_key_and_category(&label) { + // Loop through the stores we expect to find this metric in, and if we + // find it then just return the full metric identifier that was found + for store in &meta.inner.send_in_pings { + if glean.storage().has_metric(meta.inner.lifetime, store, key) { + return combine_base_identifier_and_labels(base_identifier, key, category); + } + } - let label = format!("{new_key}{RECORD_SEPARATOR}{new_category}"); - if errors == 0 { - LabelCheck::Label(label) + // Count the number of distinct keys and categories already recorded, we can figure out which + // one(s) to check based on the label variant. + let (seen_keys, seen_categories) = get_seen_keys_and_categories(meta, glean); + match label { + DynamicLabelType::Label(ref label) => { + record_error( + glean, + meta, + ErrorType::InvalidLabel, + format!("Invalid `DualLabeledCounter` label format: {label:?}"), + None, + ); + key = OTHER_LABEL; + category = OTHER_LABEL; + } + DynamicLabelType::KeyOnly(_) => { + if (!seen_keys.contains(key) && seen_keys.len() >= MAX_LABELS) + || !label_is_valid(key, glean, meta) + { + key = OTHER_LABEL; + } + } + DynamicLabelType::CategoryOnly(_) => { + if (!seen_categories.contains(category) && seen_categories.len() >= MAX_LABELS) + || !label_is_valid(category, glean, meta) + { + category = OTHER_LABEL; + } + } + DynamicLabelType::KeyAndCategory(_) => { + if (!seen_keys.contains(key) && seen_keys.len() >= MAX_LABELS) + || !label_is_valid(key, glean, meta) + { + key = OTHER_LABEL; + } + if (!seen_categories.contains(category) && seen_categories.len() >= MAX_LABELS) + || !label_is_valid(category, glean, meta) + { + category = OTHER_LABEL; + } + } + } + combine_base_identifier_and_labels(base_identifier, key, category) } else { - LabelCheck::Error(label, errors) + record_error( + glean, + meta, + ErrorType::InvalidLabel, + "Invalid `DualLabeledCounter` label format, unable to determine key and/or category", + None, + ); + combine_base_identifier_and_labels(base_identifier, OTHER_LABEL, OTHER_LABEL) } } -fn label_is_valid(label: &str, metric_id: &str) -> bool { +fn label_is_valid(label: &str, glean: &Glean, meta: &CommonMetricDataInternal) -> bool { if label.len() > MAX_LABEL_LENGTH { - log::warn!( - "Metric {:?}: label length {} exceeds maximum of {}", - metric_id, + let msg = format!( + "label length {} exceeds maximum of {}", label.len(), MAX_LABEL_LENGTH ); - false - } else if label.contains(RECORD_SEPARATOR) { - log::warn!("Metric {metric_id:?}: Label cannot contain the ASCII record separator character (0x1E)"); + record_error(glean, meta, ErrorType::InvalidLabel, msg, None); false } else { true } } + +fn get_seen_keys_and_categories( + meta: &CommonMetricDataInternal, + glean: &Glean, +) -> (HashSet, HashSet) { + let base_identifier = &meta.base_identifier(); + let prefix = format!("{base_identifier}{RECORD_SEPARATOR}"); + let mut seen_keys: HashSet = HashSet::new(); + let mut seen_categories: HashSet = HashSet::new(); + let mut snapshotter = |metric_id: &[u8], _: &Metric| { + let metric_id_str = String::from_utf8_lossy(metric_id); + + // Split full identifier on the ASCII Record Separator (\x1e) + let parts: Vec<&str> = metric_id_str.split(RECORD_SEPARATOR).collect(); + + if parts.len() == 2 { + seen_keys.insert(parts[0].into()); + seen_categories.insert(parts[1].into()); + } else { + record_error( + glean, + meta, + ErrorType::InvalidLabel, + "Dual Labeled Counter label doesn't contain exactly 2 parts".to_string(), + None, + ); + } + }; + + let lifetime = meta.inner.lifetime; + for store in &meta.inner.send_in_pings { + glean + .storage() + .iter_store_from(lifetime, store, Some(&prefix), &mut snapshotter); + } + + (seen_keys, seen_categories) +} diff --git a/glean-core/src/metrics/experiment.rs b/glean-core/src/metrics/experiment.rs index 2d49bc91d7..334c85e2e1 100644 --- a/glean-core/src/metrics/experiment.rs +++ b/glean-core/src/metrics/experiment.rs @@ -9,7 +9,7 @@ use std::sync::atomic::AtomicU8; use crate::common_metric_data::CommonMetricDataInternal; use crate::error_recording::{record_error, ErrorType}; use crate::metrics::{Metric, MetricType, RecordedExperiment}; -use crate::storage::INTERNAL_STORAGE; +use crate::storage::{StorageManager, INTERNAL_STORAGE}; use crate::util::{truncate_string_at_boundary, truncate_string_at_boundary_with_error}; use crate::Lifetime; use crate::{CommonMetricData, Glean}; @@ -205,7 +205,12 @@ impl ExperimentMetric { /// /// The stored value or `None` if nothing stored. pub fn test_get_value(&self, glean: &Glean) -> Option { - match glean.storage().get_metric(self.meta(), INTERNAL_STORAGE) { + match StorageManager.snapshot_metric( + glean.storage(), + INTERNAL_STORAGE, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Experiment(e)) => Some(e), _ => None, } diff --git a/glean-core/src/metrics/labeled.rs b/glean-core/src/metrics/labeled.rs index d8cd4a8093..ba5d2855d2 100644 --- a/glean-core/src/metrics/labeled.rs +++ b/glean-core/src/metrics/labeled.rs @@ -4,21 +4,23 @@ use std::any::Any; use std::borrow::Cow; +use std::collections::HashSet; use std::collections::{hash_map::Entry, HashMap}; use std::mem; use std::sync::{Arc, Mutex}; use malloc_size_of::MallocSizeOf; -use rusqlite::{params, Transaction}; -use crate::common_metric_data::{CommonMetricData, LabelCheck, MetricLabel}; -use crate::error_recording::{test_get_num_recorded_errors, ErrorType}; +use crate::common_metric_data::{CommonMetricData, CommonMetricDataInternal, DynamicLabelType}; +use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::histogram::HistogramType; use crate::metrics::{ BooleanMetric, CounterMetric, CustomDistributionMetric, MemoryDistributionMetric, MemoryUnit, - MetricType, QuantityMetric, StringMetric, TestGetValue, TimeUnit, TimingDistributionMetric, + Metric, MetricType, QuantityMetric, StringMetric, TestGetValue, TimeUnit, + TimingDistributionMetric, }; use crate::storage::StorageManager; +use crate::Glean; const MAX_LABELS: usize = 16; const OTHER_LABEL: &str = "__other__"; @@ -251,18 +253,16 @@ where /// Creates a new metric with a specific label. /// /// This is used for static labels where we can just set the name to be `name/label`. - fn new_metric_with_label(&self, label: MetricLabel) -> T { - self.submetric.with_label(label) + fn new_metric_with_name(&self, name: String) -> T { + self.submetric.with_name(name) } /// Creates a new metric with a specific label. /// /// This is used for dynamic labels where we have to actually validate and correct the /// label later when we have a Glean object. - /// - /// TODO: Consolidate with `new_metric_with_label` above. - fn new_metric_with_dynamic_label(&self, label: MetricLabel) -> T { - self.submetric.with_label(label) + fn new_metric_with_dynamic_label(&self, label: DynamicLabelType) -> T { + self.submetric.with_dynamic_label(label) } /// Creates a static label. @@ -320,11 +320,13 @@ where let metric = match self.labels { Some(_) => { let label = self.static_label(label); - self.new_metric_with_label(MetricLabel::Static(label.to_string())) - } - None => { - self.new_metric_with_dynamic_label(MetricLabel::Label(label.to_string())) + self.new_metric_with_name(combine_base_identifier_and_label( + &self.submetric.meta().inner.name, + label, + )) } + None => self + .new_metric_with_dynamic_label(DynamicLabelType::Label(label.to_string())), }; let metric = Arc::new(metric); entry.insert(Arc::clone(&metric)); @@ -369,7 +371,7 @@ where StorageManager.snapshot_labels( glean.storage(), queried_ping_name, - &self.submetric.meta().base_identifier(), + &self.submetric.meta().identifier(glean), self.submetric.meta().inner.lifetime, ) }); @@ -383,48 +385,73 @@ where } } -pub fn validate_dynamic_label_sqlite( - tx: &Transaction, +/// Combines a metric's base identifier and label +pub fn combine_base_identifier_and_label(base_identifer: &str, label: &str) -> String { + format!("{}/{}", base_identifer, label) +} + +/// Strips the label off of a complete identifier +pub fn strip_label(identifier: &str) -> &str { + identifier.split_once('/').map_or(identifier, |s| s.0) +} + +/// Validates a dynamic label, changing it to `OTHER_LABEL` if it's invalid. +/// +/// Checks the requested label against limitations, such as the label length and allowed +/// characters. +/// +/// # Arguments +/// +/// * `label` - The requested label +/// +/// # Returns +/// +/// The entire identifier for the metric, including the base identifier and the corrected label. +/// The errors are logged. +pub fn validate_dynamic_label( + glean: &Glean, + meta: &CommonMetricDataInternal, base_identifier: &str, label: &str, -) -> LabelCheck { - let existing_labels_sql = "SELECT DISTINCT labels FROM telemetry WHERE id = ?1"; - - let mut label_already_used = false; - let mut label_count = 0; - { - let Ok(mut stmt) = tx.prepare(existing_labels_sql) else { - // If we can't fetch from the database, assume the label is ok to use - return LabelCheck::Label(label.to_string()); - }; - - let Ok(mut rows) = stmt.query(params![base_identifier]) else { - // If we can't fetch from the database, assume the label is ok to use - return LabelCheck::Label(label.to_string()); - }; +) -> String { + let key = combine_base_identifier_and_label(base_identifier, label); + for store in &meta.inner.send_in_pings { + if glean.storage().has_metric(meta.inner.lifetime, store, &key) { + return key; + } + } - while let Ok(Some(row)) = rows.next() { - let existing_label: String = row.get(0).unwrap(); + let mut labels = HashSet::new(); + let prefix = &key[..=base_identifier.len()]; + let mut snapshotter = |metric_id: &[u8], _: &Metric| { + labels.insert(metric_id.to_vec()); + }; - label_count += 1; - if existing_label == label { - label_already_used = true; - break; - } - } + let lifetime = meta.inner.lifetime; + for store in &meta.inner.send_in_pings { + glean + .storage() + .iter_store_from(lifetime, store, Some(prefix), &mut snapshotter); } - if !label_already_used && label_count >= MAX_LABELS { - LabelCheck::Label(String::from(OTHER_LABEL)) + let label_count = labels.len(); + let error = if label_count >= MAX_LABELS { + true } else if label.len() > MAX_LABEL_LENGTH { - log::warn!( - "Metric {:?}: label length {} exceeds maximum of {}", - base_identifier, + let msg = format!( + "label length {} exceeds maximum of {}", label.len(), MAX_LABEL_LENGTH ); - LabelCheck::Error(String::from(OTHER_LABEL), 1) + record_error(glean, meta, ErrorType::InvalidLabel, msg, None); + true + } else { + false + }; + + if error { + combine_base_identifier_and_label(base_identifier, OTHER_LABEL) } else { - LabelCheck::Label(label.to_string()) + key } } diff --git a/glean-core/src/metrics/memory_distribution.rs b/glean-core/src/metrics/memory_distribution.rs index 9349537cf4..3513dabf3d 100644 --- a/glean-core/src/metrics/memory_distribution.rs +++ b/glean-core/src/metrics/memory_distribution.rs @@ -5,11 +5,12 @@ use std::mem; use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::histogram::{Functional, Histogram}; use crate::metrics::memory_unit::MemoryUnit; use crate::metrics::{DistributionData, Metric, MetricType}; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -63,9 +64,9 @@ impl MetricType for MemoryDistributionMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), memory_unit: self.memory_unit, @@ -256,7 +257,12 @@ impl MemoryDistributionMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::MemoryDistribution(hist)) => Some(snapshot(&hist)), _ => None, } diff --git a/glean-core/src/metrics/mod.rs b/glean-core/src/metrics/mod.rs index dd1518b0cf..455abc94be 100644 --- a/glean-core/src/metrics/mod.rs +++ b/glean-core/src/metrics/mod.rs @@ -31,7 +31,7 @@ mod quantity; mod rate; mod recorded_experiment; mod remote_settings_config; -pub(crate) mod string; +mod string; mod string_list; mod text; mod time_unit; @@ -41,7 +41,7 @@ mod url; mod uuid; use crate::common_metric_data::CommonMetricDataInternal; -pub use crate::common_metric_data::MetricLabel; +pub use crate::common_metric_data::DynamicLabelType; pub use crate::event_database::RecordedEvent; use crate::histogram::{Functional, Histogram, PrecomputedExponential, PrecomputedLinear}; pub use crate::metrics::datetime::Datetime; @@ -196,7 +196,7 @@ pub trait MetricType { } /// Create a new metric from this with a specific label. - fn with_label(&self, _label: MetricLabel) -> Self + fn with_dynamic_label(&self, _label: DynamicLabelType) -> Self where Self: Sized, { @@ -255,13 +255,17 @@ pub trait MetricType { let remote_settings_config = &glean.remote_settings_config.lock().unwrap(); // Get the value from the remote configuration if it is there, otherwise return the default value. let current_disabled = { - let identifier = self.meta().base_identifier(); + let base_id = self.meta().base_identifier(); + let identifier = base_id + .split_once('/') + .map(|split| split.0) + .unwrap_or(&base_id); // NOTE: The `!` preceding the `*is_enabled` is important for inverting the logic since the // underlying property in the metrics.yaml is `disabled` and the outward API is treating it as // if it were `enabled` to make it easier to understand. if !remote_settings_config.metrics_enabled.is_empty() { - if let Some(is_enabled) = remote_settings_config.metrics_enabled.get(&identifier) { + if let Some(is_enabled) = remote_settings_config.metrics_enabled.get(identifier) { u8::from(!*is_enabled) } else { u8::from(self.meta().inner.disabled) @@ -284,7 +288,7 @@ pub trait MetricType { /// identifier (category, name, label) for a metric pub trait MetricIdentifier<'a> { /// Retrieve the category, name and (maybe) label of the metric - fn get_identifiers(&'a self) -> (&'a str, &'a str, Option); + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option<&'a str>); } /// [`TestGetValue`] describes an interface for retrieving the value for a given metric @@ -315,13 +319,9 @@ impl<'a, T> MetricIdentifier<'a> for T where T: MetricType, { - fn get_identifiers(&'a self) -> (&'a str, &'a str, Option) { + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option<&'a str>) { let meta = &self.meta().inner; - ( - &meta.category, - &meta.name, - meta.label.as_ref().map(|label| label.to_string()), - ) + (&meta.category, &meta.name, meta.dynamic_label.as_deref()) } } diff --git a/glean-core/src/metrics/object.rs b/glean-core/src/metrics/object.rs index 9739f6e06d..cbb5af36c7 100644 --- a/glean-core/src/metrics/object.rs +++ b/glean-core/src/metrics/object.rs @@ -9,6 +9,7 @@ use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorTy use crate::metrics::JsonValue; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -117,7 +118,12 @@ impl ObjectMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Object(o)) => Some(o), _ => None, } diff --git a/glean-core/src/metrics/quantity.rs b/glean-core/src/metrics/quantity.rs index d20f6ddc90..137569b53e 100644 --- a/glean-core/src/metrics/quantity.rs +++ b/glean-core/src/metrics/quantity.rs @@ -4,10 +4,11 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -32,9 +33,9 @@ impl MetricType for QuantityMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), } @@ -101,7 +102,12 @@ impl QuantityMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Quantity(i)) => Some(i), _ => None, } diff --git a/glean-core/src/metrics/rate.rs b/glean-core/src/metrics/rate.rs index 37fc66d268..40e920fd2a 100644 --- a/glean-core/src/metrics/rate.rs +++ b/glean-core/src/metrics/rate.rs @@ -6,6 +6,7 @@ use crate::common_metric_data::CommonMetricDataInternal; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -146,7 +147,12 @@ impl RateMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Rate(n, d)) => Some((n, d).into()), _ => None, } diff --git a/glean-core/src/metrics/string.rs b/glean-core/src/metrics/string.rs index a4dda47f26..528c682ad9 100644 --- a/glean-core/src/metrics/string.rs +++ b/glean-core/src/metrics/string.rs @@ -4,15 +4,16 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::util::truncate_string_at_boundary_with_error; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; -pub const MAX_LENGTH_VALUE: usize = 255; +const MAX_LENGTH_VALUE: usize = 255; /// A string metric. /// @@ -36,9 +37,9 @@ impl MetricType for StringMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), } @@ -95,7 +96,12 @@ impl StringMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::String(s)) => Some(s), _ => None, } @@ -161,7 +167,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - label: None, + dynamic_label: None, in_session: false, }); diff --git a/glean-core/src/metrics/string_list.rs b/glean-core/src/metrics/string_list.rs index 257196c567..ad2a1b6244 100644 --- a/glean-core/src/metrics/string_list.rs +++ b/glean-core/src/metrics/string_list.rs @@ -8,6 +8,7 @@ use crate::common_metric_data::CommonMetricDataInternal; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::util::truncate_string_at_boundary_with_error; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -154,7 +155,12 @@ impl StringListMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::StringList(values)) => Some(values), _ => None, } diff --git a/glean-core/src/metrics/text.rs b/glean-core/src/metrics/text.rs index c81c0c6bb0..12c3c5bd61 100644 --- a/glean-core/src/metrics/text.rs +++ b/glean-core/src/metrics/text.rs @@ -4,10 +4,11 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::util::truncate_string_at_boundary_with_error; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -38,9 +39,9 @@ impl MetricType for TextMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), } @@ -99,7 +100,12 @@ impl TextMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Text(s)) => Some(s), _ => None, } @@ -165,7 +171,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - label: None, + dynamic_label: None, in_session: false, }); diff --git a/glean-core/src/metrics/timespan.rs b/glean-core/src/metrics/timespan.rs index 4f3d6fddd1..5ab5f769a4 100644 --- a/glean-core/src/metrics/timespan.rs +++ b/glean-core/src/metrics/timespan.rs @@ -10,6 +10,7 @@ use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorTy use crate::metrics::time_unit::TimeUnit; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -253,7 +254,12 @@ impl TimespanMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Timespan(time, time_unit)) => Some(time_unit.duration_convert(time)), _ => None, } diff --git a/glean-core/src/metrics/timing_distribution.rs b/glean-core/src/metrics/timing_distribution.rs index 5d8dd9c053..5620cd3a1e 100644 --- a/glean-core/src/metrics/timing_distribution.rs +++ b/glean-core/src/metrics/timing_distribution.rs @@ -10,11 +10,12 @@ use std::time::Duration; use malloc_size_of_derive::MallocSizeOf; -use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; +use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::histogram::{Functional, Histogram}; use crate::metrics::time_unit::TimeUnit; use crate::metrics::{DistributionData, Metric, MetricType}; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -110,9 +111,9 @@ impl MetricType for TimingDistributionMetric { } } - fn with_label(&self, label: MetricLabel) -> Self { + fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.label = Some(label); + meta.inner.dynamic_label = Some(label); Self { meta: Arc::new(meta), time_unit: self.time_unit, @@ -484,7 +485,6 @@ impl TimingDistributionMetric { /// Use [`accumulate_raw_samples_nanos`](Self::accumulate_raw_samples_nanos) instead. #[doc(hidden)] pub fn accumulate_raw_samples_nanos_sync(&self, glean: &Glean, samples: &[u64]) { - log::debug!("logging samples: {samples:?}"); if !self.should_record(glean) { return; } @@ -542,7 +542,12 @@ impl TimingDistributionMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::TimingDistribution(hist)) => Some(snapshot(&hist)), _ => None, } diff --git a/glean-core/src/metrics/url.rs b/glean-core/src/metrics/url.rs index 871fe46dee..93c063ed5e 100644 --- a/glean-core/src/metrics/url.rs +++ b/glean-core/src/metrics/url.rs @@ -8,6 +8,7 @@ use crate::common_metric_data::CommonMetricDataInternal; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::util::truncate_string_at_boundary_with_error; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -114,7 +115,12 @@ impl UrlMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Url(s)) => Some(s), _ => None, } @@ -178,7 +184,9 @@ mod test { category: "test".into(), send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, - ..Default::default() + disabled: false, + dynamic_label: None, + in_session: false, }); let sample_url = "glean://test".to_string(); @@ -195,7 +203,9 @@ mod test { category: "test".into(), send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, - ..Default::default() + disabled: false, + dynamic_label: None, + in_session: false, }); // Whenever the URL is longer than our MAX_URL_LENGTH, we truncate the URL to the @@ -233,7 +243,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - label: None, + dynamic_label: None, in_session: false, }); @@ -257,7 +267,9 @@ mod test { category: "test".into(), send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, - ..Default::default() + disabled: false, + dynamic_label: None, + in_session: false, }); let incorrects = vec![ diff --git a/glean-core/src/metrics/uuid.rs b/glean-core/src/metrics/uuid.rs index 291938a57f..30ed56eb46 100644 --- a/glean-core/src/metrics/uuid.rs +++ b/glean-core/src/metrics/uuid.rs @@ -10,6 +10,7 @@ use crate::common_metric_data::CommonMetricDataInternal; use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; use crate::metrics::Metric; use crate::metrics::MetricType; +use crate::storage::StorageManager; use crate::Glean; use crate::{CommonMetricData, TestGetValue}; @@ -113,7 +114,12 @@ impl UuidMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match glean.storage().get_metric(self.meta(), queried_ping_name) { + match StorageManager.snapshot_metric( + glean.storage(), + queried_ping_name, + &self.meta.identifier(glean), + self.meta.inner.lifetime, + ) { Some(Metric::Uuid(uuid)) => Uuid::parse_str(&uuid).ok(), _ => None, } diff --git a/glean-core/src/ping/mod.rs b/glean-core/src/ping/mod.rs index c1d0859903..9e35fc016e 100644 --- a/glean-core/src/ping/mod.rs +++ b/glean-core/src/ping/mod.rs @@ -83,7 +83,12 @@ impl PingMaker { ..Default::default() }); - let current_seq = match glean.storage().get_metric(seq.meta(), INTERNAL_STORAGE) { + let current_seq = match StorageManager.snapshot_metric( + glean.storage(), + INTERNAL_STORAGE, + &seq.meta().identifier(glean), + seq.meta().inner.lifetime, + ) { Some(Metric::Counter(i)) => i, _ => 0, }; @@ -279,6 +284,16 @@ impl PingMaker { info!("Collecting {}", ping.name()); let database = glean.storage(); + // HACK: Only for metrics pings we add the ping timings. + // But we want that to persist until the next metrics ping is actually sent. + let write_samples = database.write_timings.replace(Vec::with_capacity(64)); + if !write_samples.is_empty() { + glean + .database_metrics + .write_time + .accumulate_samples_sync(glean, &write_samples); + } + let mut metrics_data = StorageManager.snapshot_as_json(database, ping.name(), true); let events_data = glean diff --git a/glean-core/src/storage/mod.rs b/glean-core/src/storage/mod.rs index 21c79275eb..14198f1fa2 100644 --- a/glean-core/src/storage/mod.rs +++ b/glean-core/src/storage/mod.rs @@ -10,7 +10,8 @@ use std::collections::HashMap; use serde_json::{json, Value as JsonValue}; -use crate::database::sqlite::Database; +use crate::database::Database; +use crate::metrics::dual_labeled_counter::RECORD_SEPARATOR; use crate::metrics::Metric; use crate::Lifetime; @@ -28,7 +29,6 @@ pub struct StorageManager; fn snapshot_labeled_metrics( snapshot: &mut HashMap>, metric_id: &str, - label: &str, metric: &Metric, ) { // Explicit match for supported labeled metrics, avoiding the formatting string @@ -45,6 +45,9 @@ fn snapshot_labeled_metrics( }; let map = snapshot.entry(ping_section).or_default(); + // Safe unwrap, the function is only called when the id does contain a '/' + let (metric_id, label) = metric_id.split_once('/').unwrap(); + let obj = map.entry(metric_id.into()).or_insert_with(|| json!({})); let obj = obj.as_object_mut().unwrap(); // safe unwrap, we constructed the object above obj.insert(label.into(), metric.as_json()); @@ -58,21 +61,20 @@ fn snapshot_labeled_metrics( fn snapshot_dual_labeled_metrics( snapshot: &mut HashMap>, metric_id: &str, - key: &str, - category: &str, metric: &Metric, ) { let ping_section = format!("dual_labeled_{}", metric.ping_section()); let map = snapshot.entry(ping_section).or_default(); + let parts = metric_id.split(RECORD_SEPARATOR).collect::>(); let obj = map - .entry(metric_id.into()) + .entry(parts[0].into()) .or_insert_with(|| json!({})) .as_object_mut() .unwrap(); // safe unwrap, we constructed the object above - let key_obj = obj.entry(key).or_insert_with(|| json!({})); + let key_obj = obj.entry(parts[1].to_string()).or_insert_with(|| json!({})); let key_obj = key_obj.as_object_mut().unwrap(); - key_obj.insert(category.into(), metric.as_json()); + key_obj.insert(parts[2].into(), metric.as_json()); } impl StorageManager { @@ -118,42 +120,25 @@ impl StorageManager { ) -> Option { let mut snapshot: HashMap> = HashMap::new(); - let mut snapshotter = |metric_id: &[u8], labels: &[&str], metric: &Metric| { + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { let metric_id = String::from_utf8_lossy(metric_id).into_owned(); - match labels { - [] | [""] => { - let map = snapshot.entry(metric.ping_section().into()).or_default(); - map.insert(metric_id, metric.as_json()); - } - [label] => { - snapshot_labeled_metrics(&mut snapshot, &metric_id, label, metric); - } - [key, category] => { - snapshot_dual_labeled_metrics(&mut snapshot, &metric_id, key, category, metric); - } - other => { - log::error!( - "Unsupported list of labels encountered for metric {metric_id:?}: {other:?}. Metric will be ignored." - ); - } + if metric_id.contains('/') { + snapshot_labeled_metrics(&mut snapshot, &metric_id, metric); + } else if metric_id.split(RECORD_SEPARATOR).count() == 3 { + snapshot_dual_labeled_metrics(&mut snapshot, &metric_id, metric); + } else { + let map = snapshot.entry(metric.ping_section().into()).or_default(); + map.insert(metric_id, metric.as_json()); } }; - if let Err(e) = storage.iter_store(Lifetime::Ping, store_name, &mut snapshotter) { - log::debug!("could not snapshot ping lifetime store: {e:?}"); - } - if let Err(e) = storage.iter_store(Lifetime::Application, store_name, &mut snapshotter) { - log::debug!("could not snapshot application lifetime store: {e:?}"); - } - if let Err(e) = storage.iter_store(Lifetime::User, store_name, &mut snapshotter) { - log::debug!("could not snapshot user lifetime store: {e:?}"); - } + storage.iter_store_from(Lifetime::Ping, store_name, None, &mut snapshotter); + storage.iter_store_from(Lifetime::Application, store_name, None, &mut snapshotter); + storage.iter_store_from(Lifetime::User, store_name, None, &mut snapshotter); // Add send in all pings client.annotations if store_name != "glean_client_info" { - if let Err(e) = storage.iter_store(Lifetime::Application, "all-pings", snapshotter) { - log::debug!("could not snapshot metrics for 'all-pings': {e:?}"); - } + storage.iter_store_from(Lifetime::Application, "all-pings", None, snapshotter); } if clear_store { @@ -180,7 +165,7 @@ impl StorageManager { /// # Returns /// /// The decoded metric or `None` if no data is found. - pub fn _snapshot_metric( + pub fn snapshot_metric( &self, storage: &Database, store_name: &str, @@ -189,16 +174,15 @@ impl StorageManager { ) -> Option { let mut snapshot: Option = None; - let mut snapshotter = |id: &[u8], _labels: &[&str], metric: &Metric| { + let mut snapshotter = |id: &[u8], metric: &Metric| { let id = String::from_utf8_lossy(id).into_owned(); if id == metric_id { snapshot = Some(metric.clone()) } }; - storage - .iter_store(metric_lifetime, store_name, &mut snapshotter) - .ok()?; + storage.iter_store_from(metric_lifetime, store_name, None, &mut snapshotter); + snapshot } @@ -223,15 +207,17 @@ impl StorageManager { ) -> Vec { let mut labels = Vec::new(); - let mut snapshotter = |id: &[u8], found_labels: &[&str], _metric: &Metric| { - let id = String::from_utf8_lossy(id); - // Not doing this for dual-labeled metrics. - if id == metric_id && found_labels.len() == 1 { - labels.push(found_labels[0].to_string()); + let mut snapshotter = |id: &[u8], _metric: &Metric| { + let id = String::from_utf8_lossy(id).into_owned(); + if let Some((base_id, label)) = id.split_once('/') { + if base_id == metric_id { + labels.push(label.to_owned()); + } } }; - _ = storage.iter_store(metric_lifetime, store_name, &mut snapshotter); + storage.iter_store_from(metric_lifetime, store_name, None, &mut snapshotter); + labels } @@ -266,7 +252,7 @@ impl StorageManager { ) -> Option { let mut snapshot: HashMap = HashMap::new(); - let mut snapshotter = |metric_id: &[u8], _labels: &[&str], metric: &Metric| { + let mut snapshotter = |metric_id: &[u8], metric: &Metric| { let metric_id = String::from_utf8_lossy(metric_id).into_owned(); if metric_id.ends_with("#experiment") { let (name, _) = metric_id.split_once('#').unwrap(); // safe unwrap, we ensured there's a `#` in the string @@ -274,9 +260,7 @@ impl StorageManager { } }; - storage - .iter_store(Lifetime::Application, store_name, &mut snapshotter) - .ok()?; + storage.iter_store_from(Lifetime::Application, store_name, None, &mut snapshotter); if snapshot.is_empty() { None diff --git a/glean-core/tests/77ca0472-5124-4f6b-971d-4a2a928fb158.safe.bin b/glean-core/tests/77ca0472-5124-4f6b-971d-4a2a928fb158.safe.bin deleted file mode 100644 index 86ebf4231c9c5fd527a375acbef3fd3adfda408d..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1155 zcma)5%T9za6rJ(S#>B@;jWHP?3j=9O8HBZ;;K~i@K*wo<9c+unzqf^3A3?`yAm|lOhafi zjnLRpb*t4}IrQ!O*daZC!d2${?Fvwjqrjq??i{F{9%3+u?Yqy!? zD9LFO;g}MV_HAz1VmB+uJ~uk5nH5#L>$&Ewyk0w@5zl;W{$IzG5Q2LCPN>geY=nG* H(j9yNUA)yg diff --git a/glean-core/tests/clientid_textfile.rs b/glean-core/tests/clientid_textfile.rs index fec5a3e730..0f1ebf71ab 100644 --- a/glean-core/tests/clientid_textfile.rs +++ b/glean-core/tests/clientid_textfile.rs @@ -8,7 +8,8 @@ use crate::common::*; use std::fs; use std::path::Path; -use rusqlite::params; +use rkv::Rkv; +use rkv::StoreOptions; use uuid::Uuid; use glean_core::metrics::*; @@ -22,7 +23,7 @@ fn clientid_metric() -> UuidMetric { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - label: None, + dynamic_label: None, in_session: false, }) } @@ -126,9 +127,16 @@ fn clientid_regen_issue_with_existing_db() { // We modify the database and ONLY clear out the client id. { - let path = temp.path().join("db").join("glean.sqlite"); - let conn = rusqlite::Connection::open(path).unwrap(); - _ = conn.execute("DELETE FROM telemetry WHERE id = 'client_id'", ()); + let path = temp.path().join("db"); + let rkv = Rkv::new::(&path).unwrap(); + let user_store = rkv.open_single("user", StoreOptions::create()).unwrap(); + + // We know this. + let client_id_key = "glean_client_info#client_id"; + + let mut writer = rkv.write().unwrap(); + user_store.delete(&mut writer, client_id_key).unwrap(); + writer.commit().unwrap(); } let (glean, temp) = new_glean(Some(temp)); @@ -194,18 +202,23 @@ fn c0ffee_in_db_gets_overwritten_by_stored_client_id() { // We modify the database and ONLY set the client id to c0ffee. { - let path = temp.path().join("db").join("glean.sqlite"); - let conn = rusqlite::Connection::open(path).unwrap(); + let path = temp.path().join("db"); + let rkv = Rkv::new::(&path).unwrap(); + let user_store = rkv.open_single("user", StoreOptions::create()).unwrap(); + + // We know this. + let client_id_key = "glean_client_info#client_id"; - let encoded = rmp_serde::to_vec(&Metric::Uuid(String::from( + let mut writer = rkv.write().unwrap(); + let encoded = bincode::serialize(&Metric::Uuid(String::from( "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0", ))) .unwrap(); - - _ = conn.execute( - "UPDATE telemetry SET value = ? WHERE id = 'client_id'", - params![encoded], - ); + let known_client_id = rkv::Value::Blob(&encoded); + user_store + .put(&mut writer, client_id_key, &known_client_id) + .unwrap(); + writer.commit().unwrap(); } let (glean, temp) = new_glean(Some(temp)); diff --git a/glean-core/tests/collection_enabled.rs b/glean-core/tests/collection_enabled.rs index de36f415a7..78ae96dcfe 100644 --- a/glean-core/tests/collection_enabled.rs +++ b/glean-core/tests/collection_enabled.rs @@ -9,7 +9,6 @@ use glean_core::metrics::*; use glean_core::CommonMetricData; use glean_core::Glean; use glean_core::Lifetime; -use glean_core::{test_get_num_recorded_errors, ErrorType}; fn nofollows_ping(glean: &mut Glean) -> PingType { // When `follows_collection_enabled=false` then by default `enabled=false` @@ -199,39 +198,3 @@ fn queued_nofollows_pings_are_not_removed() { let (glean, _t) = new_glean(Some(t)); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); } - -#[test] -fn label_errors_when_collection_disabled() { - let (mut glean, _t) = new_glean(None); - - let manual_ping = manual_ping(&mut glean); - glean.set_ping_enabled(&manual_ping, false); - - let labeled = LabeledCounter::new( - LabeledMetricData::Common { - cmd: CommonMetricData { - name: "labeled_metric".into(), - category: "telemetry".into(), - send_in_pings: vec!["manual".into()], - disabled: false, - lifetime: Lifetime::Ping, - ..Default::default() - }, - }, - None, - ); - - let long_label = "a".repeat(112); - let metric = labeled.get(&long_label); - - // Attempt to increment the counter with zero - metric.add_sync(&glean, 1); - // Check that nothing was recorded. - assert!(metric.get_value(&glean, Some("manual")).is_none()); - - // Make sure that the error has been recorded - assert_eq!( - Ok(1), - test_get_num_recorded_errors(&glean, metric.meta(), ErrorType::InvalidLabel) - ); -} diff --git a/glean-core/tests/metrics_sync.rs b/glean-core/tests/metrics_sync.rs index a9cc2bd3d9..98c6c3b869 100644 --- a/glean-core/tests/metrics_sync.rs +++ b/glean-core/tests/metrics_sync.rs @@ -29,8 +29,6 @@ static DEFINITION_ONLY: &[&str] = &[ "glean.session_start", /* in foreign language wrapper */ "glean.validation.foreground_count", - /* kept active while Rkv-powered Glean is in use */ - "glean.database.rkv_load_error", ]; #[derive(Clone, Default, Debug, Eq, PartialEq)] diff --git a/glean-core/tests/ping.rs b/glean-core/tests/ping.rs index 50eff79746..dc2622bfc5 100644 --- a/glean-core/tests/ping.rs +++ b/glean-core/tests/ping.rs @@ -136,7 +136,7 @@ fn test_pings_submitted_metric() { send_in_pings: vec!["metrics".into(), "baseline".into()], lifetime: Lifetime::Ping, disabled: false, - label: None, + dynamic_label: None, ..Default::default() }, }, diff --git a/glean-core/tests/sqlite.rs b/glean-core/tests/sqlite.rs deleted file mode 100644 index 242e82ed3c..0000000000 --- a/glean-core/tests/sqlite.rs +++ /dev/null @@ -1,232 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -mod common; -use std::fs; - -use crate::common::*; - -use glean_core::metrics::*; -use glean_core::CommonMetricData; -use glean_core::Lifetime; -use rusqlite::params; -use rusqlite::TransactionBehavior; -use uuid::uuid; - -fn clientid_metric() -> UuidMetric { - UuidMetric::new(CommonMetricData { - name: "client_id".into(), - category: "".into(), - send_in_pings: vec!["glean_client_info".into()], - lifetime: Lifetime::User, - ..Default::default() - }) -} - -fn load_error_metric() -> StringMetric { - StringMetric::new(CommonMetricData { - name: "load_error".into(), - category: "glean.database".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }) -} - -#[test] -fn database_file_is_not_sqlite() { - let temp = { - let (glean, temp) = new_glean(None); - drop(glean); - temp - }; - - { - let path = temp.path().join("db").join("glean.sqlite"); - fs::remove_file(&path).unwrap(); - fs::write(&path, "not sqlite").unwrap(); - } - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None); - assert!(client_id.is_some()); -} - -#[test] -fn database_contains_wrong_table() { - let temp = { - let (glean, temp) = new_glean(None); - drop(glean); - temp - }; - - { - let path = temp.path().join("db").join("glean.sqlite"); - fs::remove_file(&path).unwrap(); - - let conn = rusqlite::Connection::open(path).unwrap(); - conn.execute("CREATE TABLE telemetry (a TEXT)", ()).unwrap(); - } - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None); - assert!(client_id.is_some()); - - let load_error = load_error_metric().get_value(&glean, None); - assert!(load_error.is_some()); -} - -#[test] -#[ignore] -fn database_contains_correct_user_version_but_wrong_table() { - let temp = { - let (glean, temp) = new_glean(None); - drop(glean); - temp - }; - - { - let path = temp.path().join("db").join("glean.sqlite"); - let conn = rusqlite::Connection::open(path).unwrap(); - conn.execute("DROP TABLE telemetry", ()).unwrap(); - conn.execute("CREATE TABLE telemetry (a TEXT)", ()).unwrap(); - } - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None); - assert!(client_id.is_some()); - - let load_error = load_error_metric().get_value(&glean, None); - assert!(load_error.is_some()); -} - -#[test] -fn invalid_msgpack_value() { - let (first_client_id, temp) = { - let (glean, temp) = new_glean(None); - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - drop(glean); - (client_id, temp) - }; - - { - let path = temp.path().join("db").join("glean.sqlite"); - let conn = rusqlite::Connection::open(path).unwrap(); - conn.execute( - "UPDATE telemetry SET value = ?1 WHERE id = 'client_id'", - params![b"c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0"], - ) - .unwrap(); - - // Also remove the client_id.txt so the client_id is not re-set from it. - fs::remove_file(temp.path().join("client_id.txt")).unwrap(); - } - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - let known_id = uuid!("c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0"); - assert_ne!(known_id, client_id); - assert_ne!(first_client_id, client_id); -} - -#[test] -fn higher_user_version_upgrade_does_not_crash() { - let (first_client_id, temp) = { - let (glean, temp) = new_glean(None); - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - drop(glean); - (client_id, temp) - }; - - { - let path = temp.path().join("db").join("glean.sqlite"); - let conn = rusqlite::Connection::open(path).unwrap(); - conn.execute_batch("PRAGMA user_version = 2").unwrap(); - } - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - assert_eq!(first_client_id, client_id); -} - -// Permissions only really work on Unix systems, definitely not on Windows -#[cfg(unix)] -mod unix { - use glean_core::Glean; - - use super::*; - - #[test] - fn database_permission_error() { - let temp = tempfile::tempdir().unwrap(); - - let db_path = temp.path().join("db"); - fs::create_dir_all(&db_path).unwrap(); - let path = db_path.join("glean.sqlite"); - fs::write(&path, "").unwrap(); - let attr = fs::metadata(&path).unwrap(); - let original_permissions = attr.permissions(); - let mut permissions = original_permissions.clone(); - permissions.set_readonly(true); - fs::set_permissions(&path, permissions).unwrap(); - - let cfg = glean_core::InternalConfiguration { - data_path: path.display().to_string(), - application_id: GLOBAL_APPLICATION_ID.into(), - language_binding_name: "Rust".into(), - upload_enabled: true, - max_events: None, - delay_ping_lifetime_io: false, - app_build: "Unknown".into(), - use_core_mps: false, - trim_data_to_registered_pings: false, - log_level: None, - rate_limit: None, - enable_event_timestamps: false, - experimentation_id: None, - enable_internal_pings: true, - ping_schedule: Default::default(), - ping_lifetime_threshold: 0, - ping_lifetime_max_time: 0, - max_pending_pings_count: None, - max_pending_pings_directory_size: None, - session_mode: glean_core::SessionMode::Auto, - session_sample_rate: 1.0, - session_inactivity_timeout_ms: 1_800_000, - }; - let glean = Glean::new(cfg); - assert!(glean.is_err()); - } -} - -// TODO(bug 2049295): -// This currently fails. -// The database is locked, so Glean can't access it. -// It's unclear how we should handle that. -// It's not a particular likely case to happen in practice. -#[test] -#[ignore] -fn database_externally_locked() { - let temp = { - let (glean, temp) = new_glean(None); - drop(glean); - temp - }; - - let path = temp.path().join("db").join("glean.sqlite"); - let mut conn = rusqlite::Connection::open(path).unwrap(); - let _tx = conn - .transaction_with_behavior(TransactionBehavior::Immediate) - .unwrap(); - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None); - assert!(client_id.is_some()); -} diff --git a/glean-core/tests/sqlite_migration.rs b/glean-core/tests/sqlite_migration.rs deleted file mode 100644 index 11fa1c32e1..0000000000 --- a/glean-core/tests/sqlite_migration.rs +++ /dev/null @@ -1,220 +0,0 @@ -mod common; -use std::fs; - -use crate::common::*; - -use glean_core::metrics::*; -use glean_core::CommonMetricData; -use glean_core::Lifetime; -use rkv::{Rkv, StoreOptions}; -use uuid::uuid; - -static RKV_DATABASE: &[u8] = include_bytes!("77ca0472-5124-4f6b-971d-4a2a928fb158.safe.bin"); - -fn clientid_metric() -> UuidMetric { - UuidMetric::new(CommonMetricData { - name: "client_id".into(), - category: "".into(), - send_in_pings: vec!["glean_client_info".into()], - lifetime: Lifetime::User, - ..Default::default() - }) -} - -/// Copy of parts of `DatabaseMetrics` in `glean-core/src/internal_metrics.rs` for ease of testing. -struct MigrationMetrics { - metrics_in_sqlite: CounterMetric, - migrated_metrics: CounterMetric, - failed_metrics: CounterMetric, - migration_duration: TimingDistributionMetric, - migration_error: CounterMetric, -} - -impl MigrationMetrics { - fn new() -> Self { - Self { - migrated_metrics: CounterMetric::new(CommonMetricData { - name: "migrated_metrics".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), - - metrics_in_sqlite: CounterMetric::new(CommonMetricData { - name: "metrics_in_sqlite".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), - - failed_metrics: CounterMetric::new(CommonMetricData { - name: "failed_metrics".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), - - migration_duration: TimingDistributionMetric::new( - CommonMetricData { - name: "migration_duration".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }, - TimeUnit::Millisecond, - ), - migration_error: CounterMetric::new(CommonMetricData { - name: "error".into(), - category: "glean.migration".into(), - send_in_pings: vec!["metrics".into(), "health".into()], - lifetime: Lifetime::Ping, - ..Default::default() - }), - } - } -} - -#[test] -fn migration_succeeds() { - let temp = tempfile::tempdir().unwrap(); - let db_path = temp.path().join("db"); - fs::create_dir_all(&db_path).unwrap(); - - let safe_bin = db_path.join("data.safe.bin"); - // File has been generated from essentially: - // - // ```rust - // let tmpname = PathBuf::new("/tmp/glean-fc"); - // let cfg = ConfigurationBuilder::new(true, tmpname.clone(), "glean-fc") - // .with_server_endpoint("invalid-test-host") - // .with_use_core_mps(false) - // .build(); - // glean::initialize(cfg, client_info); - // glean::shutdown(); - // ``` - // - // All ping-specific metrics have been removed. - // Only client_info metrics are migrated, including the client ID. - fs::write(safe_bin, RKV_DATABASE).unwrap(); - let exp_client_id = uuid!("77ca0472-5124-4f6b-971d-4a2a928fb158"); - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - assert_eq!(exp_client_id, client_id); - - let metrics = MigrationMetrics::new(); - assert_eq!(Some(13), metrics.migrated_metrics.get_value(&glean, None)); - assert_eq!(Some(13), metrics.metrics_in_sqlite.get_value(&glean, None)); - assert_eq!(None, metrics.failed_metrics.get_value(&glean, None)); - assert!(metrics.migration_duration.get_value(&glean, None).is_some()); - assert_eq!(None, metrics.migration_error.get_value(&glean, None)); -} - -#[test] -fn migration_skipped_if_database_exists() { - let (first_client_id, temp) = { - let (glean, temp) = new_glean(None); - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - drop(glean); - (client_id, temp) - }; - - let safe_bin = temp.path().join("db").join("data.safe.bin"); - fs::write( - &safe_bin, - include_bytes!("77ca0472-5124-4f6b-971d-4a2a928fb158.safe.bin"), - ) - .unwrap(); - let rkv_client_id = uuid!("77ca0472-5124-4f6b-971d-4a2a928fb158"); - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - assert_eq!( - first_client_id, client_id, - "Client ID should be the one first generated" - ); - assert_ne!( - rkv_client_id, client_id, - "Client ID should not be one from the Rkv database" - ); - assert!(safe_bin.exists(), "Rkv file should not have been deleted"); - - let metrics = MigrationMetrics::new(); - assert_eq!(None, metrics.migrated_metrics.get_value(&glean, None)); - assert_eq!(None, metrics.metrics_in_sqlite.get_value(&glean, None)); - assert_eq!(None, metrics.failed_metrics.get_value(&glean, None)); - assert_eq!(None, metrics.migration_duration.get_value(&glean, None)); - assert_eq!(None, metrics.migration_error.get_value(&glean, None)); -} - -#[test] -fn migration_succeeds_with_failures() { - let temp = tempfile::tempdir().unwrap(); - let db_path = temp.path().join("db"); - fs::create_dir_all(&db_path).unwrap(); - - let safe_bin = db_path.join("data.safe.bin"); - // Reusing the same database file from above. - fs::write(safe_bin, RKV_DATABASE).unwrap(); - let exp_client_id = uuid!("77ca0472-5124-4f6b-971d-4a2a928fb158"); - - // Modifying the database to force migration errors. - { - let rkv = Rkv::new::(&db_path).unwrap(); - let ping_store = rkv - .open_single(Lifetime::Ping.as_str(), StoreOptions::create()) - .unwrap(); - let mut writer = rkv.write().unwrap(); - - let key = "metrics#a.broken.metric"; - let value = rkv::Value::Blob(b"not a value"); - ping_store.put(&mut writer, key, &value).unwrap(); - - let key = "baseline#second.broken.metric"; - let value = rkv::Value::I64(31); - ping_store.put(&mut writer, key, &value).unwrap(); - - writer.commit().unwrap(); - } - - let (glean, _temp) = new_glean(Some(temp)); - - let client_id = clientid_metric().get_value(&glean, None).unwrap(); - assert_eq!(exp_client_id, client_id); - - let metrics = MigrationMetrics::new(); - assert_eq!(Some(13), metrics.migrated_metrics.get_value(&glean, None)); - assert_eq!(Some(13), metrics.metrics_in_sqlite.get_value(&glean, None)); - - // We injected 2 broken metrics. - assert_eq!(Some(2), metrics.failed_metrics.get_value(&glean, None)); - - assert!(metrics.migration_duration.get_value(&glean, None).is_some()); - assert_eq!(None, metrics.migration_error.get_value(&glean, None)); -} - -#[test] -fn migration_fails() { - let temp = tempfile::tempdir().unwrap(); - let db_path = temp.path().join("db"); - fs::create_dir_all(&db_path).unwrap(); - - let safe_bin = db_path.join("data.safe.bin"); - fs::write(safe_bin, b"\0").unwrap(); - - let (glean, _temp) = new_glean(Some(temp)); - - let metrics = MigrationMetrics::new(); - assert_eq!(Some(1), metrics.migration_error.get_value(&glean, None)); - - assert_eq!(None, metrics.migrated_metrics.get_value(&glean, None)); - assert_eq!(None, metrics.metrics_in_sqlite.get_value(&glean, None)); - assert_eq!(None, metrics.failed_metrics.get_value(&glean, None)); - assert_eq!(None, metrics.migration_duration.get_value(&glean, None)); -} diff --git a/samples/glean-sym-test/Cargo.lock b/samples/glean-sym-test/Cargo.lock index 7d4035a2b6..367a205c05 100644 --- a/samples/glean-sym-test/Cargo.lock +++ b/samples/glean-sym-test/Cargo.lock @@ -360,18 +360,6 @@ dependencies = [ "windows-sys", ] -[[package]] -name = "fallible-iterator" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" - -[[package]] -name = "fallible-streaming-iterator" -version = "0.1.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" - [[package]] name = "fastrand" version = "2.4.1" @@ -465,8 +453,6 @@ dependencies = [ "once_cell", "oslog", "rkv", - "rmp-serde", - "rusqlite", "serde", "serde_json", "thiserror", @@ -518,15 +504,6 @@ version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4f467dd6dccf739c208452f8014c75c18bb8301b050ad1cfb27153803edb0f51" -[[package]] -name = "hashlink" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7382cf6263419f2d8df38c55d7da83da5c18aef87fc7a7fc1fb1e344edfe14c1" -dependencies = [ - "hashbrown 0.15.5", -] - [[package]] name = "heck" version = "0.5.0" @@ -769,17 +746,6 @@ dependencies = [ "windows-targets", ] -[[package]] -name = "libsqlite3-sys" -version = "0.35.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "133c182a6a2c87864fe97778797e46c7e999672690dc9fa3ee8e241aa4a9c13f" -dependencies = [ - "cc", - "pkg-config", - "vcpkg", -] - [[package]] name = "linux-raw-sys" version = "0.12.1" @@ -902,12 +868,6 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" -[[package]] -name = "pkg-config" -version = "0.3.33" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e" - [[package]] name = "plain" version = "0.2.3" @@ -1023,39 +983,6 @@ dependencies = [ "wr_malloc_size_of", ] -[[package]] -name = "rmp" -version = "0.8.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ba8be72d372b2c9b35542551678538b562e7cf86c3315773cae48dfbfe7790c" -dependencies = [ - "num-traits", -] - -[[package]] -name = "rmp-serde" -version = "1.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72f81bee8c8ef9b577d1681a70ebbc962c232461e397b22c208c43c04b67a155" -dependencies = [ - "rmp", - "serde", -] - -[[package]] -name = "rusqlite" -version = "0.37.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "165ca6e57b20e1351573e3729b958bc62f0e48025386970b6e4d29e7a7e71f3f" -dependencies = [ - "bitflags", - "fallible-iterator", - "fallible-streaming-iterator", - "hashlink", - "libsqlite3-sys", - "smallvec", -] - [[package]] name = "rustc-hash" version = "2.1.2" @@ -1507,12 +1434,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "vcpkg" -version = "0.2.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426" - [[package]] name = "wasip2" version = "1.0.2+wasi-0.2.9" diff --git a/samples/glean-sym-test/Makefile b/samples/glean-sym-test/Makefile index 2bce144b2a..934b69d250 100644 --- a/samples/glean-sym-test/Makefile +++ b/samples/glean-sym-test/Makefile @@ -14,5 +14,5 @@ help: .PHONY: run run: ## Run the sample app - cargo build --locked + cargo build env $(LIB_PATH_NAME)=target/debug python3 app.py diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml index 365ad2ca92..17e13a4072 100644 --- a/supply-chain/audits.toml +++ b/supply-chain/audits.toml @@ -295,11 +295,6 @@ criteria = "safe-to-deploy" delta = "2.4.0 -> 2.4.1" notes = "Only allowing new clippy lints" -[[audits.bitflags]] -who = "Jan-Erik Rediger " -criteria = "safe-to-deploy" -delta = "2.9.4 -> 2.10.0" - [[audits.bstr]] who = "Jan-Erik Rediger " criteria = "safe-to-run" @@ -326,11 +321,6 @@ who = "Jan-Erik Rediger " criteria = "safe-to-deploy" delta = "1.0.78 -> 1.0.83" -[[audits.cc]] -who = "Jan-Erik Rediger " -criteria = "safe-to-deploy" -delta = "1.2.41 -> 1.2.53" - [[audits.chrono]] who = "Lars Eggert " criteria = "safe-to-deploy" @@ -391,16 +381,6 @@ who = "Lars Eggert " criteria = "safe-to-deploy" delta = "0.3.11 -> 0.4.0" -[[audits.fallible-iterator]] -who = "Jan-Erik Rediger " -criteria = "safe-to-deploy" -version = "0.2.0" - -[[audits.fallible-streaming-iterator]] -who = "Jan-Erik Rediger " -criteria = "safe-to-deploy" -version = "0.1.9" - [[audits.fd-lock]] who = "Jan-Erik Rediger " criteria = "safe-to-deploy" @@ -412,11 +392,6 @@ who = "Jan-Erik Rediger " criteria = "safe-to-deploy" delta = "3.0.13 -> 4.0.4" -[[audits.find-msvc-tools]] -who = "Jan-Erik Rediger " -criteria = "safe-to-deploy" -delta = "0.1.4 -> 0.1.8" - [[audits.flate2]] who = "Jan-Erik Rediger " criteria = "safe-to-deploy" @@ -596,16 +571,6 @@ criteria = "safe-to-deploy" delta = "0.19.0 -> 0.20.0" notes = "Removed all LMDB-specific code, added malloc_size_of integration" -[[audits.rmp]] -who = "Jan-Erik Rediger " -criteria = "safe-to-deploy" -delta = "0.8.14 -> 0.8.15" - -[[audits.rmp-serde]] -who = "Jan-Erik Rediger " -criteria = "safe-to-deploy" -delta = "1.3.0 -> 1.3.1" - [[audits.rustversion]] who = "Jan-Erik Rediger " criteria = "safe-to-deploy" @@ -838,18 +803,6 @@ criteria = "safe-to-deploy" delta = "1.3.0 -> 1.4.0" notes = "Maintained by me. I have written or reviewed all of the code." -[[trusted.cc]] -criteria = "safe-to-deploy" -user-id = 55123 # rust-lang-owner -start = "2022-10-29" -end = "2027-02-20" - -[[trusted.find-msvc-tools]] -criteria = "safe-to-deploy" -user-id = 539 # Josh Stone (cuviper) -start = "2025-08-29" -end = "2027-02-20" - [[trusted.hashbrown]] criteria = "safe-to-deploy" user-id = 55123 # rust-lang-owner diff --git a/supply-chain/config.toml b/supply-chain/config.toml index f1fa789c77..f1822213c3 100644 --- a/supply-chain/config.toml +++ b/supply-chain/config.toml @@ -86,7 +86,7 @@ criteria = "safe-to-deploy" [[exemptions.hashlink]] version = "0.7.0" -criteria = "safe-to-deploy" +criteria = "safe-to-run" [[exemptions.humantime]] version = "2.1.0" @@ -112,10 +112,6 @@ criteria = "safe-to-deploy" version = "0.8.6" criteria = "safe-to-deploy" -[[exemptions.libsqlite3-sys]] -version = "0.26.0" -criteria = "safe-to-deploy" - [[exemptions.memchr]] version = "2.5.0" criteria = "safe-to-deploy" @@ -181,10 +177,6 @@ criteria = "safe-to-run" version = "0.6.27" criteria = "safe-to-run" -[[exemptions.rusqlite]] -version = "0.27.0" -criteria = "safe-to-deploy" - [[exemptions.scroll]] version = "0.11.0" criteria = "safe-to-deploy" diff --git a/supply-chain/imports.lock b/supply-chain/imports.lock index e3726fa030..3abeb822f0 100644 --- a/supply-chain/imports.lock +++ b/supply-chain/imports.lock @@ -8,12 +8,6 @@ user-id = 696 user-login = "fitzgen" user-name = "Nick Fitzgerald" -[[publisher.cc]] -version = "1.2.30" -when = "2025-07-18" -user-id = 55123 -user-login = "rust-lang-owner" - [[publisher.encoding_rs]] version = "0.8.35" when = "2024-10-24" @@ -21,13 +15,6 @@ user-id = 4484 user-login = "hsivonen" user-name = "Henri Sivonen" -[[publisher.find-msvc-tools]] -version = "0.1.0" -when = "2025-08-29" -user-id = 539 -user-login = "cuviper" -user-name = "Josh Stone" - [[publisher.hashbrown]] version = "0.15.4" when = "2025-06-07" @@ -365,27 +352,6 @@ Nothing outside the realm of what one would expect from a bitflags generator, all as expected. """ -[[audits.bytecode-alliance.audits.bitflags]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -delta = "2.4.1 -> 2.6.0" -notes = """ -Changes in how macros are invoked and various bits and pieces of macro-fu. -Otherwise no major changes and nothing dealing with `unsafe`. -""" - -[[audits.bytecode-alliance.audits.bitflags]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -delta = "2.7.0 -> 2.9.4" -notes = "Tweaks to the macro, nothing out of order." - -[[audits.bytecode-alliance.audits.bitflags]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -delta = "2.10.0 -> 2.11.1" -notes = "Minor updates, nothing awry here." - [[audits.bytecode-alliance.audits.camino]] who = "Pat Hickey " criteria = "safe-to-deploy" @@ -418,8 +384,8 @@ notes = "Dependency updates and minor changes, nothing suspicious." [[audits.bytecode-alliance.audits.cc]] who = "Alex Crichton " criteria = "safe-to-deploy" -delta = "1.2.30 -> 1.2.41" -notes = "This is a trusted rust-lang/rust crate" +version = "1.0.73" +notes = "I am the author of this crate." [[audits.bytecode-alliance.audits.cfg-if]] who = "Alex Crichton " @@ -466,16 +432,6 @@ criteria = "safe-to-deploy" version = "0.1.2" notes = "This should be portable to any POSIX system and seems like it should be part of the libc crate, but at any rate it's safe as is." -[[audits.bytecode-alliance.audits.fallible-iterator]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -delta = "0.2.0 -> 0.3.0" -notes = """ -This major version update has a few minor breaking changes but everything -this crate has to do with iterators and `Result` and such. No `unsafe` or -anything like that, all looks good. -""" - [[audits.bytecode-alliance.audits.fastrand]] who = "Alex Crichton " criteria = "safe-to-deploy" @@ -491,11 +447,23 @@ criteria = "safe-to-deploy" delta = "2.1.1 -> 2.3.0" notes = "Minor refactoring, nothing new." -[[audits.bytecode-alliance.audits.find-msvc-tools]] -who = "Alex Crichton " +[[audits.bytecode-alliance.audits.fd-lock]] +who = "Pat Hickey " criteria = "safe-to-deploy" -delta = "0.1.0 -> 0.1.4" -notes = "Nothing out of the ordinary for a crate finding MSVC tooling." +version = "3.0.9" +notes = "This crate uses unsafe to make Windows syscalls, to borrow an Fd with an appropriate lifetime, and to zero a windows API structure that appears to have a valid representation with zeroed memory." + +[[audits.bytecode-alliance.audits.fd-lock]] +who = "Pat Hickey " +criteria = "safe-to-deploy" +delta = "3.0.9 -> 3.0.10" +notes = "Just a dependency version bump" + +[[audits.bytecode-alliance.audits.fd-lock]] +who = "Dan Gohman " +criteria = "safe-to-deploy" +delta = "3.0.10 -> 3.0.12" +notes = "Just a dependency version bump" [[audits.bytecode-alliance.audits.foldhash]] who = "Alex Crichton " @@ -648,12 +616,6 @@ criteria = "safe-to-deploy" delta = "0.7.1 -> 0.8.0" notes = "Minor updates, using new Rust features like `const`, no major changes." -[[audits.bytecode-alliance.audits.num-traits]] -who = "Andrew Brown " -criteria = "safe-to-deploy" -version = "0.2.19" -notes = "As advertised: a numeric library. The only `unsafe` is from some float-to-int conversions, which seems expected." - [[audits.bytecode-alliance.audits.percent-encoding]] who = "Alex Crichton " criteria = "safe-to-deploy" @@ -664,44 +626,12 @@ a few `unsafe` blocks related to utf-8 validation which are locally verifiable as correct and otherwise this crate is good to go. """ -[[audits.bytecode-alliance.audits.pkg-config]] -who = "Pat Hickey " -criteria = "safe-to-deploy" -version = "0.3.25" -notes = "This crate shells out to the pkg-config executable, but it appears to sanitize inputs reasonably." - -[[audits.bytecode-alliance.audits.pkg-config]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -delta = "0.3.26 -> 0.3.29" -notes = """ -No `unsafe` additions or anything outside of the purview of the crate in this -change. -""" - -[[audits.bytecode-alliance.audits.pkg-config]] -who = "Chris Fallin " -criteria = "safe-to-deploy" -delta = "0.3.29 -> 0.3.32" - [[audits.bytecode-alliance.audits.semver]] who = "Pat Hickey " criteria = "safe-to-deploy" version = "1.0.17" notes = "plenty of unsafe pointer and vec tricks, but in well-structured and commented code that appears to be correct" -[[audits.bytecode-alliance.audits.shlex]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -version = "1.1.0" -notes = "Only minor `unsafe` code blocks which look valid and otherwise does what it says on the tin." - -[[audits.bytecode-alliance.audits.smallvec]] -who = "Alex Crichton " -criteria = "safe-to-deploy" -delta = "1.13.2 -> 1.14.0" -notes = "Minor new feature, nothing out of the ordinary." - [[audits.bytecode-alliance.audits.static_assertions]] who = "Andrew Brown " criteria = "safe-to-deploy" @@ -768,12 +698,6 @@ is similar to what it once was back then. Skimming over the crate there is nothing suspicious and it's everything you'd expect a Rust URL parser to be. """ -[[audits.bytecode-alliance.audits.vcpkg]] -who = "Pat Hickey " -criteria = "safe-to-deploy" -version = "0.2.15" -notes = "no build.rs, no macros, no unsafe. It reads the filesystem and makes copies of DLLs into OUT_DIR." - [[audits.bytecode-alliance.audits.walkdir]] who = "Andrew Brown " criteria = "safe-to-deploy" @@ -1823,12 +1747,6 @@ delta = "1.0.218 -> 1.0.219" notes = "Minor changes (clippy tweaks, using `mem::take` instead of `mem::replace`)." aggregated-from = "https://chromium.googlesource.com/chromium/src/+/main/third_party/rust/chromium_crates_io/supply-chain/audits.toml?format=TEXT" -[[audits.google.audits.smallvec]] -who = "Manish Goregaokar " -criteria = "safe-to-deploy" -version = "1.13.2" -aggregated-from = "https://chromium.googlesource.com/chromium/src/+/main/third_party/rust/chromium_crates_io/supply-chain/audits.toml?format=TEXT" - [[audits.google.audits.synstructure]] who = "Manish Goregaokar " criteria = "safe-to-deploy" @@ -2085,13 +2003,6 @@ start = "2025-03-18" end = "2027-04-23" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.android-tzdata]] -who = "Mark Hammond " -criteria = "safe-to-deploy" -version = "0.1.1" -notes = "Small crate parsing a file. No unsafe code" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - [[audits.mozilla.audits.android_logger]] who = "Jan-Erik Rediger " criteria = "safe-to-deploy" @@ -2224,13 +2135,10 @@ criteria = "safe-to-deploy" delta = "2.3.3 -> 2.4.0" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.bitflags]] -who = [ - "Teodor Tanasoaia ", - "Erich Gubler ", -] +[[audits.mozilla.audits.cc]] +who = "Mike Hommey " criteria = "safe-to-deploy" -delta = "2.6.0 -> 2.7.0" +delta = "1.0.73 -> 1.0.78" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" [[audits.mozilla.audits.chrono]] @@ -2404,30 +2312,6 @@ criteria = "safe-to-deploy" delta = "0.2.171 -> 0.2.176" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.libsqlite3-sys]] -who = "Mark Hammond " -criteria = "safe-to-deploy" -delta = "0.26.0 -> 0.27.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.libsqlite3-sys]] -who = "Mark Hammond " -criteria = "safe-to-deploy" -delta = "0.27.0 -> 0.28.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.libsqlite3-sys]] -who = "Erich Gubler " -criteria = "safe-to-deploy" -delta = "0.28.0 -> 0.31.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.libsqlite3-sys]] -who = "Mark Hammond " -criteria = "safe-to-deploy" -delta = "0.31.0 -> 0.35.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - [[audits.mozilla.audits.malloc_size_of_derive]] who = "Bobby Holley " criteria = "safe-to-deploy" @@ -2439,6 +2323,13 @@ but convinced myself that any generated code will be entirely safe to deploy. """ aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" +[[audits.mozilla.audits.num-traits]] +who = "Josh Stone " +criteria = "safe-to-deploy" +version = "0.2.15" +notes = "All code written or reviewed by Josh Stone." +aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" + [[audits.mozilla.audits.once_cell]] who = "Erich Gubler " criteria = "safe-to-deploy" @@ -2458,12 +2349,6 @@ criteria = "safe-to-deploy" delta = "1.20.3 -> 1.21.1" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.pkg-config]] -who = "Mike Hommey " -criteria = "safe-to-deploy" -delta = "0.3.25 -> 0.3.26" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - [[audits.mozilla.audits.rayon]] who = "Josh Stone " criteria = "safe-to-deploy" @@ -2495,60 +2380,6 @@ criteria = "safe-to-deploy" version = "0.18.4" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.rmp]] -who = "Ben Dean-Kawamura " -criteria = "safe-to-deploy" -version = "0.8.14" -notes = """ -Very popular crate. 1 instance of unsafe code, which is used to adjust a slice to work around -lifetime issues. No network or file access. -""" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.rmp-serde]] -who = "Ben Dean-Kawamura " -criteria = "safe-to-deploy" -version = "1.3.0" -notes = "Very popular crate. No unsafe code, network or file access." -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.rusqlite]] -who = "Mike Hommey " -criteria = "safe-to-deploy" -delta = "0.27.0 -> 0.28.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.rusqlite]] -who = "Ben Dean-Kawamura " -criteria = "safe-to-deploy" -delta = "0.28.0 -> 0.29.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.rusqlite]] -who = "Mark Hammond " -criteria = "safe-to-deploy" -delta = "0.29.0 -> 0.30.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.rusqlite]] -who = "Mark Hammond " -criteria = "safe-to-deploy" -delta = "0.30.0 -> 0.31.0" -notes = "Mostly build and dependency related changes, and bump to sqlite version" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.rusqlite]] -who = "Erich Gubler " -criteria = "safe-to-deploy" -delta = "0.31.0 -> 0.33.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.rusqlite]] -who = "Mark Hammond " -criteria = "safe-to-deploy" -delta = "0.33.0 -> 0.37.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - [[audits.mozilla.audits.rustc-hash]] who = "Bobby Holley " criteria = "safe-to-deploy" @@ -2588,12 +2419,6 @@ version = "1.0.3" notes = "Relatively simple Serde trait implementations. No IO or unsafe code." aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.shlex]] -who = "Max Inden " -criteria = "safe-to-deploy" -delta = "1.1.0 -> 1.3.0" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - [[audits.mozilla.audits.siphasher]] who = "Emilio Cobos Álvarez " criteria = "safe-to-deploy" @@ -2601,18 +2426,6 @@ delta = "0.3.11 -> 1.0.1" notes = "Only change to the crate source is adding documentation." aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" -[[audits.mozilla.audits.smallvec]] -who = "Erich Gubler " -criteria = "safe-to-deploy" -delta = "1.14.0 -> 1.15.1" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - -[[audits.mozilla.audits.smallvec]] -who = "Mike Hommey " -criteria = "safe-to-deploy" -delta = "1.15.1 -> 1.15.2" -aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" - [[audits.mozilla.audits.tempfile]] who = "Mike Hommey " criteria = "safe-to-deploy" From 239e999f914473ebac2432e52a88c5da911066f5 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Fri, 19 Jun 2026 15:56:35 +0200 Subject: [PATCH 2/4] Restore migration metrics These are removed by reverting the SQLite merge. We need to keep them alive, because we will get them back later again when we re-merge SQLite. --- glean-core/metrics.yaml | 111 +++++++++++++++++++++++++++++++ glean-core/tests/metrics_sync.rs | 7 ++ 2 files changed, 118 insertions(+) diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml index 3a6fb3fc79..27176681ae 100644 --- a/glean-core/metrics.yaml +++ b/glean-core/metrics.yaml @@ -972,6 +972,23 @@ glean.database: - glean-team@mozilla.com expires: never + load_error: + type: string + description: | + If there was an error loading the sqlite database, record it. + send_in_pings: + - metrics + - health + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_sensitivity: + - technical + notification_emails: + - glean-team@mozilla.com + expires: never + write_time: type: timing_distribution time_unit: microsecond @@ -1408,3 +1425,97 @@ glean.health: - permission-denied - io - not-found + +glean.migration: + failed_metrics: + type: counter + lifetime: ping + description: | + Number of metrics that failed to deserialize from storage + while iterating the Rkv database for migration. + notification_emails: + - glean-team@mozilla.com + - jrediger@mozilla.com + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_sensitivity: + - technical + expires: never + send_in_pings: + - metrics + - health + metrics_in_sqlite: + type: counter + lifetime: ping + description: | + The number of metrics stored in SQLite after a migration run. + notification_emails: + - glean-team@mozilla.com + - jrediger@mozilla.com + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_sensitivity: + - technical + expires: never + send_in_pings: + - metrics + - health + migrated_metrics: + type: counter + lifetime: ping + description: | + The number of metrics migrated from Rkv storage to SQLite storage. + notification_emails: + - glean-team@mozilla.com + - jrediger@mozilla.com + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_sensitivity: + - technical + expires: never + send_in_pings: + - metrics + - health + migration_duration: + type: timespan + lifetime: ping + time_unit: millisecond + description: | + The duration for one full migration run at startup. + notification_emails: + - glean-team@mozilla.com + - jrediger@mozilla.com + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_sensitivity: + - technical + expires: never + send_in_pings: + - metrics + - health + error: + type: counter + lifetime: ping + description: | + The number of times a migration was attempted and failed. + notification_emails: + - glean-team@mozilla.com + - jrediger@mozilla.com + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=2034605 + data_sensitivity: + - technical + expires: never + send_in_pings: + - metrics + - health diff --git a/glean-core/tests/metrics_sync.rs b/glean-core/tests/metrics_sync.rs index 98c6c3b869..ba91d07070 100644 --- a/glean-core/tests/metrics_sync.rs +++ b/glean-core/tests/metrics_sync.rs @@ -29,6 +29,13 @@ static DEFINITION_ONLY: &[&str] = &[ "glean.session_start", /* in foreign language wrapper */ "glean.validation.foreground_count", + /* sqlite migration metrics */ + "glean.database.load_error", + "glean.migration.error", + "glean.migration.failed_metrics", + "glean.migration.metrics_in_sqlite", + "glean.migration.migrated_metrics", + "glean.migration.migration_duration", ]; #[derive(Clone, Default, Debug, Eq, PartialEq)] From 6756cb8a7d35936788661ce7c1c0166bbb706c7a Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Mon, 22 Jun 2026 17:33:31 +0200 Subject: [PATCH 3/4] Restore changelog for v69.0.0 --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28f96ba6ae..f8bf33ff66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ [Full changelog](https://github.com/mozilla/glean/compare/v68.0.0...v69.0.0) +* General + * BREAKING CHANGE: Replaced the client-side storage backend with a SQLite-powered one. + This is major change and large refactoring of the Glean SDK internals, but has very limited breaking changes in the + public API (breaking changes in the Rust API only). + Data is migrated from the old rkv-powered storage to the SQLite database on first start with this Glean SDK version. + The Rkv data is retained for now. Only the SQLite-backed data is written to for new metric recordings. + New `glean.migration.*` metrics were added to monitor and validate the migration process. + Other monitoring metrics are preserved. + The `glean.database.rkv_load_error` has been replaced by a more generic `glean.database.load_error` with similar semantics. + # v68.0.0 (2026-06-19) [Full changelog](https://github.com/mozilla/glean/compare/v67.5.0...v68.0.0) From f4426773d34208558efaa927b4efce5dc1dd1d49 Mon Sep 17 00:00:00 2001 From: Jan-Erik Rediger Date: Fri, 19 Jun 2026 16:08:32 +0200 Subject: [PATCH 4/4] Remove left-over SQLite files --- glean-core/src/database/mod.rs | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/glean-core/src/database/mod.rs b/glean-core/src/database/mod.rs index 8408aa1f18..8da37e7675 100644 --- a/glean-core/src/database/mod.rs +++ b/glean-core/src/database/mod.rs @@ -19,6 +19,8 @@ use crate::ErrorKind; use malloc_size_of::MallocSizeOf; use rkv::{StoreError, StoreOptions}; +const SQLITE_DATABASE_FILES: &[&str] = &["glean.sqlite", "glean.sqlite-wal", "glean.sqlite-shm"]; + /// Unwrap a `Result`s `Ok` value or do the specified action. /// /// This is an alternative to the question-mark operator (`?`), @@ -265,6 +267,14 @@ impl Database { db.load_ping_lifetime_data(); + // SQLite storage cleanup -- removing potential left-over files from previous migration. + for file in SQLITE_DATABASE_FILES { + let path = path.join(file); + if fs::remove_file(&path).is_ok() { + log::debug!("Removed SQLite file: {}", path.display()); + } + } + Ok(db) } @@ -1619,5 +1629,33 @@ mod test { "Load error recorded" ); } + + #[test] + fn deletes_leftover_sqlite_files() { + let dir = tempdir().unwrap(); + + // Create database directory structure. + let database_dir = dir.path().join("db"); + fs::create_dir_all(&database_dir).expect("create database dir"); + + // Create left-over SQLite files. + for &file in SQLITE_DATABASE_FILES { + let path = database_dir.join(file); + fs::write(path, "").expect("write to database file"); + } + + let _db = Database::new(dir.path(), false, 0, Duration::ZERO).unwrap(); + + assert!(dir.path().exists()); + for &file in SQLITE_DATABASE_FILES { + let path = database_dir.join(file); + assert!( + !path.exists(), + "{} should have been removed when opening the database", + path.display() + ); + fs::write(path, "").expect("write to database file"); + } + } } }