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

Jump to bottom

Make sure ping=1 does not create duplicated actions #8218

Closed
tsteur opened this issue Jun 25, 2015 · 2 comments
Closed

Make sure ping=1 does not create duplicated actions #8218

tsteur opened this issue Jun 25, 2015 · 2 comments
Assignees
@mattab
Labels
Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Milestone
2.14.0

Comments

@tsteur
Copy link
Member

tsteur commented Jun 25, 2015

Recently we implemented new tracker feature ping=1 in #8069 .

While reading #8202 I noticed there might be a problem with the implementation of ping request during tracking. Problem might be in Visit.php here: https://github.com/piwik/piwik/pull/8069/files#diff-9bca48e482124d44a3d910acc809194eR168

Imagine a ping request is sent and a new visit is detected. This can happen for many reasons, eg it is now after midnight, no ping was sent for last 15 minutes and user configured a shorter "timeout", a custom dimensions causes a new visit etc.

In this case we might track the same action again as it never goes in if !$isNewVisit. It is a new visit and therefore we create a new visit including action etc for this ping request but I reckon it should not?

A solution could be to do something like

if ($this->isPingRequest() && $isNewVisit) {
   return; // do not track anything?
} else if ($this->isPingRequest() && !$isNewVisit) {
  $action= null;
}

I'm not sure about expected behaviour in this case and wasn't sure if I should comment on already merged PR or not. So created this issue. Feel free to close if it is invalid

@mattab @diosmosis
@tsteur tsteur added the Bug For errors / faults / flaws / inconsistencies etc. label Jun 25, 2015
@tsteur tsteur modified the milestones: 2.14.0, 2.14.1 Jun 25, 2015
@mattab mattab modified the milestones: 2.14.0, 2.14.1 Jun 26, 2015
@mattab mattab added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jun 26, 2015
@mattab
Copy link
Member

mattab commented Jun 26, 2015

It is a new visit and therefore we create a new visit including action etc for this ping request but I reckon it should not?

After thinking about it, it's a good point.

  • it is not desired to create new visit with "unknown action".
  • we should ignore ping requests that would otherwise create these new visits with unknown action.
  • note to self: check whether goal conversions/ecommerce could be double counted.
    • Edit: looks OK and no double counting should be possible

I'll work on this one

@mattab mattab self-assigned this Jun 26, 2015
mattab added a commit that referenced this issue Jun 26, 2015
@mattab
Refs #8218 Do not create a new visit when it is a ping request.
56e1bc7
@mattab
Copy link
Member

mattab commented Jun 26, 2015

fixed in #8223

@mattab mattab closed this as completed Jun 26, 2015
@mattab mattab mentioned this issue Jun 26, 2015
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mattab mattab

Labels
Bug For errors / faults / flaws / inconsistencies etc. not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
2.14.0
Development

No branches or pull requests
2 participants
@tsteur @mattab