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

Track a unique id for each pageview #10499

Merged
merged 4 commits into from Sep 30, 2016

Conversation

Projects
None yet
2 participants
@tsteur
Copy link
Member

commented Sep 13, 2016

While debugging tracking requests on a server a while ago I thought it would be really useful to have a unique id in the tracking request for a specific pageview. We cannot easily grep access logs for visitorId etc as they are stored as binary in the DB etc. Also this would give us all requests for a specific user but not a specific tracking request.

This PR generates a new, unique pageview ID for each pageview. All tracking requests that are related to this pageview will use the same ID. This would eventually allow us to exactly know which events, impressions, etc were tracked during which pageview. Useful eg for visitor log but also for other potentially new reports in the future.

This is a proposal and therefore WIP and Needs review / RFC. If this is something we'd merge I'd finish the feature in terms of adding tests for the tracker etc. For now we would only collect the data but not really use it yet apart from debugging. Because we can likely not make any schema changes for a year or so after Piwik 3.0 is released I'm proposing it already now to have it in the DB and to already collect this data. So we could eventually use it.

What's the thoughts on this one?

@tsteur tsteur added this to the 3.0.0-b1 milestone Sep 13, 2016

@mattab

This comment has been minimized.

Copy link
Member

commented Sep 19, 2016

Good idea and concept. as discussed, it will likely be useful in the future for some new functionality, and it will be useful already now for troubleshooting some complex tracking issues.

This is a proposal and therefore WIP and Needs review / RFC. If this is something we'd merge I'd finish the feature in terms of adding tests for the tracker etc.

Sounds good!

@tsteur

This comment has been minimized.

Copy link
Member Author

commented Sep 19, 2016

The tests should be fixed. Also issued PR for matomo-org/matomo-php-tracker#21

There is no system test for it yet to test it actually persists the idpageview. I can possibly add one as soon as the other PR is merged. Will need to see how to add a useful test as it simply uses the value from pv_id

@mattab

This comment has been minimized.

Copy link
Member

commented Sep 23, 2016

@tsteur it looks good, but (only) now I'm thinking: to save space, wouldn't be enough to store 8 chars instead of 16? twice as less overhead for this "extra" un-used field, would be beneficial. I could make the change if you want!

@tsteur

This comment has been minimized.

Copy link
Member Author

commented Sep 23, 2016

It just increases the chance that we generate duplicates re 8 characters. maybe it works though if we later use it more like idvisit, idpageview when identifying which pageview the actions belong to but it will make everything harder. maybe lets have a chat next week?

@mattab

This comment has been minimized.

Copy link
Member

commented Sep 23, 2016

maybe lets have a chat next week?

OK

@tsteur

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2016

reduced it to 6 bytes, let me know if it looks good and I will rebase PR to it can be merged

@mattab

This comment has been minimized.

Copy link
Member

commented Sep 27, 2016

LGTM

Needs to be documented as well in:

@tsteur tsteur force-pushed the idpageview2 branch from 06b9b90 to b471c84 Sep 27, 2016

@tsteur tsteur referenced this pull request Sep 27, 2016

Merged

Ddded docs for pageviewid #149

@tsteur

This comment has been minimized.

Copy link
Member Author

commented Sep 27, 2016

Rebased, added to dev changelog and created https://github.com/piwik/developer-documentation/pull/149/files

@mattab

This comment has been minimized.

Copy link
Member

commented Sep 30, 2016

Awesome @tsteur that's perfect!

@mattab mattab merged commit 65bc19b into 3.x-dev Sep 30, 2016

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@mattab mattab deleted the idpageview2 branch Sep 30, 2016

@mattab mattab added the Enhancement label Oct 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.