Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Base Macros #62

Merged

Conversation

georgewoodhead
Copy link
Contributor

@georgewoodhead georgewoodhead commented Nov 15, 2023

Description & motivation

Update the package to use the latest snowplow utils base macros. This enables you to specify your session and user identifiers used in the incremental building of the models.

Changes to the default session identifier

  • We’ve changed the default session identifier from being the domain_sessionid, to now be the media session id (or page/screen view id if the media session entity is not set).

  • Previously if a user paused some content and switched to another tab for an hour or so before coming back to finish watching the content, it could mean these subsequent media events would be disregarded. This was because the original domain_sessionid would have expired and the stray media events attached to the new domain_sessionid would be removed in the models deduplication steps (similar to the stray page pings). In this scenario the media session id and page_view_id remain the same enabling us to model the complete media play.

  • We have added a new field to the base and ad views tables, domain_sessionid_array, which shows all the domain_sessionid values attached to a media play, thus keeping the ability to perform session level analysis.

We have also added the option to passthrough fields to the base and ad views models.

Checklist

  • I have verified that these changes work locally
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)
  • I have raised a documentation PR if applicable (Link here if required)

@snowplowcla snowplowcla added the cla:yes [Auto generated] Snowplow Contributor License Agreement has been signed. label Nov 15, 2023
@georgewoodhead georgewoodhead force-pushed the feature/base-macros branch 2 times, most recently from 3c0e407 to e7518c4 Compare November 22, 2023 18:03
Copy link
Contributor

@rlh1994 rlh1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good! Literally just 2 questions!

macros/identifiers.sql Outdated Show resolved Hide resolved
Copy link
Contributor

@rlh1994 rlh1994 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good! Literally just 2 questions!

@georgewoodhead georgewoodhead marked this pull request as ready for review November 24, 2023 09:58
Copy link
Contributor

@matus-tomlein matus-tomlein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks amazing, great work!

macros/identifiers.sql Outdated Show resolved Hide resolved
models/base/scratch/base_scratch.yml Show resolved Hide resolved
models/media_ad_views/media_ad_views.yml Show resolved Hide resolved
models/media_base/media_base.yml Show resolved Hide resolved
models/media_plays/media_plays.yml Show resolved Hide resolved
CHANGELOG Outdated Show resolved Hide resolved
@georgewoodhead georgewoodhead merged commit bcd6e3a into release/snowplow-media-player/0.7.0 Nov 28, 2023
4 checks passed
@emielver emielver deleted the feature/base-macros branch December 1, 2023 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes [Auto generated] Snowplow Contributor License Agreement has been signed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants