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 1827852 - Extend start/stop time of a ping to millisecond precision #2456

Merged
merged 6 commits into from
Oct 19, 2023
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
Expand Up @@ -5,6 +5,8 @@
* General
* BREAKING CHANGE: Adding `0` to a `counter` or `labeled_counter` metric will be silently ignored instead of raising an `invalid_value` error ([bug 1762859](https://bugzilla.mozilla.org/show_bug.cgi?id=1762859))
* Trigger the uploader thread after scanning the pending pings directory ([bug 1847950](https://bugzilla.mozilla.org/show_bug.cgi?id=1847950))
* Extend start/stop time of a ping to millisecond precision. Custom pings can opt-out using `precise_timestamps: false` ([#2456](https://github.com/mozilla/glean/pull/2456))
* Update `glean_parser` to v10.0.0. Disallow `unit` field for anything but quantity, disallows `ping` lifetime metrics on the events ping, allows to configure precise timestamps in pings ([release notes](https://github.com/mozilla/glean_parser/releases/tag/v10.0.0))

# v54.0.0 (2023-09-12)

Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ docs-python: build-python ## Build the Python documentation
.PHONY: docs rust-docs swift-docs

docs-metrics: setup-python ## Build the internal metrics documentation
$(GLEAN_PYENV)/bin/pip install glean_parser~=8.1
$(GLEAN_PYENV)/bin/pip install glean_parser~=10.0
$(GLEAN_PYENV)/bin/glean_parser translate --allow-reserved \
-f markdown \
-o ./docs/user/user/collected-metrics \
Expand Down
11 changes: 9 additions & 2 deletions docs/user/user/collected-metrics/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

> If you are looking for the metrics collected by Glean.js,
> refer to the documentation over on the [`@mozilla/glean.js`](https://github.com/mozilla/glean.js/blob/main/docs/reference/metrics.md) repository.
<!-- AUTOGENERATED BY glean_parser v7.0.0. DO NOT EDIT. -->
<!-- AUTOGENERATED BY glean_parser v10.0.0. DO NOT EDIT. -->

# Metrics

Expand All @@ -29,6 +29,7 @@ In addition to those built-in metrics, the following metrics are added to the pi

| Name | Type | Description | Data reviews | Extras | Expiration | [Data Sensitivity](https://wiki.mozilla.org/Firefox/Data_Collection) |
| --- | --- | --- | --- | --- | --- | --- |
| glean.client.annotation.experimentation_id |[string](https://mozilla.github.io/glean/book/user/metrics/string.html) |An experimentation identifier derived and provided by the application for the purpose of experimentation enrollment. |[Bug 1848201](https://bugzilla.mozilla.org/show_bug.cgi?id=1848201#c5)||never |1 |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Don't we recommend folks use the Glean Dictionary for this stuff? (Non-blocking, just thinking out loud about filing a bug)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we do. for others. We still have it and whether we should or not should go into a separate bug.

| glean.error.invalid_label |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a metric was set with an invalid label. The labels are the `category.name` identifier of the metric. |[Bug 1499761](https://bugzilla.mozilla.org/show_bug.cgi?id=1499761#c5)||never |1 |
| glean.error.invalid_overflow |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a metric was set a value that overflowed. The labels are the `category.name` identifier of the metric. |[Bug 1591912](https://bugzilla.mozilla.org/show_bug.cgi?id=1591912#c3)||never |1 |
| glean.error.invalid_state |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of times a timing metric was used incorrectly. The labels are the `category.name` identifier of the metric. |[Bug 1499761](https://bugzilla.mozilla.org/show_bug.cgi?id=1499761#c5)||never |1 |
Expand Down Expand Up @@ -164,13 +165,19 @@ In addition to those built-in metrics, the following metrics are added to the pi
| glean.error.preinit_tasks_overflow |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |The number of tasks that overflowed the pre-initialization buffer. Only sent if the buffer ever overflows. In Version 0 this reported the total number of tasks enqueued. |[Bug 1609482](https://bugzilla.mozilla.org/show_bug.cgi?id=1609482#c3)||never |1 |
| glean.upload.deleted_pings_after_quota_hit |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |The number of pings deleted after the quota for the size of the pending pings directory or number of files is hit. Since quota is only calculated for the pending pings directory, and deletion request ping live in a different directory, deletion request pings are never deleted. |[Bug 1601550](https://bugzilla.mozilla.org/show_bug.cgi?id=1601550#c3)||never |1 |
| glean.upload.discarded_exceeding_pings_size |[memory_distribution](https://mozilla.github.io/glean/book/user/metrics/memory_distribution.html) |The size of pings that exceeded the maximum ping size allowed for upload. |[Bug 1597761](https://bugzilla.mozilla.org/show_bug.cgi?id=1597761#c10)||never |1 |
| glean.upload.in_flight_pings_dropped |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |How many pings were dropped because we found them already in-flight. |[Bug 1816401](https://bugzilla.mozilla.org/show_bug.cgi?id=1816401)||never |1 |
| glean.upload.missing_send_ids |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |How many ping upload responses did we not record as a success or failure (in `glean.upload.send_success` or `glean.upload.send_failue`, respectively) due to an inconsistency in our internal bookkeeping? |[Bug 1816400](https://bugzilla.mozilla.org/show_bug.cgi?id=1816400)||never |1 |
| glean.upload.pending_pings |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |The total number of pending pings at startup. This does not include deletion-request pings. |[Bug 1665041](https://bugzilla.mozilla.org/show_bug.cgi?id=1665041#c23)||never |1 |
| glean.upload.pending_pings_directory_size |[memory_distribution](https://mozilla.github.io/glean/book/user/metrics/memory_distribution.html) |The size of the pending pings directory upon initialization of Glean. This does not include the size of the deletion request pings directory. |[Bug 1601550](https://bugzilla.mozilla.org/show_bug.cgi?id=1601550#c3)||never |1 |
| glean.upload.ping_upload_failure |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |Counts the number of ping upload failures, by type of failure. This includes failures for all ping types, though the counts appear in the next successfully sent `metrics` ping. |[Bug 1589124](https://bugzilla.mozilla.org/show_bug.cgi?id=1589124#c1)|<ul><li>status_code_4xx</li><li>status_code_5xx</li><li>status_code_unknown</li><li>unrecoverable</li><li>recoverable</li></ul>|never |1 |
| glean.upload.send_failure |[timing_distribution](https://mozilla.github.io/glean/book/user/metrics/timing_distribution.html) |Time needed for a failed send of a ping to the servers and getting a reply back. |[Bug 1814592](https://bugzilla.mozilla.org/show_bug.cgi?id=1814592#c3)||never |1 |
| glean.upload.send_success |[timing_distribution](https://mozilla.github.io/glean/book/user/metrics/timing_distribution.html) |Time needed for a successful send of a ping to the servers and getting a reply back |[Bug 1814592](https://bugzilla.mozilla.org/show_bug.cgi?id=1814592#c3)||never |1 |
| glean.validation.foreground_count |[counter](https://mozilla.github.io/glean/book/user/metrics/counter.html) |On mobile, the number of times the application went to foreground. |[Bug 1683707](https://bugzilla.mozilla.org/show_bug.cgi?id=1683707#c2)||never |1 |
| glean.validation.pings_submitted |[labeled_counter](https://mozilla.github.io/glean/book/user/metrics/labeled_counters.html) |A count of the pings submitted, by ping type. This metric appears in both the metrics and baseline pings. - On the metrics ping, the counts include the number of pings sent since the last metrics ping (including the last metrics ping) - On the baseline ping, the counts include the number of pings send since the last baseline ping (including the last baseline ping) |[Bug 1586764](https://bugzilla.mozilla.org/show_bug.cgi?id=1586764#c3)||never |1 |
| glean.validation.shutdown_dispatcher_wait |[timing_distribution](https://mozilla.github.io/glean/book/user/metrics/timing_distribution.html) |Time waited for the dispatcher to unblock during shutdown. Most samples are expected to be below the 10s timeout used. |[Bug 1828066](https://bugzilla.mozilla.org/show_bug.cgi?id=1828066#c7)||never |1 |
| glean.validation.shutdown_wait |[timing_distribution](https://mozilla.github.io/glean/book/user/metrics/timing_distribution.html) |Time waited for the uploader at shutdown. |[Bug 1814592](https://bugzilla.mozilla.org/show_bug.cgi?id=1814592#c3)||never |1 |

Data categories are [defined here](https://wiki.mozilla.org/Firefox/Data_Collection).

<!-- AUTOGENERATED BY glean_parser v7.0.0. DO NOT EDIT. -->
<!-- AUTOGENERATED BY glean_parser v10.0.0. DO NOT EDIT. -->

10 changes: 6 additions & 4 deletions docs/user/user/pings/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,18 @@ _Type: [Datetime](../../reference/metrics/datetime.md),
Lifetime: [User](../../reference/yaml/metrics.md#user)_

The time of the start of collection of the data in the ping, in local time
and with minute precision, including timezone information.
and with millisecond precision (by default), including timezone information.
(_Note_: Custom pings can opt-out of precise timestamps and use minute precision.)

#### `end_time`

_Type: [Datetime](../../reference/metrics/datetime.md),
Lifetime: [Ping](../../reference/yaml/metrics.md#ping-default)_

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.
The time of the end of collection of the data in the ping, in local time
and with millisecond precision (by default), including timezone information.
This is also the time this ping was generated and is likely well before ping transmission time.
(_Note_: Custom pings can opt-out of precise timestamps and use minute precision.)

#### `reason` _(optional)_

Expand Down
2 changes: 1 addition & 1 deletion glean-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ include = [
rust-version = "1.62"

[package.metadata.glean]
glean-parser = "8.1.0"
glean-parser = "10.0.0"

[badges]
circle-ci = { repository = "mozilla/glean", branch = "main" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class PingType<ReasonCodesEnum> (
name: String,
includeClientId: Boolean,
sendIfEmpty: Boolean,
preciseTimestamps: Boolean,
val reasonCodes: List<String>,
) where ReasonCodesEnum : Enum<ReasonCodesEnum>, ReasonCodesEnum : ReasonCode {
private var testCallback: ((ReasonCodesEnum?) -> Unit)? = null
Expand All @@ -58,6 +59,7 @@ class PingType<ReasonCodesEnum> (
name = name,
includeClientId = includeClientId,
sendIfEmpty = sendIfEmpty,
preciseTimestamps = preciseTimestamps,
reasonCodes = reasonCodes,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ class GleanTest {
name = pingName,
includeClientId = true,
sendIfEmpty = false,
preciseTimestamps = true,
reasonCodes = listOf(),
)
val stringMetric = StringMetricType(
Expand Down Expand Up @@ -843,6 +844,7 @@ class GleanTest {
name = pingName,
includeClientId = true,
sendIfEmpty = false,
preciseTimestamps = true,
reasonCodes = listOf(),
)
val stringMetric = StringMetricType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ class GleanDebugActivityTest {
name = "custom",
includeClientId = false,
sendIfEmpty = true,
preciseTimestamps = true,
reasonCodes = listOf(),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class CustomPingTest {
name = "custom-ping",
includeClientId = true,
sendIfEmpty = true,
preciseTimestamps = true,
reasonCodes = emptyList(),
)

Expand Down Expand Up @@ -104,6 +105,7 @@ class CustomPingTest {
name = "custom-ping",
includeClientId = true,
sendIfEmpty = true,
preciseTimestamps = true,
reasonCodes = emptyList(),
)

Expand Down Expand Up @@ -176,6 +178,7 @@ class CustomPingTest {
name = pingName,
includeClientId = true,
sendIfEmpty = true,
preciseTimestamps = true,
reasonCodes = emptyList(),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,7 @@ class EventMetricTypeTest {
name = pingName,
includeClientId = true,
sendIfEmpty = false,
preciseTimestamps = true,
reasonCodes = listOf(),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class PingTypeTest {
name = "custom",
includeClientId = true,
sendIfEmpty = false,
preciseTimestamps = true,
reasonCodes = listOf(),
)

Expand Down Expand Up @@ -118,6 +119,7 @@ class PingTypeTest {
name = "custom_ping",
includeClientId = true,
sendIfEmpty = false,
preciseTimestamps = true,
reasonCodes = listOf(),
)

Expand Down Expand Up @@ -164,6 +166,7 @@ class PingTypeTest {
name = "custom-ping",
includeClientId = true,
sendIfEmpty = false,
preciseTimestamps = true,
reasonCodes = listOf(),
)

Expand Down Expand Up @@ -210,6 +213,7 @@ class PingTypeTest {
name = "custom",
includeClientId = false,
sendIfEmpty = false,
preciseTimestamps = true,
reasonCodes = listOf(),
)

Expand Down Expand Up @@ -258,6 +262,7 @@ class PingTypeTest {
includeClientId = false,
sendIfEmpty = false,
reasonCodes = listOf(),
preciseTimestamps = true,
)

val counter = CounterMetricType(
Expand Down
2 changes: 1 addition & 1 deletion glean-core/build/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "glean-build"
version = "8.1.0"
version = "10.0.0"
edition = "2021"
description = "Glean SDK Rust build helper"
repository = "https://github.com/mozilla/glean"
Expand Down
2 changes: 1 addition & 1 deletion glean-core/build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use std::env;

use xshell_venv::{Result, Shell, VirtualEnv};

const GLEAN_PARSER_VERSION: &str = "8.1.0";
const GLEAN_PARSER_VERSION: &str = "10.0.0";

/// A Glean Rust bindings generator.
pub struct Builder {
Expand Down
9 changes: 8 additions & 1 deletion glean-core/ios/Glean/Metrics/Ping.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,20 @@ public class Ping<ReasonCodesEnum: ReasonCodes> {
var testCallback: ((ReasonCodesEnum?) throws -> Void)?

/// The public constructor used by automatically generated metrics.
public init(name: String, includeClientId: Bool, sendIfEmpty: Bool, reasonCodes: [String]) {
public init(
name: String,
includeClientId: Bool,
sendIfEmpty: Bool,
preciseTimestamps: Bool,
reasonCodes: [String]
) {
self.name = name
self.reasonCodes = reasonCodes
self.innerPing = PingType(
name,
includeClientId,
sendIfEmpty,
preciseTimestamps,
reasonCodes
)
}
Expand Down
2 changes: 2 additions & 0 deletions glean-core/ios/GleanTests/GleanTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ class GleanTests: XCTestCase {
name: "custom",
includeClientId: true,
sendIfEmpty: false,
preciseTimestamps: true,
reasonCodes: []
)

Expand Down Expand Up @@ -412,6 +413,7 @@ class GleanTests: XCTestCase {
name: "custom",
includeClientId: true,
sendIfEmpty: false,
preciseTimestamps: true,
reasonCodes: []
)

Expand Down
4 changes: 4 additions & 0 deletions glean-core/ios/GleanTests/Metrics/PingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class PingTests: XCTestCase {
name: "custom",
includeClientId: true,
sendIfEmpty: false,
preciseTimestamps: true,
reasonCodes: []
)

Expand Down Expand Up @@ -84,6 +85,7 @@ class PingTests: XCTestCase {
name: "custom",
includeClientId: false,
sendIfEmpty: false,
preciseTimestamps: true,
reasonCodes: []
)

Expand Down Expand Up @@ -119,6 +121,7 @@ class PingTests: XCTestCase {
name: "custom",
includeClientId: false,
sendIfEmpty: true,
preciseTimestamps: true,
reasonCodes: []
)

Expand Down Expand Up @@ -209,6 +212,7 @@ class PingTests: XCTestCase {
name: "custom2",
includeClientId: true,
sendIfEmpty: true,
preciseTimestamps: true,
reasonCodes: ["was_tested"]
)

Expand Down
2 changes: 1 addition & 1 deletion glean-core/ios/sdk_generator.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

set -e

GLEAN_PARSER_VERSION=8.1
GLEAN_PARSER_VERSION=10.0

# CMDNAME is used in the usage text below.
# shellcheck disable=SC2034
Expand Down
10 changes: 5 additions & 5 deletions glean-core/metrics.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ glean.internal.metrics:

start_time:
type: datetime
time_unit: minute
time_unit: millisecond
lifetime: user
send_in_pings:
- glean_internal_info
Expand All @@ -346,6 +346,7 @@ glean.internal.metrics:
in local time and with minute precision, including timezone information.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1556966
- https://bugzilla.mozilla.org/show_bug.cgi?id=1827852
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3
data_sensitivity:
Expand All @@ -356,7 +357,7 @@ glean.internal.metrics:

end_time:
type: datetime
time_unit: minute
time_unit: millisecond
lifetime: ping
send_in_pings:
- glean_internal_info
Expand All @@ -367,6 +368,7 @@ glean.internal.metrics:
and is likely well before ping transmission time.
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1556966
- https://bugzilla.mozilla.org/show_bug.cgi?id=1827852
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1512938#c3
data_sensitivity:
Expand Down Expand Up @@ -402,7 +404,7 @@ glean.client.annotation:
- all-pings
description: |
An experimentation identifier derived and provided by the application
for the purpose of experimenation enrollment.
for the purpose of experimentation enrollment.
badboy marked this conversation as resolved.
Show resolved Hide resolved
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1848201
data_reviews:
Expand Down Expand Up @@ -498,8 +500,6 @@ glean.error:
Only sent if the buffer ever overflows.

In Version 0 this reported the total number of tasks enqueued.
unit:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

o_O. Where did this come from? Ahh, cool! Even though unit has not meaning here, it still shows up in the Glean dictionary. Maybe this would be okay to leave as I kinda like the explicit documentation this metadata provides (though the description should do that, too) 🤔

No strong opinions on this, happy either way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was equally surprised!
We just disallowed this in glean_parser. So we would need to retract that decision, which would require a bug, a revert and a new release.

tasks
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1609482
data_reviews:
Expand Down
2 changes: 1 addition & 1 deletion glean-core/python/glean/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
__email__ = "glean-team@mozilla.com"


GLEAN_PARSER_VERSION = "8.1.0"
GLEAN_PARSER_VERSION = "10.0.0"
parser_version = VersionInfo.parse(GLEAN_PARSER_VERSION)
parser_version_next_major = parser_version.bump_major()

Expand Down
1 change: 1 addition & 0 deletions glean-core/python/glean/_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"range_min",
"reason_codes",
"send_in_pings",
"precise_timestamps",
"time_unit",
]

Expand Down
3 changes: 2 additions & 1 deletion glean-core/python/glean/metrics/ping.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ def __init__(
name: str,
include_client_id: bool,
send_if_empty: bool,
precise_timestamps: bool,
reason_codes: List[str],
):
"""
Expand All @@ -25,7 +26,7 @@ def __init__(
"""
self._reason_codes = reason_codes
self._inner = GleanPingType(
name, include_client_id, send_if_empty, reason_codes
name, include_client_id, send_if_empty, precise_timestamps, reason_codes
)
self._test_callback = None # type: Optional[Callable[[Optional[str]], None]]

Expand Down
Loading
Loading