Closed Bug 1595723 Opened 5 years ago Closed 5 years ago

We need a new metric type for coarse time measurements

Categories

(Data Platform and Tools :: Glean Metric Types, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dexter, Assigned: mdroettboom)

References

Details

(Whiteboard: [telemetry:glean-rs:backlog][new-metric])

Attachments

(1 file)

107 bytes, text/x-google-doc
emtwo
: review+
frank
: review+
brizental
: review+
chutten
: review+
Details

Proposal for changing an existing or adding a new Glean metric type

Who is the individual/team requesting this change?

Daosheng Mu, Firefox Reality

Is this about changing an existing metric type or creating a new one?

Creating a new metric type.

Can you describe the data that needs to be recorded?

We need to measure coarser times, such as "how long was the tab open?": this could span from a few minutes to days (for "user-time" centered measurements).

Can you provide a raw sample of the data that needs to be recorded (this is in the abstract, and not any particular implementation details about its representation in the payload or the database)

It's a distribution of samples, e.g. a tab was opened for 5 hours, another for 1 hour, an additional one for 4 days. What really matters is not the specific value, but the shape of the distribution.

What is the business question/use-case that requires the data to be recorded?

We need to understand what is the distribution of tab life across our users.

How would the data be consumed?

We would like to see aggregated counts available on the GLAM dashboard or, in the meanwhile, through custom SQL queries.

Why existing metric types are not enough?

The Glean SDK supports the Timing Distribution metric type, which allows to measure times under 10 minutes with great nanoseconds-precision (for "cpu-time" centered measurements) . The lifetime of a browser tab can be much longer than that.

What is the timeline by which the data needs to be collected?

TBD.

Note: this came up during the data migration planning for Firefox Reality.

Priority: -- → P3
Whiteboard: [telemetry:glean-rs:m?]
Depends on: 1570386

I am curious if we can just use TimeSpan that we don't need to care about the limitation of 10 mins from Time Distribution.

If we're only recording a single span of time we should of course use a Timespan. But, taking Alessio's example of how long tabs are open... for that we need multiple timespans and are interested in their distribution. That's normally what a time distribution would do, if it weren't for that 10min limit.

Whiteboard: [telemetry:glean-rs:m?] → [telemetry:glean-rs:backlog][new-metric]
Depends on: 1608137

I reframed comment 0 to conform to the new template, and moved this bug to the right component.

Daosheng, are the information in comment 0 correct? Is this still needed? This can now move as we finalized the process for adding new metric types.

Component: Glean: SDK → Glean Metric Types
Flags: needinfo?(dmu)

(In reply to Alessio Placitelli [:Dexter] from comment #4)

I reframed comment 0 to conform to the new template, and moved this bug to the right component.

Daosheng, are the information in comment 0 correct? Is this still needed? This can now move as we finalized the process for adding new metric types.

It looks great to me. This is what we're looking for.

Flags: needinfo?(dmu)
Attached file Discussion Doc

Here's the document for discussing the addition of this new metric type related to this bug, as highlighted in the process document.

I'm flagging all of you for review on the request. Please examine it and see if there's any blocker from your point of view.

Feedback is due by March 2nd, 2020.

Note This is the first run of the process, so please be patient and do not hesitate to leave feedback about the process too!

Attachment #9128459 - Flags: review?(msamuel)
Attachment #9128459 - Flags: review?(mreid)
Attachment #9128459 - Flags: review?(mgorlick)
Attachment #9128459 - Flags: review?(mdroettboom)
Attachment #9128459 - Flags: review?(chutten)
Attachment #9128459 - Flags: review?(brizental)
Attachment #9128459 - Flags: review?(mreid) → review?(fbertsch)

I added the start of a concrete proposal to the doc, since it was kind of missing (we're learning about the holes is this process as we go). We can probably have the back and forth in the doc, and once more final, will give the rest of use something concrete to consider.

I left a comment on the doc.

One minor nit regarding the document: it would be nice, when possible, to not copy paste the markdown to gdocs as it doesn't format that properly. I find it hard to scan through the document without the proper formatting :) That's easily fixable by copying the parsed content from the bugzilla comment.

Comment on attachment 9128459 [details] Discussion Doc Nothing blocking from a Data Steward front. (( Being nosy I _did_ leave a note wondering if we could do without this with a careful application of cleverness. ))
Attachment #9128459 - Flags: review?(chutten) → review+
Comment on attachment 9128459 [details] Discussion Doc There needs to be a wider discussion as to how this type will be implemented, or even if it should be a new metric type or an addition to the current timing distribution type. This discussion can take place after this proposal, because it shouldn’t change the decision made here. r+
Attachment #9128459 - Flags: review?(brizental) → review+
Comment on attachment 9128459 [details] Discussion Doc From the perspective of Data Tools / Glam, the way this would be aggregated would be similar to other types of distributions so it should not pose any major issues there. As long as there is a clear way to differentiate in the data what the granularity is, this would be fine. On a separate note, it's confusing that the document and the top comment in this bug say different things. For example, the top of this bug says it's a request for a new metric type, and the document says it's about changing a metric type.
Attachment #9128459 - Flags: review?(msamuel) → review+

Thanks Marina. I'll address your comment about inconsistency in the doc.

Comment on attachment 9128459 [details] Discussion Doc r+ for the "change min/max" buckets. That was a good set of iteration on the problem and this solution is both easy and extendable.
Attachment #9128459 - Flags: review?(fbertsch) → review+
Flags: needinfo?(loines)
Assignee: nobody → mdroettboom

Approved the proposal, and created bug 1630997 for the implementation.

Assignee: mdroettboom → nobody
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Assignee: nobody → mdroettboom
Attachment #9128459 - Flags: review?(mgorlick) → review?
Flags: needinfo?(loines)
Attachment #9128459 - Flags: review?(mdroettboom)
Attachment #9128459 - Flags: review?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: