1799509 - Missing columns with "." in names showing up

Status: RESOLVED FIXED

(Data Platform and Tools :: General, defect, P1)

Description

[Anna Scholtz [:ascholtz]]

During the health check we noticed missing columns with "." in their field names showing up for multiple Glean apps. For example: metrics.timespan.extensions.startup_cache_load_time.time_unit or metrics.memory_distribution.glean.upload.pending_pings_directory_size.count This indicates a potential problem with either the schema generator or the way these fields are sent from the clients. Usually dots within field names get transformed to underscores.

Comment 1

[Anna Scholtz [:ascholtz]]

I did some investigating and noticed that none of these fields actually appear in any of the schemas. Since these fields appear across all Glean applications, I wonder if this is more of a client SDK issue?
:chutten maybe you have some insight? We are seeing a bunch of metrics being sent from all kinds of Glean applications that are not part of the schemas. Some of these metrics are:

• metrics.timespan.extensions.startup_cache_load_time.time_unit
• metrics.memory_distribution.glean.upload.pending_pings_directory_size.count
• metrics.timing_distribution.wr.renderer_time.count
• metrics.timing_distribution.javascript.gc.minor_time.count
• metrics.timing_distribution.geckoview.content_process_lifetime.count
• metrics.string.geckoview.validation.build_id
• metrics.string.search.engine.defatlt.engine_id
• metrics.timing_distribution.perf.startup.home_fragment_on_view_created.count

Comment 2

[Chris H-C :chutten]

The payload is expected to have nested structures with identifiers that are sometimes permitted to have . characters like

"metrics": {
  "timespan": {
    "extensions.startup_cache_load_time": {
      "value": 1,
      "time_unit": "minute"
    }
  }
}

I'm not sure what would result in a decoding that assumes there'd be a column that contains the . between extensions.startup_cache_load_time. I'm not sure what you mean by "Since these fields appear across all Glean applications, I wonder if this is more of a client SDK issue?" -- the nested time_unit field is present in schemas. (the count field is not (or is at least not documented as such), though it is present in test APIs).

Naively I'd assume there's something that translates extensions.startup_cache_load_time to its column name extensions_startup_cache_load_time and maybe therein lies the fault?

Comment 3

[Anna Scholtz [:ascholtz]]

The majority (ca. 90%) of the missing columns seems to be related to the count field. Is there a reason why this should not be part of the schemas? Can we just add it?

Comment 4

[Chris H-C :chutten]

That one I don't know anything about. Might be related to bug 1790501 if it only started happening with Glean v51.5.0+

Comment 5

[Jan-Erik Rediger [:janerik]]

We indeed accidentally added count to the payload of distributions.
I'll work on removing that again.
Nonetheless we should allow it in the schema, as we will receive it for all those clients indefinitely.

Comment 6

[BMO Automation]

Attachment: Bug 1799509 - Do not serialize a histogram's count into the payload

Comment 7

[Jan-Erik Rediger [:janerik]]

Anna, can you help with the schema part?
I don't fully understand where these things show up.
And thus I don't really know where we would need to fix this.

Comment 8

[Bruno Rosa [:brosa]]

Talked with Jan-Erik about this. I am going to check today if Glean.js is doing the same thing, if so I will work on getting that fixed.

Comment 9

[Jan-Erik Rediger [:janerik]]

badboy merged PR #2267: "Bug 1799509 - Do not serialize a histogram's count into the payload" in 8f1da81.

Comment 10

[Bruno Rosa [:brosa]]

I believe Glean.js is ok. I don't see count showing up in distribution metrics for pings. I will confirm with Jan-Erik tomorrow.

Here is an example of a ping I sent using Glean.js - https://debug-ping-preview.firebaseapp.com/rawPing/8e0927a3-0d2b-40c9-b211-b49a7f0f6d4f

Comment 11

[BMO Automation]

Attachment: [Bug 1799509] Add count to timing_distribution

Comment 12

[Anna Scholtz [:ascholtz]]

Attachment: GitHub Pull Request

Comment 13

[Jan-Erik Rediger [:janerik]]

badboy merged PR #2278: "[Bug 1799509] Add count to timing_distribution" in c665774.

Comment 14

[Jan-Erik Rediger [:janerik]]

scholtzan merged PR #750: "[Bug 1799509] Add count field to Glean timing_distribution" in f6e9491.

Comment 15

[Jan-Erik Rediger [:janerik]]

With the above PRs merged this should be fixed now.

Comment 16

[BMO Github Automation]

Attachment: [mozilla-services/mozilla-pipeline-schemas] Bug 1799509 - Add description to Glean `_distribution` metrics' count field (#782)