diff options
Diffstat (limited to 'toolkit/components/glean')
30 files changed, 393 insertions, 377 deletions
diff --git a/toolkit/components/glean/Cargo.toml b/toolkit/components/glean/Cargo.toml index ba563f514a..ff65f74143 100644 --- a/toolkit/components/glean/Cargo.toml +++ b/toolkit/components/glean/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" license = "MPL-2.0" [dependencies] -glean = "59.0.0" +glean = "60.0.1" log = "0.4" nserror = { path = "../../../xpcom/rust/nserror" } nsstring = { path = "../../../xpcom/rust/nsstring" } diff --git a/toolkit/components/glean/api/Cargo.toml b/toolkit/components/glean/api/Cargo.toml index af507f499f..62e3477adb 100644 --- a/toolkit/components/glean/api/Cargo.toml +++ b/toolkit/components/glean/api/Cargo.toml @@ -9,7 +9,7 @@ license = "MPL-2.0" [dependencies] bincode = "1.0" chrono = "0.4.10" -glean = "59.0.0" +glean = "60.0.1" inherent = "1.0.0" log = "0.4" nsstring = { path = "../../../../xpcom/rust/nsstring", optional = true } diff --git a/toolkit/components/glean/api/src/common_test.rs b/toolkit/components/glean/api/src/common_test.rs index 46062736f2..1e02bfccf4 100644 --- a/toolkit/components/glean/api/src/common_test.rs +++ b/toolkit/components/glean/api/src/common_test.rs @@ -43,7 +43,7 @@ fn setup_glean(tempdir: Option<tempfile::TempDir>) -> tempfile::TempDir { rate_limit: None, enable_event_timestamps: false, experimentation_id: None, - enable_internal_pings: true + enable_internal_pings: true, }; let client_info = glean::ClientInfoMetrics { diff --git a/toolkit/components/glean/api/src/private/ping.rs b/toolkit/components/glean/api/src/private/ping.rs index 7e03c1ff00..5defa9b5c6 100644 --- a/toolkit/components/glean/api/src/private/ping.rs +++ b/toolkit/components/glean/api/src/private/ping.rs @@ -31,6 +31,8 @@ impl Ping { send_if_empty: bool, precise_timestamps: bool, include_info_sections: bool, + enabled: bool, + schedules_pings: Vec<String>, reason_codes: Vec<String>, ) -> Self { if need_ipc() { @@ -42,6 +44,8 @@ impl Ping { send_if_empty, precise_timestamps, include_info_sections, + enabled, + schedules_pings, reason_codes, )) } @@ -105,7 +109,7 @@ mod test { // Smoke test for what should be the generated code. static PROTOTYPE_PING: Lazy<Ping> = - Lazy::new(|| Ping::new("prototype", false, true, true, true, vec![])); + Lazy::new(|| Ping::new("prototype", false, true, true, true, true, vec![], vec![])); #[test] fn smoke_test_custom_ping() { diff --git a/toolkit/components/glean/bindings/GleanMetric.h b/toolkit/components/glean/bindings/GleanMetric.h index c6bf6b4066..f2b96d3cce 100644 --- a/toolkit/components/glean/bindings/GleanMetric.h +++ b/toolkit/components/glean/bindings/GleanMetric.h @@ -21,6 +21,8 @@ enum class ScalarID : uint32_t; namespace mozilla::glean { +typedef uint64_t TimerId; + class GleanMetric : public nsISupports, public nsWrapperCache { public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS; diff --git a/toolkit/components/glean/bindings/jog/JOG.cpp b/toolkit/components/glean/bindings/jog/JOG.cpp index 56119cef0e..c5b2bad6b7 100644 --- a/toolkit/components/glean/bindings/jog/JOG.cpp +++ b/toolkit/components/glean/bindings/jog/JOG.cpp @@ -12,6 +12,7 @@ #include "mozilla/ClearOnShutdown.h" #include "mozilla/glean/bindings/jog/jog_ffi_generated.h" #include "mozilla/Logging.h" +#include "mozilla/IntegerPrintfMacros.h" #include "mozilla/StaticPrefs_telemetry.h" #include "mozilla/AppShutdown.h" #include "nsDirectoryServiceDefs.h" @@ -22,8 +23,7 @@ namespace mozilla::glean { -using mozilla::LogLevel; -static mozilla::LazyLogModule sLog("jog"); +static LazyLogModule sLog("jog"); // Storage // Thread Safety: Only used on the main thread. diff --git a/toolkit/components/glean/bindings/jog/src/lib.rs b/toolkit/components/glean/bindings/jog/src/lib.rs index b62e54f6e8..e1ea60b7ad 100644 --- a/toolkit/components/glean/bindings/jog/src/lib.rs +++ b/toolkit/components/glean/bindings/jog/src/lib.rs @@ -139,6 +139,8 @@ pub extern "C" fn jog_test_register_ping( send_if_empty: bool, precise_timestamps: bool, include_info_sections: bool, + enabled: bool, + schedules_pings: &ThinVec<nsCString>, reason_codes: &ThinVec<nsCString>, ) -> u32 { let ping_name = name.to_string(); @@ -146,12 +148,18 @@ pub extern "C" fn jog_test_register_ping( .iter() .map(|reason| reason.to_string()) .collect(); + let schedules_pings = schedules_pings + .iter() + .map(|ping| ping.to_string()) + .collect(); create_and_register_ping( ping_name, include_client_id, send_if_empty, precise_timestamps, include_info_sections, + enabled, + schedules_pings, reason_codes, ) .expect("Creation or registration of ping failed.") // permitted to panic in test-only method. @@ -163,6 +171,8 @@ fn create_and_register_ping( send_if_empty: bool, precise_timestamps: bool, include_info_sections: bool, + enabled: bool, + schedules_pings: Vec<String>, reason_codes: Vec<String>, ) -> Result<u32, Box<dyn std::error::Error>> { let ns_name = nsCString::from(&ping_name); @@ -172,6 +182,8 @@ fn create_and_register_ping( send_if_empty, precise_timestamps, include_info_sections, + enabled, + schedules_pings, reason_codes, ); extern "C" { @@ -219,6 +231,8 @@ struct PingDefinitionData { send_if_empty: bool, precise_timestamps: bool, include_info_sections: bool, + enabled: bool, + schedules_pings: Option<Vec<String>>, reason_codes: Option<Vec<String>>, } @@ -266,6 +280,8 @@ pub extern "C" fn jog_load_jogfile(jogfile_path: &nsAString) -> bool { ping.send_if_empty, ping.precise_timestamps, ping.include_info_sections, + ping.enabled, + ping.schedules_pings.unwrap_or_else(Vec::new), ping.reason_codes.unwrap_or_else(Vec::new), ); } diff --git a/toolkit/components/glean/bindings/private/TimingDistribution.h b/toolkit/components/glean/bindings/private/TimingDistribution.h index ae4a796279..ecf8689fad 100644 --- a/toolkit/components/glean/bindings/private/TimingDistribution.h +++ b/toolkit/components/glean/bindings/private/TimingDistribution.h @@ -21,8 +21,6 @@ struct GleanDistributionData; namespace mozilla::glean { -typedef uint64_t TimerId; - namespace impl { class TimingDistributionMetric { public: diff --git a/toolkit/components/glean/build_scripts/glean_parser_ext/jog.py b/toolkit/components/glean/build_scripts/glean_parser_ext/jog.py index b42827989e..08418bbce5 100644 --- a/toolkit/components/glean/build_scripts/glean_parser_ext/jog.py +++ b/toolkit/components/glean/build_scripts/glean_parser_ext/jog.py @@ -56,6 +56,8 @@ known_ping_args = [ "send_if_empty", "precise_timestamps", "include_info_sections", + "enabled", + "schedules_pings", "reason_codes", ] diff --git a/toolkit/components/glean/build_scripts/glean_parser_ext/run_glean_parser.py b/toolkit/components/glean/build_scripts/glean_parser_ext/run_glean_parser.py index bc9f09f0d3..436cca974c 100644 --- a/toolkit/components/glean/build_scripts/glean_parser_ext/run_glean_parser.py +++ b/toolkit/components/glean/build_scripts/glean_parser_ext/run_glean_parser.py @@ -12,7 +12,7 @@ import cpp import jinja2 import jog import rust -from glean_parser import lint, parser, translate, util +from glean_parser import lint, metrics, parser, translate, util from mozbuild.util import FileAvoidWrite, memoize from util import generate_metric_ids @@ -189,6 +189,19 @@ def output_gifft_map(output_fd, probe_type, all_objs, cpp_fd): file=sys.stderr, ) sys.exit(1) + # We only support mirrors for lifetime: ping + # If you understand and are okay with how Legacy Telemetry has no + # mechanism to which to mirror non-ping lifetimes, + # you may use `no_lint: [GIFFT_NON_PING_LIFETIME]` + elif ( + metric.lifetime != metrics.Lifetime.ping + and "GIFFT_NON_PING_LIFETIME" not in metric.no_lint + ): + print( + f"Glean lifetime semantics are not mirrored. {category_name}.{metric.name}'s lifetime of {metric.lifetime} is not supported.", + file=sys.stderr, + ) + sys.exit(1) env = jinja2.Environment( loader=jinja2.PackageLoader("run_glean_parser", "templates"), diff --git a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/jog_factory.jinja2 b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/jog_factory.jinja2 index a31bdbabf0..ef2088bec4 100644 --- a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/jog_factory.jinja2 +++ b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/jog_factory.jinja2 @@ -148,10 +148,12 @@ pub fn create_and_register_ping( send_if_empty: bool, precise_timestamps: bool, include_info_sections: bool, + enabled: bool, + schedules_pings: Vec<String>, reason_codes: Vec<String>, ) -> Result<u32, Box<dyn std::error::Error>> { let ping_id = NEXT_PING_ID.fetch_add(1, Ordering::SeqCst); - let ping = Ping::new(ping_name, include_client_id, send_if_empty, precise_timestamps, include_info_sections, reason_codes); + let ping = Ping::new(ping_name, include_client_id, send_if_empty, precise_timestamps, include_info_sections, enabled, schedules_pings, reason_codes); assert!( __jog_metric_maps::PING_MAP.write()?.insert(ping_id.into(), ping).is_none(), "We should never insert a runtime ping with an already-used id." diff --git a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust.jinja2 b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust.jinja2 index 5723ff5d58..3b29a0f252 100644 --- a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust.jinja2 +++ b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust.jinja2 @@ -20,10 +20,13 @@ Jinja2 template is not. Please file bugs! #} pub struct {{ name }} { {% for itemname, val in struct.properties.items() %} {% if val.type == "object" %} - pub {{itemname|snake_case}}: {{ name ~ "Item" ~ itemname|Camelize ~ "Object" }}, + #[serde(skip_serializing_if = "Option::is_none")] + pub {{itemname|snake_case}}: Option<{{ name ~ "Item" ~ itemname|Camelize ~ "Object" }}>, {% elif val.type == "array" %} + #[serde(skip_serializing_if = "Vec::is_empty", default = "Vec::new")] pub {{itemname|snake_case}}: {{ name ~ "Item" ~ itemname|Camelize }}, {% else %} + #[serde(skip_serializing_if = "Option::is_none")] pub {{itemname|snake_case}}: Option<{{val.type|structure_type_name}}>, {% endif %} {% endfor %} diff --git a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust_pings.jinja2 b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust_pings.jinja2 index 8afdae61ae..1a95b8f25c 100644 --- a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust_pings.jinja2 +++ b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust_pings.jinja2 @@ -21,6 +21,8 @@ pub static {{ obj.name|snake_case }}: Lazy<Ping> = Lazy::new(|| { {{ obj.send_if_empty|rust }}, {{ obj.precise_timestamps|rust }}, {{ obj.include_info_sections|rust }}, + {{ obj.enabled|rust }}, + {{ obj.schedules_pings|rust }}, {{ obj.reason_codes|rust }}, ) }); diff --git a/toolkit/components/glean/build_scripts/mach_commands.py b/toolkit/components/glean/build_scripts/mach_commands.py index b8d270c088..627f9f3c37 100644 --- a/toolkit/components/glean/build_scripts/mach_commands.py +++ b/toolkit/components/glean/build_scripts/mach_commands.py @@ -15,56 +15,39 @@ GENERATED_HEADER = """ """ -@Command( - "data-review", - category="misc", - description="Generate a skeleton data review request form for a given bug's data", -) -@CommandArgument( - "bug", default=None, nargs="?", type=str, help="bug number or search pattern" -) -def data_review(command_context, bug=None): - # Get the metrics_index's list of metrics indices - # by loading the index as a module. - import sys - from os import path +DATA_REVIEW_HELP = """ +Beginning 2024-05-07[1], data reviews for projects in mozilla-central are now +conducted on Phabricator. Simply duplicate your bug URL from the `bugs` list to +the `data_reviews` list in your metrics and pings definitions, and push for code +review in the normal way[2]. - sys.path.append(path.join(path.dirname(__file__), path.pardir)) - from pathlib import Path +More details about this process can be found in the in-tree docs[3] and wiki[4]. - from glean_parser import data_review - from metrics_index import metrics_yamls +If you'd like to generate a Data Review Request template anyway (if, for +instance, you can't use Phabricator for your data review or you need a Data +Review Request to aid in a Sensitive Data Review process. Or you're just +curious), you can invoke glean_parser directly: - return data_review.generate( - bug, [Path(command_context.topsrcdir) / x for x in metrics_yamls] - ) +./mach python -m glean_parser data-review + +[1]: https://groups.google.com/a/mozilla.org/g/firefox-dev/c/7z-i6UhPoKY +[2]: https://firefox-source-docs.mozilla.org/contributing/index.html +[3]: https://firefox-source-docs.mozilla.org/contributing/data-review.html +[4]: https://wiki.mozilla.org/Data_Collection +""" @Command( - "perf-data-review", + "data-review", category="misc", - description="Generate a skeleton performance data review request form for a given bug's data", -) -@CommandArgument( - "bug", default=None, nargs="?", type=str, help="bug number or search pattern" + description="Describe how Data Review works in mozilla-central", ) -def perf_data_review(command_context, bug=None): - # Get the metrics_index's list of metrics indices - # by loading the index as a module. - import sys - from os import path +def data_review(command_context): + # Data Review happens in Phabricator now + # (https://groups.google.com/a/mozilla.org/g/firefox-dev/c/7z-i6UhPoKY) + # so explain how to do it. - sys.path.append(path.join(path.dirname(__file__), path.pardir)) - from metrics_index import metrics_yamls - - sys.path.append(path.dirname(__file__)) - from pathlib import Path - - import perf_data_review - - return perf_data_review.generate( - bug, [Path(command_context.topsrcdir) / x for x in metrics_yamls] - ) + print(DATA_REVIEW_HELP) @Command( diff --git a/toolkit/components/glean/docs/user/geckoview_streaming_migration.md b/toolkit/components/glean/docs/user/geckoview_streaming_migration.md deleted file mode 100644 index 6dede3e90e..0000000000 --- a/toolkit/components/glean/docs/user/geckoview_streaming_migration.md +++ /dev/null @@ -1,262 +0,0 @@ -# Migrating Telemetry Collected via Geckoview Streaming to Glean - -With Geckoview Streaming (GVST) having been deprecated, -this is a guide to migrating collections to [Glean][book-of-glean] -via [Firefox on Glean](../index.md). - -```{contents} -``` - -## Before we Begin - -You should familiarize yourself with the guide on -[Adding New Metrics to Firefox Desktop](./new_definitions_file.md). - -You should also read through the guide for -[Migrating Metrics from Legacy Telemetry to Glean](./migration.md). - -This guide assumes some basic familiarity with the above. -The [Glean book][book-of-glean] has a full API reference, as well. - -## Process - -There are 3 main steps: - -1. Move the metric definition and make necessary updates -2. Update the call to use the Glean API -3. Update the tests to use the Glean test API - -### Move and Update the Metric Definition - -Existing metrics that make use of the GVST will already have a fully specified YAML -entry that we will use as a starting point. -This is convenient, but we want to make some minor updates rather than take it fully as is. -At a minimum we need to move the definition out of the -[Geckoview metrics.yaml][gv-metrics-yaml] file and change from GVST to [GIFFT](./gifft.md). -It can go into whichever metrics.yaml file you feel is most appropriate. -If an appropriate one does not exist, create a new one [following this guide][new-yaml]. -Completely remove the metric definition from the Geckoview `metrics.yaml`. - -For all metric types other than `labeled counters` the first step is to change the key -of the `gecko_datapoint` entry to `telemetry_mirror`. -Next, update the value as per the rules outlined in the [GIFFT guide][telemetry-mirror-doc]. -This change is required to keep data flowing in the Legacy Telemetry version of the metric. -Doing so will ensure that downstream analyses do not break unintentionally. -It is not necessary to modify the [Histograms.json][histograms-json] or -[Scalars.yaml][scalars-yaml] file. - -To migrate `labeled counters` instead fully remove the `gecko_datapoint` entry. -Note that our overall treatment of this type is slightly different. - -Next add the bug that covers the migration to the `bugs` field. -Update the `description` field as well to indicate the metric used to be collected -via the Geckoview Streaming API. Other fields should be updated as makes sense. -For example, you may need to update the field for `notification_emails`. -Since you are not changing the collection a new data review is not necessary. -However, if you notice a metric is set to expire soon and it should continue to be collected, -complete a [data review renewal][dr-renewal] form. - -Do not change the name or metric type. -**If you need to change or one both you are creating a new collection.** - -### Update Calls to Use the Glean API - -The next step is to update the metric calls to the Glean API. -Fortunately, for the vast matjority of metricsthis is a 1:1 swapout, -or for `labeled counters` (which are `categorical histograms` in legacy) we add a second call. -We identify the Glean API, remove the old call, and put in its place the Glean call. -You can find a full API reference in the [Glean][book-of-glean], but we'll look at how to record -values for the types that have existing GVST metrics. - -One way to mentally organize the metrics is to break them into two groups, those that are set, -and those that are accumulated to. As when you use the Legacy Telemetry API for GVST, -they are invoked slightly differently. - -To record in C++, you need to include `#include "mozilla/glean/GleanMetrics.h"`. -In Javascript, it is extremely unlikely that you will not already have access to `Glean`. -If you do not, please reach out to a Data Collection Tools team member on -[the #glean:mozilla.org Matrix channel](https://chat.mozilla.org/#/room/#glean:mozilla.org). - -Let's try a few examples. - -#### Migrating a Set Value (string) in C++ - -Let's look at the case of the Graphics Adaptor Vendor ID. -This is a String, -and it's recorded via GVST in C++ - -GVST YAML entry (slightly truncated): - -```YAML -geckoview: - version: - description: > - The version of the Gecko engine, example: 74.0a1 - type: string - gecko_datapoint: gecko.version -``` - -And this is recorded: - -```CPP -Telemetry::ScalarSet(Telemetry::ScalarID::GECKO_VERSION, - NS_ConvertASCIItoUTF16(gAppData->version)); -``` - -To migrate this, let's update our YAML entry, again moving it out of the GVST -metrics.yaml into the most appropriate one: - -```YAML -geckoview: - version: - description: > - The version of the Gecko engine, example: 74.0a1 - (Migrated from geckoview.gfx.adapter.primary) - type: string - telemetry_mirror: GECKO_VERSION -``` - -Notice how we've checked all of our boxes: - -* Made sure our category makes sense -* Changed gecko_datapoint to telemetry_mirror -* Updated the description to note that it was migrated from another collection -* Kept the type identical. - -Now we can update our call: - -```CPP -mozilla::glean::geckoview::version.Set(nsDependentCString(gAppData->version)); -``` - -#### Migrating a Labeled Counter in C++ - -Let's look at probably the most complicated scenario, one where we need to accumulate -to a labeled collection. Because the glean `labeled counter` and legacy `categorical histogram` -type do not support GIFFT, we will add a second call. -Let's take a look at an elided version of how this would be done with GVST: - -```CPP -switch (aResult.as<NonDecoderResult>()) { - case NonDecoderResult::SizeOverflow: - AccumulateCategorical(LABELS_AVIF_DECODE_RESULT::size_overflow); - return; - case NonDecoderResult::OutOfMemory: - AccumulateCategorical(LABELS_AVIF_DECODE_RESULT::out_of_memory); - return; - case NonDecoderResult::PipeInitError: - AccumulateCategorical(LABELS_AVIF_DECODE_RESULT::pipe_init_error); - return; -} -``` - -And we update it by adding a call with the FOG API: - -```CPP -switch (aResult.as<NonDecoderResult>()) { - case NonDecoderResult::SizeOverflow: - AccumulateCategorical(LABELS_AVIF_DECODE_RESULT::size_overflow); - mozilla::glean::avif::decode_result.EnumGet(avif::DecodeResult::eSizeOverflow).Add(); - return; - case NonDecoderResult::OutOfMemory: - AccumulateCategorical(LABELS_AVIF_DECODE_RESULT::out_of_memory); - mozilla::glean::avif::decode_result.EnumGet(avif::DecodeResult::eOutOfMemory).Add(); - return; - case NonDecoderResult::PipeInitError: - AccumulateCategorical(LABELS_AVIF_DECODE_RESULT::pipe_init_error); - mozilla::glean::avif::decode_result.EnumGet(avif::DecodeResult::ePipeInitError).Add(); - return; -} -``` - -#### Migrating an Accumulated Value (Histogram) in Javascript - -Javascript follows the same pattern. Consider the case when want to record the number -of uniqiue site origins. Here's the original JS implementation: - -```Javascript -let originCount = this.computeSiteOriginCount(aWindows, aIsGeckoView); -let histogram = Services.telemetry.getHistogramById( - "FX_NUMBER_OF_UNIQUE_SITE_ORIGINS_ALL_TABS", -); - -if (!this._lastRecordSiteOrigin) { - this._lastRecordSiteOrigin = currentTime; -} else if (currentTime >= this._lastRecordSiteOrigin + this.min_interval) { - this._lastRecordSiteOrigin = currentTime; - - histogram.add(originCount); -} -``` - -And here is the direct Glean version - -```Javascript -let originCount = this.computeSiteOriginCount(aWindows, aIsGeckoView); - -if (!this._lastRecordSiteOrigin) { - this._lastRecordSiteOrigin = currentTime; -} else if (currentTime >= this._lastRecordSiteOrigin + this.min_interval) { - this._lastRecordSiteOrigin = currentTime; - - Glean.tabs.uniqueSiteOriginsAllTabs.accumulateSamples([originCount]); -} - -``` - -Note that we don't have to call into Services to get the histogram object. - -### Update the tests to use the Glean test API - -The last piece is updating tests. If tests don't exist -(which is often the case since testing metrics collected via GVST can be challenging), -we recommend that you write them as the -[the Glean test API is quite straightforward](./instrumentation_tests.md). - -The main test method is `testGetValue()`. Returning to our earlier example of -Number of Unique Site Origins, in a Javascript test we can invoke: - -```Javascript -let result = Glean.tabs.uniqueSiteOriginsAllTabs.testGetValue(); - -// This collection is a histogram, we can check the sum for this test -Assert.equal(result.sum, 144); -``` - -If your collection is in a child process, it can be helpful to invoke -`await Services.fog.testFlushAllChildren();` - -If you wish to write a C++ test, `testGetValue()` is also our main method: - -```CPP -#include "mozilla/glean/GleanMetrics.h" - -ASSERT_EQ(1, - mozilla::glean::avif::image_decode_result - .EnumGet(avif::DecodeResult::eSizeOverflow) - .TestGetValue() - .unwrap() - .ref()); - -ASSERT_EQ(3, - mozilla::glean::avif::image_decode_result - .EnumGet(avif::DecodeResult::eOutOfMemory) - .TestGetValue() - .unwrap() - .ref()); - -ASSERT_EQ(0, - mozilla::glean::avif::image_decode_result - .EnumGet(avif::DecodeResult::ePipeInitError) - .TestGetValue() - .unwrap() - .ref()); -``` - -[book-of-glean]: https://mozilla.github.io/glean/book/index.html -[gv-metrics-yaml]: https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/geckoview/streaming/metrics.yaml -[histograms-json]: https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/Histograms.json -[scalars-yaml]: https://searchfox.org/mozilla-central/source/toolkit/components/telemetry/Scalars.yaml -[new-yaml]: ./new_definitions_file.md#where-do-i-define-new-metrics-and-pings -[dr-renewal]: https://github.com/mozilla/data-review/blob/main/renewal_request.md -[telemetry-mirror-doc]: https://firefox-source-docs.mozilla.org/toolkit/components/glean/user/gifft.html#the-telemetry-mirror-property-in-metrics-yaml diff --git a/toolkit/components/glean/docs/user/migration.md b/toolkit/components/glean/docs/user/migration.md index f3c86a182b..5743b488e6 100644 --- a/toolkit/components/glean/docs/user/migration.md +++ b/toolkit/components/glean/docs/user/migration.md @@ -138,7 +138,7 @@ So for a Histogram that records timing samples like this: ``` "GC_MS": { "record_in_processes": ["main", "content"], - "products": ["firefox", "geckoview_streaming"], + "products": ["firefox"], "alert_emails": ["dev-telemetry-gc-alerts@mozilla.org", "jcoppeard@mozilla.com"], "expires_in_version": "never", "releaseChannelCollection": "opt-out", @@ -259,7 +259,7 @@ So for a Histogram with artisinal samples like: ``` "CHECKERBOARD_SEVERITY": { "record_in_processes": ["main", "content", "gpu"], - "products": ["firefox", "fennec", "geckoview_streaming"], + "products": ["firefox"], "alert_emails": ["gfx-telemetry-alerts@mozilla.com", "botond@mozilla.com"], "bug_numbers": [1238040, 1539309, 1584109], "releaseChannelCollection": "opt-out", @@ -326,7 +326,7 @@ For example, for a Histogram of kind `categorical` like: ``` "AVIF_DECODE_RESULT": { "record_in_processes": ["main", "content"], - "products": ["firefox", "geckoview_streaming"], + "products": ["firefox"], "alert_emails": ["cchang@mozilla.com", "jbauman@mozilla.com"], "expires_in_version": "never", "releaseChannelCollection": "opt-out", @@ -560,7 +560,7 @@ gfx.display: - gfx-telemetry-alerts@mozilla.com - ktaeleman@mozilla.com products: - - 'geckoview_streaming' + - 'firefox' record_in_processes: - 'main' release_channel_collection: opt-out diff --git a/toolkit/components/glean/docs/user/ohttp.md b/toolkit/components/glean/docs/user/ohttp.md new file mode 100644 index 0000000000..ec73612db6 --- /dev/null +++ b/toolkit/components/glean/docs/user/ohttp.md @@ -0,0 +1,77 @@ +# Using Oblivious HTTP in Firefox on Glean + +[Oblivious HTTP (RFC 9458)][ohttp-spec] +is an Internet standard transport that permits a separation of privacy concerns. + +A client sending an HTTP(S) request necessarily exposes both +their network address and the request's contents to the destination server. +OHTTP allows, through the introduction of encapsulation and a relay, +for a system by which a third-party relay may learn only the network address and not the contents, +and the server may learn only the request contents and not the network address. + +This can be a useful risk mitigation for data collections we do not wish to associate with an IP address. + +## Can I use OHTTP for my Data? + +Any data collection that meets the following criteria can use OHTTP: +* Your data must be solely collected on Firefox Desktop + * At this time, no other Mozilla project supports OHTTP. +* Your data must be recorded via Glean. + * It is the sole data collection system at Mozilla that supports OHTTP. +* Your data must be in its own [custom ping][custom-ping-doc]. + * OHTTP is a transport-level decision and pings are Glean's transport payload. +* Your data (like all new or expanded data collections in Mozilla projects) + must have gone through [Data Collection Review][data-review]. + * If you're considering OHTTP it's likely because the data you intend to collect is sensitive. + That'll mean you'll probably specifically need to go through + [Sensitive Data Collection Review][sensitive-review]. +* Your data must not need to be associated with an id that is sent without OHTTP. + * This includes `client_id` and the Mozilla Accounts identifier. + The `client_id` and other fingerprinting information are explicitly excluded + from pings using OHTTP. + +## How can I use OHTTP for my Data? + +### Short Version: add two metadata fields to your ping definition + +Most simply, you opt a ping into using OHTTP by augmenting its +`pings.yaml` definition with these three lines: + +```yaml + metadata: + include_info_sections: false + use_ohttp: true +``` + +[Here is a convenience link to a searchfox search for `use_ohttp: true`][use-ohttp-searchfox] +if you'd like to see existing uses in tree. + +### Longer Version + +0. Ensure you've followed the necessary steps for + [adding new instrumentation to Firefox Desktop][new-instrumentation-doc]: + * Name your ping, + * Design and implement your instrumentation, + * Design and implement your ping submission schedule, + * Arrange for [data review][data-review] (probably [sensitive][sensitive-review]). +1. Augment your ping's definition in its `pings.yaml` with + `metadata.include_info_sections: false` and + `metadata.use_ohttp: true`: + * `include_info_sections: false` ensures that there is no + `client_id` or fingerprintable pieces of `client_info` or `ping_info` + fields that would allow us to trivially map this ping to a specific client. + * `use_ohttp: true` signals to Firefox on Glean's (FOG's) `glean_parser` extensions to + generate the necessary code to recognize this ping as needing OHTTP transport. + It is read in FOG's uploader to ensure the ping is only sent using OHTTP. +2. [Test your instrumentation][instrumentation-tests]. + +And that's it! + + +[ohttp-spec]: https://datatracker.ietf.org/doc/rfc9458/ +[custom-ping-doc]: https://mozilla.github.io/glean/book/reference/pings/index.html +[data-review]: https://wiki.mozilla.org/Data_Collection +[sensitive-review]: https://wiki.mozilla.org/Data_Collection#Step_3:_Sensitive_Data_Collection_Review_Process +[use-ohttp-searchfox]: https://searchfox.org/mozilla-central/search?q=use_ohttp%3A%20true +[new-instrumentation-doc]: ./new_definitions_file.md +[instrumentation-tests]: ./instrumentation_tests.md diff --git a/toolkit/components/glean/metrics_index.py b/toolkit/components/glean/metrics_index.py index 28759a15fa..3390f711ab 100644 --- a/toolkit/components/glean/metrics_index.py +++ b/toolkit/components/glean/metrics_index.py @@ -35,6 +35,7 @@ gecko_metrics = [ "netwerk/protocol/http/metrics.yaml", "security/certverifier/metrics.yaml", "security/manager/ssl/metrics.yaml", + "toolkit/components/antitracking/bouncetrackingprotection/metrics.yaml", "toolkit/components/cookiebanners/metrics.yaml", "toolkit/components/extensions/metrics.yaml", "toolkit/components/formautofill/metrics.yaml", @@ -67,6 +68,7 @@ firefox_desktop_metrics = [ "browser/components/urlbar/metrics.yaml", "browser/modules/metrics.yaml", "dom/media/platforms/wmf/metrics.yaml", + "toolkit/components/contentrelevancy/metrics.yaml", "toolkit/components/crashes/metrics.yaml", "toolkit/components/nimbus/metrics.yaml", "toolkit/components/search/metrics.yaml", @@ -128,6 +130,7 @@ firefox_desktop_pings = [ "browser/components/pocket/pings.yaml", "browser/components/search/pings.yaml", "browser/components/urlbar/pings.yaml", + "browser/modules/pings.yaml", "toolkit/components/crashes/pings.yaml", "toolkit/components/resistfingerprinting/pings.yaml", "toolkit/components/telemetry/pings.yaml", diff --git a/toolkit/components/glean/src/lib.rs b/toolkit/components/glean/src/lib.rs index b682f43066..6c9af50183 100644 --- a/toolkit/components/glean/src/lib.rs +++ b/toolkit/components/glean/src/lib.rs @@ -197,12 +197,12 @@ pub extern "C" fn fog_test_get_experiment_data( /// /// See [`glean_core::Glean::set_metrics_disabled_config`]. #[no_mangle] -pub extern "C" fn fog_set_metrics_feature_config(config_json: &nsACString) { +pub extern "C" fn fog_apply_server_knobs_config(config_json: &nsACString) { // Normalize null and empty strings to a stringified empty map if config_json == "null" || config_json.is_empty() { - glean::glean_set_metrics_enabled_config("{}".to_owned()); + glean::glean_apply_server_knobs_config("{}".to_owned()); } - glean::glean_set_metrics_enabled_config(config_json.to_string()); + glean::glean_apply_server_knobs_config(config_json.to_string()); } /// Performs Glean tasks when client state changes to inactive diff --git a/toolkit/components/glean/tags.yaml b/toolkit/components/glean/tags.yaml index c935371b28..2313eda344 100644 --- a/toolkit/components/glean/tags.yaml +++ b/toolkit/components/glean/tags.yaml @@ -9,6 +9,8 @@ --- $schema: moz://mozilla.org/schemas/glean/tags/1-0-0 +'Application Services :: Relevancy': + description: The Bugzilla component which applies to this object. 'Cloud Services :: Firefox: Common': description: The Bugzilla component which applies to this object. 'Conduit :: mots': diff --git a/toolkit/components/glean/tests/pytest/jogfile_output b/toolkit/components/glean/tests/pytest/jogfile_output index 510b4995c5..7456bb1ffd 100644 --- a/toolkit/components/glean/tests/pytest/jogfile_output +++ b/toolkit/components/glean/tests/pytest/jogfile_output @@ -25,7 +25,7 @@ [ "metrics" ], - "application", + "ping", false, { "bucket_count": 100, @@ -129,7 +129,7 @@ [ "metrics" ], - "application", + "ping", false, { "memory_unit": "kilobyte" @@ -180,7 +180,7 @@ [ "metrics" ], - "application", + "ping", false, { "time_unit": "nanosecond" @@ -313,6 +313,8 @@ false, true, true, + true, + [], [ "background", "dirty_startup", @@ -325,6 +327,8 @@ true, true, true, + true, + [], [] ], [ @@ -333,6 +337,8 @@ false, true, true, + true, + [], [ "background", "max_capacity", @@ -345,6 +351,8 @@ false, true, true, + true, + [], [ "overdue", "reschedule", @@ -359,6 +367,8 @@ true, true, false, + true, + [], [] ] ] diff --git a/toolkit/components/glean/tests/pytest/metrics_test.yaml b/toolkit/components/glean/tests/pytest/metrics_test.yaml index 6d9a4f8c0e..2bdcd1877f 100644 --- a/toolkit/components/glean/tests/pytest/metrics_test.yaml +++ b/toolkit/components/glean/tests/pytest/metrics_test.yaml @@ -6,11 +6,6 @@ # automatically converted to platform-specific code at build time using the # `glean_parser` PyPI package. -# This file is presently for Internal FOG Use Only. -# You should not add metrics here until probably about January of 2021. -# If you're looking for the metrics.yaml for Geckoveiw Streaming Telemetry, -# you can find that one in toolkit/components/telemetry/geckoview/streaming. - --- $schema: moz://mozilla.org/schemas/glean/metrics/2-0-0 @@ -29,6 +24,8 @@ test: data_reviews: - https://example.com telemetry_mirror: SOME_BOOL_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME labeled_boolean_metric: type: labeled_boolean @@ -44,6 +41,8 @@ test: data_reviews: - https://example.com telemetry_mirror: SOME_KEYED_BOOL_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME labeled_boolean_metric_labels: type: labeled_boolean @@ -70,6 +69,8 @@ test: - nine_labels - ten_labels telemetry_mirror: SOME_OTHER_KEYED_BOOL_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME counter_metric: type: counter @@ -85,6 +86,8 @@ test: data_reviews: - https://example.com telemetry_mirror: SOME_UINT_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME labeled_counter_metric: type: labeled_counter @@ -100,6 +103,8 @@ test: data_reviews: - https://example.com telemetry_mirror: SOME_KEYED_UINT_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME labeled_counter_metric_labels: type: labeled_counter @@ -118,6 +123,8 @@ test: - one_label - two_labels telemetry_mirror: SOME_OTHER_KEYED_UINT_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME string_metric: type: string @@ -133,6 +140,8 @@ test: data_reviews: - https://example.com telemetry_mirror: SOME_STRING_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME labeled_string_metric: type: labeled_string @@ -179,6 +188,8 @@ test: data_reviews: - https://example.com telemetry_mirror: YET_ANOTHER_KEYED_BOOL_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME text_metric: type: text @@ -209,6 +220,8 @@ test: data_reviews: - https://example.com telemetry_mirror: SOME_OTHER_UINT_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME timing_distribution_metric: type: timing_distribution @@ -216,7 +229,6 @@ test: description: | A multi-line description - lifetime: application notification_emails: - glean-team@mozilla.com bugs: @@ -231,7 +243,6 @@ test: description: | A multi-line description - lifetime: application notification_emails: - glean-team@mozilla.com bugs: @@ -247,7 +258,6 @@ test: description: | A multi-line description - lifetime: application notification_emails: - glean-team@mozilla.com bugs: @@ -275,6 +285,8 @@ test.nested: data_reviews: - https://example.com telemetry_mirror: SOME_OTHER_STRING_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME datetime_metric: type: datetime @@ -290,6 +302,8 @@ test.nested: data_reviews: - https://example.com telemetry_mirror: SOME_STILL_OTHER_STRING_SCALAR + no_lint: + - GIFFT_NON_PING_LIFETIME event_metric: type: event diff --git a/toolkit/components/glean/tests/pytest/metrics_test_output b/toolkit/components/glean/tests/pytest/metrics_test_output index 0f93eb032d..79a1b487fa 100644 --- a/toolkit/components/glean/tests/pytest/metrics_test_output +++ b/toolkit/components/glean/tests/pytest/metrics_test_output @@ -62,7 +62,7 @@ pub mod test { name: "custom_distribution_metric".into(), category: "test".into(), send_in_pings: vec!["metrics".into()], - lifetime: Lifetime::Application, + lifetime: Lifetime::Ping, disabled: false, ..Default::default() }, 0, 100, 100, HistogramType::Linear) @@ -270,7 +270,7 @@ pub mod test { name: "memory_distribution_metric".into(), category: "test".into(), send_in_pings: vec!["metrics".into()], - lifetime: Lifetime::Application, + lifetime: Lifetime::Ping, disabled: false, ..Default::default() }, MemoryUnit::Kilobyte) @@ -350,7 +350,7 @@ pub mod test { name: "timing_distribution_metric".into(), category: "test".into(), send_in_pings: vec!["metrics".into()], - lifetime: Lifetime::Application, + lifetime: Lifetime::Ping, disabled: false, ..Default::default() }, TimeUnit::Nanosecond) @@ -373,7 +373,9 @@ pub mod test_nested { #[derive(Debug, Hash, Eq, PartialEq, ::glean::traits::__serde::Serialize, ::glean::traits::__serde::Deserialize)] #[serde(deny_unknown_fields)] pub struct AnObjectObjectItem { + #[serde(skip_serializing_if = "Option::is_none")] pub colour: Option<String>, + #[serde(skip_serializing_if = "Option::is_none")] pub diameter: Option<i64>, } diff --git a/toolkit/components/glean/tests/pytest/pings_test_output b/toolkit/components/glean/tests/pytest/pings_test_output index 97c0793b1f..7214cd1756 100644 --- a/toolkit/components/glean/tests/pytest/pings_test_output +++ b/toolkit/components/glean/tests/pytest/pings_test_output @@ -21,6 +21,8 @@ pub static not_baseline: Lazy<Ping> = Lazy::new(|| { false, true, true, + true, + vec![], vec!["background".into(), "dirty_startup".into(), "foreground".into()], ) }); @@ -38,6 +40,8 @@ pub static not_deletion_request: Lazy<Ping> = Lazy::new(|| { true, true, true, + true, + vec![], vec![], ) }); @@ -53,6 +57,8 @@ pub static not_events: Lazy<Ping> = Lazy::new(|| { false, true, true, + true, + vec![], vec!["background".into(), "max_capacity".into(), "startup".into()], ) }); @@ -72,6 +78,8 @@ pub static not_metrics: Lazy<Ping> = Lazy::new(|| { false, true, true, + true, + vec![], vec!["overdue".into(), "reschedule".into(), "today".into(), "tomorrow".into(), "upgrade".into()], ) }); @@ -85,6 +93,8 @@ pub static not_ohttp: Lazy<Ping> = Lazy::new(|| { true, true, false, + true, + vec![], vec![], ) }); diff --git a/toolkit/components/glean/tests/test_metrics.yaml b/toolkit/components/glean/tests/test_metrics.yaml index 47587241b1..3db601610d 100644 --- a/toolkit/components/glean/tests/test_metrics.yaml +++ b/toolkit/components/glean/tests/test_metrics.yaml @@ -423,6 +423,42 @@ test_only: diameter: type: number + crash_stack: + type: object + description: A not-real crash stack + bugs: + - https://bugzilla.mozilla.org/1839640 + data_reviews: + - http://example.com/reviews + notification_emails: + - CHANGE-ME@example.com + expires: never + structure: + type: object + properties: + status: + type: string + main_module: + type: number + crash_info: + type: object + properties: + typ: + type: string + address: + type: string + crashing_thread: + type: number + modules: + type: array + items: + type: object + properties: + base_addr: + type: string + end_addr: + type: string + test_only.ipc: a_counter: type: counter diff --git a/toolkit/components/glean/tests/xpcshell/test_Glean.js b/toolkit/components/glean/tests/xpcshell/test_Glean.js index 18b450a69b..f5661f91b6 100644 --- a/toolkit/components/glean/tests/xpcshell/test_Glean.js +++ b/toolkit/components/glean/tests/xpcshell/test_Glean.js @@ -514,7 +514,7 @@ add_task(async function test_fog_object_works() { let expected = [ { colour: "red", diameter: 5 }, { colour: "blue", diameter: 7 }, - { colour: "orange", diameter: null }, + { colour: "orange" }, ]; Assert.deepEqual(expected, result); @@ -528,10 +528,10 @@ add_task(async function test_fog_object_works() { Glean.testOnly.balloons.set(balloons); result = Glean.testOnly.balloons.testGetValue(); expected = [ - { colour: "inf", diameter: null }, - { colour: "negative-inf", diameter: null }, - { colour: "nan", diameter: null }, - { colour: "undef", diameter: null }, + { colour: "inf" }, + { colour: "negative-inf" }, + { colour: "nan" }, + { colour: "undef" }, ]; Assert.deepEqual(expected, result); @@ -562,3 +562,69 @@ add_task(async function test_fog_object_works() { "Should throw because last object was invalid." ); }); + +add_task(async function test_fog_complex_object_works() { + if (!Glean.testOnly.crashStack) { + // FIXME(bug 1883857): object metric type not available, e.g. in artifact builds. + // Skipping this test. + return; + } + + Assert.equal( + undefined, + Glean.testOnly.crashStack.testGetValue(), + "No object stored" + ); + + Glean.testOnly.crashStack.set({}); + let result = Glean.testOnly.crashStack.testGetValue(); + Assert.deepEqual({}, result); + + let stack = { + status: "OK", + crash_info: { + typ: "main", + address: "0xf001ba11", + crashing_thread: 1, + }, + main_module: 0, + modules: [ + { + base_addr: "0x00000000", + end_addr: "0x00004000", + }, + ], + }; + + Glean.testOnly.crashStack.set(stack); + result = Glean.testOnly.crashStack.testGetValue(); + Assert.deepEqual(stack, result); + + stack = { + status: "OK", + modules: [ + { + base_addr: "0x00000000", + end_addr: "0x00004000", + }, + ], + }; + Glean.testOnly.crashStack.set(stack); + result = Glean.testOnly.crashStack.testGetValue(); + Assert.deepEqual(stack, result); + + stack = { + status: "OK", + modules: [], + }; + Glean.testOnly.crashStack.set(stack); + result = Glean.testOnly.crashStack.testGetValue(); + Assert.deepEqual({ status: "OK" }, result); + + stack = { + status: "OK", + }; + Glean.testOnly.crashStack.set(stack); + result = Glean.testOnly.crashStack.testGetValue(); + Assert.deepEqual(stack, result); +}); diff --git a/toolkit/components/glean/tests/xpcshell/test_GleanServerKnobs.js b/toolkit/components/glean/tests/xpcshell/test_GleanServerKnobs.js index 31ed262798..6b36d74ec4 100644 --- a/toolkit/components/glean/tests/xpcshell/test_GleanServerKnobs.js +++ b/toolkit/components/glean/tests/xpcshell/test_GleanServerKnobs.js @@ -21,9 +21,11 @@ add_task(function test_fog_metrics_disabled_remotely() { // Create and set a feature configuration that disables the test metric. const feature_config = { - "test_only.cheesy_string": false, + metrics_enabled: { + "test_only.cheesy_string": false, + }, }; - Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config)); + Services.fog.applyServerKnobsConfig(JSON.stringify(feature_config)); // Attempt to set another cheesy string in the test metric. This should not // record because of the override to the metric's default value in the @@ -49,10 +51,12 @@ add_task(function test_fog_multiple_metrics_disabled_remotely() { // Create and set a feature configuration that disables multiple test // metrics. var feature_config = { - "test_only.cheesy_string": false, - "test_only.meaning_of_life": false, + metrics_enabled: { + "test_only.cheesy_string": false, + "test_only.meaning_of_life": false, + }, }; - Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config)); + Services.fog.applyServerKnobsConfig(JSON.stringify(feature_config)); // Attempt to set the metrics again. This should not record because of the // override to the metrics' default value in the feature configuration. @@ -65,10 +69,12 @@ add_task(function test_fog_multiple_metrics_disabled_remotely() { // Change the feature configuration to re-enable the `cheesy_string` metric. feature_config = { - "test_only.cheesy_string": true, - "test_only.meaning_of_life": false, + metrics_enabled: { + "test_only.cheesy_string": true, + "test_only.meaning_of_life": false, + }, }; - Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config)); + Services.fog.applyServerKnobsConfig(JSON.stringify(feature_config)); // Attempt to set the metrics again. This should only record `cheesy_string` // because of the most recent feature configuration. @@ -101,9 +107,11 @@ add_task(function test_fog_metrics_feature_config_api_handles_null_values() { // Create and set a feature configuration that disables the test metric. const feature_config = { - "test_only.cheesy_string": false, + metrics_enabled: { + "test_only.cheesy_string": false, + }, }; - Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config)); + Services.fog.applyServerKnobsConfig(JSON.stringify(feature_config)); // Attempt to set another cheesy string in the test metric. This should not // record because of the override to the metric's default value in the @@ -113,7 +121,7 @@ add_task(function test_fog_metrics_feature_config_api_handles_null_values() { Assert.equal(str1, Glean.testOnly.cheesyString.testGetValue("test-ping")); // Set the configuration to `null`. - Services.fog.setMetricsFeatureConfig(null); + Services.fog.applyServerKnobsConfig(null); // Attempt to set another cheesy string in the test metric. This should now // record because `null` doesn't change already existing configuration. @@ -122,7 +130,7 @@ add_task(function test_fog_metrics_feature_config_api_handles_null_values() { // Set the configuration to `""` to replicate getting an empty string from // Nimbus. - Services.fog.setMetricsFeatureConfig(""); + Services.fog.applyServerKnobsConfig(""); // Attempt to set another cheesy string in the test metric. This should now // record again because `""` doesn't change already existing configuration. @@ -140,9 +148,11 @@ add_task(function test_fog_metrics_disabled_reset_fog_behavior() { // Create and set a feature configuration that disables the test metric. const feature_config = { - "test_only.cheesy_string": false, + metrics_enabled: { + "test_only.cheesy_string": false, + }, }; - Services.fog.setMetricsFeatureConfig(JSON.stringify(feature_config)); + Services.fog.applyServerKnobsConfig(JSON.stringify(feature_config)); // Attempt to set another cheesy string in the test metric. This should not // record because of the override to the metric's default value in the diff --git a/toolkit/components/glean/tests/xpcshell/test_JOG.js b/toolkit/components/glean/tests/xpcshell/test_JOG.js index 00f3ded135..2017d7a52e 100644 --- a/toolkit/components/glean/tests/xpcshell/test_JOG.js +++ b/toolkit/components/glean/tests/xpcshell/test_JOG.js @@ -289,7 +289,16 @@ add_task(async function test_jog_custom_pings() { `"ping"`, false ); - Services.fog.testRegisterRuntimePing("jog-ping", true, true, true, true, []); + Services.fog.testRegisterRuntimePing( + "jog-ping", + true, + true, + true, + true, + true, + [], + [] + ); Assert.ok("jogPing" in GleanPings); let submitted = false; Glean.jogCat.jogPingBool.set(false); @@ -639,9 +648,16 @@ add_task(function test_jog_dotted_categories_work() { add_task(async function test_jog_ping_works() { const kReason = "reason-1"; - Services.fog.testRegisterRuntimePing("my-ping", true, true, true, true, [ - kReason, - ]); + Services.fog.testRegisterRuntimePing( + "my-ping", + true, + true, + true, + true, + true, + [], + [kReason] + ); let submitted = false; GleanPings.myPing.testBeforeNextSubmit(reason => { submitted = true; @@ -653,9 +669,16 @@ add_task(async function test_jog_ping_works() { add_task(async function test_jog_noinfo_ping_works() { const kReason = "reason-1"; - Services.fog.testRegisterRuntimePing("noinfo-ping", true, true, true, false, [ - kReason, - ]); + Services.fog.testRegisterRuntimePing( + "noinfo-ping", + true, + true, + true, + false, + true, + [], + [kReason] + ); let submitted = false; GleanPings.noinfoPing.testBeforeNextSubmit(reason => { submitted = true; diff --git a/toolkit/components/glean/xpcom/FOG.cpp b/toolkit/components/glean/xpcom/FOG.cpp index 955f2511b6..1517bd0597 100644 --- a/toolkit/components/glean/xpcom/FOG.cpp +++ b/toolkit/components/glean/xpcom/FOG.cpp @@ -11,7 +11,6 @@ #include "mozilla/ClearOnShutdown.h" #include "mozilla/dom/Promise.h" #include "mozilla/FOGIPC.h" -#include "mozilla/browser/NimbusFeatures.h" #include "mozilla/glean/bindings/Common.h" #include "mozilla/glean/bindings/jog/jog_ffi_generated.h" #include "mozilla/glean/fog_ffi_generated.h" @@ -119,8 +118,7 @@ extern "C" bool FOG_TooLateToSend(void) { // This allows us to pass the configurable maximum ping limit (in pings per // minute) to Rust. Default value is 15. extern "C" uint32_t FOG_MaxPingLimit(void) { - return NimbusFeatures::GetInt("gleanInternalSdk"_ns, - "gleanMaxPingsPerMinute"_ns, 15); + return Preferences::GetInt("telemetry.glean.internal.maxPingsPerMinute", 15); } // Called when knowing if we're in automation is necessary. @@ -134,8 +132,8 @@ FOG::InitializeFOG(const nsACString& aDataPathOverride, gInitializeCalled = true; RunOnShutdown( [&] { - if (NimbusFeatures::GetBool("gleanInternalSdk"_ns, "finalInactive"_ns, - false)) { + if (Preferences::GetBool("telemetry.glean.internal.finalInactive", + false)) { glean::impl::fog_internal_glean_handle_client_inactive(); } }, @@ -314,14 +312,14 @@ FOG::TestGetExperimentData(const nsACString& aExperimentId, JSContext* aCx, } NS_IMETHODIMP -FOG::SetMetricsFeatureConfig(const nsACString& aJsonConfig) { +FOG::ApplyServerKnobsConfig(const nsACString& aJsonConfig) { #ifdef MOZ_GLEAN_ANDROID NS_WARNING( "Don't set metric feature configs from Gecko in Android. Ignoring."); return NS_OK; #else MOZ_ASSERT(XRE_IsParentProcess()); - glean::impl::fog_set_metrics_feature_config(&aJsonConfig); + glean::impl::fog_apply_server_knobs_config(&aJsonConfig); return NS_OK; #endif } @@ -410,17 +408,16 @@ FOG::TestRegisterRuntimeMetric( } NS_IMETHODIMP -FOG::TestRegisterRuntimePing(const nsACString& aName, - const bool aIncludeClientId, - const bool aSendIfEmpty, - const bool aPreciseTimestamps, - const bool aIncludeInfoSections, - const nsTArray<nsCString>& aReasonCodes, - uint32_t* aPingIdOut) { +FOG::TestRegisterRuntimePing( + const nsACString& aName, const bool aIncludeClientId, + const bool aSendIfEmpty, const bool aPreciseTimestamps, + const bool aIncludeInfoSections, const bool aEnabled, + const nsTArray<nsCString>& aSchedulesPings, + const nsTArray<nsCString>& aReasonCodes, uint32_t* aPingIdOut) { *aPingIdOut = 0; *aPingIdOut = glean::jog::jog_test_register_ping( &aName, aIncludeClientId, aSendIfEmpty, aPreciseTimestamps, - aIncludeInfoSections, &aReasonCodes); + aIncludeInfoSections, aEnabled, &aSchedulesPings, &aReasonCodes); return NS_OK; } diff --git a/toolkit/components/glean/xpcom/nsIFOG.idl b/toolkit/components/glean/xpcom/nsIFOG.idl index 67041d6228..86409b2f6d 100644 --- a/toolkit/components/glean/xpcom/nsIFOG.idl +++ b/toolkit/components/glean/xpcom/nsIFOG.idl @@ -113,7 +113,7 @@ interface nsIFOG : nsISupports * {metric_base_identifier: boolean,} * which may contain multiple metric object entries. */ - void setMetricsFeatureConfig(in ACString aJsonConfig); + void applyServerKnobsConfig(in ACString aJsonConfig); /** * ** Test-only Method ** @@ -184,6 +184,7 @@ interface nsIFOG : nsISupports * @param aSendIfEmpty - Whether the ping should send even if empty. * @param aIncludeInfoSections - Whether the ping should include * {client|ping}_info sections. + * TODO(jer): docs * @param aReasonCodes - The list of valid reasons for ping submission. */ uint32_t testRegisterRuntimePing(in ACString aName, @@ -191,5 +192,7 @@ interface nsIFOG : nsISupports in boolean aSendIfEmpty, in boolean aPreciseTimestamps, in boolean aIncludeInfoSections, + in boolean aEnabled, + in Array<ACString> aSchedulesPings, in Array<ACString> aReasonCodes); }; |