snowplow / data-models Goto Github PK

pr_check and e2e scripts inadvertently overwrite creds for the great_expectations suite, resulting in 'password authentication failed' errors.

Find cause of the issue and fix.

The BQ v1 release will introduce a different way of testing page_view_in_session_index, which adds testing for new edge cases. Update Redshift version to use the same (and remove existing test).

In Bigquery, columns need to be coalesced with schema evolutions, since even minor schema version bumps create new columns. Create a mechanism to handle this gracefully.

Great expectations' datasource new won't work if the file doesn't exist as templates are referenced elsewhere. Add config_variables.yml file with defaults in order to ensure testing workflow can be established easily.

Additionally, the .test/share directory should be ignored.

Original metadata table design and naming follows a model structure which we have diverted from.

Table should be renamed to datamodel_metadata.

Instead of module_name and step_name we should have model and module. Values of model should all be web (when base module becomes generalised to other models, this can change). Values of module should be module name.

A rare edge case can occur with current 'exclude all' duplicates strategy, when an event is processed, and a subsequent run contains duplicates of that event along with other, legitimate events. For example:

A run contains a page view with event id 123 - this event has page view in session index = 1.

A subsequent run contains a duplicate of that event, along with another, legitimate page view event in the same session. The data from that session in this run will be:

page view event - event ID: 123
page view event - event ID: 123
page view event - event ID: 456

In this second run, the already processed event 123 will be removed by deduplication, and the new one 456 will be assigned page view in session index of 1.

Page View 123 won't be removed from the table, so we will have a session with two page views of page view in session index of 1.

We might solve this by using session_id to update the table, but this feels somewhat fragile.

We can also solve it by implementing better deduplication logic - to keep the first event_id (by collector_tstamp).

The tricky part is that ideally we only keep the first event IF the collector_tstamp is not duplicated also, and remove both otherwise (to avoid cartesian join). However, if we remove both we still have a chance to hit this issue.

One way out of that is to implement a mechanism to apply the incremental logic to all relevant atomic tables (thereby creating deduplicated _staged tables for every join that might be involved in a customisation).

Having a changelog per-model makes sense. However, changes to scripts and other non-model-specific resources shouldn't be tied to a model's changelog. So, we should add a global one to track those.

I think the best structure is as follows:

• Any commits that aren't specific to a particular model are listed with issue reference.
• If those commits are part of a specific web release that release is referenced with a reference to the changelog.
• If those commits aren't tied to a release they are listed under a maintenance 'release'

