1879888 - Investigate if string metric type is adequate to capture gfx.adapter.primary.description

Status: RESOLVED FIXED

(Data Platform and Tools :: Monitoring & Alerting, defect)

Description

[Perry McManis [:perry.mcmanis]]

We recently migrated this metric from Geckoview Streaming to a pure Glean impl. We are now seeing errors on Fenix because the descriptions are too long. Speculatively, we may not have seen them before because they were truncated inside of Legacy before being passed to Glean.

Investigate what's going on with this field, and see if there are other weird data things we are now seeing that may have been missed in a prior state where things were collected with GVST.

Comment 1

[Perry McManis [:perry.mcmanis]]

It is readily apparent that this description field is being truncated on Fenix. This metric type needs to change or we need to accept the limitation.

see:
https://console.cloud.google.com/bigquery?ws=!1m7!1m6!12m5!1m3!1smozdata!2sus-west1!3s4cce5919-9edb-4aac-b9ae-c537508d4d99!2e1

For desktop it is much, much rarer and at twice the number of (human) characters:
https://console.cloud.google.com/bigquery?ws=!1m7!1m6!12m5!1m3!1smozdata!2sus-west1!3sde94f76b-dffd-431a-9537-3fdd19ce02cb!2e1

Comment 2

[Chris H-C :chutten]

Important context:

• string metrics are presently limited to 100B of UTF-8. Exceeding that results in an invalid_overflow error and the value to be set is truncated and set.
  • Scalars of kind: string would truncate to 50 characters in GVST before relaying it to Glean, so previously we didn't get errors (and didn't get more than 50 characters).
• timing_distribution metrics overflow at different durations depending on the time_unit. For millisecond this value is approximately 19 years. Exceeding this records an invalid_overflow
  • Accumulating a negative durations result in an invalid_value

Other migrated metrics that were previously being silently truncated but are now reporting errors in their data in Firefox Desktop:

• string metrics:
  • gfx.adapter.primary.driver_files about half of all clients are reporting overflows.
• timing_distribution metrics:
  • gfx.checkerboard.potential_duration about half of all clients are reporting overflows.
  • network.dns_start quickly rising, currently a third of all nightly clients are reporting overflows.
  • network.tls_handshake moderately rising, currently 9% of all nightly clients reporting overflows.
  • performance.pageload.fcp_responsestart slowly rising, currently 3% of all nightly clients reporting overflows.
  • performance.time.response_start slowly rising, currently 3% of all nightly clients reporting overflows.

Comment 3

[Perry McManis [:perry.mcmanis]]

Hey Jamie, question for you (or if not you someone you might point us to):

after investigation, we have concluded that gfx.adapter.primary.description was always sending truncated data, especially relevant on Fenix. Now that we've switched over to Glean, this hasn't changed, but we DO get errors now letting us know this is happening. We'd like some input on what you all over in GFX might like to do. Here are the most obvious options:

1. we do nothing, and continue to get a lot of errors (not desirable, probably)
2. we truncate before submitting to glean. same data comes out, but none of the overflow errors
3. we migrate from a string metric to the text metric type, which is far less susceptible to this
4. remove the entire collection.

Please note that 3. means we are going to an entirely NEW data collection and continuity would be broken. It is probably worth taking a look at the actual values in the query to see if you think that this truncation is unacceptable before we commit to 3. if that's even the option we want. Options 1 and 4 are probably both undesirable but both are possible.

Also happy to consider alternatives.

Comment 4

[Jamie Nicol [:jnicol]]

The contents of the adapter description is pretty inconsistent between platforms.

• On Macos we return an empty string
• On Linux we return the GL renderer string. Note this is also reported as gfx.adapter.primary.device_id
• On Windows we return the DeviceString. I'm not entirely sure what that is, but we don't report it anywhere else so presumably that is actually useful.
• On Android it is a combination of several Android SDK Build constants, as well as the OpenGL Renderer, Vendor, and Version strings. No wonder it is over the limit!! Note that the GL strings are already reported in other metrics, as are some of the Build constants eg device_model.

So on Android, Mac and Linux this telemetry doesn't seem particularly useful. On Windows, I presume it is and therefore we want to preserve it. I think we should therefore check whether the data is ever too long on Windows. If so we might want to take option 3 so that the data is reported correctly. If not, option 2 will suffice.

Jeff, you use telemetry a lot. Do we use this metric on windows? Would we be impacted by option 3 if it broke continuity?

Comment 5

[Jamie Nicol [:jnicol]]

According to this query Perry made for me, the longest value on windows is 82 bytes.

