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

Fix total time of visit is not the sum of times of page visits if heartbeat feature is used #9617

Merged
merged 7 commits into from Jan 22, 2016

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jan 22, 2016

We can fix the incorrect action time by not updating visit_last_action_time see problem explained in #9610 (comment) . A ping is not really an action so it should not update visit_last_action_time.

This changes behaviour of heart beat in a way that the "time spent on last action" can be max 30 minutes (or custom value specified in visit_standard_length config setting). Why? So far we updated visit_last_action_time on each ping request and set it to now. This kept the visitors session active and the VisitorRecognizer would find an existing visit even if the user did not click within 30 minutes but if there were ping request.

Now the visit_last_action_time is no longer updated. This means after 30 minutes of sending ping requests the VisitorRecognizer will no longer find an existing visit and therefore all further ping requests will be aborted by this line https://github.com/piwik/piwik/blob/2.16.0-b6/plugins/Heartbeat/Tracker/PingRequestProcessor.php#L33-L35

fixes #9610

@tsteur tsteur added not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Needs Review PRs that need a code review labels Jan 22, 2016
@tsteur tsteur added this to the 2.16.0 milestone Jan 22, 2016
@@ -407,6 +407,10 @@ public function setParam($name, $value)
{
$this->params[$name] = $value;
unset($this->paramsCache[$name]);

Copy link
Member Author

Choose a reason for hiding this comment

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

this change is kinda unrelated but noticed it while fixing this issue

@tsteur
Copy link
Member Author

tsteur commented Jan 22, 2016

Ideally, we would fix this in Piwik 3.0 the proper way.

For example we could have a visit_last_ping_time and let users decide whether a ping should extend the current session or not. Ideally we would directly update the value for the time spent on the last action see #9539 . To do this we would need to fetch the last action during tracking time. To make this fast we would need something like #8476 to store active, ongoing visits and actions in a separate table to make it blazing fast to fetch and update them.

@mattab
Copy link
Member

mattab commented Jan 22, 2016

LGTM. updated tests files, when build is green we can merge!

mattab pushed a commit that referenced this pull request Jan 22, 2016
Fix total time of visit is not the sum of times of page visits if heartbeat feature is used
@mattab mattab merged commit 3d0684c into master Jan 22, 2016
@mattab mattab deleted the 9610 branch January 22, 2016 11:32
tsteur added a commit to matomo-org/developer-documentation that referenced this pull request Apr 19, 2018
mattab pushed a commit to matomo-org/developer-documentation that referenced this pull request Apr 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants