Fenix: Find what is causing glean.error.invalid_value / glean.error.invalid_label in metric pings

[boek]

https://bugzilla.mozilla.org/show_bug.cgi?id=1552507

[boek]

@Dexterp37

After some investigation:

It looks like there are actually 10 search engines with a suggestion URL > 100 characters the largest of which hits 136.

And then just about every search engine outside of the en-us locale fails the regex validation :(

[Dexterp37]

@boek thanks for looking into this.

• With respect to the URL limit, I think it would be sane to check with Ryan Harter if that has any impact on ongoing analyses. We're considering adding a new metric type, UrlMetricType, which has much higher length and requires to validate to an URL. We have bug 1557299 for this, but it's not a short-term solution. Another solution could be to bump the limit in the Glean SDK once again, but that would be.. not ideal, see bug 1552905.
• With respect to the search_counts, I filed bug 1566764. This might be rather important, as is search data, so we bumped it to top priority on our end. Pending a discussion with Ryan Harter, I think that on our end we should investigate allowing - in label names. Fenix, if I understood the problem correctly, should switch to use the code rather than the name (also for consistency with Desktop, if that's a required thing).
• I recommend writing a unit test for these key metrics, using the Glean SDK testing API.

Note: I had a look at e0d0817 and it looks good overall. It might be a good thing to land independently of the search_counts, IMHO. Feel free to r? me or anyone else on the Glean team if you want.

[Dexterp37]

Flagging @harterrt for any thought about the above comment :)

[harterrt]

What are the 10 engines and URLs with >100 characters? What do we report in these cases - truncated URLs?

@Dexterp37's suggestion for search_counts looks good to me. One note from an offline conversation, the engine we report should not include a dot. The histogram key is of the form engine.source. Adding an additional dot in the engine name will break the ETL. Alessio's suggestion of using code instead of engine should fix part of this but we should confirm there are no exceptions.

[Dexterp37]

@boek any chance you could see if the problem is fixed with #4146 ?

[AndiAJ]

Hi @boek, not really sure if I've tested this the proper way, and perhaps you could help me out with some suggestions.

As per the attached video, I've set the language to Russian, meaning that it will automatically change the search engine to Yandex and did 3 searches using the Cyrillic alphabet (actually used the search suggestions because I had no idea what I was typing)

Baseline Ping 3ee5b6cf-eb77-4a03-9124-40f3ae72b3f5
Events Ping a34e65d3-6f17-4d79-a84f-b84e7c9bbb31
Metrics Ping e9417f92-e2de-4d94-ab3d-d95b74315f45

The only error encountered was the one regarding the glean.baseline.duration

Video
Logcat
Dashboard

Looking forward or your suggestions and confirmation!

[Dexterp37]

The only error encountered was the one regarding the glean.baseline.duration

The above error is not related to this issue, it's a bug in the Glean SDK and it's being investigated in this bug.

[boek]

Awesome! Looks good to me :+1:

[AndiAJ]

Hi verified as fixed on RC 1.2-rc2 using a Google Pixel 3a (Android 9).
No errors encountered.

Please find attached and review:
Logcat
Glean Dashboard

[Dexterp37]

Hi verified as fixed on RC 1.2-rc2 using a Google Pixel 3a (Android 9).
No errors encountered.

Please find attached and review:
Logcat
Glean Dashboard

Perfect! Thanks Andi!