1839640 - Implement the 'object' metric type

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, task, P2)

Description

[Jan-Erik Rediger [:janerik]]

Proposal: https://docs.google.com/document/d/1ov2txT70sO4u3m7rj-HQdcJLY7VC-Otx2pFiCA-a9fc/edit#heading=h.l4ya7hop8p4q
(public link)

A first rough PoC is available in https://github.com/mozilla/glean/pull/2489
with parser changes here: https://github.com/mozilla/glean_parser/pull/587

Far from done.
This will also need some work on the pipeline to properly ingest the data into a JSON column.

Comment 1

[Jan-Erik Rediger [:janerik]]

Attachment: glean_parser PR 587: Bug 1839640 - Add new metric type: object

Comment 2

[Jan-Erik Rediger [:janerik]]

Attachment: Glean PR 2489: Bug 1839640 - Add new metric type: object

Comment 3

[Jan-Erik Rediger [:janerik]]

Using this bug as a big of a worklog, as this change requires multiple PRs to different projects and some coordination to land in full:

1. The above PRs add the necessary changes to glean_parser and glean-core, implementing the type only for the RLB for now. They are in decent shape (minus some naming, tests and docs).
2. I have a hacky patch compiling adding the object metric to m-c with a JS API; the C++ API is half-broken and needs some work.
3. Patches to mozilla-schema-generator and mozilla-pipeline-schemas are easy.
4. We probably need to touch jsonschema-transpiler to get us the right schema using a JSON column
5. and then we need to modify ingestion to transform the data correctly

1-3 are nearing completion.
4 I will take a look at, I don't expect much work.
For 5 relud pointed me here: https://github.com/mozilla/gcp-ingestion/blob/5e33bdbf86da7fbee5699de32b6695aec0a093e8/ingestion-core/src/main/java/com/mozilla/telemetry/ingestion/core/transform/PubsubMessageToObjectNode.java#L328
That might be the trickiest part because I never touched that code but maybe I can rely on some help from relud.

Comment 4

[Jan-Erik Rediger [:janerik]]

Attachment: WIP: Bug 1839640 - Implement the new object metric type (JS only)

For now this only provides a JavaScript API, as it's easier to generate
and to use (we just use JavaScript objects, validation is done by
(de)serialization on the Rust side).

No JOG support. Parent-process only right now.

Todo:

• The test API should return an object, not a JSON string

Comment 5

[Jan-Erik Rediger [:janerik]]

As called out I will need help with the related changes across the pipeline.

1. mozilla-pipeline-schemas

Added new metric type: https://github.com/badboy/mozilla-pipeline-schemas/commit/35b0870fbf304662d179d26dc934b1217663c73b

No further schema limits applied. However this breaks mozilla-schema-generator later.

Tried to fix this with: https://github.com/badboy/mozilla-pipeline-schemas/commit/10984502191a513957f552921dbc1d8adf0992b9
Note: this is wrong. type: any is not valid jsonschema.

Help needed. Changes depend on next part.

2. mozilla-schema-generator

First: URLs from which to fetch schemas/repos is hard-coded, so I had to manually change that to test it against local checkouts.

MSG requires items for type: array

• items then cannot be empty, as it requires types in size check: https://github.com/badboy/mozilla-schema-generator/blob/b696aec9a4f5423a141069612365af7c492e1e42/mozilla_schema_generator/schema.py#L111
• any is not a valid jsonschema type, so this probably will break down the line?

I'm not sure how to handle this here.

Help needed.

3. jsonschema-transpiler

I have a PR up: https://github.com/mozilla/jsonschema-transpiler/pull/107
This breaks test, I can fix these.

But: I manually created what I think is the schema and tested it against that, so this might be wrong. We need to fix MPS and MSG first to generate a schema to test with, then fix the transpiler.

I can handle this when 1. and 2. are handled.

4. gcp-ingestion

I think it's already able to handle putting JSON into a JSON column.
Ihave no idea how to test it. And I don't have the dev environment for it set up either.
I went through the code:

