Closed Bug 1595899 Opened 5 years ago Closed 4 years ago

Missing main summary columns in main ping table schema

Categories

(Data Platform and Tools :: General, task, P2)

Product:
Data Platform and Tools
Component:
General
Type:
task
Points:
2

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: relud, Assigned: relud)

References

(Depends on 1 open bug)

Details

Some columns are needed for main_summary to stop using additional_properties, but have not been added due to union types or insufficient schema.

I think we should do the following:

  • Handle union types under environment.addons (STRING for version, INT64 for the others)
  • Treat payload.addonDetails.XPI as a map without value, i.e. an array of structs with a key field but no value field. This should allow us to determine the value type at a later date.
  • Treat payload.processes.dynamic.scalars as a string map, and payload.processes.dynamic.keyedScalars as a map of string maps. Negative side-effect: values that are a string of a boolean or integer would be bucketed into non-string columns when generating main_summary.
  • Add fields to the payload.simpleMeasurements schema as integers for the specific fields.

Fields that need to be added:

Field Schema Type New BigQuery Type Main Summary Type
environment.addons.activeAddons[].userDisabled BOOL or INT64 INT64 BOOL
environment.addons.activeAddons[].version STRING or INT64 STRING STRING
environment.addons.activeAddons[].foreignInstall INT64 or BOOL INT64 BOOL
environment.addons.theme.foreignInstall BOOL or INT64 INT64 BOOL
payload.addonDetails.XPI MAP<STRING,ANY> ARRAY<STRUCT<STRING>> MAP<STRING,ANY> (value not used)
payload.processes.dynamic.scalars MAP<STRING,ANY> MAP<STRING,STRING> STRUCT<MAP<STRING,STRING>, MAP<STRING,INT64>, MAP<STRING,BOOL>>
payload.processes.dynamic.keyedScalars MAP<STRING, MAP<STRING,ANY>> MAP<STRING, MAP<STRING,STRING>> STRUCT< MAP<STRING,MAP<STRING,STRING>>, MAP<STRING,MAP<STRING,INT64>>, MAP<STRING,MAP<STRING,BOOL>>>
payload.simpleMeasurements.activeTicks ANY INT64 INT64
payload.simpleMeasurements.main ANY INT64 INT64
payload.simpleMeasurements.firstPaint ANY INT64 INT64
payload.simpleMeasurements.sessionRestored ANY INT64 INT64
payload.simpleMeasurements.totalTime ANY INT64 INT64
payload.simpleMeasurements.blankWindowShown ANY INT64 INT64
No longer depends on: 1595584
Assignee: nobody → dthorn
Depends on: 1596600

it turns out payload.processes.dynamic.scalars and payload.processes.dynamic.keyedScalars already exist with a handful of probes, so we will have to determine a different solution for those, in a separate bug.

In order to unblock the remaining fields from bug 1596600 adding XPI was moved to a separate pull request: https://github.com/mozilla/mozilla-schema-generator/pull/80

Points: --- → 2
Priority: -- → P1

Looks like the 3 PRs from comment 1 have been merged. Daniel, is there more to do here or can we close this out?

Flags: needinfo?(dthorn)

https://github.com/mozilla/mozilla-schema-generator/pull/80 is currently introducing some unexpected changes that need to be addressed, so it still hasn't been merged.

Flags: needinfo?(dthorn)
Priority: P1 → P2

I've closed the PR to add payload.addonDetails.XPI to per https://github.com/mozilla/mozilla-schema-generator/pull/80#issuecomment-759815434

Upon further inspection this change is unnecessary.

This was implemented in hopes of removing references to additional_properties from main_summary_v4, but that still won't happen because payload.processes.dynamic.scalars and payload.processes.dynamic.keyedScalars are still needed and can't be added the to schema.

We can revisit this decision in the future if payload.processes.dynamic.scalars and payload.processes.dynamic.keyedScalars are fixed, but until then I'm going to resolve this bug as fixed.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.