maintenance (2021-02-04)
---------------------------
Patch broken link in README (#127)

snowflake/web/1.0.0 (2021-02-01)
------------------------------------------
Snowflake web model release ([changelog](web/snowflake/CHANGELOG))
Fix bug in run_config.sh script (#124)

redshift/web/1.1.0 (2020-12-01)
---------------------------------------
Update main readme (#123)
Redshift web model release ([changelog](web/redshift/CHANGELOG))

The second example customisation isn't well instrumented. On review, it's a more complicated use case than we need for a basic tier of examples. Should be removed or replaced with something more straightforward.

The example customisation is confusing and not a good example for those unfamiliar with the structures. Replace with a more fluent example.

In the documentation you refer to data Documentation for the data models can be found on our documentation site. and this link send me to a website where there is a link sending me back to the readme. Documentation Inception :D

The filters applied to the dom_interactive, dom_content_loaded_event_start and dom_content_loaded_event_end fields in the web model result in more page views than expected showing NULL for total_time_in_ms.

The question is, are these filters too strict?

• If they are then the solution is simple, all we need to do is relax the filters.
• If the results are unexpected, then maybe this needs to be looked into further.

The example customisation is confusing and not a good example for those unfamiliar with the structures. Replace with a more fluent example.

Using MERGE instead of DELETE FROM, INSERT INTO might well prove more performant and cost efficient, particularly in the case of BigQuery.

We should test this, and implement MERGE if it proves to be the better option (without affecting logic obviously).

I think this line should be commented out by default and only uncommented by users after the initial run of the script: https://github.com/snowplow/snowplow/blob/master/5-data-modeling/deduplication-queries/sql/02-events-without-fingerprint.sql#L143

page-view-events should include page_view_id IS NOT NULL in the WHERE clause.

Readmes make reference to testing scripts - should be updated with the new updates to the scripts in v1 BigQuery release.

2020 -> 2020-2021

Table definitions for the Redshift model define CHAR/VARCHAR columns inconsistently through the model - should be both internally consistent and consistent with external definitions

docs.snowplowanalytics.com should be the home for all the docs...

Commands provided are missing the -t argument to point to a target template. https://github.com/snowplow/data-models/tree/master/.scripts#run_configsh

Missing ev. in this line leads to failure when YAUAA is enabled.

In some instances the screen_id generated by the screen view context during a screen view event does not equal the screen_view_id which is incorrect.

This is thought to be state management issue within the mobile trackers rather than within the data modelling. As such, it should be resolved once a fix is applied to the tracker.

Bigquery will introduce config and script to run multiple playbooks. Redshift should get similar treatment once BQ is released (as well as Snowflake when that gets released).

Scripts should be more generic and should suit all platforms better

With the bigquery release the playbook readmes turn out to be nearly identical to Redshift - Snowflake will likely turn out the same.

It might be easier to reason about and maintain if we have a docs folder for playbook configuration instead.

This line of code:

https://github.com/snowplow/data-models/blob/master/web/v1/redshift/sql-runner/sql/standard/01-base/01-main/10-commit-base.sql#L26

Should also contain a where clause.

WHERE page_view_id IS NOT NULL

Otherwise the model can fail on this step.

In the README the download link is still pointing to Bintray which is soon to be deprecated!

https://github.com/snowplow/data-models#prerequisites

Not sure if there are references in other documentation but would be good to move them all.

The example customisation is confusing and not a good example for those unfamiliar with the structures. Replace with a more fluent example.

The other modules have the module name in the setup file, this one looks to have been an oversight.

Example in README doesn't use limits, it should.

Eg.:

All the destroy steps have a copy paste error for columnCheckQuery.

Playbook readmes have mangled formatting for lists of playbook variables. Needs a newline after each entry. Eg:

`:input_schema:`       name of atomic schema

`:scratch_schema:`     name of scratch schema  

`:output_schema:`      name of derived schema

`:entropy:`            string to append to all tables, to test without affecting prod tables (eg. `_test` produces tables like `events_staged_test`). Must match entropy value used for all other modules in a given run. Populate with an empty string if no entropy value is needed.

`:stage_next:`         update staging tables - set to true if running the next module. If true, make sure that the next module includes a 'complete' step.

`:start_date:`         Start date, used to seed manifest.

`:lookback_window:`    Defaults to 6. Period of time (in hours) to look before the latest event in manifest - to account for late arriving data, which comes out of order.

`:days_late_allowed:`  Defaults to 3.  Period of time (in days) for which we should include late data. If the difference between collector tstamps for the session start and new event is greater than this value, data for that session will not be processed.

`:update_cadence:`     Defaults to 7. Period of time (in days) in the future (from the latest event in manifest) to look for new events.

`:session_lookback:`   Defaults to 365. Period of time (in days) to limit scan on session manifest. Exists to improve performance of model when we have a lot of sessions. Should be set to as large a number as practical.

Scan limits are no longer required in the engaged time step, but seem to have been left in.

No impact on performance or logic, so should be cleaned up along with the next release.

All newly implemented customers only have data for yauaa version 1-0-1

https://github.com/snowplow/data-models/blob/master/web/v1/bigquery/sql-runner/sql/standard/02-page-views/01-main/01-
page-view-events.sql#L131

A coalesce w/ version 1-0-0 might be useful for backwards compatibility - but not sure if this would break anything downstream.

Line 17 of web/v1/README.md

More detail on each module can be found in the 'Modules detail' section below.

Should point to the web/v1/sql-runner/README.

More declarative of the value to be specified

Add a link to try-snowplow to front README to help guide those who arrive here looking to explore what Snowplow data can do.

Related: snowplow/snowplow#4425

Adding the _prod1 suffix caused more pain than it saved.

New schema versions create new columns in BigQuery - which need to be coalesced, but also pose the problem that some versions might not exist in the Database.

We solve this issue for the core enrichment contexts in #52 by using a stored procedure to extract the relevant data into a scratch table.

For the sake of solving the problem at hand, the initial implementation only handles top-level fields which aren't arrays or structs, and only uses the first item in the array.

This or a similar pattern could be amended to handle those more complicated cases, and offer a generic means of handling schema evolution for any custom BQ column.

The trickiest part being that a changing datatype in a struct or array of structs makes the column incompatible with its previous form. If we solve this problem, we can solve the single biggest pain point of working with Snowplow data in BigQuery.

Adding something like this to the relevant parts of the base module should do it:

WHERE
      a.collector_tstamp >= LOWER_LIMIT

  {{if eq .partition_derived true}}
      AND a.derived_tstamp >= LOWER_LIMIT
  {{end}}