diff --git a/Cargo.lock b/Cargo.lock index 344b205ab7..a9c3fafce7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -160,11 +160,11 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.4.1" +version = "2.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "327762f6e5a765692301e5bb513e0d9fef63be86bbc14528052b1cd3e6f03e07" +checksum = "812e12b5285cc515a9c72a5c1d3b6d46a19dac5acfef5265968c166106e31dd3" dependencies = [ - "serde", + "serde_core", ] [[package]] @@ -230,11 +230,12 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" [[package]] name = "cc" -version = "1.0.83" +version = "1.2.53" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1174fb0b6ec23863f8b971027804a42614e347eafb0a95bf0b12cdae21fc4d0" +checksum = "755d2fce177175ffca841e9a06afdb2c4ab0f593d53b4dee48147dfaade85932" dependencies = [ - "libc", + "find-msvc-tools", + "shlex", ] [[package]] @@ -477,6 +478,18 @@ 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" @@ -494,6 +507,12 @@ dependencies = [ "windows-sys", ] +[[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" @@ -599,6 +618,8 @@ dependencies = [ "pulldown-cmark", "pulldown-cmark-to-cmark", "rkv", + "rmp-serde", + "rusqlite", "serde", "serde_json", "tempfile", @@ -619,6 +640,7 @@ dependencies = [ "env_logger", "flate2", "glean", + "glean-build", "libc", "log", "once_cell", @@ -833,6 +855,17 @@ version = "0.2.176" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "58f929b4d672ea937a23a1ab494143d968337a5f47e56d0815df1e0890ddf174" +[[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" @@ -889,9 +922,9 @@ dependencies = [ [[package]] name = "num-traits" -version = "0.2.15" +version = "0.2.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "578ede34cf02f8924ab9447f50c28075b4d3e5b269972345e7e0372b38c6cdcd" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" dependencies = [ "autocfg", ] @@ -944,6 +977,12 @@ 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" @@ -1020,7 +1059,7 @@ version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e8bbe1a966bd2f362681a44f6edce3c2310ac21e4d5067a6e7ec396297a6ea0" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.10.0", "memchr", "unicase", ] @@ -1115,7 +1154,7 @@ checksum = "0f67a9dbc634fcd36a2d1d800ca818065dcf71a1d907dc35130c2d1552c6e1dc" dependencies = [ "arrayref", "bincode", - "bitflags 2.4.1", + "bitflags 2.10.0", "id-arena", "lazy_static", "log", @@ -1129,6 +1168,39 @@ 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.10.0", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "smallvec", +] + [[package]] name = "rustc-hash" version = "2.1.1" @@ -1141,7 +1213,7 @@ version = "0.38.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "67ce50cb2e16c2903e30d1cbccfd8387a74b9d4c938b6a4c5ec6cc7556f7a8a0" dependencies = [ - "bitflags 2.4.1", + "bitflags 2.10.0", "errno", "libc", "linux-raw-sys", @@ -1261,12 +1333,24 @@ 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.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67b1b7a3b5fe4f1376887184045fcf45c69e92af734b7aaddc05fb777b6fbd03" + [[package]] name = "smawk" version = "0.3.2" @@ -1601,6 +1685,12 @@ 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 447fb4b67e..71f5bdd897 100644 --- a/glean-core/Cargo.toml +++ b/glean-core/Cargo.toml @@ -44,6 +44,8 @@ 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 37be0ce779..a11de53917 100644 --- a/glean-core/benchmark/Cargo.lock +++ b/glean-core/benchmark/Cargo.lock @@ -477,6 +477,18 @@ 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" @@ -499,6 +511,12 @@ 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" @@ -543,6 +561,8 @@ dependencies = [ "once_cell", "oslog", "rkv", + "rmp-serde", + "rusqlite", "serde", "serde_json", "thiserror", @@ -624,12 +644,30 @@ 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" @@ -781,7 +819,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7714e70437a7dc3ac8eb7e6f8df75fd8eb422675fc7678aff7364301092b1017" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.16.1", "serde", "serde_core", ] @@ -833,6 +871,17 @@ 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" @@ -961,6 +1010,12 @@ 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" @@ -1131,6 +1186,39 @@ 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" @@ -1601,6 +1689,12 @@ 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/metrics.yaml b/glean-core/metrics.yaml index 0f4b8aac5a..eb61cca5ef 100644 --- a/glean-core/metrics.yaml +++ b/glean-core/metrics.yaml @@ -924,17 +924,17 @@ glean.database: - glean-team@mozilla.com expires: never - rkv_load_error: + load_error: type: string description: | - If there was an error loading the RKV database, record it. + 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=1815253 + - https://bugzilla.mozilla.org/show_bug.cgi?id=TODO data_reviews: - - https://bugzilla.mozilla.org/show_bug.cgi?id=1815253 + - https://bugzilla.mozilla.org/show_bug.cgi?id=TODO data_sensitivity: - technical notification_emails: diff --git a/glean-core/python/glean/_loader.py b/glean-core/python/glean/_loader.py index 00cb48c9ec..ca50f388cb 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 "dynamic_label" not in args: - args["dynamic_label"] = None + if "label" not in args: + args["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 9e3ed68f8e..ba0733e024 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"], - dynamic_label=None, + label=None, ) ) @@ -35,7 +35,7 @@ def test_disabled_booleans_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="boolean_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_counter.py b/glean-core/python/tests/metrics/test_counter.py index a8231bd26e..fd6a2f9d24 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"], - dynamic_label=None, + label=None, ) ) @@ -43,7 +43,7 @@ def test_disabled_counters_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -78,7 +78,7 @@ def test_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1", "store2"], - dynamic_label=None, + label=None, ) ) @@ -104,7 +104,7 @@ def test_negative_values_are_not_counted(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_datetime.py b/glean-core/python/tests/metrics/test_datetime.py index b58d6ccf87..d8f0d72694 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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 398861e362..f6b5c35564 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ), allowed_extra_keys=["object_id"], ) @@ -191,7 +191,7 @@ class EventKeys(enum.Enum): lifetime=Lifetime.PING, name="click", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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 eac757826f..3ba30baec3 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) ) @@ -64,7 +64,7 @@ def test_labeled_boolean_type(): lifetime=Lifetime.APPLICATION, name="labeled_boolean_metric", send_in_pings=["metrics"], - dynamic_label=None, + label=None, ) ) ) @@ -86,7 +86,7 @@ def test_labeled_string_type(): lifetime=Lifetime.APPLICATION, name="labeled_string_metric", send_in_pings=["metrics"], - dynamic_label=None, + label=None, ) ) ) @@ -108,7 +108,7 @@ def test_labeled_quantity_type(): lifetime=Lifetime.APPLICATION, name="labeled_quantity_metric", send_in_pings=["metrics"], - dynamic_label=None, + label=None, ) ) ) @@ -129,7 +129,7 @@ def test_other_label_with_predefined_labels(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) ) @@ -214,7 +214,7 @@ def test_invalid_labels_go_to_other(): lifetime=Lifetime.APPLICATION, name="labeled_counter_metric", send_in_pings=["metrics"], - dynamic_label=None, + label=None, ) ) ) @@ -251,7 +251,7 @@ def test_rapidly_recreating_labeled_metrics_does_not_crash(): send_in_pings=["metrics"], lifetime=Lifetime.APPLICATION, disabled=False, - dynamic_label=None, + 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 a4b4d4dc52..b346027039 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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 15ee6a8556..b83432ee45 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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 80a49841b5..84ba6b54c9 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"], - dynamic_label=None, + label=None, ) ) @@ -42,7 +42,7 @@ def test_disabled_quantities_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -77,7 +77,7 @@ def test_api_saves_to_secondary_pings(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1", "store2"], - dynamic_label=None, + label=None, ) ) @@ -101,7 +101,7 @@ def test_negative_values_are_not_counted(): lifetime=Lifetime.APPLICATION, name="quantity_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_rate.py b/glean-core/python/tests/metrics/test_rate.py index 770cddcbe8..3650d5ed9e 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"], - dynamic_label=None, + label=None, disabled=False, ), ) @@ -55,7 +55,7 @@ def test_numerator_smoke(): name="numerator", lifetime=Lifetime.PING, send_in_pings=["store1"], - dynamic_label=None, + label=None, disabled=False, ), ) @@ -87,7 +87,7 @@ def test_denominator_smoke(): name="rate1", lifetime=Lifetime.PING, send_in_pings=["store1"], - dynamic_label=None, + label=None, disabled=False, ) @@ -96,7 +96,7 @@ def test_denominator_smoke(): name="rate2", lifetime=Lifetime.PING, send_in_pings=["store1"], - dynamic_label=None, + label=None, disabled=False, ) @@ -107,7 +107,7 @@ def test_denominator_smoke(): name="counter", lifetime=Lifetime.PING, send_in_pings=["store1"], - dynamic_label=None, + 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 c5bc118454..d985e0c438 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"], - dynamic_label=None, + label=None, ) ) @@ -37,7 +37,7 @@ def test_disabled_strings_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="string_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -92,7 +92,7 @@ def test_setting_a_string_as_none(): lifetime=Lifetime.APPLICATION, name="string_metric", send_in_pings=["store1", "store2"], - dynamic_label=None, + 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 1309100cc0..102bab37ae 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -84,7 +84,7 @@ def test_test_get_value_throws(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -124,7 +124,7 @@ def test_long_string_lists_are_truncated(): lifetime=Lifetime.APPLICATION, name="string_list_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_text.py b/glean-core/python/tests/metrics/test_text.py index 8eb52f07e7..41b1fd0a6c 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"], - dynamic_label=None, + label=None, disabled=False, ), ) @@ -37,7 +37,7 @@ def test_text_truncation(): name="text", lifetime=Lifetime.PING, send_in_pings=["store1"], - dynamic_label=None, + 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 59f8b9a196..aa0d4e4e7c 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ), time_unit=TimeUnit.SECOND, ) @@ -248,7 +248,7 @@ def test_measure(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -267,7 +267,7 @@ def test_measure_exception(): lifetime=Lifetime.APPLICATION, name="timespan_metric", send_in_pings=["store1"], - dynamic_label=None, + 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 7ce9f37993..74a05b5afe 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -120,7 +120,7 @@ def test_measure(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["baseline"], - dynamic_label=None, + label=None, ), time_unit=TimeUnit.NANOSECOND, ) @@ -143,7 +143,7 @@ def test_measure_exception(): lifetime=Lifetime.APPLICATION, name="timing_distribution", send_in_pings=["baseline"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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 c9cf82210a..fe2d87bc47 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"], - dynamic_label=None, + label=None, ) ) @@ -37,7 +37,7 @@ def test_disabled_urls_must_not_record_data(): lifetime=Lifetime.APPLICATION, name="url_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -108,7 +108,7 @@ def test_setting_a_url_as_none(): lifetime=Lifetime.APPLICATION, name="url_metric", send_in_pings=["store1", "store2"], - dynamic_label=None, + label=None, ) ) diff --git a/glean-core/python/tests/metrics/test_uuid.py b/glean-core/python/tests/metrics/test_uuid.py index 071c16e9de..9402264869 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"], - dynamic_label=None, + label=None, ) ) @@ -52,7 +52,7 @@ def test_disabled_uuids_must_not_record_data(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -107,7 +107,7 @@ def test_invalid_uuid_must_not_crash(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) @@ -126,7 +126,7 @@ def test_invalid_uuid_string(): lifetime=Lifetime.PING, name="uuid_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) diff --git a/glean-core/python/tests/test_collection_enabled.py b/glean-core/python/tests/test_collection_enabled.py index a1dbb0470e..5925290597 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"], - dynamic_label=None, + label=None, ) ) @@ -93,7 +93,7 @@ def test_loader_sets_flags(tmpdir, helpers): lifetime=Lifetime.PING, name="counter", send_in_pings=["nofollows-defined"], - dynamic_label=None, + label=None, ) ) diff --git a/glean-core/python/tests/test_glean.py b/glean-core/python/tests/test_glean.py index 65013eab23..860d303721 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"], - dynamic_label=None, + label=None, ) ) @@ -105,7 +105,7 @@ def test_disabling_upload_should_disable_metrics_recording(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) @@ -198,7 +198,7 @@ def test_queued_recorded_metrics_correctly_during_init(): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - dynamic_label=None, + 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"], - dynamic_label=None, + 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], - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) @@ -503,7 +503,7 @@ def test_configuration_property(safe_httpserver): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["baseline"], - dynamic_label=None, + label=None, ) ) @@ -676,7 +676,7 @@ def test_clear_application_lifetime_metrics(tmpdir): lifetime=Lifetime.APPLICATION, name="lifetime_reset", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) @@ -881,7 +881,7 @@ def test_sending_of_custom_pings(safe_httpserver): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) @@ -952,7 +952,7 @@ def test_max_events_overflow(tmpdir, helpers): lifetime=Lifetime.APPLICATION, name="event", send_in_pings=["events"], - dynamic_label=None, + label=None, ), allowed_extra_keys=[], ) @@ -1001,7 +1001,7 @@ def test_glean_shutdown(safe_httpserver): send_in_pings=["custom"], lifetime=Lifetime.APPLICATION, disabled=False, - dynamic_label=None, + label=None, ) ) @@ -1056,7 +1056,7 @@ def test_uploader_capabilities_reported(tmpdir, helpers): lifetime=Lifetime.APPLICATION, name="counter_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) @@ -1107,7 +1107,7 @@ def test_uploader_capabilities_empty_not_reported(tmpdir, helpers): lifetime=Lifetime.PING, name="stringlist_metric", send_in_pings=["store1"], - dynamic_label=None, + label=None, ) ) @@ -1243,7 +1243,7 @@ def test_uploader_capabilities_in_events_ping(tmpdir, helpers): name="custom", lifetime=Lifetime.PING, send_in_pings=["custom-events"], - dynamic_label=None, + label=None, ), allowed_extra_keys=[], ) diff --git a/glean-core/python/tests/test_network.py b/glean-core/python/tests/test_network.py index 89bd5d750c..d84d7fea0d 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, - dynamic_label=None, + 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"], - dynamic_label=None, + label=None, ) ) counter_metric.add(10) diff --git a/glean-core/rlb-tests/Cargo.toml b/glean-core/rlb-tests/Cargo.toml index 4a879203af..77be9081fb 100644 --- a/glean-core/rlb-tests/Cargo.toml +++ b/glean-core/rlb-tests/Cargo.toml @@ -17,3 +17,6 @@ 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 new file mode 100644 index 0000000000..8a17552ca4 --- /dev/null +++ b/glean-core/rlb-tests/build.rs @@ -0,0 +1,9 @@ +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 new file mode 100644 index 0000000000..f8c0146131 --- /dev/null +++ b/glean-core/rlb-tests/metrics.yaml @@ -0,0 +1,257 @@ +# 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/. + +# This file defines the metrics that are recorded by the Glean SDK. They are +# automatically converted to Rust code at build time using the `glean_parser` +# PyPI package. + +--- + +$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 new file mode 100644 index 0000000000..5abc28a675 --- /dev/null +++ b/glean-core/rlb-tests/pings.yaml @@ -0,0 +1,22 @@ +# 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/. + +# This file defines the built-in pings that are recorded by the Glean SDK. They +# are automatically converted to Rust code at build time using the +# `glean_parser` PyPI package. + +--- + +$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 new file mode 100644 index 0000000000..1ddd5933b8 --- /dev/null +++ b/glean-core/rlb-tests/src/bin/verify-data.rs @@ -0,0 +1,254 @@ +// 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, + }; + + _ = &*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 ddf712b995..71bc65a68a 100644 --- a/glean-core/rlb-tests/tests/it.rs +++ b/glean-core/rlb-tests/tests/it.rs @@ -295,3 +295,19 @@ 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 new file mode 100644 index 0000000000..8858f9339d Binary files /dev/null and b/glean-core/rlb-tests/tests/rkv-database.safe.bin differ diff --git a/glean-core/rlb/src/private/event.rs b/glean-core/rlb/src/private/event.rs index 3ad16c11d5..378f7bad66 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<&'a str>) { + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option) { self.inner.get_identifiers() } } diff --git a/glean-core/rlb/src/private/object.rs b/glean-core/rlb/src/private/object.rs index 214e4175a8..5356a4a486 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<&'a str>) { + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option) { self.inner.get_identifiers() } } diff --git a/glean-core/rlb/src/test.rs b/glean-core/rlb/src/test.rs index 76e88fd678..651e9e9401 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, DynamicLabelType, LabeledCounter}; +use glean_core::{glean_test_get_experimentation_id, LabeledCounter, MetricLabel}; 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, - dynamic_label: None, + label: None, }); crate::set_upload_enabled(false); @@ -437,7 +437,7 @@ fn queued_recorded_metrics_correctly_record_during_init() { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); // This will queue 3 tasks that will add to the metric value once Glean is initialized @@ -1258,7 +1258,7 @@ fn test_a_ping_before_submission() { send_in_pings: vec!["custom1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); metric.add(1); @@ -1288,7 +1288,7 @@ fn test_boolean_get_num_errors() { send_in_pings: vec!["custom1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: Some(DynamicLabelType::Label(str::to_string("asdf"))), + label: Some(MetricLabel::Label(str::to_string("asdf"))), }); // Check specifically for an invalid label @@ -1374,7 +1374,7 @@ fn test_text_can_hold_long_string() { send_in_pings: vec!["custom1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: Some(DynamicLabelType::Label(str::to_string("text"))), + label: Some(MetricLabel::Label(str::to_string("text"))), }); // 216 characters, which would overflow StringMetric diff --git a/glean-core/rlb/tests/metric_metadata.rs b/glean-core/rlb/tests/metric_metadata.rs index 803290bb03..c0cba456a8 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::DynamicLabelType; + use glean_core::MetricLabel; 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()], - dynamic_label: Some(DynamicLabelType::Label("ah_ah_ah".into())), + label: Some(MetricLabel::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()], - dynamic_label: Some(DynamicLabelType::Label("111th".into())), + label: Some(MetricLabel::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")); + assert_eq!(label, Some("ah_ah_ah".to_string())); // 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")); + assert_eq!(label, Some("111th".to_string())); 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 6fa77cbaa0..5fd07d55a3 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, - dynamic_label: None, + label: None, }, TimeUnit::Millisecond, ) @@ -70,7 +70,7 @@ pub mod metrics { send_in_pings: vec!["metrics".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, TimeUnit::Millisecond, ) @@ -85,7 +85,7 @@ pub mod metrics { send_in_pings: vec!["metrics".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, TimeUnit::Millisecond, ) diff --git a/glean-core/src/common_metric_data.rs b/glean-core/src/common_metric_data.rs index 6e9c1dc596..c99f7b4d18 100644 --- a/glean-core/src/common_metric_data.rs +++ b/glean-core/src/common_metric_data.rs @@ -2,15 +2,17 @@ // 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::ops::Deref; +use std::fmt::Display; use std::sync::atomic::{AtomicU8, Ordering}; use malloc_size_of_derive::MallocSizeOf; +use rusqlite::Transaction; use crate::error::{Error, ErrorKind}; -use crate::metrics::dual_labeled_counter::validate_dynamic_key_and_or_category; -use crate::metrics::labeled::validate_dynamic_label; -use crate::Glean; +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 serde::{Deserialize, Serialize}; /// The supported metrics' lifetimes. @@ -68,44 +70,53 @@ pub struct CommonMetricData { /// /// Disabled metrics are never recorded. pub disabled: bool, - /// Dynamic label. + /// Label for this metric. /// - /// 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, + /// 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 DynamicLabelType { +pub enum MetricLabel { + /// Static Label -- no validation required + Static(String), /// A dynamic label applied from a `LabeledMetric` Label(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 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 and category - KeyAndCategory(String), + KeyAndCategory(String, String), } -impl Default for DynamicLabelType { +impl Default for MetricLabel { fn default() -> Self { Self::Label(String::new()) } } -impl Deref for DynamicLabelType { - type Target = str; - - fn deref(&self) -> &Self::Target { +impl Display for MetricLabel { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use crate::metrics::dual_labeled_counter::RECORD_SEPARATOR; match self { - DynamicLabelType::Label(label) => label, - DynamicLabelType::KeyOnly(key) => key, - DynamicLabelType::CategoryOnly(category) => category, - DynamicLabelType::KeyAndCategory(key_and_category) => key_and_category, + MetricLabel::Static(label) => write!(f, "{label}"), + MetricLabel::Label(label) => write!(f, "{label}"), + MetricLabel::KeyOnly(key, category) => { + write!(f, "{key}{RECORD_SEPARATOR}{category}") + } + MetricLabel::CategoryOnly(key, category) => { + write!(f, "{key}{RECORD_SEPARATOR}{category}") + } + MetricLabel::KeyAndCategory(key, category) => { + write!(f, "{key}{RECORD_SEPARATOR}{category}") + } } } } @@ -135,6 +146,54 @@ impl From for CommonMetricDataInternal { } } +pub enum LabelCheck { + NoLabel, + Label(String), + Error(String, i32), +} + +impl LabelCheck { + pub fn label(&self) -> &str { + use LabelCheck::*; + match self { + NoLabel => "", + Label(label) | Error(label, _) => label, + } + } + + 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, + ); + } + + fn map(self, mut f: impl FnMut(String) -> String) -> Self { + use LabelCheck::*; + + match self { + NoLabel => NoLabel, + Label(s) => Label(f(s)), + // shoud use `MAX_LABEL_LENGTH`, but we can't const-format this + Error(s, cnt) => Error(f(s), cnt), + } + } +} + impl CommonMetricDataInternal { /// Creates a new metadata object. pub fn new, B: Into, C: Into>( @@ -165,27 +224,33 @@ impl CommonMetricDataInternal { } } - /// The metric's unique identifier, including the category, name and label. + /// TODO /// /// 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 { + pub(crate) fn check_labels(&self, tx: &Transaction<'_>) -> LabelCheck { let base_identifier = self.base_identifier(); - if let Some(label) = &self.inner.dynamic_label { + if let Some(label) = &self.inner.label { match label { - DynamicLabelType::Label(label) => { - validate_dynamic_label(glean, self, &base_identifier, 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) } - _ => validate_dynamic_key_and_or_category( - glean, - self, - &base_identifier, - label.clone(), - ), } } else { - base_identifier + LabelCheck::NoLabel } } diff --git a/glean-core/src/core/mod.rs b/glean-core/src/core/mod.rs index e38798ffb4..f145af411d 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::Database; +use crate::database::sqlite::Database; use crate::debug::DebugOptions; use crate::error::ClientIdFileError; use crate::event_database::EventDatabase; @@ -341,7 +341,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 { @@ -680,14 +680,12 @@ impl Glean { .accumulate_sync(self, size.get() as i64) } - if let Some(rkv_load_state) = self + if let Some(load_state) = self .data_store .as_ref() - .and_then(|database| database.rkv_load_state()) + .and_then(|database| database.load_state()) { - self.database_metrics - .rkv_load_error - .set_sync(self, rkv_load_state) + self.database_metrics.load_error.set_sync(self, load_state) } } @@ -1289,12 +1287,10 @@ 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 StorageManager.snapshot_metric( - self.storage(), - INTERNAL_STORAGE, - &dirty_bit_metric.meta().identifier(self), - dirty_bit_metric.meta().inner.lifetime, - ) { + match self + .storage() + .get_metric(dirty_bit_metric.meta(), INTERNAL_STORAGE) + { Some(Metric::Boolean(b)) => b, _ => false, } diff --git a/glean-core/src/database/migration.rs b/glean-core/src/database/migration.rs new file mode 100644 index 0000000000..9550dc6c0c --- /dev/null +++ b/glean-core/src/database/migration.rs @@ -0,0 +1,332 @@ +// 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 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 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, +} + +impl Database { + /// 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)`. + pub fn iter_store(&self, lifetime: Lifetime, mut transaction_fn: F) + where + F: FnMut(String, &Metric), + { + let Ok(reader) = self.rkv.read() else { return }; + let Ok(mut iter) = self.get_store(lifetime).iter_start(&reader) else { + log::debug!("No store for {lifetime:?}"); + return; + }; + + 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"); + continue; + }; + value + } + _ => { + log::debug!("Blob for key {key:?} is not a valid blob"); + continue; + } + }; + transaction_fn(key, &metric); + } + } +} + +#[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 }) +} + +/// Migrate the `rkv` database to SQL. +/// +/// Returns the number of migrated metrics. +fn migrate(rkv: &Database, sql_db: &sqlite::Database, tx: &mut Transaction) -> usize { + let mut migrated_metrics = 0; + let mut migrate_metric = |lifetime: Lifetime, key: String, metric: &Metric| { + let Some(metric_id) = split_key(&key) else { + log::debug!("Invalid metric key: {key:?}"); + 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); + migrated_metrics += 1; + }; + + let snapshotter_user = + |key: String, metric: &Metric| migrate_metric(Lifetime::User, key, metric); + rkv.iter_store(Lifetime::User, snapshotter_user); + + let snapshotter_app = + |key: String, metric: &Metric| migrate_metric(Lifetime::Application, key, metric); + rkv.iter_store(Lifetime::Application, snapshotter_app); + + let snapshotter_ping = + |key: String, metric: &Metric| migrate_metric(Lifetime::Ping, key, metric); + rkv.iter_store(Lifetime::Ping, snapshotter_ping); + + migrated_metrics +} + +pub fn try_migrate(data_path: &Path, db: &sqlite::Database) -> Result<()> { + use super::migration::{self, Database as RkvDatabase}; + + 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(()); + } + + let Ok(rkv) = RkvDatabase::new(data_path) else { + log::debug!("Can't open rkv database. No migration."); + return Ok(()); + }; + let count = db.conn.write(|tx| { + let count = migration::migrate(&rkv, db, tx); + Ok::<_, Error>(count) + })?; + + log::info!("{count} metrics migrated to sqlite"); + + log::debug!( + "Data migrated. Would be removing Rkv database at {}", + rkv_file.display() + ); + + Ok(()) +} + +#[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 8408aa1f18..3c1fee301a 100644 --- a/glean-core/src/database/mod.rs +++ b/glean-core/src/database/mod.rs @@ -2,1622 +2,5 @@ // 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::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" - ); - } - } -} +pub mod migration; +pub mod sqlite; diff --git a/glean-core/src/database/sqlite.rs b/glean-core/src/database/sqlite.rs new file mode 100644 index 0000000000..9b954fe666 --- /dev/null +++ b/glean-core/src/database/sqlite.rs @@ -0,0 +1,649 @@ +// 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; +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), +} + +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, +} + +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 + } +} + +impl std::fmt::Debug for Database { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + fmt.debug_struct("Database") + .field("conn", &self.conn) + .finish() + } +} + +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 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: 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 db = Self { + conn, + file_size, + load_state, + }; + + if sqlite_exists { + log::debug!("SQLite database already exists. Not trying to migrate Rkv"); + } else { + _ = migration::try_migrate(&path, &db); + } + + 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(()) + }) + } + + /// TODO + pub fn get_metric( + &self, + data: &CommonMetricDataInternal, + storage_name: &str, + ) -> Option { + 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: 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, + { + 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 = ?1 AND ping = ?2"; + self.conn.write(|tx| { + let mut stmt = tx.prepare_cached(clear_sql)?; + stmt.execute([Lifetime::Ping.as_str(), 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 new file mode 100644 index 0000000000..2e8aa54869 --- /dev/null +++ b/glean-core/src/database/sqlite/connection.rs @@ -0,0 +1,111 @@ +// 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 new file mode 100644 index 0000000000..d9082779a2 --- /dev/null +++ b/glean-core/src/database/sqlite/schema.rs @@ -0,0 +1,69 @@ +// 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( + "PRAGMA journal_mode = WAL; + PRAGMA synchronous = NORMAL; + PRAGMA journal_size_limit = 512000; -- 512 KB. + PRAGMA temp_store = MEMORY; + 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 1694cae04e..58823a93d9 100644 --- a/glean-core/src/error.rs +++ b/glean-core/src/error.rs @@ -9,6 +9,8 @@ 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 @@ -65,6 +67,12 @@ 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. @@ -121,6 +129,8 @@ 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), } } } @@ -155,6 +165,27 @@ 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 24dcedd3e8..3488e3bbad 100644 --- a/glean-core/src/error_recording.rs +++ b/glean-core/src/error_recording.rs @@ -13,14 +13,16 @@ //! 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::labeled::{combine_base_identifier_and_label, strip_label}; -use crate::metrics::CounterMetric; -use crate::CommonMetricData; +use crate::metrics::{CounterMetric, Metric}; use crate::Glean; use crate::Lifetime; +use crate::{CommonMetricData, MetricLabel}; /// The possible error types for metric recording. /// @@ -92,8 +94,7 @@ 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 identifier = meta.base_identifier(); - let name = strip_label(&identifier); + let name = meta.base_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(); @@ -103,10 +104,11 @@ fn get_error_metric_for_metric(meta: &CommonMetricDataInternal, error: ErrorType } CounterMetric::new(CommonMetricData { - name: combine_base_identifier_and_label(error.as_str(), name), + name: error.as_str().to_string(), category: "glean.error".into(), lifetime: Lifetime::Ping, send_in_pings, + label: Some(MetricLabel::Label(name.to_string())), ..Default::default() }) } @@ -142,6 +144,51 @@ 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, +) { + assert!(num_errors > 0); + + // 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 f63c714088..b1aafbe874 100644 --- a/glean-core/src/glean.udl +++ b/glean-core/src/glean.udl @@ -354,7 +354,7 @@ interface PingType { void set_enabled(boolean enabled); }; -typedef enum DynamicLabelType; +typedef enum MetricLabel; // The common set of data shared across all different metric types. dictionary CommonMetricData { @@ -373,12 +373,12 @@ dictionary CommonMetricData { // Disabled metrics are never recorded. boolean disabled; - // Dynamic label. + // Label for this metric. // // When a labeled metric 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. - DynamicLabelType? dynamic_label = null; + // labels are stored in the metric data + // so that it can validated against the database later. + MetricLabel? label = null; }; interface CounterMetric { diff --git a/glean-core/src/internal_metrics.rs b/glean-core/src/internal_metrics.rs index fd46c642b1..b418b93316 100644 --- a/glean-core/src/internal_metrics.rs +++ b/glean-core/src/internal_metrics.rs @@ -58,7 +58,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), first_run_date: DatetimeMetric::new( @@ -68,7 +68,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }, TimeUnit::Day, ), @@ -79,7 +79,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }), attribution_source: StringMetric::new(CommonMetricData { @@ -88,7 +88,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), attribution_medium: StringMetric::new(CommonMetricData { @@ -97,7 +97,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), attribution_campaign: StringMetric::new(CommonMetricData { @@ -106,7 +106,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), attribution_term: StringMetric::new(CommonMetricData { @@ -115,7 +115,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), attribution_content: StringMetric::new(CommonMetricData { @@ -124,7 +124,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), distribution_name: StringMetric::new(CommonMetricData { @@ -133,7 +133,7 @@ impl CoreMetrics { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), } } @@ -148,7 +148,7 @@ impl AdditionalMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), pings_submitted: LabeledMetric::::new( @@ -159,7 +159,7 @@ impl AdditionalMetrics { send_in_pings: vec!["metrics".into(), "baseline".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, }, None, @@ -172,7 +172,7 @@ impl AdditionalMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, TimeUnit::Millisecond, ), @@ -184,7 +184,7 @@ impl AdditionalMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, TimeUnit::Millisecond, ), @@ -203,7 +203,7 @@ impl AdditionalMetrics { send_in_pings: vec!["all-pings".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }), event_timestamp_clamped: CounterMetric::new(CommonMetricData { @@ -212,7 +212,7 @@ impl AdditionalMetrics { send_in_pings: vec!["health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), server_knobs_config: ObjectMetric::new(CommonMetricData { @@ -221,7 +221,7 @@ impl AdditionalMetrics { send_in_pings: vec!["glean_internal_info".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }), } } @@ -251,7 +251,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, }, Some(vec![ @@ -271,7 +271,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, MemoryUnit::Kilobyte, ), @@ -283,7 +283,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, MemoryUnit::Kilobyte, ), @@ -294,7 +294,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), pending_pings: CounterMetric::new(CommonMetricData { @@ -303,7 +303,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), send_success: TimingDistributionMetric::new( @@ -313,7 +313,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, TimeUnit::Millisecond, ), @@ -325,7 +325,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, TimeUnit::Millisecond, ), @@ -336,7 +336,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), missing_send_ids: CounterMetric::new(CommonMetricData { @@ -345,7 +345,7 @@ impl UploadMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), } } @@ -355,8 +355,8 @@ impl UploadMetrics { pub struct DatabaseMetrics { pub size: MemoryDistributionMetric, - /// RKV's load result, indicating success or relaying the detected error. - pub rkv_load_error: StringMetric, + /// sqlite's load result, indicating success or relaying the detected error. + pub load_error: StringMetric, /// The time it takes for a write-commit for the Glean database. pub write_time: TimingDistributionMetric, @@ -372,18 +372,18 @@ impl DatabaseMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, MemoryUnit::Byte, ), - rkv_load_error: StringMetric::new(CommonMetricData { - name: "rkv_load_error".into(), + load_error: StringMetric::new(CommonMetricData { + name: "load_error".into(), category: "glean.database".into(), send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), write_time: TimingDistributionMetric::new( @@ -393,7 +393,7 @@ impl DatabaseMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: true, - dynamic_label: None, + label: None, }, TimeUnit::Microsecond, ), @@ -450,7 +450,7 @@ impl HealthMetrics { send_in_pings: vec!["metrics".into(), "health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), init_count: CounterMetric::new(CommonMetricData { name: "init_count".into(), @@ -458,7 +458,7 @@ impl HealthMetrics { send_in_pings: vec!["health".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }), exception_state: StringMetric::new(CommonMetricData { name: "exception_state".into(), @@ -466,7 +466,7 @@ impl HealthMetrics { send_in_pings: vec!["health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), recovered_client_id: UuidMetric::new(CommonMetricData { name: "recovered_client_id".into(), @@ -474,7 +474,7 @@ impl HealthMetrics { send_in_pings: vec!["health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }), file_read_error: LabeledMetric::::new( LabeledMetricData::Common { @@ -484,7 +484,7 @@ impl HealthMetrics { send_in_pings: vec!["health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, }, Some(vec![ @@ -503,7 +503,7 @@ impl HealthMetrics { send_in_pings: vec!["health".into()], lifetime: Lifetime::Ping, disabled: false, - dynamic_label: None, + label: None, }, }, Some(vec![ diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index 7f47159a23..b835465b15 100644 --- a/glean-core/src/lib.rs +++ b/glean-core/src/lib.rs @@ -63,7 +63,7 @@ mod util; #[cfg(all(not(target_os = "android"), not(target_os = "ios")))] mod fd_logger; -pub use crate::common_metric_data::{CommonMetricData, DynamicLabelType, Lifetime}; +pub use crate::common_metric_data::{CommonMetricData, Lifetime, MetricLabel}; 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 97df93f2b7..aa454de082 100644 --- a/glean-core/src/lib_unit_tests.rs +++ b/glean-core/src/lib_unit_tests.rs @@ -1190,10 +1190,10 @@ fn records_database_file_size() { // We should see the database containing some data. assert!(data.sum > 0); - let rkv_load_state = &glean.database_metrics.rkv_load_error; - let rkv_load_error = rkv_load_state.get_value(&glean, "metrics"); + let load_state = &glean.database_metrics.load_error; + let load_error = load_state.get_value(&glean, "metrics"); - assert_eq!(rkv_load_error, None); + assert_eq!(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 504f3347a5..2614ba1f3a 100644 --- a/glean-core/src/metrics/boolean.rs +++ b/glean-core/src/metrics/boolean.rs @@ -4,11 +4,10 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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; @@ -33,9 +32,9 @@ impl MetricType for BooleanMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), } @@ -89,12 +88,7 @@ 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 StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { Some(Metric::Boolean(b)) => Some(b), _ => None, } diff --git a/glean-core/src/metrics/counter.rs b/glean-core/src/metrics/counter.rs index 300a088472..407426a66b 100644 --- a/glean-core/src/metrics/counter.rs +++ b/glean-core/src/metrics/counter.rs @@ -5,11 +5,10 @@ use std::cmp::Ordering; use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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}; @@ -35,9 +34,9 @@ impl MetricType for CounterMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), } @@ -126,12 +125,7 @@ impl CounterMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 a94af73a08..418c06b2d7 100644 --- a/glean-core/src/metrics/custom_distribution.rs +++ b/glean-core/src/metrics/custom_distribution.rs @@ -5,11 +5,10 @@ use std::mem; use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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}; @@ -55,9 +54,9 @@ impl MetricType for CustomDistributionMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), range_min: self.range_min, @@ -218,13 +217,7 @@ impl CustomDistributionMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - 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 + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 b6b39a4909..82b9642c40 100644 --- a/glean-core/src/metrics/datetime.rs +++ b/glean-core/src/metrics/datetime.rs @@ -10,7 +10,6 @@ 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}; @@ -218,12 +217,7 @@ impl DatetimeMetric { ) -> Option<(ChronoDatetime, TimeUnit)> { let queried_ping_name = ping_name.unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 1d343e7d40..c25d00d4c4 100644 --- a/glean-core/src/metrics/denominator.rs +++ b/glean-core/src/metrics/denominator.rs @@ -8,7 +8,6 @@ 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}; @@ -96,12 +95,7 @@ impl DenominatorMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta().identifier(glean), - self.meta().inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 597c129878..970b88158c 100644 --- a/glean-core/src/metrics/dual_labeled_counter.rs +++ b/glean-core/src/metrics/dual_labeled_counter.rs @@ -8,10 +8,14 @@ use std::collections::{HashMap, HashSet}; use std::mem; use std::sync::{Arc, Mutex}; -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}; +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; const MAX_LABELS: usize = 16; const OTHER_LABEL: &str = "__other__"; @@ -110,32 +114,25 @@ impl DualLabeledCounterMetric { match (&self.keys, &self.categories) { (None, None) => self .counter - .with_dynamic_label(DynamicLabelType::KeyAndCategory( - make_label_from_key_and_category(key, category), - )), + .with_label(MetricLabel::KeyAndCategory(key.into(), category.into())), (None, _) => { let static_category = self.static_category(category); - self.counter.with_dynamic_label(DynamicLabelType::KeyOnly( - make_label_from_key_and_category(key, static_category), - )) + self.counter + .with_label(MetricLabel::KeyOnly(key.into(), static_category.into())) } (_, None) => { let static_key = self.static_key(key); - self.counter - .with_dynamic_label(DynamicLabelType::CategoryOnly( - make_label_from_key_and_category(static_key, category), - )) + self.counter.with_label(MetricLabel::CategoryOnly( + static_key.into(), + category.into(), + )) } (_, _) => { // Both labels are static and can be validated now let static_key = self.static_key(key); let static_category = self.static_category(category); - let name = combine_base_identifier_and_labels( - self.counter.meta().inner.name.as_str(), - static_key, - static_category, - ); - self.counter.with_name(name) + let label = format!("{static_key}{RECORD_SEPARATOR}{static_category}"); + self.counter.with_label(MetricLabel::Static(label)) } } } @@ -251,181 +248,89 @@ impl TestGetValue for DualLabeledCounterMetric { } } -/// Combines a metric's base identifier and label -pub fn combine_base_identifier_and_labels( - base_identifer: &str, +pub fn validate_dual_label_sqlite( + tx: &Transaction, + base_identifier: &str, key: &str, category: &str, -) -> String { - format!( - "{}{}", - base_identifer, - make_label_from_key_and_category(key, category) - ) -} +) -> LabelCheck { + let existing_labels_sql = "SELECT DISTINCT labels FROM telemetry WHERE id = ?1"; + + // TODO: 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!("Label cannot contain the ASCII record separator character (0x1E)"); + return LabelCheck::Error(format!("{OTHER_LABEL}{RECORD_SEPARATOR}{OTHER_LABEL}"), 1); + } -/// 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 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; + }; -/// 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 - ) -} + 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; + }; -/// 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); + 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 { + log::debug!("Database contains invalid dual-label: {existing_labels:?}"); + continue; + }; + + existing_keys.insert(existing_key.to_string()); + existing_categories.insert(existing_category.to_string()); + } } - // 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 mut errors = 0; + let new_key = if (existing_keys.contains(key) || existing_keys.len() < MAX_LABELS) + && label_is_valid(key) + { + key + } else { + errors += 1; + OTHER_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) + let new_category = if (existing_categories.contains(category) + || existing_categories.len() < MAX_LABELS) + && label_is_valid(category) + { + category } else { - 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) + errors += 1; + OTHER_LABEL + }; + + let label = format!("{new_key}{RECORD_SEPARATOR}{new_category}"); + if errors == 0 { + LabelCheck::Label(label) + } else { + LabelCheck::Error(label, errors) } } -fn label_is_valid(label: &str, glean: &Glean, meta: &CommonMetricDataInternal) -> bool { +fn label_is_valid(label: &str) -> bool { if label.len() > MAX_LABEL_LENGTH { - let msg = format!( + log::warn!( "label length {} exceeds maximum of {}", label.len(), MAX_LABEL_LENGTH ); - record_error(glean, meta, ErrorType::InvalidLabel, msg, None); + false + } else if label.contains(RECORD_SEPARATOR) { + log::warn!("Label cannot contain the ASCII record separator character (0x1E)"); 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 334c85e2e1..2d49bc91d7 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::{StorageManager, INTERNAL_STORAGE}; +use crate::storage::INTERNAL_STORAGE; use crate::util::{truncate_string_at_boundary, truncate_string_at_boundary_with_error}; use crate::Lifetime; use crate::{CommonMetricData, Glean}; @@ -205,12 +205,7 @@ impl ExperimentMetric { /// /// The stored value or `None` if nothing stored. pub fn test_get_value(&self, glean: &Glean) -> Option { - match StorageManager.snapshot_metric( - glean.storage(), - INTERNAL_STORAGE, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), INTERNAL_STORAGE) { Some(Metric::Experiment(e)) => Some(e), _ => None, } diff --git a/glean-core/src/metrics/labeled.rs b/glean-core/src/metrics/labeled.rs index ba5d2855d2..deeaf2158f 100644 --- a/glean-core/src/metrics/labeled.rs +++ b/glean-core/src/metrics/labeled.rs @@ -4,23 +4,21 @@ 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, CommonMetricDataInternal, DynamicLabelType}; -use crate::error_recording::{record_error, test_get_num_recorded_errors, ErrorType}; +use crate::common_metric_data::{CommonMetricData, LabelCheck, MetricLabel}; +use crate::error_recording::{test_get_num_recorded_errors, ErrorType}; use crate::histogram::HistogramType; use crate::metrics::{ BooleanMetric, CounterMetric, CustomDistributionMetric, MemoryDistributionMetric, MemoryUnit, - Metric, MetricType, QuantityMetric, StringMetric, TestGetValue, TimeUnit, - TimingDistributionMetric, + MetricType, QuantityMetric, StringMetric, TestGetValue, TimeUnit, TimingDistributionMetric, }; use crate::storage::StorageManager; -use crate::Glean; const MAX_LABELS: usize = 16; const OTHER_LABEL: &str = "__other__"; @@ -253,16 +251,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_name(&self, name: String) -> T { - self.submetric.with_name(name) + fn new_metric_with_label(&self, label: MetricLabel) -> T { + self.submetric.with_label(label) } /// 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. - fn new_metric_with_dynamic_label(&self, label: DynamicLabelType) -> T { - self.submetric.with_dynamic_label(label) + fn new_metric_with_dynamic_label(&self, label: MetricLabel) -> T { + self.submetric.with_label(label) } /// Creates a static label. @@ -320,13 +318,11 @@ where let metric = match self.labels { Some(_) => { let label = self.static_label(label); - self.new_metric_with_name(combine_base_identifier_and_label( - &self.submetric.meta().inner.name, - label, - )) + self.new_metric_with_label(MetricLabel::Static(label.to_string())) + } + None => { + self.new_metric_with_dynamic_label(MetricLabel::Label(label.to_string())) } - None => self - .new_metric_with_dynamic_label(DynamicLabelType::Label(label.to_string())), }; let metric = Arc::new(metric); entry.insert(Arc::clone(&metric)); @@ -371,7 +367,7 @@ where StorageManager.snapshot_labels( glean.storage(), queried_ping_name, - &self.submetric.meta().identifier(glean), + &self.submetric.meta().base_identifier(), self.submetric.meta().inner.lifetime, ) }); @@ -385,73 +381,47 @@ where } } -/// 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, +pub fn validate_dynamic_label_sqlite( + tx: &Transaction, base_identifier: &str, label: &str, -) -> 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; - } - } +) -> 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 mut labels = HashSet::new(); - let prefix = &key[..=base_identifier.len()]; - let mut snapshotter = |metric_id: &[u8], _: &Metric| { - labels.insert(metric_id.to_vec()); - }; + 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()); + }; + + while let Ok(Some(row)) = rows.next() { + let existing_label: String = row.get(0).unwrap(); - let lifetime = meta.inner.lifetime; - for store in &meta.inner.send_in_pings { - glean - .storage() - .iter_store_from(lifetime, store, Some(prefix), &mut snapshotter); + label_count += 1; + if existing_label == label { + label_already_used = true; + break; + } + } } - let label_count = labels.len(); - let error = if label_count >= MAX_LABELS { - true + if !label_already_used && label_count >= MAX_LABELS { + LabelCheck::Label(String::from(OTHER_LABEL)) } else if label.len() > MAX_LABEL_LENGTH { - let msg = format!( + log::warn!( "label length {} exceeds maximum of {}", label.len(), MAX_LABEL_LENGTH ); - record_error(glean, meta, ErrorType::InvalidLabel, msg, None); - true - } else { - false - }; - - if error { - combine_base_identifier_and_label(base_identifier, OTHER_LABEL) + LabelCheck::Error(String::from(OTHER_LABEL), 1) } else { - key + LabelCheck::Label(label.to_string()) } } diff --git a/glean-core/src/metrics/memory_distribution.rs b/glean-core/src/metrics/memory_distribution.rs index 3513dabf3d..9349537cf4 100644 --- a/glean-core/src/metrics/memory_distribution.rs +++ b/glean-core/src/metrics/memory_distribution.rs @@ -5,12 +5,11 @@ use std::mem; use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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}; @@ -64,9 +63,9 @@ impl MetricType for MemoryDistributionMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), memory_unit: self.memory_unit, @@ -257,12 +256,7 @@ impl MemoryDistributionMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 a916d017d5..ddb1bee98c 100644 --- a/glean-core/src/metrics/mod.rs +++ b/glean-core/src/metrics/mod.rs @@ -41,7 +41,7 @@ mod url; mod uuid; use crate::common_metric_data::CommonMetricDataInternal; -pub use crate::common_metric_data::DynamicLabelType; +pub use crate::common_metric_data::MetricLabel; 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_dynamic_label(&self, _label: DynamicLabelType) -> Self + fn with_label(&self, _label: MetricLabel) -> Self where Self: Sized, { @@ -233,17 +233,13 @@ 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 base_id = self.meta().base_identifier(); - let identifier = base_id - .split_once('/') - .map(|split| split.0) - .unwrap_or(&base_id); + let identifier = self.meta().base_identifier(); // 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) @@ -266,7 +262,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<&'a str>); + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option); } /// [`TestGetValue`] describes an interface for retrieving the value for a given metric @@ -297,9 +293,13 @@ impl<'a, T> MetricIdentifier<'a> for T where T: MetricType, { - fn get_identifiers(&'a self) -> (&'a str, &'a str, Option<&'a str>) { + fn get_identifiers(&'a self) -> (&'a str, &'a str, Option) { let meta = &self.meta().inner; - (&meta.category, &meta.name, meta.dynamic_label.as_deref()) + ( + &meta.category, + &meta.name, + meta.label.as_ref().map(|label| label.to_string()), + ) } } diff --git a/glean-core/src/metrics/object.rs b/glean-core/src/metrics/object.rs index cbb5af36c7..9739f6e06d 100644 --- a/glean-core/src/metrics/object.rs +++ b/glean-core/src/metrics/object.rs @@ -9,7 +9,6 @@ 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}; @@ -118,12 +117,7 @@ impl ObjectMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { Some(Metric::Object(o)) => Some(o), _ => None, } diff --git a/glean-core/src/metrics/quantity.rs b/glean-core/src/metrics/quantity.rs index 137569b53e..d20f6ddc90 100644 --- a/glean-core/src/metrics/quantity.rs +++ b/glean-core/src/metrics/quantity.rs @@ -4,11 +4,10 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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}; @@ -33,9 +32,9 @@ impl MetricType for QuantityMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), } @@ -102,12 +101,7 @@ impl QuantityMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { Some(Metric::Quantity(i)) => Some(i), _ => None, } diff --git a/glean-core/src/metrics/rate.rs b/glean-core/src/metrics/rate.rs index 40e920fd2a..37fc66d268 100644 --- a/glean-core/src/metrics/rate.rs +++ b/glean-core/src/metrics/rate.rs @@ -6,7 +6,6 @@ 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}; @@ -147,12 +146,7 @@ impl RateMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 2e33068686..0bbbe645d4 100644 --- a/glean-core/src/metrics/string.rs +++ b/glean-core/src/metrics/string.rs @@ -4,11 +4,10 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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}; @@ -37,9 +36,9 @@ impl MetricType for StringMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), } @@ -96,12 +95,7 @@ impl StringMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { Some(Metric::String(s)) => Some(s), _ => None, } @@ -167,7 +161,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); let sample_string = "0123456789".repeat(26); diff --git a/glean-core/src/metrics/string_list.rs b/glean-core/src/metrics/string_list.rs index ad2a1b6244..257196c567 100644 --- a/glean-core/src/metrics/string_list.rs +++ b/glean-core/src/metrics/string_list.rs @@ -8,7 +8,6 @@ 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}; @@ -155,12 +154,7 @@ impl StringListMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { Some(Metric::StringList(values)) => Some(values), _ => None, } diff --git a/glean-core/src/metrics/text.rs b/glean-core/src/metrics/text.rs index ae47fd34bc..06900dbd33 100644 --- a/glean-core/src/metrics/text.rs +++ b/glean-core/src/metrics/text.rs @@ -4,11 +4,10 @@ use std::sync::Arc; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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}; @@ -39,9 +38,9 @@ impl MetricType for TextMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), } @@ -100,12 +99,7 @@ impl TextMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { Some(Metric::Text(s)) => Some(s), _ => None, } @@ -171,7 +165,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); let sample_string = "0123456789".repeat(200 * 1024); diff --git a/glean-core/src/metrics/timespan.rs b/glean-core/src/metrics/timespan.rs index 5ab5f769a4..4f3d6fddd1 100644 --- a/glean-core/src/metrics/timespan.rs +++ b/glean-core/src/metrics/timespan.rs @@ -10,7 +10,6 @@ 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}; @@ -254,12 +253,7 @@ impl TimespanMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 5620cd3a1e..d7ed5735bb 100644 --- a/glean-core/src/metrics/timing_distribution.rs +++ b/glean-core/src/metrics/timing_distribution.rs @@ -10,12 +10,11 @@ use std::time::Duration; use malloc_size_of_derive::MallocSizeOf; -use crate::common_metric_data::{CommonMetricDataInternal, DynamicLabelType}; +use crate::common_metric_data::{CommonMetricDataInternal, MetricLabel}; 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}; @@ -111,9 +110,9 @@ impl MetricType for TimingDistributionMetric { } } - fn with_dynamic_label(&self, label: DynamicLabelType) -> Self { + fn with_label(&self, label: MetricLabel) -> Self { let mut meta = (*self.meta).clone(); - meta.inner.dynamic_label = Some(label); + meta.inner.label = Some(label); Self { meta: Arc::new(meta), time_unit: self.time_unit, @@ -542,12 +541,7 @@ impl TimingDistributionMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 d9d0b46525..58a1cf674e 100644 --- a/glean-core/src/metrics/url.rs +++ b/glean-core/src/metrics/url.rs @@ -8,7 +8,6 @@ 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}; @@ -115,12 +114,7 @@ impl UrlMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { Some(Metric::Url(s)) => Some(s), _ => None, } @@ -185,7 +179,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); let sample_url = "glean://test".to_string(); @@ -203,7 +197,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); // Whenever the URL is longer than our MAX_URL_LENGTH, we truncate the URL to the @@ -241,7 +235,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); let test_url = "data:application/json"; @@ -265,7 +259,7 @@ mod test { send_in_pings: vec!["store1".into()], lifetime: Lifetime::Application, disabled: false, - dynamic_label: None, + label: None, }); let incorrects = vec![ diff --git a/glean-core/src/metrics/uuid.rs b/glean-core/src/metrics/uuid.rs index 30ed56eb46..291938a57f 100644 --- a/glean-core/src/metrics/uuid.rs +++ b/glean-core/src/metrics/uuid.rs @@ -10,7 +10,6 @@ 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}; @@ -114,12 +113,7 @@ impl UuidMetric { .into() .unwrap_or_else(|| &self.meta().inner.send_in_pings[0]); - match StorageManager.snapshot_metric( - glean.storage(), - queried_ping_name, - &self.meta.identifier(glean), - self.meta.inner.lifetime, - ) { + match glean.storage().get_metric(self.meta(), queried_ping_name) { 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 ce21b26e0d..35761b63a7 100644 --- a/glean-core/src/ping/mod.rs +++ b/glean-core/src/ping/mod.rs @@ -83,12 +83,7 @@ impl PingMaker { ..Default::default() }); - let current_seq = match StorageManager.snapshot_metric( - glean.storage(), - INTERNAL_STORAGE, - &seq.meta().identifier(glean), - seq.meta().inner.lifetime, - ) { + let current_seq = match glean.storage().get_metric(seq.meta(), INTERNAL_STORAGE) { Some(Metric::Counter(i)) => i, _ => 0, }; @@ -284,16 +279,6 @@ 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 14198f1fa2..c47eb901df 100644 --- a/glean-core/src/storage/mod.rs +++ b/glean-core/src/storage/mod.rs @@ -10,8 +10,7 @@ use std::collections::HashMap; use serde_json::{json, Value as JsonValue}; -use crate::database::Database; -use crate::metrics::dual_labeled_counter::RECORD_SEPARATOR; +use crate::database::sqlite::Database; use crate::metrics::Metric; use crate::Lifetime; @@ -29,6 +28,7 @@ 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,9 +45,6 @@ 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()); @@ -61,20 +58,21 @@ fn snapshot_labeled_metrics( fn snapshot_dual_labeled_metrics( snapshot: &mut HashMap>, metric_id: &str, + label1: &str, + label2: &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(parts[0].into()) + .entry(metric_id.into()) .or_insert_with(|| json!({})) .as_object_mut() .unwrap(); // safe unwrap, we constructed the object above - let key_obj = obj.entry(parts[1].to_string()).or_insert_with(|| json!({})); + let key_obj = obj.entry(label1).or_insert_with(|| json!({})); let key_obj = key_obj.as_object_mut().unwrap(); - key_obj.insert(parts[2].into(), metric.as_json()); + key_obj.insert(label2.into(), metric.as_json()); } impl StorageManager { @@ -120,25 +118,44 @@ impl StorageManager { ) -> Option { let mut snapshot: HashMap> = HashMap::new(); - let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + let mut snapshotter = |metric_id: &[u8], labels: &[&str], metric: &Metric| { let metric_id = String::from_utf8_lossy(metric_id).into_owned(); - 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()); + 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); + } + [label1, label2] => { + snapshot_dual_labeled_metrics( + &mut snapshot, + &metric_id, + label1, + label2, + metric, + ); + } + _ => panic!("uh wat?"), } }; - 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); + 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 ping lifetime store: {e:?}"); + } + if let Err(e) = storage.iter_store(Lifetime::User, store_name, &mut snapshotter) { + log::debug!("could not snapshot ping lifetime store: {e:?}"); + } // Add send in all pings client.annotations if store_name != "glean_client_info" { - storage.iter_store_from(Lifetime::Application, "all-pings", None, snapshotter); + if let Err(e) = storage.iter_store(Lifetime::Application, "all-pings", snapshotter) { + log::debug!("could not snapshot metrics for 'all-pings': {e:?}"); + } } if clear_store { @@ -165,7 +182,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, @@ -174,15 +191,16 @@ impl StorageManager { ) -> Option { let mut snapshot: Option = None; - let mut snapshotter = |id: &[u8], metric: &Metric| { + let mut snapshotter = |id: &[u8], _labels: &[&str], metric: &Metric| { let id = String::from_utf8_lossy(id).into_owned(); if id == metric_id { snapshot = Some(metric.clone()) } }; - storage.iter_store_from(metric_lifetime, store_name, None, &mut snapshotter); - + storage + .iter_store(metric_lifetime, store_name, &mut snapshotter) + .ok()?; snapshot } @@ -207,17 +225,15 @@ impl StorageManager { ) -> Vec { let mut labels = Vec::new(); - 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()); - } + 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()); } }; - storage.iter_store_from(metric_lifetime, store_name, None, &mut snapshotter); - + _ = storage.iter_store(metric_lifetime, store_name, &mut snapshotter); labels } @@ -252,7 +268,7 @@ impl StorageManager { ) -> Option { let mut snapshot: HashMap = HashMap::new(); - let mut snapshotter = |metric_id: &[u8], metric: &Metric| { + let mut snapshotter = |metric_id: &[u8], _labels: &[&str], 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 @@ -260,7 +276,9 @@ impl StorageManager { } }; - storage.iter_store_from(Lifetime::Application, store_name, None, &mut snapshotter); + storage + .iter_store(Lifetime::Application, store_name, &mut snapshotter) + .ok()?; 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 new file mode 100644 index 0000000000..86ebf4231c Binary files /dev/null and b/glean-core/tests/77ca0472-5124-4f6b-971d-4a2a928fb158.safe.bin differ diff --git a/glean-core/tests/clientid_textfile.rs b/glean-core/tests/clientid_textfile.rs index 41b9a34d5d..3dbbc51180 100644 --- a/glean-core/tests/clientid_textfile.rs +++ b/glean-core/tests/clientid_textfile.rs @@ -8,8 +8,7 @@ use crate::common::*; use std::fs; use std::path::Path; -use rkv::Rkv; -use rkv::StoreOptions; +use rusqlite::params; use uuid::Uuid; use glean_core::metrics::*; @@ -23,7 +22,7 @@ fn clientid_metric() -> UuidMetric { send_in_pings: vec!["glean_client_info".into()], lifetime: Lifetime::User, disabled: false, - dynamic_label: None, + label: None, }) } @@ -126,16 +125,9 @@ fn clientid_regen_issue_with_existing_db() { // We modify the database and ONLY clear out the 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 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 (glean, temp) = new_glean(Some(temp)); @@ -201,23 +193,18 @@ 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"); - 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 path = temp.path().join("db").join("glean.sqlite"); + let conn = rusqlite::Connection::open(path).unwrap(); - let mut writer = rkv.write().unwrap(); - let encoded = bincode::serialize(&Metric::Uuid(String::from( + let encoded = rmp_serde::to_vec(&Metric::Uuid(String::from( "c0ffeec0-ffee-c0ff-eec0-ffeec0ffeec0", ))) .unwrap(); - let known_client_id = rkv::Value::Blob(&encoded); - user_store - .put(&mut writer, client_id_key, &known_client_id) - .unwrap(); - writer.commit().unwrap(); + + _ = conn.execute( + "UPDATE telemetry SET value = ? WHERE id = 'client_id'", + params![encoded], + ); } let (glean, temp) = new_glean(Some(temp)); diff --git a/glean-core/tests/collection_enabled.rs b/glean-core/tests/collection_enabled.rs index 78ae96dcfe..de36f415a7 100644 --- a/glean-core/tests/collection_enabled.rs +++ b/glean-core/tests/collection_enabled.rs @@ -9,6 +9,7 @@ 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` @@ -198,3 +199,39 @@ 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/ping.rs b/glean-core/tests/ping.rs index cb63092160..d1ad8408c0 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, - dynamic_label: None, + label: None, }, }, None, diff --git a/glean-core/tests/sqlite.rs b/glean-core/tests/sqlite.rs new file mode 100644 index 0000000000..3f678f6661 --- /dev/null +++ b/glean-core/tests/sqlite.rs @@ -0,0 +1,212 @@ +// 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, + disabled: false, + label: None, + }) +} + +#[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()); +} + +#[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()); +} + +#[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, + }; + let glean = Glean::new(cfg); + assert!(glean.is_err()); + } +} + +// TODO: +// 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 new file mode 100644 index 0000000000..9d6697c590 --- /dev/null +++ b/glean-core/tests/sqlite_migration.rs @@ -0,0 +1,87 @@ +mod common; +use std::fs; + +use crate::common::*; + +use glean_core::metrics::*; +use glean_core::CommonMetricData; +use glean_core::Lifetime; +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, + disabled: false, + label: None, + }) +} + +#[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, + include_bytes!("77ca0472-5124-4f6b-971d-4a2a928fb158.safe.bin"), + ) + .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); + + // TODO: validate migration metrics +} + +#[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"); +} diff --git a/supply-chain/audits.toml b/supply-chain/audits.toml index efb37606f6..e050266a99 100644 --- a/supply-chain/audits.toml +++ b/supply-chain/audits.toml @@ -295,6 +295,11 @@ 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" @@ -321,6 +326,11 @@ 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" @@ -381,12 +391,27 @@ 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" delta = "3.0.12 -> 3.0.13" notes = "Dependency updates only" +[[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" @@ -545,6 +570,16 @@ 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" @@ -754,6 +789,18 @@ criteria = "safe-to-deploy" delta = "1.1.0 -> 1.2.0" notes = "Added a file lock on the created directory" +[[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 c41551eb22..4d432db878 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-run" +criteria = "safe-to-deploy" [[exemptions.hermit-abi]] version = "0.2.6" @@ -112,6 +112,10 @@ criteria = "safe-to-run" version = "0.2.139" 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" @@ -172,6 +176,10 @@ 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 9b91ce80a7..cd4432f75e 100644 --- a/supply-chain/imports.lock +++ b/supply-chain/imports.lock @@ -8,6 +8,12 @@ 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" @@ -15,6 +21,13 @@ 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" @@ -352,6 +365,21 @@ 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.camino]] who = "Pat Hickey " criteria = "safe-to-deploy" @@ -384,8 +412,8 @@ notes = "Dependency updates and minor changes, nothing suspicious." [[audits.bytecode-alliance.audits.cc]] who = "Alex Crichton " criteria = "safe-to-deploy" -version = "1.0.73" -notes = "I am the author of this crate." +delta = "1.2.30 -> 1.2.41" +notes = "This is a trusted rust-lang/rust crate" [[audits.bytecode-alliance.audits.cfg-if]] who = "Alex Crichton " @@ -432,6 +460,16 @@ 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" @@ -465,6 +503,12 @@ criteria = "safe-to-deploy" delta = "3.0.10 -> 3.0.12" notes = "Just a dependency version bump" +[[audits.bytecode-alliance.audits.find-msvc-tools]] +who = "Alex Crichton " +criteria = "safe-to-deploy" +delta = "0.1.0 -> 0.1.4" +notes = "Nothing out of the ordinary for a crate finding MSVC tooling." + [[audits.bytecode-alliance.audits.foldhash]] who = "Alex Crichton " criteria = "safe-to-deploy" @@ -611,6 +655,12 @@ 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" @@ -621,12 +671,44 @@ 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" @@ -693,6 +775,12 @@ 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" @@ -1718,6 +1806,12 @@ 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" @@ -2113,10 +2207,13 @@ 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.cc]] -who = "Mike Hommey " +[[audits.mozilla.audits.bitflags]] +who = [ + "Teodor Tanasoaia ", + "Erich Gubler ", +] criteria = "safe-to-deploy" -delta = "1.0.73 -> 1.0.78" +delta = "2.6.0 -> 2.7.0" aggregated-from = "https://hg.mozilla.org/mozilla-central/raw-file/tip/supply-chain/audits.toml" [[audits.mozilla.audits.chrono]] @@ -2272,6 +2369,30 @@ 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" @@ -2283,13 +2404,6 @@ 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" @@ -2309,6 +2423,12 @@ 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" @@ -2340,6 +2460,60 @@ 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" @@ -2379,6 +2553,12 @@ 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" @@ -2386,6 +2566,12 @@ 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.tempfile]] who = "Mike Hommey " criteria = "safe-to-deploy"