1. Ingestion starts basically here: https://github.com/mozilla/gcp-ingestion/blob/5e33bdbf86da7fbee5699de32b6695aec0a093e8/ingestion-core/src/main/java/com/mozilla/telemetry/ingestion/core/transform/PubsubMessageToObjectNode.java#L328
2. Goes to processField
3. Ends up in coerceSingleValueToBqType
4. It checks against the table schema. Should drop to last else branch, which gets us Optional.of(o).
   • I think this is then fine for a colum of JSON type.

Help needed. Does not depend on the other parts. I can provide a sample payload and table schema.

---

Pinging arkadiusz and relud for help.
We can split the work in individual bugs for easier tracking if you want.

Comment 6

[Daniel Thorn [:relud]]

type: any is not valid jsonschema.

MSG requires items for type: array

I think a hint needs to be added to indicate the JSON type to mozilla-schema-generator and jsonschema-transpiler. format seems like a good choice, but it is technically only a string property, so I'm not sure if it would break anything.

Comment 7

[Jan-Erik Rediger [:janerik]]

(In reply to Daniel Thorn [:relud] from comment #6)

type: any is not valid jsonschema.

MSG requires items for type: array

I think a hint needs to be added to indicate the JSON type to mozilla-schema-generator and jsonschema-transpiler. format seems like a good choice, but it is technically only a string property, so I'm not sure if it would break anything.

That works and is relatively easy to implement. So far it doesn't seem to break anything.

Comment 8

[Jan-Erik Rediger [:janerik]]

I spoke too soon. It works with all our tools, but as far as I can tell is not proper jsonschema if I use format with also type: [array, object].

Comment 9

[Arkadiusz Komarzewski [:akomar]]

Clearing NI as we discussed this in person (store object paths in json schemas and use a side input to jsonschema-transpiler to tell it where to stop parsing).

Comment 10

[Jan-Erik Rediger [:janerik]]

I have something working now.

First off branches and commits for all projects involved here.

• Fenix: https://github.com/mozilla-mobile/firefox-android/compare/main...badboy:firefox-android:object-metric-type-test
  • branch: object-metric-type-test
  • commit: 6fda51ef62d935f2d0ee5304fab955000cbbed52
• probe-scraper: https://github.com/mozilla/probe-scraper/compare/main...badboy:probe-scraper:object-metric-type-test
  • branch: object-metric-type-test
  • commit: a4bbb42be6c0d1ba0a21e4501ce0d0b1320835c2
• jsonschema-transpiler: https://github.com/mozilla/jsonschema-transpiler/pull/107
  • branch: object-metric-type-json-column
  • commit: 8450041a1df544877ba5bc546fab9c23ed3dd0d2
• mozilla-pipeline-schemas: https://github.com/mozilla-services/mozilla-pipeline-schemas/compare/main...badboy:mozilla-pipeline-schemas:object-metric-type-2
  • branch: object-metric-type-2
  • commit: b1973563667e2f091277f99c9798e26937239f66
• mozilla-schema-generator: https://github.com/mozilla/mozilla-schema-generator/compare/main...badboy:mozilla-schema-generator:object-type
  • branch: object-type
  • commit: 9f90876b23bd01e5af6b7e06c31101bec089dda1

Clone probe-scraper, checkout the mentioned commit, then run and host probe-scraper:

python3 -m probe_scraper.runner --dry-run --cache-dir tmp/cache --out-dir tmp/out --glean --glean-repo fenix --glean-repo glean-core

cd tmp/out && httplz -p 8000 # or: python3 -m http.server

Clone MSG, checkout the mentioned commit, then run mozilla-schema-generator:

rm -rf .probe_cache # just in case.
mozilla-schema-generator generate-glean-pings --out-dir out --repo org-mozilla-fenix --pretty --mps-branch object-metric-type-2

Install and run the transpiler (this is what bin/generate_commit does)

cargo install --git https://github.com/mozilla/jsonschema-transpiler --branch object-metric-type-json-column

jsonschema-transpiler --resolve drop --type bigquery --normalize-case --force-nullable --tuple-struct out/org-mozilla-fenix/baseline/baseline.1.schema.json > out/org-mozilla-fenix/baseline/baseline.1.bq

Result:

$ rg -C3 crash.stack out/org-mozilla-fenix/metrics/metrics.1.bq
1436-          {
1437-            "description": "All threads' frame information.",
1438-            "mode": "NULLABLE",
1439:            "name": "crash_stack_threads",
1440-            "type": "JSON"
1441-          }
1442-        ],

