From af912502794790a18904ebcca6037dbd325cd776 Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Tue, 14 Jan 2020 16:21:22 -0500 Subject: [PATCH] 1557048: Add reason codes to the metrics ping This adds support for sending reason codes along with pings. The reason codes are defined as an enumeration in the pings.yaml file, and only these values are allowed on specific pings. Additionally, this builds on that to add reason codes to the metrics ping. --- CHANGELOG.md | 6 +- docs/user/pings/index.md | 2 +- docs/user/pings/metrics.md | 80 ++++--------- glean-core/README.md | 4 +- glean-core/android/build.gradle | 2 +- .../java/mozilla/telemetry/glean/Glean.kt | 49 ++++---- .../glean/debug/GleanDebugActivity.kt | 2 +- .../telemetry/glean/private/PingType.kt | 47 ++++++-- .../telemetry/glean/rust/LibGleanFFI.kt | 14 ++- .../glean/scheduler/MetricsPingScheduler.kt | 52 ++++++--- .../java/mozilla/telemetry/glean/GleanTest.kt | 19 ++- .../java/mozilla/telemetry/glean/TestUtil.kt | 4 +- .../glean/private/EventMetricTypeTest.kt | 2 +- .../glean/private/LabeledMetricTypeTest.kt | 4 +- .../telemetry/glean/private/PingTypeTest.kt | 22 ++-- .../scheduler/MetricsPingSchedulerTest.kt | 108 ++++++++++++------ glean-core/benchmark/benches/bench_basic.rs | 4 +- glean-core/examples/sample.rs | 8 +- glean-core/ffi/examples/glean_app.c | 4 +- glean-core/ffi/glean.h | 10 +- glean-core/ffi/src/lib.rs | 15 +-- glean-core/ffi/src/ping_type.rs | 6 +- .../ios/Glean/Debug/GleanDebugTools.swift | 2 +- glean-core/ios/Glean/Glean.swift | 46 ++++---- glean-core/ios/Glean/GleanFfi.h | 10 +- glean-core/ios/Glean/Metrics/Ping.swift | 58 ++++++++-- .../Scheduler/MetricsPingScheduler.swift | 61 ++++++++-- .../GleanTests/Metrics/EventMetricTests.swift | 2 +- .../ios/GleanTests/Metrics/PingTests.swift | 16 ++- .../Scheduler/MetricsPingSchedulerTests.swift | 14 ++- glean-core/ios/sdk_generator.sh | 2 +- glean-core/metrics.yaml | 14 +++ glean-core/pings.yaml | 16 +++ glean-core/preview/README.md | 4 +- glean-core/preview/examples/prototype.rs | 5 +- glean-core/preview/src/lib.rs | 21 +--- glean-core/preview/src/metrics/ping.rs | 20 +++- glean-core/python/glean/_ffi.py | 6 +- glean-core/python/glean/_loader.py | 9 +- glean-core/python/glean/glean.py | 34 +++--- glean-core/python/glean/metrics/ping.py | 30 ++++- glean-core/python/setup.py | 2 +- glean-core/python/tests/metrics/test_event.py | 2 +- glean-core/python/tests/test_glean.py | 14 ++- glean-core/src/event_database/mod.rs | 4 +- glean-core/src/internal_pings.rs | 19 ++- glean-core/src/lib.rs | 41 +++---- glean-core/src/metrics/ping.rs | 28 ++++- glean-core/src/ping/mod.rs | 28 ++++- glean-core/tests/event.rs | 2 +- glean-core/tests/ping.rs | 18 +-- glean-core/tests/ping_maker.rs | 44 +++---- .../GleanGradlePlugin.groovy | 2 +- 53 files changed, 673 insertions(+), 365 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8c8fad2f8..b01bfad621 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,11 @@ in the baseline ping ([`glean.baseline.locale`](https://github.com/mozilla/glean/blob/c261205d6e84d2ab39c50003a8ffc3bd2b763768/glean-core/metrics.yaml#L28-L42)) is redundant and will be removed by the end of the quarter. * Drop the Glean handle and move state into glean-core ([#664](https://github.com/mozilla/glean/pull/664)) - * If an experiment includes no `extra` fields, it will no longer include `{"extra": null}` in the JSON paylod. + * If an experiment includes no `extra` fields, it will no longer include `{"extra": null}` in the JSON payload. + * Support for ping `reason` codes was added. + * The metrics ping will now include `reason` codes that indicate why it was + submitted. + * The version of `glean_parser` has been upgraded to 1.17.2 * Android: * Collections performed before initialization (preinit tasks) are now dispatched off the main thread during initialization. diff --git a/docs/user/pings/index.md b/docs/user/pings/index.md index d3a8de567f..70c2292dc9 100644 --- a/docs/user/pings/index.md +++ b/docs/user/pings/index.md @@ -31,6 +31,7 @@ Optional fields are marked accordingly. | `experiments` | Object | *Optional*. A dictionary of [active experiments](#the-experiments-object) | | `start_time` | Datetime | The time of the start of collection of the data in the ping, in local time and with minute precision, including timezone information. | | `end_time` | Datetime | The time of the end of collection of the data in the ping, in local time and with minute precision, including timezone information. This is also the time this ping was generated and is likely well before ping transmission time. | +| `reason` | String | The optional reason the ping was submitted. The specific set of values and their meanings are defined for each metric type in the `reasons` field in the `pings.yaml` file. | All the metrics surviving application restarts (e.g. `seq`, ...) are removed once the application using the Glean SDK is uninstalled. @@ -104,4 +105,3 @@ These docs refer to application 'background' state in several places. This specifically means when the activity is no longer visible to the user, it has entered the Stopped state, and the system invokes the [`onStop()`](https://developer.android.com/reference/android/app/Activity.html#onStop()) callback. This may occur, for example, when a newly launched activity covers the entire screen. The system may also call `onStop()` when the activity has finished running, and is about to be terminated. - diff --git a/docs/user/pings/metrics.md b/docs/user/pings/metrics.md index 1eaa1ecf7f..1beb148897 100644 --- a/docs/user/pings/metrics.md +++ b/docs/user/pings/metrics.md @@ -1,29 +1,36 @@ # The `metrics` ping ## Description -The `metrics` ping is intended for all of the metrics that are explicitly set by the application or are included in the application's `metrics.yaml` file (except events). -The reported data is tied to the ping's *measurement window*, which is the time between the collection of two `metrics` ping. -Ideally, this window is expected to be about 24 hours, given that the collection is scheduled daily at 4AM. -Data in the [`ping_info`](index.md#the-ping_info-section) section of the ping can be used to infer the length of this window. +The `metrics` ping is intended for all of the metrics that are explicitly set by the application or are included in the application's `metrics.yaml` file (except events). +The reported data is tied to the ping's *measurement window*, which is the time between the collection of two `metrics` ping. +Ideally, this window is expected to be about 24 hours, given that the collection is scheduled daily at 4AM. +However, the metrics ping is only submitted while the application is actually running, so in practice, it may not meet the 4AM target very frequently. +Data in the [`ping_info`](index.md#the-ping_info-section) section of the ping can be used to infer the length of this window and the reason that triggered the ping to be submitted. If the application crashes, unsent recorded metrics are sent along with the next `metrics` ping. +Additionally, it is undesirable to mix metric recording from different versions of the application. Therefore, if a version upgrade is detected, the `metrics` ping is collected immediately before further metrics from the new version are recorded. + > **Note:** As the `metrics` ping was specifically designed for mobile operating systems, it is not sent when using the Glean Python bindings. ## Scheduling -The desired behavior is to collect the ping at the first available opportunity after 4AM local time on a new calendar day. +The desired behavior is to collect the ping at the first available opportunity after 4AM local time on a new calendar day, but given constraints of the platform, it can only be submitted while the application is running. This breaks down into three scenarios: 1. the application was just installed; -2. the application was just started (after a crash or a long inactivity period); -3. the application was open and the 4AM due time was hit. +2. the application was just upgraded (the version of the app is different from the last time the app was run); +3. the application was just started (after a crash or a long inactivity period); +4. the application was running at 4AM. + +In the first case, since the application was just installed, if the due time for the current calendar day has passed, a `metrics` ping is immediately generated and scheduled for sending (reason code `overdue`). Otherwise, if the due time for the current calendar day has not passed, a ping collection is scheduled for that time (reason code `today`). -In the first case, since the application was just installed, if the due time for the current calendar day has passed, a `metrics` ping is immediately generated and scheduled for sending. Otherwise, if the due time for the current calendar day has not passed, a ping collection is scheduled for that time. +In the second case, if a version change is detected at startup, the metrics ping is immediately submitted so that metrics from one version are not aggregated with metrics from another version (reason code `upgrade`). -In the second case, if the `metrics` ping was already collected on the current calendar day, a new collection will be scheduled for the next calendar day, at 4AM. -If no collection happened yet, and the due time for the current calendar day has passed, a `metrics` ping is immediately generated and scheduled for sending. +In the third case, if the `metrics` ping was not already collected on the current calendar day, and it is before 4AM, a collection is scheduled for 4AM on the current calendar day (reason code `today`). +If it is after 4AM, a new collection is scheduled immediately (reason code `overdue`). +Lastly, if a ping was already collected on the current calendar day, the next one is scheduled for collecting at 4AM on the next calendar day (reason code `tomorrow`). -In the third case, similarly to the previous case, if the `metrics` ping was already collected on the current calendar day when we hit 4AM, then a new collection is scheduled for the next calendar day. -Otherwise, the `metrics` is immediately collected and scheduled for sending. +In the fourth and last case, the application is running during a scheduled ping collection time. +The next ping is scheduled for 4AM the next calendar day (reason code `reschedule`). More [scheduling examples](#scheduling-examples) are included below. @@ -35,53 +42,8 @@ Additionally, error metrics in the `glean.error` category are included in the `m The `metrics` ping shall also include the common [`ping_info`](index.md#the-ping_info-section) and ['client_info'](index.md#the-client_info-section) sections. ### Querying ping contents -A quick note about querying ping contents (i.e. for [sql.telemetry.mozilla.org](https://sql.telemetry.mozilla.org)): Each metric in the metrics ping is organized by its metric type, and uses a namespace of 'glean.metrics'. -For instance, in order to select a String field called `test` you would use `metrics.string['glean.metrics.test']`. - -### Example metrics ping - -```json -{ - "ping_info": { - "ping_type": "metrics", - "experiments": { - "third_party_library": { - "branch": "enabled" - } - }, - "seq": 0, - "start_time": "2019-03-29T09:50-04:00", - "end_time": "2019-03-29T10:02-04:00" - }, - "client_info": { - "telemetry_sdk_build": "0.49.0", - "first_run_date": "2019-03-29-04:00", - "os": "Android", - "android_sdk_version": "27", - "os_version": "8.1.0", - "device_manufacturer": "Google", - "device_model": "Android SDK built for x86", - "architecture": "x86", - "app_build": "1", - "app_display_version": "1.0", - "client_id": "35dab852-74db-43f4-8aa0-88884211e545" - }, - "metrics": { - "counter": { - "sample_metrics.test": 1 - }, - "string": { - "basic.os": "Android" - }, - "timespan": { - "test.test_timespan": { - "time_unit": "microsecond", - "value": 181908 - } - } - } -} -``` + +Information about query ping contents is available in [Accessing Glean data](https://docs.telemetry.mozilla.org/concepts/glean/accessing_glean_data.html) in the Firefox data docs. ## Scheduling Examples diff --git a/glean-core/README.md b/glean-core/README.md index 56e041364e..68868f2565 100644 --- a/glean-core/README.md +++ b/glean-core/README.md @@ -32,7 +32,7 @@ let cfg = Configuration { max_events: None, }; let mut glean = Glean::new(cfg).unwrap(); -let ping = PingType::new("sample", true); +let ping = PingType::new("sample", true, true, vec![]); glean.register_ping_type(&ping); let call_counter: CounterMetric = CounterMetric::new(CommonMetricData { @@ -44,7 +44,7 @@ let call_counter: CounterMetric = CounterMetric::new(CommonMetricData { call_counter.add(&glean, 1); -glean.submit_ping(&ping).unwrap(); +glean.submit_ping(&ping, None).unwrap(); ``` ## License diff --git a/glean-core/android/build.gradle b/glean-core/android/build.gradle index e851adeeee..993b81a77c 100644 --- a/glean-core/android/build.gradle +++ b/glean-core/android/build.gradle @@ -27,7 +27,7 @@ apply plugin: 'jacoco' * Everytime this hash is changed it should also be changed in * glean-core/python/tests/conftest.py */ -String GLEAN_PING_SCHEMA_GIT_HASH = "f2a7ce4" +String GLEAN_PING_SCHEMA_GIT_HASH = "ee08e7c" String GLEAN_PING_SCHEMA_URL = "https://raw.githubusercontent.com/mozilla-services/mozilla-pipeline-schemas/$GLEAN_PING_SCHEMA_GIT_HASH/schemas/glean/glean/glean.1.schema.json" // Set configuration for the glean_parser diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt index 776b771bcc..6a4e742b50 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/Glean.kt @@ -27,7 +27,7 @@ import mozilla.telemetry.glean.GleanMetrics.GleanBaseline import mozilla.telemetry.glean.GleanMetrics.GleanInternalMetrics import mozilla.telemetry.glean.GleanMetrics.Pings import mozilla.telemetry.glean.net.BaseUploader -import mozilla.telemetry.glean.private.PingType +import mozilla.telemetry.glean.private.PingTypeBase import mozilla.telemetry.glean.private.RecordedExperimentData import mozilla.telemetry.glean.scheduler.GleanLifecycleObserver import mozilla.telemetry.glean.scheduler.DeletionPingUploadWorker @@ -78,7 +78,7 @@ open class GleanInternalAPI internal constructor () { internal lateinit var metricsPingScheduler: MetricsPingScheduler // Keep track of ping types that have been registered before Glean is initialized. - private val pingTypeQueue: MutableSet = mutableSetOf() + private val pingTypeQueue: MutableSet = mutableSetOf() // This is used to cache the process state and is used by the function `isMainProcess()` @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) @@ -468,7 +468,7 @@ open class GleanInternalAPI internal constructor () { * Collect a ping and return a string */ @VisibleForTesting(otherwise = VisibleForTesting.NONE) - internal fun testCollect(ping: PingType): String? { + internal fun testCollect(ping: PingTypeBase): String? { return LibGleanFFI.INSTANCE.glean_ping_collect(ping.handle)?.getAndConsumeRustString() } @@ -476,11 +476,12 @@ open class GleanInternalAPI internal constructor () { * Handle the background event and send the appropriate pings. */ internal fun handleBackgroundEvent() { - submitPings(listOf(Pings.baseline, Pings.events)) + Pings.baseline.submit() + Pings.events.submit() } /** - * Collect and submit a list of pings for eventual upload. + * Collect and submit a ping for eventual upload. * * The ping content is assembled as soon as possible, but upload is not * guaranteed to happen immediately, as that depends on the upload @@ -489,18 +490,18 @@ open class GleanInternalAPI internal constructor () { * If the ping currently contains no content, it will not be assembled and * queued for sending. * - * @param pings List of pings to submit. + * @param ping Ping to submit. + * @param reason The reason the ping is being submitted. * @return The async [Job] performing the work of assembling the ping */ - internal fun submitPings(pings: List): Job? { - val pingNames = pings.map { it.name } - return submitPingsByName(pingNames) + internal fun submitPing(ping: PingTypeBase, reason: String? = null): Job? { + return submitPingByName(ping.name, reason) } /** - * Collect and submit a list of pings for eventual upload by name. + * Collect and submit a ping for eventual upload by name. * - * Each ping will be looked up in the known instances of [PingType]. If the + * The ping will be looked up in the known instances of [PingType]. If the * ping isn't known, an error is logged and the ping isn't queued for uploading. * * The ping content is assembled as soon as possible, but upload is not @@ -511,18 +512,19 @@ open class GleanInternalAPI internal constructor () { * queued for sending, unless explicitly specified otherwise in the registry * file. * - * @param pingNames List of ping names to submit. + * @param pingName Name of the ping to submit. + * @param reason The reason the ping is being submitted. * @return The async [Job] performing the work of assembling the ping */ @Suppress("EXPERIMENTAL_API_USAGE") - internal fun submitPingsByName(pingNames: List) = Dispatchers.API.launch { - submitPingsByNameSync(pingNames) + internal fun submitPingByName(pingName: String, reason: String? = null) = Dispatchers.API.launch { + submitPingByNameSync(pingName, reason) } /** - * Collect and submit a list of pings for eventual upload by name, synchronously. + * Collect and submit a ping (by its name) for eventual upload, synchronously. * - * Each ping will be looked up in the known instances of [PingType]. If the + * The ping will be looked up in the known instances of [PingType]. If the * ping isn't known, an error is logged and the ping isn't queued for uploading. * * The ping content is assembled as soon as possible, but upload is not @@ -533,9 +535,10 @@ open class GleanInternalAPI internal constructor () { * queued for sending, unless explicitly specified otherwise in the registry * file. * - * @param pingNames List of ping names to submit. + * @param pingName Name of the ping to submit. + * @param reason The reason the ping is being submitted. */ - internal fun submitPingsByNameSync(pingNames: List) { + internal fun submitPingByNameSync(pingName: String, reason: String? = null) { if (!isInitialized()) { Log.e(LOG_TAG, "Glean must be initialized before submitting pings.") return @@ -546,11 +549,9 @@ open class GleanInternalAPI internal constructor () { return } - val pingArray = StringArray(pingNames.toTypedArray(), "utf-8") - val pingArrayLen = pingNames.size - val submittedPing = LibGleanFFI.INSTANCE.glean_submit_pings_by_name( - pingArray, - pingArrayLen + val submittedPing = LibGleanFFI.INSTANCE.glean_submit_ping_by_name( + pingName, + reason ).toBoolean() if (submittedPing) { @@ -640,7 +641,7 @@ open class GleanInternalAPI internal constructor () { * Register a [PingType] in the registry associated with this [Glean] object. */ @Synchronized - internal fun registerPingType(pingType: PingType) { + internal fun registerPingType(pingType: PingTypeBase) { if (this.isInitialized()) { LibGleanFFI.INSTANCE.glean_register_ping_type( pingType.handle diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/debug/GleanDebugActivity.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/debug/GleanDebugActivity.kt index 91b31e455b..6aac8e4f5e 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/debug/GleanDebugActivity.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/debug/GleanDebugActivity.kt @@ -104,7 +104,7 @@ class GleanDebugActivity : Activity() { Glean.configuration = debugConfig intent.getStringExtra(SEND_PING_EXTRA_KEY)?.let { - Glean.submitPingsByName(listOf(it)) + Glean.submitPingByName(it) } } diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/private/PingType.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/private/PingType.kt index 33d0b674e5..f244b9d89e 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/private/PingType.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/private/PingType.kt @@ -4,10 +4,34 @@ package mozilla.telemetry.glean.private +import com.sun.jna.StringArray import mozilla.telemetry.glean.Glean import mozilla.telemetry.glean.rust.LibGleanFFI import mozilla.telemetry.glean.rust.toByte +/** + * An enum with no values for convenient use as the default set of reason codes. + */ +@Suppress("EmptyClassBlock") +enum class NoReasonCodes( + /** + * @suppress + */ + val value: Int +) { + // deliberately empty +} + +/** + * The base class of all PingTypes with just enough to track their registration, so + * we can create a heterogeneous collection of ping types. + */ +open class PingTypeBase( + internal val name: String +) { + internal var handle: Long = 0L +} + /** * This implements the developer facing API for custom pings. * @@ -15,18 +39,22 @@ import mozilla.telemetry.glean.rust.toByte * * The Ping API only exposes the [send] method, which schedules a ping for sending. */ -class PingType( - internal val name: String, +class PingType> ( + name: String, includeClientId: Boolean, - sendIfEmpty: Boolean -) { - internal var handle: Long + sendIfEmpty: Boolean, + val reasonCodes: List +) : PingTypeBase(name) { init { + val ffiReasonList = StringArray(reasonCodes.toTypedArray(), "utf-8") + val ffiReasonListLen = reasonCodes.size this.handle = LibGleanFFI.INSTANCE.glean_new_ping_type( name = name, include_client_id = includeClientId.toByte(), - send_if_empty = sendIfEmpty.toByte() + send_if_empty = sendIfEmpty.toByte(), + reason_codes = ffiReasonList, + reason_codes_len = ffiReasonListLen ) Glean.registerPingType(this) } @@ -48,9 +76,12 @@ class PingType( * There are no guarantees that this will happen immediately. * * If the ping currently contains no content, it will not be queued. + * + * @param reason The reason the ping is being submitted. */ - fun submit() { - Glean.submitPings(listOf(this)) + fun submit(reason: ReasonCodesEnum? = null) { + val reasonString = reason?.let { this.reasonCodes[it.ordinal] } + Glean.submitPing(this, reasonString) } /** diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt index 4622441eef..ed9eaa415c 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/rust/LibGleanFFI.kt @@ -86,9 +86,9 @@ internal interface LibGleanFFI : Library { fun glean_ping_collect(ping_type_handle: Long): Pointer? - fun glean_submit_pings_by_name( - ping_names: StringArray, - ping_names_len: Int + fun glean_submit_ping_by_name( + ping_name: String, + reason: String? ): Byte fun glean_set_experiment_active( @@ -107,7 +107,13 @@ internal interface LibGleanFFI : Library { // Ping type - fun glean_new_ping_type(name: String, include_client_id: Byte, send_if_empty: Byte): Long + fun glean_new_ping_type( + name: String, + include_client_id: Byte, + send_if_empty: Byte, + reason_codes: StringArray?, + reason_codes_len: Int + ): Long fun glean_destroy_ping_type(handle: Long) diff --git a/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt b/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt index 1b9b9e0b3b..4ad3ef5538 100644 --- a/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt +++ b/glean-core/android/src/main/java/mozilla/telemetry/glean/scheduler/MetricsPingScheduler.kt @@ -47,6 +47,13 @@ internal class MetricsPingScheduler( } init { + // In testing mode, set the "last seen version" as the same as this one. + // Otherwise, all we will ever send is pings for the "upgrade" reason. + @Suppress("EXPERIMENTAL_API_USAGE") + if (Dispatchers.API.testingMode) { + isDifferentVersion() + } + // When performing the data migration from glean-ac, this scheduler might be // provided with a date the 'metrics' ping was last sent. If so, save that in // the new storage and use it in this scheduler. @@ -63,9 +70,14 @@ internal class MetricsPingScheduler( * or to attempt to schedule it for the current calendar day. If the latter and * we're overdue for the expected collection time, the task is scheduled for immediate * execution. + * @param reason The reason the metrics ping is being submitted. */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal fun schedulePingCollection(now: Calendar, sendTheNextCalendarDay: Boolean) { + internal fun schedulePingCollection( + now: Calendar, + sendTheNextCalendarDay: Boolean, + reason: Pings.metricsReasonCodes + ) { // Compute how many milliseconds until the next time the metrics ping // needs to collect data. val millisUntilNextDueTime = getMillisecondsUntilDueTime(sendTheNextCalendarDay, now) @@ -76,7 +88,7 @@ internal class MetricsPingScheduler( cancel() timer = Timer("glean.MetricsPingScheduler") - timer?.schedule(MetricsPingTimer(this), millisUntilNextDueTime) + timer?.schedule(MetricsPingTimer(this, reason), millisUntilNextDueTime) } /** @@ -187,9 +199,11 @@ internal class MetricsPingScheduler( // schedule the metrics ping for immediate collection. We only need to perform // this check at startup (when overduePingAsFirst is true). if (isDifferentVersion()) { + Log.i(LOG_TAG, "The application just updated. Send metrics ping now.") + @Suppress("EXPERIMENTAL_API_USAGE") Dispatchers.API.executeTask { - collectPingAndReschedule(now, startupPing = true) + collectPingAndReschedule(now, startupPing = true, reason = Pings.metricsReasonCodes.upgrade) } return } @@ -215,12 +229,13 @@ internal class MetricsPingScheduler( // The metrics ping was already sent today. Schedule it for the next // calendar day. This addresses (1). Log.i(LOG_TAG, "The 'metrics' ping was already sent today, ${now.time}.") - schedulePingCollection(now, sendTheNextCalendarDay = true) + schedulePingCollection(now, sendTheNextCalendarDay = true, reason = Pings.metricsReasonCodes.tomorrow) } // The ping wasn't already sent today. Are we overdue or just waiting for // the right time? This covers (2) isAfterDueTime(now) -> { Log.i(LOG_TAG, "The 'metrics' ping is scheduled for immediate collection, ${now.time}") + // **IMPORTANT** // // The reason why we're collecting the "metrics" ping in the `Dispatchers.API` @@ -235,16 +250,18 @@ internal class MetricsPingScheduler( // `Dispatchers.API` thread pool, before any other enqueued task. For more // context, see bug 1604861 and the implementation of // `collectPingAndReschedule`. + @Suppress("EXPERIMENTAL_API_USAGE") Dispatchers.API.executeTask { // This addresses (2). - collectPingAndReschedule(now, startupPing = true) + collectPingAndReschedule(now, startupPing = true, reason = Pings.metricsReasonCodes.overdue) } } else -> { // This covers (3). Log.i(LOG_TAG, "The 'metrics' collection is scheduled for today, ${now.time}") - schedulePingCollection(now, sendTheNextCalendarDay = false) + + schedulePingCollection(now, sendTheNextCalendarDay = false, reason = Pings.metricsReasonCodes.today) } } } @@ -254,10 +271,16 @@ internal class MetricsPingScheduler( * next collection. * * @param now a [Calendar] instance representing the current time. + * @param startupUp When true, this is a startup ping that should be submitted synchronously. + * @param reason The reason the ping is being submitted. */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) - internal fun collectPingAndReschedule(now: Calendar, startupPing: Boolean = false) { - Log.i(LOG_TAG, "Collecting the 'metrics' ping, now = ${now.time}, startup = $startupPing") + internal fun collectPingAndReschedule(now: Calendar, startupPing: Boolean, reason: Pings.metricsReasonCodes) { + val reasonString = Pings.metrics.reasonCodes[reason.ordinal] + Log.i( + LOG_TAG, + "Collecting the 'metrics' ping, now = ${now.time}, startup = $startupPing, reason = $reasonString" + ) if (startupPing) { // **IMPORTANT** // @@ -273,15 +296,15 @@ internal class MetricsPingScheduler( // // * Do not change this line without checking what it implies for the above wall // of text. * - Glean.submitPingsByNameSync(listOf("metrics")) + Glean.submitPingByNameSync("metrics", reasonString) } else { - Pings.metrics.submit() + Pings.metrics.submit(reason) } // Update the collection date: we don't really care if we have data or not, let's // always update the sent date. updateSentDate(getISOTimeString(now, truncateTo = TimeUnit.Day)) // Reschedule the collection. - schedulePingCollection(now, sendTheNextCalendarDay = true) + schedulePingCollection(now, sendTheNextCalendarDay = true, reason = Pings.metricsReasonCodes.reschedule) } /** @@ -339,7 +362,10 @@ internal class MetricsPingScheduler( * [MetricsPingScheduler.schedulePingCollection] for scheduling the collection of the * "metrics" ping at the due hour. */ -internal class MetricsPingTimer(val scheduler: MetricsPingScheduler) : TimerTask() { +internal class MetricsPingTimer( + val scheduler: MetricsPingScheduler, + val reason: Pings.metricsReasonCodes +) : TimerTask() { companion object { private const val LOG_TAG = "glean/MetricsPingTimer" } @@ -351,6 +377,6 @@ internal class MetricsPingTimer(val scheduler: MetricsPingScheduler) : TimerTask // Perform the actual work. val now = scheduler.getCalendarInstance() Log.d(LOG_TAG, "MetricsPingTimerTask run(), now = ${now.time}") - scheduler.collectPingAndReschedule(now) + scheduler.collectPingAndReschedule(now, false, reason) } } diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt index 7217316947..ee8f4fde58 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/GleanTest.kt @@ -21,8 +21,11 @@ import mozilla.telemetry.glean.private.CounterMetricType import mozilla.telemetry.glean.private.EventMetricType import mozilla.telemetry.glean.private.Lifetime import mozilla.telemetry.glean.private.NoExtraKeys +import mozilla.telemetry.glean.private.NoReasonCodes import mozilla.telemetry.glean.private.PingType import mozilla.telemetry.glean.private.StringMetricType +import mozilla.telemetry.glean.rust.LibGleanFFI +import mozilla.telemetry.glean.rust.toBoolean import mozilla.telemetry.glean.scheduler.GleanLifecycleObserver import mozilla.telemetry.glean.scheduler.DeletionPingUploadWorker import mozilla.telemetry.glean.scheduler.PingUploadWorker @@ -112,7 +115,7 @@ class GleanTest { @Test fun `sending an empty ping doesn't queue work`() { - Glean.submitPings(listOf(Pings.metrics)) + Pings.metrics.submit() assertFalse(getWorkerStatus(context, PingUploadWorker.PING_WORKER_TAG).isEnqueued) } @@ -312,9 +315,8 @@ class GleanTest { gleanSpy.testDestroyGleanHandle() runBlocking { - gleanSpy.handleBackgroundEvent() + assertFalse(LibGleanFFI.INSTANCE.glean_submit_ping_by_name("events", null).toBoolean()) } - assertFalse(getWorkerStatus(context, PingUploadWorker.PING_WORKER_TAG).isEnqueued) } @Test @@ -423,10 +425,11 @@ class GleanTest { )) val pingName = "custom_ping_1" - val ping = PingType( + val ping = PingType( name = pingName, includeClientId = true, - sendIfEmpty = false + sendIfEmpty = false, + reasonCodes = listOf() ) val stringMetric = StringMetricType( disabled = false, @@ -507,7 +510,11 @@ class GleanTest { @Test fun `Workers should be cancelled when disabling uploading`() { // Force the MetricsPingScheduler to schedule the MetricsPingWorker - Glean.metricsPingScheduler.schedulePingCollection(Calendar.getInstance(), true) + Glean.metricsPingScheduler.schedulePingCollection( + Calendar.getInstance(), + true, + Pings.metricsReasonCodes.overdue + ) // Enqueue a worker to send the baseline ping Pings.baseline.submit() diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt index 922cee6fb8..e8ac1593fc 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/TestUtil.kt @@ -20,7 +20,7 @@ import org.mockito.ArgumentMatchers import org.mockito.Mockito import mozilla.telemetry.glean.config.Configuration import mozilla.telemetry.glean.scheduler.PingUploadWorker -import mozilla.telemetry.glean.private.PingType +import mozilla.telemetry.glean.private.PingTypeBase import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer @@ -97,7 +97,7 @@ internal fun checkPingSchema(content: String): JSONObject { * @return the ping contents, in a JSONObject * @throws AssertionError If the JSON content is not valid */ -internal fun collectAndCheckPingSchema(ping: PingType): JSONObject { +internal fun collectAndCheckPingSchema(ping: PingTypeBase): JSONObject { val jsonString = Glean.testCollect(ping)!! return checkPingSchema(jsonString) } diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt index fc04c0fb01..fcdc87fa2e 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/EventMetricTypeTest.kt @@ -345,7 +345,7 @@ class EventMetricTypeTest { pingJson.getJSONArray("events").getJSONObject(0).getJSONObject("extra").getString("someExtra") ) - Glean.submitPingsByName(listOf("events")) + Glean.submitPingByName("events") // Trigger worker task to upload the pings in the background triggerWorkManager(context) diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/LabeledMetricTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/LabeledMetricTypeTest.kt index 74877c056f..926d675592 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/LabeledMetricTypeTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/LabeledMetricTypeTest.kt @@ -397,7 +397,7 @@ class LabeledMetricTypeTest { @Test(expected = IllegalStateException::class) fun `Test that labeled events are an exception`() { - val eventMetric = EventMetricType( + val eventMetric = EventMetricType( disabled = false, category = "telemetry", lifetime = Lifetime.Application, @@ -405,7 +405,7 @@ class LabeledMetricTypeTest { sendInPings = listOf("metrics") ) - val labeledEventMetric = LabeledMetricType>( + val labeledEventMetric = LabeledMetricType>( disabled = false, category = "telemetry", lifetime = Lifetime.Application, diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt index 49d7b5cf49..8b49b1ff20 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/private/PingTypeTest.kt @@ -42,10 +42,11 @@ class PingTypeTest { logPings = true )) - val customPing = PingType( + val customPing = PingType( name = "custom", includeClientId = true, - sendIfEmpty = false + sendIfEmpty = false, + reasonCodes = listOf() ) val counter = CounterMetricType( @@ -82,10 +83,11 @@ class PingTypeTest { logPings = true )) - val customPing = PingType( + val customPing = PingType( name = "custom_ping", includeClientId = true, - sendIfEmpty = false + sendIfEmpty = false, + reasonCodes = listOf() ) val counter = CounterMetricType( @@ -122,10 +124,11 @@ class PingTypeTest { logPings = true )) - val customPing = PingType( + val customPing = PingType( name = "custom-ping", includeClientId = true, - sendIfEmpty = false + sendIfEmpty = false, + reasonCodes = listOf() ) val counter = CounterMetricType( @@ -162,10 +165,11 @@ class PingTypeTest { logPings = true )) - val customPing = PingType( + val customPing = PingType( name = "custom", includeClientId = false, - sendIfEmpty = false + sendIfEmpty = false, + reasonCodes = listOf() ) val counter = CounterMetricType( @@ -213,7 +217,7 @@ class PingTypeTest { counter.add() assertTrue(counter.testHasValue()) - Glean.submitPingsByName(listOf("unknown")) + Glean.submitPingByName("unknown") assertFalse("We shouldn't have any pings scheduled", getWorkerStatus(context, PingUploadWorker.PING_WORKER_TAG).isEnqueued diff --git a/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt b/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt index 50e64a7627..b7a1056585 100644 --- a/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt +++ b/glean-core/android/src/test/java/mozilla/telemetry/glean/scheduler/MetricsPingSchedulerTest.kt @@ -10,9 +10,11 @@ import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.work.testing.WorkManagerTestInitHelper import mozilla.components.support.test.any +import mozilla.components.support.test.eq import mozilla.telemetry.glean.Dispatchers import mozilla.telemetry.glean.getContextWithMockedInfo import mozilla.telemetry.glean.Glean +import mozilla.telemetry.glean.GleanMetrics.Pings import mozilla.telemetry.glean.private.Lifetime import mozilla.telemetry.glean.resetGlean import mozilla.telemetry.glean.private.StringMetricType @@ -35,7 +37,6 @@ import org.junit.runner.RunWith import org.mockito.Mockito.anyBoolean import org.mockito.Mockito.anyString import org.mockito.Mockito.doReturn -import org.mockito.Mockito.eq import org.mockito.Mockito.never import org.mockito.Mockito.spy import org.mockito.Mockito.times @@ -221,16 +222,18 @@ class MetricsPingSchedulerTest { verify(mpsSpy, times(0)).updateSentDate(anyString()) verify(mpsSpy, times(0)).schedulePingCollection( any(), - anyBoolean() + anyBoolean(), + eq(Pings.metricsReasonCodes.overdue) ) - mpsSpy.collectPingAndReschedule(Calendar.getInstance()) + mpsSpy.collectPingAndReschedule(Calendar.getInstance(), false, Pings.metricsReasonCodes.overdue) // Verify that we correctly called in the methods. verify(mpsSpy, times(1)).updateSentDate(anyString()) verify(mpsSpy, times(1)).schedulePingCollection( any(), - anyBoolean() + anyBoolean(), + eq(Pings.metricsReasonCodes.reschedule) ) } @@ -260,7 +263,11 @@ class MetricsPingSchedulerTest { assertTrue("The initial test data must have been recorded", testMetric.testHasValue()) // Manually call the function to trigger the collection. - Glean.metricsPingScheduler.collectPingAndReschedule(Calendar.getInstance()) + Glean.metricsPingScheduler.collectPingAndReschedule( + Calendar.getInstance(), + false, + Pings.metricsReasonCodes.overdue + ) // Trigger worker task to upload the pings in the background triggerWorkManager(context) @@ -281,6 +288,11 @@ class MetricsPingSchedulerTest { .getJSONObject("string") .getString("telemetry.string_metric") ) + assertEquals( + "The reason should be 'overdue'", + "overdue", + metricsJson.getJSONObject("ping_info").getString("reason") + ) } finally { server.shutdown() } @@ -297,9 +309,14 @@ class MetricsPingSchedulerTest { val mpsSpy = spy(MetricsPingScheduler(context)) val overdueTestDate = "2015-07-05T12:36:00-06:00" + mpsSpy.updateSentDate(overdueTestDate) - verify(mpsSpy, never()).collectPingAndReschedule(any(), eq(true)) + verify(mpsSpy, never()).collectPingAndReschedule( + any(), + eq(true), + eq(Pings.metricsReasonCodes.overdue) + ) // Make sure to return the fake date when requested. doReturn(fakeNow).`when`(mpsSpy).getCalendarInstance() @@ -314,7 +331,11 @@ class MetricsPingSchedulerTest { assertEquals(fakeNow.time, mpsSpy.getLastCollectedDate()) // Verify that we're immediately collecting. - verify(mpsSpy, times(1)).collectPingAndReschedule(fakeNow, true) + verify(mpsSpy, times(1)).collectPingAndReschedule( + fakeNow, + true, + Pings.metricsReasonCodes.overdue + ) } @Test @@ -329,13 +350,9 @@ class MetricsPingSchedulerTest { val mpsSpy = spy(MetricsPingScheduler(context)) - // Inject the application version as already recorded, so we don't hit the case - // where the ping is sent due to a version change. - mpsSpy.isDifferentVersion() - mpsSpy.updateSentDate(getISOTimeString(fakeNow, truncateTo = TimeUnit.Day)) - verify(mpsSpy, never()).schedulePingCollection(any(), anyBoolean()) + verify(mpsSpy, never()).schedulePingCollection(any(), anyBoolean(), eq(Pings.metricsReasonCodes.overdue)) // Make sure to return the fake date when requested. doReturn(fakeNow).`when`(mpsSpy).getCalendarInstance() @@ -344,9 +361,17 @@ class MetricsPingSchedulerTest { mpsSpy.schedule() // Verify that we're scheduling for the next day and not collecting immediately. - verify(mpsSpy, times(1)).schedulePingCollection(fakeNow, sendTheNextCalendarDay = true) - verify(mpsSpy, never()).schedulePingCollection(fakeNow, sendTheNextCalendarDay = false) - verify(mpsSpy, never()).collectPingAndReschedule(any(), eq(true)) + verify(mpsSpy, times(1)).schedulePingCollection( + fakeNow, + sendTheNextCalendarDay = true, + reason = Pings.metricsReasonCodes.tomorrow + ) + verify(mpsSpy, never()).schedulePingCollection( + eq(fakeNow), + sendTheNextCalendarDay = eq(false), + reason = any() + ) + verify(mpsSpy, never()).collectPingAndReschedule(any(), eq(true), any()) } @Test @@ -361,10 +386,6 @@ class MetricsPingSchedulerTest { val mpsSpy = spy(MetricsPingScheduler(context)) - // Inject the application version as already recorded, so we don't hit the case - // where the ping is sent due to a version change. - mpsSpy.isDifferentVersion() - val fakeYesterday = Calendar.getInstance() fakeYesterday.time = fakeNow.time fakeYesterday.add(Calendar.DAY_OF_MONTH, -1) @@ -373,15 +394,23 @@ class MetricsPingSchedulerTest { // Make sure to return the fake date when requested. doReturn(fakeNow).`when`(mpsSpy).getCalendarInstance() - verify(mpsSpy, never()).schedulePingCollection(any(), anyBoolean()) + verify(mpsSpy, never()).schedulePingCollection(any(), anyBoolean(), any()) // Trigger the startup check. mpsSpy.schedule() // Verify that we're scheduling for today, but not collecting immediately. - verify(mpsSpy, times(1)).schedulePingCollection(fakeNow, sendTheNextCalendarDay = false) - verify(mpsSpy, never()).schedulePingCollection(fakeNow, sendTheNextCalendarDay = true) - verify(mpsSpy, never()).collectPingAndReschedule(any(), eq(true)) + verify(mpsSpy, times(1)).schedulePingCollection( + fakeNow, + sendTheNextCalendarDay = false, + reason = Pings.metricsReasonCodes.today + ) + verify(mpsSpy, never()).schedulePingCollection( + eq(fakeNow), + sendTheNextCalendarDay = eq(true), + reason = any() + ) + verify(mpsSpy, never()).collectPingAndReschedule(any(), eq(true), reason = any()) } @Test @@ -395,11 +424,10 @@ class MetricsPingSchedulerTest { val mpsSpy = spy(MetricsPingScheduler(context)) mpsSpy.sharedPreferences.edit().clear().apply() - // Inject the application version as already recorded, so we don't hit the case - // where the ping is sent due to a version change. + // Restore the version number, so we don't get an "upgrade" reason ping mpsSpy.isDifferentVersion() - verify(mpsSpy, never()).collectPingAndReschedule(any(), anyBoolean()) + verify(mpsSpy, never()).collectPingAndReschedule(any(), anyBoolean(), any()) // Make sure to return the fake date when requested. doReturn(fakeNow).`when`(mpsSpy).getCalendarInstance() @@ -408,8 +436,12 @@ class MetricsPingSchedulerTest { mpsSpy.schedule() // Verify that we're immediately collecting. - verify(mpsSpy, never()).collectPingAndReschedule(fakeNow, true) - verify(mpsSpy, times(1)).schedulePingCollection(fakeNow, sendTheNextCalendarDay = false) + verify(mpsSpy, never()).collectPingAndReschedule(eq(fakeNow), eq(true), any()) + verify(mpsSpy, times(1)).schedulePingCollection( + fakeNow, + sendTheNextCalendarDay = false, + reason = Pings.metricsReasonCodes.today + ) } @Test @@ -426,7 +458,7 @@ class MetricsPingSchedulerTest { mpsSpy.schedule() // Verify that we're immediately collecting. - verify(mpsSpy, times(1)).collectPingAndReschedule(any(), anyBoolean()) + verify(mpsSpy, times(1)).collectPingAndReschedule(any(), anyBoolean(), eq(Pings.metricsReasonCodes.upgrade)) } @Test @@ -440,8 +472,10 @@ class MetricsPingSchedulerTest { val mpsSpy = spy(MetricsPingScheduler(context)) mpsSpy.sharedPreferences.edit().clear().apply() + // Restore the version number, so we don't get an "upgrade" reason ping + mpsSpy.isDifferentVersion() - verify(mpsSpy, never()).collectPingAndReschedule(any(), anyBoolean()) + verify(mpsSpy, never()).collectPingAndReschedule(any(), anyBoolean(), any()) // Make sure to return the fake date when requested. doReturn(fakeNow).`when`(mpsSpy).getCalendarInstance() @@ -458,8 +492,8 @@ class MetricsPingSchedulerTest { ) // Verify that we're immediately collecting. - verify(mpsSpy, times(1)).collectPingAndReschedule(fakeNow, true) - verify(mpsSpy, never()).schedulePingCollection(fakeNow, sendTheNextCalendarDay = false) + verify(mpsSpy, times(1)).collectPingAndReschedule(fakeNow, true, reason = Pings.metricsReasonCodes.overdue) + verify(mpsSpy, never()).schedulePingCollection(eq(fakeNow), sendTheNextCalendarDay = eq(false), reason = any()) } @Test @@ -472,11 +506,17 @@ class MetricsPingSchedulerTest { assertNull(Glean.metricsPingScheduler.timer) // Manually schedule a collection task for today. - Glean.metricsPingScheduler.schedulePingCollection(Calendar.getInstance(), sendTheNextCalendarDay = false) + Glean.metricsPingScheduler.schedulePingCollection( + Calendar.getInstance(), + sendTheNextCalendarDay = false, + reason = Pings.metricsReasonCodes.overdue + ) // We expect the worker to be scheduled. assertNotNull(Glean.metricsPingScheduler.timer) + Glean.metricsPingScheduler.cancel() + resetGlean(clearStores = true) } @@ -485,7 +525,7 @@ class MetricsPingSchedulerTest { val context = ApplicationProvider.getApplicationContext() val mps = MetricsPingScheduler(context) - mps.schedulePingCollection(Calendar.getInstance(), true) + mps.schedulePingCollection(Calendar.getInstance(), true, reason = Pings.metricsReasonCodes.overdue) // Verify that the worker is enqueued assertNotNull("MetricsPingWorker is enqueued", diff --git a/glean-core/benchmark/benches/bench_basic.rs b/glean-core/benchmark/benches/bench_basic.rs index 755886f014..41b9dc2f69 100644 --- a/glean-core/benchmark/benches/bench_basic.rs +++ b/glean-core/benchmark/benches/bench_basic.rs @@ -18,7 +18,7 @@ pub fn criterion_benchmark(c: &mut Criterion) { let mut glean = Glean::new(cfg).unwrap(); - let ping = PingType::new("sample", true, false); + let ping = PingType::new("sample", true, false, vec![]); glean.register_ping_type(&ping); let call_counter: CounterMetric = CounterMetric::new(CommonMetricData { @@ -39,7 +39,7 @@ pub fn criterion_benchmark(c: &mut Criterion) { b.iter(|| { call_counter.add(&glean, 1); string.set(&glean, "hello world"); - glean.submit_ping(&ping).unwrap(); + glean.submit_ping(&ping, None).unwrap(); }) }); } diff --git a/glean-core/examples/sample.rs b/glean-core/examples/sample.rs index 70c05aaa85..f1d812fe08 100644 --- a/glean-core/examples/sample.rs +++ b/glean-core/examples/sample.rs @@ -25,8 +25,8 @@ fn main() { delay_ping_lifetime_io: false, }; let mut glean = Glean::new(cfg).unwrap(); - glean.register_ping_type(&PingType::new("baseline", true, false)); - glean.register_ping_type(&PingType::new("metrics", true, false)); + glean.register_ping_type(&PingType::new("baseline", true, false, vec![])); + glean.register_ping_type(&PingType::new("metrics", true, false, vec![])); let local_metric: StringMetric = StringMetric::new(CommonMetricData { name: "local_metric".into(), @@ -67,10 +67,10 @@ fn main() { let ping_maker = PingMaker::new(); let ping = ping_maker - .collect_string(&glean, glean.get_ping_by_name("baseline").unwrap()) + .collect_string(&glean, glean.get_ping_by_name("baseline").unwrap(), None) .unwrap(); println!("Baseline Ping:\n{}", ping); - let ping = ping_maker.collect_string(&glean, glean.get_ping_by_name("metrics").unwrap()); + let ping = ping_maker.collect_string(&glean, glean.get_ping_by_name("metrics").unwrap(), None); println!("Metrics Ping: {:?}", ping); } diff --git a/glean-core/ffi/examples/glean_app.c b/glean-core/ffi/examples/glean_app.c index e03ca35dd6..e5f811083c 100644 --- a/glean-core/ffi/examples/glean_app.c +++ b/glean-core/ffi/examples/glean_app.c @@ -15,7 +15,7 @@ int main(void) NULL }; glean_initialize(&cfg); - uint64_t store1 = glean_new_ping_type("store1", true, false); + uint64_t store1 = glean_new_ping_type("store1", true, false, NULL, 0); glean_register_ping_type(store1); printf("Glean upload enabled? %d\n", glean_is_upload_enabled()); @@ -31,7 +31,7 @@ int main(void) glean_counter_add(metric, 2); - char *payload = glean_ping_collect(store1); + char *payload = glean_ping_collect(store1, NULL); printf("Payload:\n%s\n", payload); glean_str_free(payload); diff --git a/glean-core/ffi/glean.h b/glean-core/ffi/glean.h index 6a757fa4b0..69d6975dbc 100644 --- a/glean-core/ffi/glean.h +++ b/glean-core/ffi/glean.h @@ -334,7 +334,11 @@ uint64_t glean_new_memory_distribution_metric(FfiStr category, uint8_t disabled, int32_t memory_unit); -uint64_t glean_new_ping_type(FfiStr ping_name, uint8_t include_client_id, uint8_t send_if_empty); +uint64_t glean_new_ping_type(FfiStr ping_name, + uint8_t include_client_id, + uint8_t send_if_empty, + RawStringArray reason_codes, + int32_t reason_codes_len); uint64_t glean_new_quantity_metric(FfiStr category, FfiStr name, @@ -382,7 +386,7 @@ uint64_t glean_new_uuid_metric(FfiStr category, uint8_t glean_on_ready_to_submit_pings(void); -char *glean_ping_collect(uint64_t ping_type_handle); +char *glean_ping_collect(uint64_t ping_type_handle, FfiStr reason); void glean_quantity_set(uint64_t metric_id, int64_t value); @@ -441,7 +445,7 @@ char *glean_string_test_get_value(uint64_t metric_id, FfiStr storage_name); uint8_t glean_string_test_has_value(uint64_t metric_id, FfiStr storage_name); -uint8_t glean_submit_pings_by_name(RawStringArray ping_names, int32_t ping_names_len); +uint8_t glean_submit_ping_by_name(FfiStr ping_name, FfiStr reason); void glean_test_clear_all_stores(void); diff --git a/glean-core/ffi/src/lib.rs b/glean-core/ffi/src/lib.rs index 6f1ee59af9..c3d1892e70 100644 --- a/glean-core/ffi/src/lib.rs +++ b/glean-core/ffi/src/lib.rs @@ -229,24 +229,21 @@ pub extern "C" fn glean_set_upload_enabled(flag: u8) { } #[no_mangle] -pub extern "C" fn glean_submit_pings_by_name( - ping_names: RawStringArray, - ping_names_len: i32, -) -> u8 { +pub extern "C" fn glean_submit_ping_by_name(ping_name: FfiStr, reason: FfiStr) -> u8 { with_glean(|glean| { - let pings = from_raw_string_array(ping_names, ping_names_len)?; - - Ok(glean.submit_pings_by_name(&pings)) + Ok(glean + .submit_ping_by_name(&ping_name.to_string_fallible()?, reason.as_opt_str()) + .unwrap_or(false)) }) } #[no_mangle] -pub extern "C" fn glean_ping_collect(ping_type_handle: u64) -> *mut c_char { +pub extern "C" fn glean_ping_collect(ping_type_handle: u64, reason: FfiStr) -> *mut c_char { with_glean_value(|glean| { PING_TYPES.call_infallible(ping_type_handle, |ping_type| { let ping_maker = glean_core::ping::PingMaker::new(); let data = ping_maker - .collect_string(&glean, ping_type) + .collect_string(glean, ping_type, reason.as_opt_str()) .unwrap_or_else(|| String::from("")); log::info!("Ping({}): {}", ping_type.name.as_str(), data); data diff --git a/glean-core/ffi/src/ping_type.rs b/glean-core/ffi/src/ping_type.rs index adfa655e14..f81b3bdc40 100644 --- a/glean-core/ffi/src/ping_type.rs +++ b/glean-core/ffi/src/ping_type.rs @@ -9,7 +9,7 @@ use glean_core::metrics::PingType; use crate::ffi_string_ext::FallibleToString; use crate::handlemap_ext::HandleMapExtension; -use crate::{with_glean_value, with_glean_value_mut}; +use crate::{from_raw_string_array, with_glean_value, with_glean_value_mut, RawStringArray}; lazy_static! { pub(crate) static ref PING_TYPES: ConcurrentHandleMap = ConcurrentHandleMap::new(); @@ -21,13 +21,17 @@ pub extern "C" fn glean_new_ping_type( ping_name: FfiStr, include_client_id: u8, send_if_empty: u8, + reason_codes: RawStringArray, + reason_codes_len: i32, ) -> u64 { PING_TYPES.insert_with_log(|| { let ping_name = ping_name.to_string_fallible()?; + let reason_codes = from_raw_string_array(reason_codes, reason_codes_len)?; Ok(PingType::new( ping_name, include_client_id != 0, send_if_empty != 0, + reason_codes, )) }) } diff --git a/glean-core/ios/Glean/Debug/GleanDebugTools.swift b/glean-core/ios/Glean/Debug/GleanDebugTools.swift index d1603f03bd..1acb84e7a6 100644 --- a/glean-core/ios/Glean/Debug/GleanDebugTools.swift +++ b/glean-core/ios/Glean/Debug/GleanDebugTools.swift @@ -68,7 +68,7 @@ class GleanDebugUtility { } if let pingName = parsedCommands.pingNameToSend { - Glean.shared.submitPingsByName(pingNames: [pingName]) + Glean.shared.submitPingByName(pingName: pingName) logger.debug("Glean debug tools triggered ping: \(pingName)") } } diff --git a/glean-core/ios/Glean/Glean.swift b/glean-core/ios/Glean/Glean.swift index 3357c08f34..7657a6e2d5 100644 --- a/glean-core/ios/Glean/Glean.swift +++ b/glean-core/ios/Glean/Glean.swift @@ -35,7 +35,7 @@ public class Glean { static let logTag = "glean/Glean" } - private var pingTypeQueue = [Ping]() + private var pingTypeQueue = [PingBase]() private let logger = Logger(tag: Constants.logTag) @@ -300,13 +300,16 @@ public class Glean { /// Handle background event and submit appropriate pings func handleBackgroundEvent() { - self.submitPingsByName(pingNames: ["baseline", "events"]) + Pings.shared.baseline.submit() + Pings.shared.events.submit() } - /// Collect and submit a list of pings by name for eventual uploading + /// Collect and submit a ping by name for eventual uploading /// /// - parameters: - /// * pingNames: List of ping names to send + /// * pingName: Name of ping to submit. + /// * reason: The reason the ping is being submitted. Must be one of the strings + /// defined in the reasons field in the ping metadata. /// /// The ping content is assembled as soon as possible, but upload is not /// guaranteed to happen immediately, as that depends on the upload @@ -314,17 +317,19 @@ public class Glean { /// /// If the ping currently contains no content, it will not be assembled and /// queued for sending. - func submitPingsByName(pingNames: [String]) { + func submitPingByName(pingName: String, reason: String? = nil) { // Queue submitting the ping behind all other metric operations to include them in the ping Dispatchers.shared.launchAPI { - self.submitPingsByNameSync(pingNames: pingNames) + self.submitPingByNameSync(pingName: pingName, reason: reason) } } - /// Collect and submit a list of pings by name for eventual uploading, synchronously + /// Collect and submit a ping by name for eventual uploading, synchronously /// /// - parameters: - /// * pingNames: List of ping names to send + /// * pingName: Name of the ping to submit. + /// * reason: The reason the ping is being submitted. Must be one of the strings + /// defined in the reasons field in the ping metadata. /// /// The ping content is assembled as soon as possible, but upload is not /// guaranteed to happen immediately, as that depends on the upload @@ -332,7 +337,7 @@ public class Glean { /// /// If the ping currently contains no content, it will not be assembled and /// queued for sending. - func submitPingsByNameSync(pingNames: [String]) { + func submitPingByNameSync(pingName: String, reason: String? = nil) { if !self.isInitialized() { self.logger.error("Glean must be initialized before sending pings") return @@ -343,23 +348,20 @@ public class Glean { return } - withArrayOfCStrings(pingNames) { pingNames in - let submittedPing = glean_submit_pings_by_name( - pingNames, - Int32(pingNames?.count ?? 0) - ) + let submittedPing = glean_submit_ping_by_name( + pingName, + reason + ) - if submittedPing != 0 { - if let config = self.configuration { - HttpPingUploader(configuration: config).process() - } + if submittedPing != 0 { + if let config = self.configuration { + HttpPingUploader(configuration: config).process() } } } - func submitPings(_ pings: [Ping]) { - let pingNames = pings.map { $0.name } - return self.submitPingsByName(pingNames: pingNames) + func submitPing(_ ping: PingBase, reason: String? = nil) { + return self.submitPingByName(pingName: ping.name, reason: reason) } /// Register the pings generated from `pings.yaml` with the Glean SDK. @@ -374,7 +376,7 @@ public class Glean { } /// Register a `Ping` in the registry associated with this `Glean` object. - func registerPingType(_ pingType: Ping) { + func registerPingType(_ pingType: PingBase) { // TODO: This might need to synchronized across multiple threads, // `initialize()` will read and clear the ping type queue. if !self.isInitialized() { diff --git a/glean-core/ios/Glean/GleanFfi.h b/glean-core/ios/Glean/GleanFfi.h index 6a757fa4b0..69d6975dbc 100644 --- a/glean-core/ios/Glean/GleanFfi.h +++ b/glean-core/ios/Glean/GleanFfi.h @@ -334,7 +334,11 @@ uint64_t glean_new_memory_distribution_metric(FfiStr category, uint8_t disabled, int32_t memory_unit); -uint64_t glean_new_ping_type(FfiStr ping_name, uint8_t include_client_id, uint8_t send_if_empty); +uint64_t glean_new_ping_type(FfiStr ping_name, + uint8_t include_client_id, + uint8_t send_if_empty, + RawStringArray reason_codes, + int32_t reason_codes_len); uint64_t glean_new_quantity_metric(FfiStr category, FfiStr name, @@ -382,7 +386,7 @@ uint64_t glean_new_uuid_metric(FfiStr category, uint8_t glean_on_ready_to_submit_pings(void); -char *glean_ping_collect(uint64_t ping_type_handle); +char *glean_ping_collect(uint64_t ping_type_handle, FfiStr reason); void glean_quantity_set(uint64_t metric_id, int64_t value); @@ -441,7 +445,7 @@ char *glean_string_test_get_value(uint64_t metric_id, FfiStr storage_name); uint8_t glean_string_test_has_value(uint64_t metric_id, FfiStr storage_name); -uint8_t glean_submit_pings_by_name(RawStringArray ping_names, int32_t ping_names_len); +uint8_t glean_submit_ping_by_name(FfiStr ping_name, FfiStr reason); void glean_test_clear_all_stores(void); diff --git a/glean-core/ios/Glean/Metrics/Ping.swift b/glean-core/ios/Glean/Metrics/Ping.swift index 94bbb0e105..35ddfefab5 100644 --- a/glean-core/ios/Glean/Metrics/Ping.swift +++ b/glean-core/ios/Glean/Metrics/Ping.swift @@ -4,22 +4,57 @@ import Foundation +public protocol ReasonCodes: Hashable { + /// The index of the reason code, used to index the string array passed at + /// `Ping` instantiation. + func index() -> Int +} + +/// Default of no reason codes for pings. +/// +/// An enum with no values for convenient use as the default set of reason codes +/// that an `Ping` can accept. +public enum NoReasonCodes: ReasonCodes { + public func index() -> Int { + return 0 + } +} + +/// A base class for functionality across all Ping +public class PingBase { + var handle: UInt64 + var name: String + + public init(name: String) { + self.name = name + self.handle = 0 + } +} + /// This implements the developer facing API for custom pings. /// /// Instances of this class type are automatically generated by the parsers at build time. /// /// The Ping API only exposes the `Ping.sumbit()` method, which collects and /// schedules a ping for eventual upload. -public class Ping { - var handle: UInt64 - let name: String +public class Ping: PingBase { let includeClientId: Bool + let reasonCodes: [String] /// The public constructor used by automatically generated metrics. - public init(name: String, includeClientId: Bool, sendIfEmpty: Bool) { - self.name = name + public init(name: String, includeClientId: Bool, sendIfEmpty: Bool, reasonCodes: [String]) { self.includeClientId = includeClientId - self.handle = glean_new_ping_type(name, includeClientId.toByte(), sendIfEmpty.toByte()) + self.reasonCodes = reasonCodes + super.init(name: name) + self.handle = withArrayOfCStrings(reasonCodes) { reasonCodesArray in + glean_new_ping_type( + name, + includeClientId.toByte(), + sendIfEmpty.toByte(), + reasonCodesArray, + Int32(reasonCodes.count) + ) + } NSLog("Registering this ping: \(name)") Glean.shared.registerPingType(self) } @@ -38,8 +73,15 @@ public class Ping { /// There are no guarantees that this will happen immediately. /// /// If the ping currently contains no content, it will not be queued. - public func submit() { - Glean.shared.submitPings([self]) + /// + /// - parameters: + /// * reason: The reason the ping is being submitted. + public func submit(reason: ReasonCodesEnum? = nil) { + var reasonString: String? + if reason != nil { + reasonString = self.reasonCodes[reason!.index()] + } + Glean.shared.submitPing(self, reason: reasonString) } /// Collect and submit the ping for eventual uploading. diff --git a/glean-core/ios/Glean/Scheduler/MetricsPingScheduler.swift b/glean-core/ios/Glean/Scheduler/MetricsPingScheduler.swift index 13ff1eb452..58a942c9af 100644 --- a/glean-core/ios/Glean/Scheduler/MetricsPingScheduler.swift +++ b/glean-core/ios/Glean/Scheduler/MetricsPingScheduler.swift @@ -21,6 +21,14 @@ class MetricsPingScheduler { var timer: Timer? + init() { + // In testing mode, set the "last seen version" as the same as this one. + // Otherwise, all we will ever send is pings for the "upgrade" reason. + if Dispatchers.shared.testingMode { + isDifferentVersion() + } + } + /// Schedules the metrics ping collection at the due time. /// /// - parameters: @@ -29,7 +37,12 @@ class MetricsPingScheduler { /// or to attempt to schedule it for the current calendar day. If the latter and /// we're overdue for the expected collection time, the task is scheduled for /// immediate execution. - func schedulePingCollection(_ now: Date, sendTheNextCalendarDay: Bool) { + /// * reason: The reason the ping is being submitted. + func schedulePingCollection( + _ now: Date, + sendTheNextCalendarDay: Bool, + reason: GleanMetrics.Pings.MetricsReasonCodes + ) { var fireDate = Calendar.current.date( bySettingHour: Constants.dueHourOfTheDay, minute: 0, @@ -60,7 +73,7 @@ class MetricsPingScheduler { self.logger.debug("MetricsPingScheduler timer fired!") // When the timer fires, call `collectPingAndReschedule` with the current // date/time. - self.collectPingAndReschedule(Date(), startupPing: false) + self.collectPingAndReschedule(Date(), startupPing: false, reason: reason) } } @@ -105,7 +118,11 @@ class MetricsPingScheduler { // this check at startup (when overduePingAsFirst is true). if isDifferentVersion() { Dispatchers.shared.serialOperationQueue.addOperation { - self.collectPingAndReschedule(now, startupPing: true) + self.collectPingAndReschedule( + now, + startupPing: true, + reason: GleanMetrics.Pings.MetricsReasonCodes.upgrade + ) } return } @@ -127,7 +144,11 @@ class MetricsPingScheduler { // The metrics ping was already sent today. Schedule it for the next // calendar day. This addresses (1). logger.info("The 'metrics' ping was already sent today, \(now).") - schedulePingCollection(now, sendTheNextCalendarDay: true) + schedulePingCollection( + now, + sendTheNextCalendarDay: true, + reason: GleanMetrics.Pings.MetricsReasonCodes.tomorrow + ) } else if isAfterDueTime(now) { logger.info("The 'metrics' ping is scheduled for immediate collection, \(now)") // **IMPORTANT** @@ -146,12 +167,20 @@ class MetricsPingScheduler { // context, see bug 1604861 and the implementation of // `collectPingAndReschedule`. Dispatchers.shared.serialOperationQueue.addOperation { - self.collectPingAndReschedule(now, startupPing: true) + self.collectPingAndReschedule( + now, + startupPing: true, + reason: GleanMetrics.Pings.MetricsReasonCodes.overdue + ) } } else { // This covers (3). logger.info("The 'metrics' collection is scheduled for today, \(now)") - schedulePingCollection(now, sendTheNextCalendarDay: false) + schedulePingCollection( + now, + sendTheNextCalendarDay: false, + reason: GleanMetrics.Pings.MetricsReasonCodes.today + ) } } @@ -159,8 +188,14 @@ class MetricsPingScheduler { /// /// - parameters: /// * now: A `Date` representing the current time - func collectPingAndReschedule(_ now: Date, startupPing: Bool = false) { - logger.info("Collecting the 'metrics' ping, now = \(now), startup = \(startupPing)") + /// * reason: The reason the ping is being submitted. + func collectPingAndReschedule( + _ now: Date, + startupPing: Bool = false, + reason: GleanMetrics.Pings.MetricsReasonCodes + ) { + let reasonString = GleanMetrics.Pings.shared.metrics.reasonCodes[reason.rawValue] + logger.info("Collecting the 'metrics' ping, now = \(now), startup = \(startupPing), reason = \(reasonString)") if startupPing { // **IMPORTANT** // @@ -176,15 +211,19 @@ class MetricsPingScheduler { // // * Do not change this line without checking what it implies for the above wall // of text. * - Glean.shared.submitPingsByNameSync(pingNames: ["metrics"]) + Glean.shared.submitPingByNameSync(pingName: "metrics", reason: reasonString) } else { - GleanMetrics.Pings.shared.metrics.submit() + GleanMetrics.Pings.shared.metrics.submit(reason: reason) } // Update the collection date: we don't really care if we have data or not, let's // always update the sent date. updateSentDate(now) // Reschedule the collection. - schedulePingCollection(now, sendTheNextCalendarDay: true) + schedulePingCollection( + now, + sendTheNextCalendarDay: true, + reason: GleanMetrics.Pings.MetricsReasonCodes.reschedule + ) } /// Get the date the metrics ping was last collected. diff --git a/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift b/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift index 02748e0037..ecc0210fed 100644 --- a/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift +++ b/glean-core/ios/GleanTests/Metrics/EventMetricTests.swift @@ -287,7 +287,7 @@ class EventMetricTypeTests: XCTestCase { setupHttpResponseStub() expectation = expectation(description: "Completed upload") - Glean.shared.submitPingsByName(pingNames: ["events"]) + Glean.shared.submitPingByName(pingName: "events") waitForExpectations(timeout: 5.0) { error in XCTAssertNil(error, "Test timed out waiting for upload: \(error!)") diff --git a/glean-core/ios/GleanTests/Metrics/PingTests.swift b/glean-core/ios/GleanTests/Metrics/PingTests.swift index 7f66e05c65..fe351182ce 100644 --- a/glean-core/ios/GleanTests/Metrics/PingTests.swift +++ b/glean-core/ios/GleanTests/Metrics/PingTests.swift @@ -43,7 +43,12 @@ class PingTests: XCTestCase { } func testSendingOfCustomPings() { - let customPing = Ping(name: "custom", includeClientId: true, sendIfEmpty: false) + let customPing = Ping( + name: "custom", + includeClientId: true, + sendIfEmpty: false, + reasonCodes: [] + ) let counter = CounterMetricType( category: "telemetry", @@ -73,7 +78,12 @@ class PingTests: XCTestCase { } func testSendingOfCustomPingsWithoutClientId() { - let customPing = Ping(name: "custom", includeClientId: false, sendIfEmpty: false) + let customPing = Ping( + name: "custom", + includeClientId: false, + sendIfEmpty: false, + reasonCodes: [] + ) let counter = CounterMetricType( category: "telemetry", @@ -119,7 +129,7 @@ class PingTests: XCTestCase { expectation = expectation(description: "Completed unexpected upload") expectation?.isInverted = true - Glean.shared.submitPingsByName(pingNames: ["unknown"]) + Glean.shared.submitPingByName(pingName: "unknown") // We wait for a timeout to happen, as we don't expect any data to be sent. waitForExpectations(timeout: 5.0) { _ in diff --git a/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift b/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift index 20868354f8..9bea75437f 100644 --- a/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift +++ b/glean-core/ios/GleanTests/Scheduler/MetricsPingSchedulerTests.swift @@ -110,7 +110,7 @@ class MetricsPingSchedulerTests: XCTestCase { let now = Date() UserDefaults.standard.set(nil, forKey: MetricsPingScheduler.Constants.lastMetricsPingSentDateTime) - mps.collectPingAndReschedule(now) + mps.collectPingAndReschedule(now, reason: GleanMetrics.Pings.MetricsReasonCodes.overdue) XCTAssertEqual( now.toISO8601String(precision: .second), mps.getLastCollectedDate()?.toISO8601String(precision: .second), @@ -310,7 +310,11 @@ class MetricsPingSchedulerTests: XCTestCase { mpsExpectation = expectation } - override func collectPingAndReschedule(_: Date, startupPing _: Bool = false) { + override func collectPingAndReschedule( + _: Date, + startupPing _: Bool = false, + reason _: GleanMetrics.Pings.MetricsReasonCodes + ) { mpsExpectation?.fulfill() } } @@ -343,7 +347,11 @@ class MetricsPingSchedulerTests: XCTestCase { // Calling `schedulePingCollection` with our `fakeNow` should cause the timer to // be set to fire in @ 5 seconds - mps.schedulePingCollection(fakeNow, sendTheNextCalendarDay: false) + mps.schedulePingCollection( + fakeNow, + sendTheNextCalendarDay: false, + reason: GleanMetrics.Pings.MetricsReasonCodes.overdue + ) waitForExpectations(timeout: 10.0) } diff --git a/glean-core/ios/sdk_generator.sh b/glean-core/ios/sdk_generator.sh index d825379ec0..9cbcde4705 100755 --- a/glean-core/ios/sdk_generator.sh +++ b/glean-core/ios/sdk_generator.sh @@ -25,7 +25,7 @@ set -e -GLEAN_PARSER_VERSION=1.15.5 +GLEAN_PARSER_VERSION=1.17.2 # CMDNAME is used in the usage text below. # shellcheck disable=SC2034 diff --git a/glean-core/metrics.yaml b/glean-core/metrics.yaml index 9d132e9124..dcde101a52 100644 --- a/glean-core/metrics.yaml +++ b/glean-core/metrics.yaml @@ -208,6 +208,20 @@ glean.internal.metrics: - glean-team@mozilla.com expires: never + ping_reason: + type: string + description: | + The optional reason the ping was submitted. + The specific values for reason are specific to each ping, and are + documented in the ping's pings.yaml file. + bugs: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1557048 + data_reviews: + - https://bugzilla.mozilla.org/show_bug.cgi?id=1609218#c4 + notification_emails: + - glean-team@mozilla.com + expires: never + glean.error: invalid_value: type: labeled_counter diff --git a/glean-core/pings.yaml b/glean-core/pings.yaml index c001e39cc3..ba48e03cd1 100644 --- a/glean-core/pings.yaml +++ b/glean-core/pings.yaml @@ -39,8 +39,24 @@ metrics: - https://bugzilla.mozilla.org/1512938 data_reviews: - https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3 + - https://bugzilla.mozilla.org/show_bug.cgi?id=1557048#c13 notification_emails: - glean-team@mozilla.com + reasons: + overdue: | + The ping wasn't submitted on the current calendar day, but it's after 4am, + so it's submitted immediately + today: | + The ping wasn't submitted on the current calendar day, but it is still + before 4am, so schedule to send on the current calendar day at 4am. + tomorrow: | + The ping was already submitted on the current calendar day, so schedule + for the next calendar day at 4am. + upgrade: | + The ping was submitted at startup because the application was just + upgraded. + reschedule: | + The ping was just submitted. Rescheduled for the next calendar day at 4am. events: description: > diff --git a/glean-core/preview/README.md b/glean-core/preview/README.md index 2b5e882f8a..11925730f7 100644 --- a/glean-core/preview/README.md +++ b/glean-core/preview/README.md @@ -33,11 +33,11 @@ let cfg = Configuration { }; glean_preview::initialize(cfg)?; -let prototype_ping = PingType::new("prototype", true, true); +let prototype_ping = PingType::new("prototype", true, true, vec![]); glean_preview::register_ping_type(&prototype_ping); -prototype_ping.submit(); +prototype_ping.submit(None); ``` ## License diff --git a/glean-core/preview/examples/prototype.rs b/glean-core/preview/examples/prototype.rs index b93d811425..e78895a873 100644 --- a/glean-core/preview/examples/prototype.rs +++ b/glean-core/preview/examples/prototype.rs @@ -11,7 +11,8 @@ use glean_preview as glean; use glean_preview::{metrics::PingType, ClientInfoMetrics, Configuration, Error}; #[allow(non_upper_case_globals)] -pub static PrototypePing: Lazy = Lazy::new(|| PingType::new("prototype", true, true)); +pub static PrototypePing: Lazy = + Lazy::new(|| PingType::new("prototype", true, true, vec![])); fn main() -> Result<(), Error> { env_logger::init(); @@ -42,7 +43,7 @@ fn main() -> Result<(), Error> { glean::initialize(cfg, client_info)?; glean::register_ping_type(&PrototypePing); - if glean::submit_ping_by_name("prototype") { + if glean::submit_ping_by_name("prototype", None) { log::info!("Successfully collected a prototype ping"); } else { log::info!("Prototype ping failed"); diff --git a/glean-core/preview/src/lib.rs b/glean-core/preview/src/lib.rs index 7909a349ad..07e27075a1 100644 --- a/glean-core/preview/src/lib.rs +++ b/glean-core/preview/src/lib.rs @@ -29,11 +29,11 @@ //! }; //! glean_preview::initialize(cfg, ClientInfoMetrics::unknown())?; //! -//! let prototype_ping = PingType::new("prototype", true, true); +//! let prototype_ping = PingType::new("prototype", true, true, vec!()); //! //! glean_preview::register_ping_type(&prototype_ping); //! -//! prototype_ping.submit(); +//! prototype_ping.submit(None); //! # Ok(()) //! # } //! ``` @@ -191,8 +191,8 @@ pub fn register_ping_type(ping: &metrics::PingType) { /// ## Return value /// /// Returns true if a ping was assembled and queued, false otherwise. -pub fn submit_ping(ping: &metrics::PingType) -> bool { - submit_ping_by_name(&ping.name) +pub fn submit_ping(ping: &metrics::PingType, reason: Option<&str>) -> bool { + submit_ping_by_name(&ping.name, reason) } /// Collect and submit a ping for eventual uploading by name. @@ -202,17 +202,8 @@ pub fn submit_ping(ping: &metrics::PingType) -> bool { /// ## Return value /// /// Returns true if a ping was assembled and queued, false otherwise. -pub fn submit_ping_by_name(ping: &str) -> bool { - submit_pings_by_name(&[ping.to_string()]) -} - -/// Collect and submit multiple pings by name for eventual uploading. -/// -/// ## Return value -/// -/// Returns true if at least one ping was assembled and queued, false otherwise. -pub fn submit_pings_by_name(pings: &[String]) -> bool { - with_glean(|glean| glean.submit_pings_by_name(pings)) +pub fn submit_ping_by_name(ping: &str, reason: Option<&str>) -> bool { + with_glean(|glean| glean.submit_ping_by_name(ping, reason).unwrap_or(false)) } #[cfg(test)] diff --git a/glean-core/preview/src/metrics/ping.rs b/glean-core/preview/src/metrics/ping.rs index 260158e3f6..6a46936e3c 100644 --- a/glean-core/preview/src/metrics/ping.rs +++ b/glean-core/preview/src/metrics/ping.rs @@ -21,10 +21,20 @@ impl PingType { /// * `name` - The name of the ping. /// * `include_client_id` - Whether to include the client ID in the assembled ping when. /// * `send_if_empty` - Whether the ping should be sent empty or not. - pub fn new>(name: A, include_client_id: bool, send_if_empty: bool) -> Self { + /// * `reason_codes` - The valid reason codes for this ping. + pub fn new>( + name: A, + include_client_id: bool, + send_if_empty: bool, + reason_codes: Vec, + ) -> Self { let name = name.into(); - let ping_type = - glean_core::metrics::PingType::new(name.clone(), include_client_id, send_if_empty); + let ping_type = glean_core::metrics::PingType::new( + name.clone(), + include_client_id, + send_if_empty, + reason_codes, + ); Self { name, ping_type } } @@ -33,7 +43,7 @@ impl PingType { /// ## Return value /// /// Returns true if a ping was assembled and queued, false otherwise. - pub fn submit(&self) -> bool { - crate::submit_ping(self) + pub fn submit(&self, reason: Option<&str>) -> bool { + crate::submit_ping(self, reason) } } diff --git a/glean-core/python/glean/_ffi.py b/glean-core/python/glean/_ffi.py index 26cb2a5029..25929e823d 100644 --- a/glean-core/python/glean/_ffi.py +++ b/glean-core/python/glean/_ffi.py @@ -5,7 +5,7 @@ from pathlib import Path import pkgutil import sys -from typing import Any, List +from typing import Any, List, Optional import weakref from cffi import FFI # type: ignore @@ -73,10 +73,12 @@ def make_config( return cfg -def ffi_encode_string(value: str) -> bytes: +def ffi_encode_string(value: Optional[str]) -> Optional[bytes]: """ Convert a Python string to a UTF-8 encoded char* for sending over FFI. """ + if value is None: + return ffi.NULL return value.encode("utf-8") diff --git a/glean-core/python/glean/_loader.py b/glean-core/python/glean/_loader.py index 29a29c0c0d..776599f775 100644 --- a/glean-core/python/glean/_loader.py +++ b/glean-core/python/glean/_loader.py @@ -50,6 +50,7 @@ "name", "range_max", "range_min", + "reason_codes", "send_in_pings", "time_unit", ] @@ -96,13 +97,19 @@ def _get_metric_objects(name: str, metric: glean_parser.metrics.Metric) -> Any: yield name, glean_metric - # Events also need to define an enumeration + # Events and Pings also need to define an enumeration if metric.type == "event": enum_name = name + "_keys" class_name = inflection.camelize(enum_name, True) values = dict((x.upper(), i) for (i, x) in enumerate(metric.allowed_extra_keys)) keys_enum = enum.Enum(class_name, values) # type: ignore yield enum_name, keys_enum + elif metric.type == "ping": + enum_name = name + "_reason_codes" + class_name = inflection.camelize(enum_name, True) + values = dict((x.upper(), i) for (i, x) in enumerate(metric.reason_codes)) + keys_enum = enum.Enum(class_name, values) # type: ignore + yield enum_name, keys_enum def load_metrics( diff --git a/glean-core/python/glean/glean.py b/glean-core/python/glean/glean.py index 7919afae9f..21e9a1be32 100644 --- a/glean-core/python/glean/glean.py +++ b/glean-core/python/glean/glean.py @@ -365,34 +365,39 @@ def get_data_dir(cls) -> Path: return cls._data_dir @classmethod - def test_collect(cls, ping: "PingType") -> str: + def test_collect(cls, ping: "PingType", reason: Optional[str] = None) -> str: """ Collect a ping and return as a string. + + Args: + ping: The PingType to submit + reason (str, optional): The reason code to record in the ping. """ - return _ffi.ffi_decode_string(_ffi.lib.glean_ping_collect(ping._handle)) + return _ffi.ffi_decode_string( + _ffi.lib.glean_ping_collect(ping._handle, _ffi.ffi_encode_string(reason)) + ) @classmethod - def _submit_pings(cls, pings: List["PingType"]): + def _submit_ping(cls, ping: "PingType", reason: Optional[str] = None): """ - Collect and submit a list of pings for eventual uploading. + Collect and submit a ping for eventual uploading. If the ping currently contains no content, it will not be assembled and queued for sending. Args: - pings (list of PingType): List of pings to submit. + ping (PingType): Pings to submit. + reason (str, optional): The reason the ping was submitted. """ - ping_names = [ping.name for ping in pings] - - cls._submit_pings_by_name(ping_names) + cls._submit_ping_by_name(ping.name, reason) @classmethod @Dispatcher.task - def _submit_pings_by_name(cls, ping_names: List[str]): + def _submit_ping_by_name(cls, ping_name: str, reason: Optional[str] = None): """ - Collect and submit a list of pings for eventual uploading by name. + Collect and submit a pings by name for eventual uploading. - Each ping will be looked up in the known instances of + The ping will be looked up in the known instances of `glean.metrics.PingType`. If the ping isn't known, an error is logged and the ping isn't queued for uploading. @@ -400,7 +405,8 @@ def _submit_pings_by_name(cls, ping_names: List[str]): queued for sending. Args: - ping_names (list of str): List of ping names to submit. + ping_name (str): Ping names to submit. + reason (str, optional): The reason code to include in the ping. """ if not cls.is_initialized(): log.error("Glean must be initialized before submitting pings.") @@ -410,8 +416,8 @@ def _submit_pings_by_name(cls, ping_names: List[str]): log.error("Glean must be enabled before submitting pings.") return - sent_ping = _ffi.lib.glean_submit_pings_by_name( - _ffi.ffi_encode_vec_string(ping_names), len(ping_names) + sent_ping = _ffi.lib.glean_submit_ping_by_name( + _ffi.ffi_encode_string(ping_name), _ffi.ffi_encode_string(reason), ) if sent_ping: diff --git a/glean-core/python/glean/metrics/ping.py b/glean-core/python/glean/metrics/ping.py index 7adfb278b4..07d6b9c370 100644 --- a/glean-core/python/glean/metrics/ping.py +++ b/glean-core/python/glean/metrics/ping.py @@ -3,12 +3,21 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. +from typing import List, Optional + + from ..glean import Glean from .. import _ffi class PingType: - def __init__(self, name: str, include_client_id: bool, send_if_empty: bool): + def __init__( + self, + name: str, + include_client_id: bool, + send_if_empty: bool, + reason_codes: List[str], + ): """ This implements the developer facing API for custom pings. @@ -16,8 +25,13 @@ def __init__(self, name: str, include_client_id: bool, send_if_empty: bool): ping for eventual uploading. """ self._name = name + self._reason_codes = reason_codes self._handle = _ffi.lib.glean_new_ping_type( - _ffi.ffi_encode_string(name), include_client_id, send_if_empty + _ffi.ffi_encode_string(name), + include_client_id, + send_if_empty, + _ffi.ffi_encode_vec_string(reason_codes), + len(reason_codes), ) Glean.register_ping_type(self) @@ -32,10 +46,18 @@ def name(self): """ return self._name - def submit(self): + def submit(self, reason: Optional[int] = None): """ Collect and submit the ping for eventual uploading. If the ping currently contains no content, it will not be sent. + + Args: + reason (enum, optional): The reason the ping was submitted. """ - Glean._submit_pings([self]) + reason_string = None # type: Optional[str] + if reason is not None: + reason_string = self._reason_codes[reason] + else: + reason_string = None + Glean._submit_ping(self, reason_string) diff --git a/glean-core/python/setup.py b/glean-core/python/setup.py index 08face1aad..5cad897952 100644 --- a/glean-core/python/setup.py +++ b/glean-core/python/setup.py @@ -39,7 +39,7 @@ parsed_toml = toml.load(cargo) version = parsed_toml["package"]["version"] -requirements = ["cffi==1.13.1", "glean_parser==1.15.5", "inflection==0.3.1"] +requirements = ["cffi==1.13.1", "glean_parser==1.17.2", "inflection==0.3.1"] setup_requirements = [] diff --git a/glean-core/python/tests/metrics/test_event.py b/glean-core/python/tests/metrics/test_event.py index ddbb62167d..2d66ff7532 100644 --- a/glean-core/python/tests/metrics/test_event.py +++ b/glean-core/python/tests/metrics/test_event.py @@ -276,7 +276,7 @@ class EventKeys(enum.Enum): assert 1 == len(event.test_get_value()) - Glean._submit_pings_by_name(["events"]) + Glean._submit_ping_by_name("events") assert 2 == len(safe_httpserver.requests) request = safe_httpserver.requests[1] diff --git a/glean-core/python/tests/test_glean.py b/glean-core/python/tests/test_glean.py index 66164407a0..b6f7ebd9c5 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): def test_submiting_an_empty_ping_doesnt_queue_work(safe_httpserver): safe_httpserver.serve_content(b"", code=200) - Glean._submit_pings_by_name(["metrics"]) + Glean._submit_ping_by_name("metrics") assert 0 == len(safe_httpserver.requests) @@ -218,7 +218,9 @@ def test_dont_schedule_pings_if_metrics_disabled(safe_httpserver): send_in_pings=["store1"], ) - custom_ping = PingType(name="store1", include_client_id=True, send_if_empty=False) + custom_ping = PingType( + name="store1", include_client_id=True, send_if_empty=False, reason_codes=[] + ) counter_metric.add(10) @@ -232,7 +234,9 @@ def test_dont_schedule_pings_if_metrics_disabled(safe_httpserver): def test_dont_schedule_pings_if_there_is_no_ping_content(safe_httpserver): safe_httpserver.serve_content(b"", code=200) - custom_ping = PingType(name="store1", include_client_id=True, send_if_empty=False) + custom_ping = PingType( + name="store1", include_client_id=True, send_if_empty=False, reason_codes=[] + ) custom_ping.submit() @@ -319,7 +323,9 @@ def test_collect(ping_schema_url): send_in_pings=["store1"], ) - custom_ping = PingType(name="store1", include_client_id=True, send_if_empty=False) + custom_ping = PingType( + name="store1", include_client_id=True, send_if_empty=False, reason_codes=[] + ) counter_metric.add(10) diff --git a/glean-core/src/event_database/mod.rs b/glean-core/src/event_database/mod.rs index ffcabb1d18..6ae54a3561 100644 --- a/glean-core/src/event_database/mod.rs +++ b/glean-core/src/event_database/mod.rs @@ -140,7 +140,7 @@ impl EventDatabase { let mut ping_sent = false; for store_name in store_names { - if let Err(err) = glean.submit_ping_by_name(&store_name) { + if let Err(err) = glean.submit_ping_by_name(&store_name, None) { log::error!( "Error flushing existing events to the '{}' ping: {}", store_name, @@ -199,7 +199,7 @@ impl EventDatabase { // If any of the event stores reached maximum size, submit the pings // containing those events immediately. for store_name in stores_to_submit { - if let Err(err) = glean.submit_ping_by_name(store_name) { + if let Err(err) = glean.submit_ping_by_name(store_name, None) { log::error!( "Got more than {} events, but could not send {} ping: {}", glean.get_max_events(), diff --git a/glean-core/src/internal_pings.rs b/glean-core/src/internal_pings.rs index ed72889152..159f20e4bc 100644 --- a/glean-core/src/internal_pings.rs +++ b/glean-core/src/internal_pings.rs @@ -21,10 +21,21 @@ pub struct InternalPings { impl InternalPings { pub fn new() -> InternalPings { InternalPings { - baseline: PingType::new("baseline", true, false), - metrics: PingType::new("metrics", true, false), - events: PingType::new("events", true, false), - deletion_request: PingType::new("deletion-request", true, true), + baseline: PingType::new("baseline", true, false, vec![]), + metrics: PingType::new( + "metrics", + true, + false, + vec![ + "overdue".to_string(), + "reschedule".to_string(), + "today".to_string(), + "tomorrow".to_string(), + "upgrade".to_string(), + ], + ), + events: PingType::new("events", true, false, vec![]), + deletion_request: PingType::new("deletion-request", true, true, vec![]), } } } diff --git a/glean-core/src/lib.rs b/glean-core/src/lib.rs index 6caf19a6e6..36a3cf17f6 100644 --- a/glean-core/src/lib.rs +++ b/glean-core/src/lib.rs @@ -118,7 +118,7 @@ pub struct Configuration { /// delay_ping_lifetime_io: false, /// }; /// let mut glean = Glean::new(cfg).unwrap(); -/// let ping = PingType::new("sample", true, false); +/// let ping = PingType::new("sample", true, false, vec![]); /// glean.register_ping_type(&ping); /// /// let call_counter: CounterMetric = CounterMetric::new(CommonMetricData { @@ -130,7 +130,7 @@ pub struct Configuration { /// /// call_counter.add(&glean, 1); /// -/// glean.submit_ping(&ping).unwrap(); +/// glean.submit_ping(&ping, None).unwrap(); /// ``` /// /// ## Note @@ -272,7 +272,7 @@ impl Glean { // When upload is disabled, submit a deletion-request ping if !flag { - if let Err(err) = self.internal_pings.deletion_request.submit(self) { + if let Err(err) = self.internal_pings.deletion_request.submit(self, None) { log::error!("Failed to send deletion-request ping on optout: {}", err); } } @@ -429,7 +429,11 @@ impl Glean { /// /// Returns true if a ping was assembled and queued, false otherwise. /// Returns an error if collecting or writing the ping to disk failed. - pub fn submit_ping(&self, ping: &PingType) -> Result { + /// + /// ## Arguments + /// * `ping`: The ping to submit + /// * `reason`: A reason code to include in the ping + pub fn submit_ping(&self, ping: &PingType, reason: Option<&str>) -> Result { if !self.is_upload_enabled() { log::error!("Glean must be enabled before sending pings."); return Ok(false); @@ -438,7 +442,7 @@ impl Glean { let ping_maker = PingMaker::new(); let doc_id = Uuid::new_v4().to_string(); let url_path = self.make_path(&ping.name, &doc_id); - match ping_maker.collect(self, &ping) { + match ping_maker.collect(self, &ping, reason) { None => { log::info!( "No content for ping '{}', therefore no ping queued.", @@ -467,25 +471,6 @@ impl Glean { } } - /// Collect and submit a ping for eventual uploading by name. - /// - /// See `submit_ping` for detailed information. - /// - /// Returns true if at least one ping was assembled and queued, false otherwise. - pub fn submit_pings_by_name(&self, ping_names: &[String]) -> bool { - // TODO: 1553813: glean-ac collects and stores pings in parallel and then joins them all before queueing the worker. - // This here is writing them out sequentially. - - let mut result = false; - - for ping_name in ping_names { - if let Ok(true) = self.submit_ping_by_name(ping_name) { - result = true; - } - } - result - } - /// Collect and submit a ping by name for eventual uploading. /// /// The ping content is assembled as soon as possible, but upload is not @@ -496,13 +481,17 @@ impl Glean { /// /// Returns true if a ping was assembled and queued, false otherwise. /// Returns an error if collecting or writing the ping to disk failed. - pub fn submit_ping_by_name(&self, ping_name: &str) -> Result { + /// + /// ## Arguments + /// * `ping_name`: The name of the ping to submit + /// * `reason`: A reason code to include in the ping + pub fn submit_ping_by_name(&self, ping_name: &str, reason: Option<&str>) -> Result { match self.get_ping_by_name(ping_name) { None => { log::error!("Attempted to submit unknown ping '{}'", ping_name); Ok(false) } - Some(ping) => self.submit_ping(ping), + Some(ping) => self.submit_ping(ping, reason), } } diff --git a/glean-core/src/metrics/ping.rs b/glean-core/src/metrics/ping.rs index 630a4cae22..ae0431aec9 100644 --- a/glean-core/src/metrics/ping.rs +++ b/glean-core/src/metrics/ping.rs @@ -17,6 +17,8 @@ pub struct PingType { pub include_client_id: bool, /// Whether the ping should be sent if it is empty pub send_if_empty: bool, + /// The "reason" codes that this ping can send + pub reason_codes: Vec, } impl PingType { @@ -28,11 +30,17 @@ impl PingType { /// * `name` - The name of the ping. /// * `include_client_id` - Whether to include the client ID in the assembled ping when. /// sending. - pub fn new>(name: A, include_client_id: bool, send_if_empty: bool) -> Self { + pub fn new>( + name: A, + include_client_id: bool, + send_if_empty: bool, + reason_codes: Vec, + ) -> Self { Self { name: name.into(), include_client_id, send_if_empty, + reason_codes, } } @@ -41,11 +49,25 @@ impl PingType { /// ## Arguments /// /// * `glean` - the Glean instance to use to send the ping. + /// * `reason` - the reason the ping was triggered. Included in the + /// `ping_info.reason` part of the payload. /// /// ## Return value /// /// See [`Glean#submit_ping`](../struct.Glean.html#method.submit_ping) for details. - pub fn submit(&self, glean: &Glean) -> Result { - glean.submit_ping(self) + pub fn submit(&self, glean: &Glean, reason: Option<&str>) -> Result { + let corrected_reason = match reason { + Some(reason) => { + if self.reason_codes.contains(&reason.to_string()) { + Some(reason) + } else { + log::error!("Invalid reason code {} for ping {}", reason, self.name); + None + } + } + None => None, + }; + + glean.submit_ping(self, corrected_reason) } } diff --git a/glean-core/src/ping/mod.rs b/glean-core/src/ping/mod.rs index 70453f0d65..9469ebd8e9 100644 --- a/glean-core/src/ping/mod.rs +++ b/glean-core/src/ping/mod.rs @@ -108,7 +108,7 @@ impl PingMaker { (start_time_data, end_time_data) } - fn get_ping_info(&self, glean: &Glean, storage_name: &str) -> JsonValue { + fn get_ping_info(&self, glean: &Glean, storage_name: &str, reason: Option<&str>) -> JsonValue { let (start_time, end_time) = self.get_start_end_times(glean, storage_name); let mut map = json!({ "ping_type": storage_name, @@ -117,6 +117,12 @@ impl PingMaker { "end_time": end_time, }); + if let Some(reason) = reason { + map.as_object_mut() + .unwrap() // safe unwrap, we created the object above + .insert("reason".to_string(), JsonValue::String(reason.to_string())); + }; + // Get the experiment data, if available. if let Some(experiment_data) = StorageManager.snapshot_experiments_as_json(glean.storage(), INTERNAL_STORAGE) @@ -162,12 +168,18 @@ impl PingMaker { /// /// * `glean` - the Glean instance to collect data from. /// * `ping` - the ping to collect for. + /// * `reason` - an optional reason code to include in the ping. /// /// ## Return value /// /// Returns a fully assembled JSON representation of the ping payload. /// If there is no data stored for the ping, `None` is returned. - pub fn collect(&self, glean: &Glean, ping: &PingType) -> Option { + pub fn collect( + &self, + glean: &Glean, + ping: &PingType, + reason: Option<&str>, + ) -> Option { info!("Collecting {}", ping.name); let metrics_data = StorageManager.snapshot_as_json(glean.storage(), &ping.name, true); @@ -181,7 +193,7 @@ impl PingMaker { info!("Storage for {} empty. Ping will still be sent.", ping.name); } - let ping_info = self.get_ping_info(glean, &ping.name); + let ping_info = self.get_ping_info(glean, &ping.name, reason); let client_info = self.get_client_info(glean, ping.include_client_id); let mut json = json!({ @@ -206,13 +218,19 @@ impl PingMaker { /// /// * `glean` - the Glean instance to collect data from. /// * `ping` - the ping to collect for. + /// * `reason` - an optional reason code to include in the ping. /// /// ## Return value /// /// Returns a fully assembled ping payload in a string encoded as JSON. /// If there is no data stored for the ping, `None` is returned. - pub fn collect_string(&self, glean: &Glean, ping: &PingType) -> Option { - self.collect(glean, ping) + pub fn collect_string( + &self, + glean: &Glean, + ping: &PingType, + reason: Option<&str>, + ) -> Option { + self.collect(glean, ping, reason) .map(|ping| ::serde_json::to_string_pretty(&ping).unwrap()) } diff --git a/glean-core/tests/event.rs b/glean-core/tests/event.rs index 8c1f2dce22..195b18dcf1 100644 --- a/glean-core/tests/event.rs +++ b/glean-core/tests/event.rs @@ -152,7 +152,7 @@ fn test_sending_of_event_ping_when_it_fills_up() { let store_names: Vec = vec!["events".into()]; for store_name in &store_names { - glean.register_ping_type(&PingType::new(store_name.clone(), true, false)); + glean.register_ping_type(&PingType::new(store_name.clone(), true, false, vec![])); } let click = EventMetric::new( diff --git a/glean-core/tests/ping.rs b/glean-core/tests/ping.rs index d82114bc3e..f95203639c 100644 --- a/glean-core/tests/ping.rs +++ b/glean-core/tests/ping.rs @@ -12,7 +12,7 @@ use glean_core::CommonMetricData; fn write_ping_to_disk() { let (mut glean, _temp) = new_glean(None); - let ping = PingType::new("metrics", true, false); + let ping = PingType::new("metrics", true, false, vec![]); glean.register_ping_type(&ping); // We need to store a metric as an empty ping is not stored. @@ -24,7 +24,7 @@ fn write_ping_to_disk() { }); counter.add(&glean, 1); - assert!(ping.submit(&glean).unwrap()); + assert!(ping.submit(&glean, None).unwrap()); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); } @@ -33,7 +33,7 @@ fn write_ping_to_disk() { fn disabling_upload_clears_pending_pings() { let (mut glean, _) = new_glean(None); - let ping = PingType::new("metrics", true, false); + let ping = PingType::new("metrics", true, false, vec![]); glean.register_ping_type(&ping); // We need to store a metric as an empty ping is not stored. @@ -45,7 +45,7 @@ fn disabling_upload_clears_pending_pings() { }); counter.add(&glean, 1); - assert!(ping.submit(&glean).unwrap()); + assert!(ping.submit(&glean, None).unwrap()); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); // At this point no deletion_request ping should exist // (that is: it's directory should not exist at all) @@ -60,7 +60,7 @@ fn disabling_upload_clears_pending_pings() { assert_eq!(0, get_queued_pings(glean.get_data_path()).unwrap().len()); counter.add(&glean, 1); - assert!(ping.submit(&glean).unwrap()); + assert!(ping.submit(&glean, None).unwrap()); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); } @@ -68,18 +68,18 @@ fn disabling_upload_clears_pending_pings() { fn empty_pings_with_flag_are_sent() { let (mut glean, _) = new_glean(None); - let ping1 = PingType::new("custom-ping1", true, true); + let ping1 = PingType::new("custom-ping1", true, true, vec![]); glean.register_ping_type(&ping1); - let ping2 = PingType::new("custom-ping2", true, false); + let ping2 = PingType::new("custom-ping2", true, false, vec![]); glean.register_ping_type(&ping2); // No data is stored in either of the custom pings // Sending this should succeed. - assert_eq!(true, ping1.submit(&glean).unwrap()); + assert_eq!(true, ping1.submit(&glean, None).unwrap()); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); // Sending this should fail. - assert_eq!(false, ping2.submit(&glean).unwrap()); + assert_eq!(false, ping2.submit(&glean, None).unwrap()); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); } diff --git a/glean-core/tests/ping_maker.rs b/glean-core/tests/ping_maker.rs index 7ce64c20d3..7ffe469779 100644 --- a/glean-core/tests/ping_maker.rs +++ b/glean-core/tests/ping_maker.rs @@ -22,7 +22,7 @@ fn set_up_basic_ping() -> (Glean, PingMaker, PingType, tempfile::TempDir) { }; let mut glean = Glean::new(cfg).unwrap(); let ping_maker = PingMaker::new(); - let ping_type = PingType::new("store1", true, false); + let ping_type = PingType::new("store1", true, false, vec![]); glean.register_ping_type(&ping_type); // Record something, so the ping will have data @@ -43,7 +43,7 @@ fn set_up_basic_ping() -> (Glean, PingMaker, PingType, tempfile::TempDir) { fn ping_info_must_contain_a_nonempty_start_and_end_time() { let (glean, ping_maker, ping_type, _t) = set_up_basic_ping(); - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let ping_info = content["ping_info"].as_object().unwrap(); let start_time_str = ping_info["start_time"].as_str().unwrap(); @@ -59,7 +59,7 @@ fn ping_info_must_contain_a_nonempty_start_and_end_time() { fn get_ping_info_must_report_all_the_required_fields() { let (glean, ping_maker, ping_type, _t) = set_up_basic_ping(); - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let ping_info = content["ping_info"].as_object().unwrap(); assert_eq!("store1", ping_info["ping_type"].as_str().unwrap()); @@ -72,7 +72,7 @@ fn get_ping_info_must_report_all_the_required_fields() { fn get_client_info_must_report_all_the_available_data() { let (glean, ping_maker, ping_type, _t) = set_up_basic_ping(); - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let client_info = content["client_info"].as_object().unwrap(); client_info["telemetry_sdk_build"].as_str().unwrap(); @@ -89,10 +89,12 @@ fn collect_must_report_none_when_no_data_is_stored() { let (mut glean, ping_maker, ping_type, _t) = set_up_basic_ping(); - let unknown_ping_type = PingType::new("unknown", true, false); + let unknown_ping_type = PingType::new("unknown", true, false, vec![]); glean.register_ping_type(&ping_type); - assert!(ping_maker.collect(&glean, &unknown_ping_type).is_none()); + assert!(ping_maker + .collect(&glean, &unknown_ping_type, None) + .is_none()); } #[test] @@ -111,8 +113,8 @@ fn seq_number_must_be_sequential() { for i in 0..=1 { for ping_name in ["store1", "store2"].iter() { - let ping_type = PingType::new(*ping_name, true, false); - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let ping_type = PingType::new(*ping_name, true, false, vec![]); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let seq_num = content["ping_info"]["seq"].as_i64().unwrap(); // Ensure sequence numbers in different stores are independent of // each other @@ -122,33 +124,33 @@ fn seq_number_must_be_sequential() { // Test that ping sequence numbers increase independently. { - let ping_type = PingType::new("store1", true, false); + let ping_type = PingType::new("store1", true, false, vec![]); // 3rd ping of store1 - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let seq_num = content["ping_info"]["seq"].as_i64().unwrap(); assert_eq!(2, seq_num); // 4th ping of store1 - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let seq_num = content["ping_info"]["seq"].as_i64().unwrap(); assert_eq!(3, seq_num); } { - let ping_type = PingType::new("store2", true, false); + let ping_type = PingType::new("store2", true, false, vec![]); // 3rd ping of store2 - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let seq_num = content["ping_info"]["seq"].as_i64().unwrap(); assert_eq!(2, seq_num); } { - let ping_type = PingType::new("store1", true, false); + let ping_type = PingType::new("store1", true, false, vec![]); // 5th ping of store1 - let content = ping_maker.collect(&glean, &ping_type).unwrap(); + let content = ping_maker.collect(&glean, &ping_type, None).unwrap(); let seq_num = content["ping_info"]["seq"].as_i64().unwrap(); assert_eq!(4, seq_num); } @@ -158,7 +160,7 @@ fn seq_number_must_be_sequential() { fn test_clear_pending_pings() { let (mut glean, _) = new_glean(None); let ping_maker = PingMaker::new(); - let ping_type = PingType::new("store1", true, false); + let ping_type = PingType::new("store1", true, false, vec![]); glean.register_ping_type(&ping_type); // Record something, so the ping will have data @@ -172,7 +174,7 @@ fn test_clear_pending_pings() { }); metric.set(&glean, true); - assert!(glean.submit_ping(&ping_type).is_ok()); + assert!(glean.submit_ping(&ping_type, None).is_ok()); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); assert!(ping_maker @@ -186,19 +188,19 @@ fn test_no_pings_submitted_if_upload_disabled() { // Regression test, bug 1603571 let (mut glean, _) = new_glean(None); - let ping_type = PingType::new("store1", true, true); + let ping_type = PingType::new("store1", true, true, vec![]); glean.register_ping_type(&ping_type); - assert!(glean.submit_ping(&ping_type).is_ok()); + assert!(glean.submit_ping(&ping_type, None).is_ok()); assert_eq!(1, get_queued_pings(glean.get_data_path()).unwrap().len()); // Disable upload, then try to sumbit glean.set_upload_enabled(false); - assert!(glean.submit_ping(&ping_type).is_ok()); + assert!(glean.submit_ping(&ping_type, None).is_ok()); assert_eq!(0, get_queued_pings(glean.get_data_path()).unwrap().len()); // Test again through the direct call - assert!(ping_type.submit(&glean).is_ok()); + assert!(ping_type.submit(&glean, None).is_ok()); assert_eq!(0, get_queued_pings(glean.get_data_path()).unwrap().len()); } diff --git a/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy b/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy index fcafad96a1..1e6df66cd3 100644 --- a/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy +++ b/gradle-plugin/src/main/groovy/mozilla/telemetry/glean-gradle-plugin/GleanGradlePlugin.groovy @@ -34,7 +34,7 @@ class GleanMetricsYamlTransform extends ArtifactTransform { @SuppressWarnings("GrPackage") class GleanPlugin implements Plugin { // The version of glean_parser to install from PyPI. - private String GLEAN_PARSER_VERSION = "1.15.5" + private String GLEAN_PARSER_VERSION = "1.17.2" // The version of Miniconda is explicitly specified. // Miniconda3-4.5.12 is known to not work on Windows. private String MINICONDA_VERSION = "4.5.11"