So I think we can take option 2 and truncate the data.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Can you elaborate more on the implications of continuity being broken if we choose option 3?

Comment 7

[Perry McManis [:perry.mcmanis]]

Sure,

so in the case of going to option 3, whether you change just the metric type or it's name, you would have to access data a bit differently because data would no longer be flowing into the metrics.string.gfx_adapter_primary_description (note the string part of that re changing type) field in SQL. So, for example, if you had your own queries, looker looks, dashboards etc. that used this, they would have to be updated or you would only get data through versions where that metric was still sent by that exact collection. You all would know if you have that sort of thing, but it's worth noting that those things might be impacted should they exist. On our end, we would be a bit concerned about Glean Dictionary as well, so only updating the type is not the best way to go here.

Typically speaking, we recommend that if you need to change the type, you go ahead and create a new collection. That collection can have a name and description that clarify why it's there. In theory, you could continue to have a smaller version of this field continue to flow into the existing metric, and have an expanded one of type text if you liked.

If you feel that

On Android it is a combination of several Android SDK Build constants, as well as the OpenGL Renderer, Vendor, and Version strings

is still something you need, this would be a good candidate for that new collection. You could then update the recording of the android value into gfx_adapter_primary_description to be something that wouldn't overflow.

Comment 8

[Perry McManis [:perry.mcmanis]]

Hey Jamie, have you all had a chance to to think about what solution you'd like for this one?

I'm happy to put some time against writing a patch with your preferred direction if it would help.

Comment 9

[Perry McManis [:perry.mcmanis]]

Attachment: Bug 1879888 - Instrument cases of duplicated timerIDs for some Geckoview timing distributions r?chutten

Comment 10

[Perry McManis [:perry.mcmanis]]

Please disregard that patch, it belongs to https://bugzilla.mozilla.org/show_bug.cgi?id=1882110

Comment 11

[Perry McManis [:perry.mcmanis]]

Attachment: Bug 1879888 - truncate android gfx primary descr and record it to a text r=jnicol!

Comment 12

[Perry McManis [:perry.mcmanis]]

Attachment: Data Review request

Comment 13

[Travis Long [:travis_]]

Comment on attachment 9388731 [details]
Data Review request

Data Review

1. Is there or will there be documentation that describes the schema for the ultimate data set in a public, complete, and accurate way?

Yes, through the metrics.yaml file and the Glean Dictionary.

2. Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, through the data preferences in the application settings.

3. If the request is for permanent data collection, is there someone who will monitor the data over time?

Permanent collection to be monitored by pmcmanis and fallback to [e-mail]

4. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical information

5. Is the data collection request for default-on or default-off?

Default-on

6. Does the instrumentation include the addition of any new identifiers (whether anonymous or otherwise; e.g., username, random IDs, etc. See the appendix for more details)?

No

7. Is the data collection covered by the existing Firefox privacy notice?

Yes

8. Does the data collection use a third-party collection tool?

No

Result

data-review+

Comment 14

[Alessio Placitelli [:Dexter]]

(In reply to Jamie Nicol [:jnicol] from comment #4)

• On Android it is a combination of several Android SDK Build constants, as well as the OpenGL Renderer, Vendor, and Version strings. No wonder it is over the limit!! Note that the GL strings are already reported in other metrics, as are some of the Build constants eg device_model.

So on Android, Mac and Linux this telemetry doesn't seem particularly useful. On Windows, I presume it is and therefore we want to preserve it. I think we should therefore check whether the data is ever too long on Windows. If so we might want to take option 3 so that the data is reported correctly. If not, option 2 will suffice.

This field seems to be reporting different data depending on the platform it's being collected on: as you mentioned Jamie, on Android this field doesn't make a ton of sense in the current form, given that it's cramming together most of the data we already report. We would not do our users a good service if we just submitted the same data again.

I recommend the following path forward:

• Document the discoveries from comment 4 in the metrics comments
• Truncate the value before recording (or live with this error).

Comment 15

[Perry McManis [:perry.mcmanis]]

Updated query to look at descriptions on Android that were truncated in the last seven days, in case it helps anyone: https://sql.telemetry.mozilla.org/queries/98062/source

Comment 16

[Alessio Placitelli [:Dexter]]

Perry, please kindly update the patch to match the path from comment 14. If the extended descriptor for Android is ever needed, a separate effort can be kicked off. For now, let's suppress the error that's messing with our dashboards.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ff8790ac05f
truncate android gfx primary descr and record it to a text r=jnicol

Comment 18

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/8ff8790ac05f