Bug 1990006 - Create an initial ETL workflow for gecko-trace component - #9753
Bug 1990006 - Create an initial ETL workflow for gecko-trace component#9753jjjalkanen wants to merge 13 commits into
Conversation
jjjalkanen
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR adds a gecko_trace SQL generator that produces three per-application derived tables (gecko_trace_events_v1, gecko_trace_traces_v1, and a gecko_trace_trace_events_v1 bridge table) plus a cross-application traces view, along with a new bqetl_gecko_trace DAG. The tables are populated by multi-statement scripts that use recursive CTEs to build Merkle-chain signatures over span events, and MERGE those signatures into accumulating aggregate tables.
The signature-chaining logic in _shared/trace_chain_ctes.sql reads well and the shared-include approach avoids duplicating it across the two scripts. My main findings are in the generated SQL and the generator plumbing: the ping CTE that every script reads from is missing, so the raw ping table is never referenced; the self-referencing CTEs need WITH RECURSIVE; the scripts are written out as query.sql rather than script.sql, which is the filename the repo's dry-run/deploy/Airflow tooling keys off; the metadata.yaml templates are missing the date_partition_parameter: null + parameters pair that every other script query in the repo sets; and the backfill.yaml entry keys aren't dates so they won't parse. There's also a design question about the accumulating MERGE versus the repeatability requirement in docs/reference/incremental.md.
Beyond that, several docs and two leftover sql/mozfun/gecko_trace/ directories still describe the earlier span-tree/JS-UDF design rather than what this PR generates. Details inline.
Per the reviewer checklist: the PR title references bug 1990006, and these are new derived tables with no obvious existing equivalent, so the "extend an existing dataset" item looks satisfied. The backfill.yaml item is covered in one of the comments below.
This comment has been minimized.
This comment has been minimized.
0ee6868 to
cee0158
Compare
This comment has been minimized.
This comment has been minimized.
cee0158 to
0bc603f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
7a6196d to
874cbdb
Compare
This comment has been minimized.
This comment has been minimized.
0ae48c8 to
a426ff7
Compare
This comment has been minimized.
This comment has been minimized.
a426ff7 to
734dc22
Compare
This comment has been minimized.
This comment has been minimized.
734dc22 to
58bbb71
Compare
This comment has been minimized.
This comment has been minimized.
58bbb71 to
6743d76
Compare
This comment has been minimized.
This comment has been minimized.
6743d76 to
a2b2f53
Compare
This comment has been minimized.
This comment has been minimized.
ec6644b to
acc5db3
Compare
This comment has been minimized.
This comment has been minimized.
acc5db3 to
2b95cfd
Compare
This comment has been minimized.
This comment has been minimized.
| FROM | ||
| `moz-fx-data-shared-prod.org_mozilla_firefox_beta_derived.gecko_trace_events_v1` | ||
| ) | ||
| GROUP BY |
There was a problem hiding this comment.
Just for future, GROUP BY ALL is a shorthand for this so you don't have to list all the fields here.
| reason: New aggregated events table, initial backfill. | ||
| watchers: | ||
| - jjalkanen@mozilla.com | ||
| status: Complete |
There was a problem hiding this comment.
This has not been merged yet so it cannot be Complete yet. Here's a docs to how to set up a backfill: https://mozilla.github.io/bigquery-etl/cookbooks/backfilling_a_table/#initiating-the-backfill
Also, please submit backfills config as a follow-up PR.
There was a problem hiding this comment.
I'll remove them for now and put them into a follow-up PR
| dag_name: bqetl_gecko_trace | ||
| task_group: org_mozilla_fenix_nightly | ||
| date_partition_parameter: null | ||
| parameters: ["submission_date:DATE:"] |
There was a problem hiding this comment.
| parameters: ["submission_date:DATE:"] | |
| parameters: ["submission_date:DATE:{{ds}}"] |
There was a problem hiding this comment.
Missing the actual value to be injected. {{ds}} is an Airflow macro which corresponds to execution_date (the logical date of the Airflow DAG run):
https://airflow.apache.org/docs/apache-airflow/1.10.3/macros.html
There was a problem hiding this comment.
There was a Jinja2 template issue, fixed now
| bigquery: | ||
| clustering: | ||
| fields: | ||
| - event_signature |
There was a problem hiding this comment.
Would you ever filter by this field or join using it when running queries against this table?
There was a problem hiding this comment.
This was a good question and got me to revisit how we actually use this in the Bugzilla submission branch. So event_signature is the MERGE key and the daily ETL uses it to join the event signatures to stable ids. Consumers should use stable_event_id through the gecko_trace_aggregates views. Daily counts are now in gecko_trace_events_daily_v1 so time-window queries don't rely on clustering.
| dag_name: bqetl_gecko_trace | ||
| task_group: org_mozilla_fenix_nightly | ||
| date_partition_parameter: null | ||
| parameters: ["submission_date:DATE:"] |
There was a problem hiding this comment.
| parameters: ["submission_date:DATE:"] | |
| parameters: ["submission_date:DATE:{{ds}}"] |
There was a problem hiding this comment.
Will it be confusing with both tables gecko_trace_trace_events_v1 and gecko_trace_events_v1 existing?
There was a problem hiding this comment.
It's definitely jargon! But trace events are the events of a trace (should use stable id`s) while events have a signature and stable id. I think it's clear enough
| - name: submission_date | ||
| type: DATE | ||
| mode: NULLABLE | ||
| description: Date this row was first inserted. |
There was a problem hiding this comment.
| description: Date this row was first inserted. | |
| description: Date when the event was received on the server side. |
There was a problem hiding this comment.
The phrasing received on the server side is now consistently used through the schema documentation.
There was a problem hiding this comment.
Please do as a follow-up PR and create it with states Initiate instead.
Initiate will actually trigger the backfill to be processed into a temporary location, Complete will trigger the move of the partitions from a temporary location over to the prod location.
There was a problem hiding this comment.
Yeah, this is the way to go. Removed now and a new PR is in progress
| dag_name: bqetl_gecko_trace | ||
| task_group: org_mozilla_fenix_nightly | ||
| date_partition_parameter: null | ||
| parameters: ["submission_date:DATE:"] |
There was a problem hiding this comment.
| parameters: ["submission_date:DATE:"] | |
| parameters: ["submission_date:DATE:{{ds}}"] |
There was a problem hiding this comment.
See my other backfills comments.
There was a problem hiding this comment.
Backfills removed, and will get their own PR in the near future
| dag_name: bqetl_gecko_trace | ||
| task_group: org_mozilla_firefox_beta | ||
| date_partition_parameter: null | ||
| parameters: ["submission_date:DATE:"] |
There was a problem hiding this comment.
| parameters: ["submission_date:DATE:"] | |
| parameters: ["submission_date:DATE:{{ds}}"] |
| dag_name: bqetl_gecko_trace | ||
| task_group: org_mozilla_firefox_beta | ||
| date_partition_parameter: null | ||
| parameters: ["submission_date:DATE:"] |
There was a problem hiding this comment.
| parameters: ["submission_date:DATE:"] | |
| parameters: ["submission_date:DATE:{{ds}}"] |
There was a problem hiding this comment.
Is everything outside of the sql_generators/ the output of the generator? If so, please remove them from the PR and only merge the changes in the generator. CI will ensure that the output is available in the prod workflows.
There was a problem hiding this comment.
The generator outputs have been removed
This patch adds an initial ETL workflow for processing traces collected by the [gecko-trace component](1) from varius Gecko based Firefox products. [1]: https://searchfox.org/firefox-main/source/toolkit/components/gecko-trace
The issues need to be sent to DOM LWS team for the time being.
Clarifies Firefox Preview to Firefox Android and changes an early start date to a recent start date plus backfill.
sqlglot doesn't understand right shift, and it appears that the UDFs can be inlined into queries when we use a standardized way to generate signatures.
…n MERGE keys. Changes also the signature column type across both accumulator tables and all three scripts.
Stop committing generated output under sql/; CI generates it. Drop the
backfill.yaml templates, to be submitted separately with Initiate status.
Escape the Airflow {{ds}} macro in metadata templates so it survives
Jinja rendering.
Restructure the tables so stable ids are actually stable. The previous
MERGE keyed on (signature, submission_date) generated a new UUID per day
and the bridge joins fanned out across days. Now:
- gecko_trace_events_v1 and gecko_trace_traces_v1 are signature -> stable
id dimensions, MERGEd on the signature and clustered on it.
- gecko_trace_traces_v1 carries trace_key, a hash of the ordered
stable_event_ids, so a new trace signature with a known trace_key
reuses the existing stable_trace_id.
- gecko_trace_trace_events_v1 stores (trace_signature, event_position,
event_signature) once per trace realization.
- gecko_trace_events_daily_v1 and gecko_trace_traces_daily_v1 hold the
daily counts, partitioned on submission_date and clustered on the
signature, so re-runs replace a partition.
- gecko_trace_aggregates views join facts and bridge to the dimensions
and expose stable ids; add events_daily and traces_daily views.
Split the shared CTE include so the event hash is defined once, and add
SQL tests for the two daily fact queries.
2b95cfd to
f046ee0
Compare
This comment has been minimized.
This comment has been minimized.
The gecko_trace ping stores the OTLP TracesData object in metrics.object.gecko_trace_traces_data; there is no top-level resource_spans column. The outer JSON_QUERY_ARRAY was lost in an earlier refactor and the MERGE scripts hid it because dry run skips DDL/DML. The new query.sql facts exposed it in CI. Test fixtures now nest the payload under the same JSON column.
This comment has been minimized.
This comment has been minimized.
BigQuery JSONPath does not accept bracket notation for keys, so $.attributes["source.file"] fails with "Invalid token in JSONPath". Use the double-quoted form $.attributes."source.file" instead. The metric structure in toolkit/components/gecko-trace/generated-metrics.yaml confirms attributes is an object keyed by source.file, source.line and result.
Integration report
|
4 participants
Description
This PR adds an initial ETL workflow for processing traces collected by the gecko-trace component from various Gecko based Firefox products. It's adopted from the original PR .
Related Tickets & Documents
https://bugzilla.mozilla.org/show_bug.cgi?id=1990006
https://docs.google.com/document/d/1HIcggXk8EZ7_4x57M20Rc9RmXenmh6cDtQ2KCJClvKo/edit?usp=sharing