Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1609968 - Remove locale from baseline ping #1016

Merged
merged 3 commits into from
Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased changes

* Remove locale from baseline ping. ([1609968](https://bugzilla.mozilla.org/show_bug.cgi?id=1609968), [#1016](https://github.com/mozilla/glean/pull/1016))

[Full changelog](https://github.com/mozilla/glean/compare/v31.2.3...main)

# v31.2.3 (2020-06-29)
Expand Down
3 changes: 2 additions & 1 deletion docs/user/collected-metrics/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ This is a built-in ping that is assembled out of the box by the Glean SDK.

See the Glean SDK documentation for the [`baseline` ping](https://mozilla.github.io/glean/book/user/pings/baseline.html).

This ping is sent if empty.

This ping includes the [client id](https://mozilla.github.io/glean/book/user/pings/index.html#the-client_info-section).

**Data reviews for this ping:**
Expand Down Expand Up @@ -65,7 +67,6 @@ The following metrics are added to the ping:
| Name | Type | Description | Data reviews | Extras | Expiration |
| --- | --- | --- | --- | --- | --- |
| glean.baseline.duration |[timespan](https://mozilla.github.io/glean/book/user/metrics/timespan.html) |The duration of the last foreground session. |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3)||never |
| glean.baseline.locale |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |The locale of the application during initialization (e.g. "es-ES"). If the locale can't be determined on the system, the value is ["und"](https://unicode.org/reports/tr35/#Unknown_or_Invalid_Identifiers), to indicate "undetermined". |[1](https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3)||never |

## deletion-request

Expand Down
6 changes: 0 additions & 6 deletions docs/user/pings/baseline.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,9 @@ The baseline ping includes the following fields:
| Field name | Type | Description |
|---|---|---|
| `duration` | Timespan | The duration, in seconds, of the last foreground session. Only available if `reason: background`. [^1] |
| `locale` | String | The locale of the application. [^2] |

[^1]: See also the [ping schedules and timing overview](ping-schedules-and-timings.html) for how the `duration` metric relates to other sources of timing in the `baseline` ping.

[^2]: The locale metric in the baseline ping is deprecated. Use [`client_info.locale`](index.html#The-client-info-section) instead.

The `baseline` ping also includes the common [ping sections](index.md#ping-sections) found in all pings.

### Querying ping contents
Expand Down Expand Up @@ -63,9 +60,6 @@ A quick note about querying ping contents (i.e. for [sql.telemetry.mozilla.org](
"client_id": "35dab852-74db-43f4-8aa0-88884211e545"
},
"metrics": {
"string": {
"glean.baseline.locale": "en-US"
},
"timespan": {
"glean.baseline.duration": {
"value": 52,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import mozilla.telemetry.glean.rust.LibGleanFFI
import mozilla.telemetry.glean.rust.getAndConsumeRustString
import mozilla.telemetry.glean.rust.toBoolean
import mozilla.telemetry.glean.rust.toByte
import mozilla.telemetry.glean.GleanMetrics.GleanBaseline
import mozilla.telemetry.glean.GleanMetrics.GleanInternalMetrics
import mozilla.telemetry.glean.GleanMetrics.Pings
import mozilla.telemetry.glean.net.BaseUploader
Expand Down Expand Up @@ -445,7 +444,6 @@ open class GleanInternalAPI internal constructor () {
// Please note that the following metrics must be set synchronously, so
// that they are guaranteed to be available with the first ping that is
// generated. We use an internal only API to do that.
GleanBaseline.locale.setSync(getLocaleTag())
// https://developer.android.com/reference/android/os/Build.VERSION
GleanInternalMetrics.androidSdkVersion.setSync(Build.VERSION.SDK_INT.toString())
GleanInternalMetrics.osVersion.setSync(Build.VERSION.RELEASE)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,19 +232,16 @@ class GleanTest {
assertEquals(1, json.getJSONArray("events").length())
} else if (docType == "baseline") {
val seq = json.getJSONObject("ping_info").getInt("seq")
val baselineMetricsObject = json.getJSONObject("metrics")
val baselineStringMetrics = baselineMetricsObject.getJSONObject("string")
assertEquals(1, baselineStringMetrics.length())
assertNotNull(baselineStringMetrics.get("glean.baseline.locale"))

// There are three baseline pings:
// - seq: 0, reason: foreground, duration: null
// - seq: 1, reason: background, duration: non-null
// - seq: 2, reason: foreground, duration: null
if (seq == 0 || seq == 2) {
assertFalse(baselineMetricsObject.has("timespan"))
assertFalse(json.has("metrics"))
Dexterp37 marked this conversation as resolved.
Show resolved Hide resolved
assertEquals("foreground", json.getJSONObject("ping_info").getString("reason"))
} else if (seq == 1) {
val baselineMetricsObject = json.getJSONObject("metrics")
assertEquals("background", json.getJSONObject("ping_info").getString("reason"))
val baselineTimespanMetrics = baselineMetricsObject.getJSONObject("timespan")
assertEquals(1, baselineTimespanMetrics.length())
Expand Down Expand Up @@ -285,14 +282,9 @@ class GleanTest {
assertEquals("dirty_startup", baselineJson.getJSONObject("ping_info")["reason"])
checkPingSchema(baselineJson)

val baselineMetricsObject = baselineJson.getJSONObject("metrics")
val baselineStringMetrics = baselineMetricsObject.getJSONObject("string")
assertEquals(1, baselineStringMetrics.length())
assertNotNull(baselineStringMetrics.get("glean.baseline.locale"))

assertFalse(
"The baseline ping from startup must not have a duration",
baselineMetricsObject.has("timespan")
"The baseline ping from startup must not have any metrics",
baselineJson.has("metrics")
)
} finally {
server.shutdown()
Expand Down
1 change: 0 additions & 1 deletion glean-core/ios/Glean/Glean.swift
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@ public class Glean {
// that they are guaranteed to be available with the first ping that is
// generated. We use an internal only API to do that.

GleanBaseline.locale.setSync(getLocaleTag())
GleanInternalMetrics.osVersion.setSync(UIDevice.current.systemVersion)
GleanInternalMetrics.deviceManufacturer.setSync(Sysctl.manufacturer)
GleanInternalMetrics.deviceModel.setSync(Sysctl.model)
Expand Down
22 changes: 4 additions & 18 deletions glean-core/ios/GleanTests/GleanTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,8 @@ class GleanTests: XCTestCase {
let pingInfo = json?["ping_info"] as? [String: Any]
XCTAssertEqual("foreground", pingInfo?["reason"] as? String)

// Ensure there is only the expected locale string metric
let metrics = json?["metrics"] as? [String: Any]
let strings = metrics?["string"] as? [String: Any]
XCTAssertEqual(1, strings?.count, "Must contain only the expected metric")
let locale = strings?["glean.baseline.locale"] as? String
XCTAssertNotNil(locale, "Locale is not nil")

// We should not have a duration for a ping with the "foreground" flag
XCTAssertNil(metrics?["timespan"], "Duration is not nil")
// We should not have any metrics for a ping with the "foreground" flag
XCTAssertNil(json?["metrics"], "metrics is not nil")

DispatchQueue.main.async {
// let the response get processed before we mark the expectation fulfilled
Expand Down Expand Up @@ -153,15 +146,8 @@ class GleanTests: XCTestCase {
let pingInfo = json?["ping_info"] as? [String: Any]
XCTAssertEqual("dirty_startup", pingInfo?["reason"] as? String)

// Ensure there is only the expected locale string metric
let metrics = json?["metrics"] as? [String: Any]
let strings = metrics?["string"] as? [String: Any]
XCTAssertEqual(1, strings?.count, "Must contain only the expected metric")
let locale = strings?["glean.baseline.locale"] as? String
XCTAssertNotNil(locale, "Locale is not nil")

// We should not have a duration for a ping with the "dirty_startup" flag
XCTAssertNil(metrics?["timespan"])
// We should not have any metrics for a ping with the "dirty_startup" flag
XCTAssertNil(json?["metrics"], "metrics is not nil")

DispatchQueue.main.async {
// let the response get processed before we mark the expectation fulfilled
Expand Down
18 changes: 0 additions & 18 deletions glean-core/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,6 @@ glean.baseline:
notification_emails:
- [email protected]
expires: never
locale:
brizental marked this conversation as resolved.
Show resolved Hide resolved
type: string
lifetime: application
send_in_pings:
- baseline
description: |
The locale of the application during initialization (e.g. "es-ES").
If the locale can't be determined on the system, the value is
["und"](https://unicode.org/reports/tr35/#Unknown_or_Invalid_Identifiers),
to indicate "undetermined".
bugs:
- https://bugzilla.mozilla.org/1512938
- https://bugzilla.mozilla.org/1525540
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3
notification_emails:
- [email protected]
expires: never

glean.internal.metrics:
os:
Expand Down
1 change: 1 addition & 0 deletions glean-core/pings.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ baseline:
The `baseline` ping is automatically sent when the application is moved to
the background.
include_client_id: true
send_if_empty: true
brizental marked this conversation as resolved.
Show resolved Hide resolved
bugs:
- https://bugzilla.mozilla.org/1512938
- https://bugzilla.mozilla.org/1599877
Expand Down
1 change: 0 additions & 1 deletion glean-core/python/glean/glean.py
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,6 @@ def _initialize_core_metrics(cls) -> None:
"""
from ._builtins import metrics

metrics.glean.baseline.locale._set_sync(_util.get_locale_tag())
metrics.glean.internal.metrics.os_version._set_sync(platform.release())
metrics.glean.internal.metrics.architecture._set_sync(platform.machine())
metrics.glean.internal.metrics.locale._set_sync(_util.get_locale_tag())
Expand Down