Success!?!

Comment 11

[Jan-Erik Rediger [:janerik]]

If the above changes will be accepted I think we need to go about it this way:

• new release of jsonschema-transpiler
• update jsonschema-transpiler in use in MSG
  • include new metric type changes
• land MPS changes
• land updates to glean_parser
  • get that into probe-scraper too
• land object metric type in Glean

Comment 12

[BMO Github Automation]

Attachment: [mozilla-services/mozilla-pipeline-schemas] Bug 1839640 - Add the object type with very shallow restrictions (#793)

Comment 13

[Jan-Erik Rediger [:janerik]]

Attachment: [mozilla/mozilla-schema-generator] Bug 1839640 - Add the new object type and additional behavior for it (#257)

Comment 14

[Jan-Erik Rediger [:janerik]]

Arkadiusz, can you take a look at https://bugzilla.mozilla.org/show_bug.cgi?id=1839640#c10 and see if that would be an acceptable way forward?

Comment 15

[Jan-Erik Rediger [:janerik]]

In the linked Jira issue we planned and executed a quick test if the pipeline correctly ingests data into JSON columns.
That test succeeded and it all works as expected, with no changes. That means we can move forward with the implementation parts.

Comment 16

[Jan-Erik Rediger [:janerik]]

Attachment: [mozilla/jsonschema-transpiler] Bug 1839640 - BigQuery: Convert json atoms to a JSON column type (#107)

Comment 17

[Jan-Erik Rediger [:janerik]]

Next steps:

1. Get reviews on glean_parser / Glean. Those things can then land and we can prepare a release
2. Update the m-c patch. It's working locally, mostly waiting for the above releases
3. Land the jsonschema-transpiler patch and get that used in our tooling
4. Land the MSG patch
5. Land the MPS patch
6. Start using object metrics!

1 & 2 can get rolling independently already.
3-5 need to land in that order then. As long as no one uses object metrics they are not risky.
Once landed we should do proper testing that it works all as expected.

Comment 18

[Arkadiusz Komarzewski [:akomar]]

Considering the pipeline ingestion side, the plan outlined in https://bugzilla.mozilla.org/show_bug.cgi?id=1839640#c11 looks good to me. To reiterate, steps are:

1. Update jsonschema-transpiler to support object metric type (https://github.com/mozilla/jsonschema-transpiler/pull/107)
2. Update mozilla-schema-generator to use the updated jsonschema-transpiler. I believe it's this PR: https://github.com/mozilla/mozilla-schema-generator/pull/257/files
3. Update glean schema in mozilla-pipeline-schemas: https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/793/files

Comment 19

[Jan-Erik Rediger [:janerik]]

badboy merged PR [mozilla/jsonschema-transpiler]: Bug 1839640 - BigQuery: Convert json atoms to a JSON column type (#107) in a47640c.

Comment 20

[Jan-Erik Rediger [:janerik]]

badboy merged PR [mozilla/mozilla-schema-generator]: Bug 1839640 - Add the new object type and additional behavior for it (#257) in 4e1796a.

Comment 21

[Jan-Erik Rediger [:janerik]]

travis79 merged PR [mozilla-services/mozilla-pipeline-schemas]: Bug 1839640 - Add the object type with very shallow restrictions (#793) in 52119e6.

Comment 22

[Jan-Erik Rediger [:janerik]]

badboy merged PR [mozilla/glean_parser]: Bug 1839640 - Add new metric type: object (#587) in a802a50.

Comment 23

[Jan-Erik Rediger [:janerik]]

badboy merged PR [mozilla/glean]: Bug 1839640 - Add new metric type: object (#2489) in 78e85ac.

---

This concludes most pieces in the SDK and pipeline.
I'm keeping this bug open as they are some dependent bugs, but those require m-c work first.
I will create followup bugs blocking this.

Comment 24

[Jan-Erik Rediger [:janerik]]

The bulk of the work landed. The remaining "Depends on" bugs can be tackled separately. This should unblock usage now.