-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 possibility to queue tracking requests so they are sent in bulk #13616
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -997,7 +997,8 @@ if (typeof JSON_PIWIK !== 'object' && typeof window.JSON === 'object' && window. | |
enableCrossDomainLinking, disableCrossDomainLinking, isCrossDomainLinkingEnabled, setCrossDomainLinkingTimeout, getCrossDomainLinkingUrlParameter, | ||
addListener, enableLinkTracking, enableJSErrorTracking, setLinkTrackingTimer, getLinkTrackingTimer, | ||
enableHeartBeatTimer, disableHeartBeatTimer, killFrame, redirectFile, setCountPreRendered, | ||
trackGoal, trackLink, trackPageView, getNumTrackedPageViews, trackRequest, trackSiteSearch, trackEvent, | ||
trackGoal, trackLink, trackPageView, getNumTrackedPageViews, trackRequest, queueRequest, trackSiteSearch, trackEvent, | ||
requests, timeout, sendRequests, queueRequest, | ||
setEcommerceView, addEcommerceItem, removeEcommerceItem, clearEcommerceCart, trackEcommerceOrder, trackEcommerceCartUpdate, | ||
deleteCookie, deleteCookies, offsetTop, offsetLeft, offsetHeight, offsetWidth, nodeType, defaultView, | ||
innerHTML, scrollLeft, scrollTop, currentStyle, getComputedStyle, querySelectorAll, splice, | ||
|
@@ -1098,6 +1099,8 @@ if (typeof window.Piwik !== 'object') { | |
|
||
coreConsentCounter = 0, | ||
|
||
trackerIdCounter = 0, | ||
|
||
isPageUnloading = false; | ||
|
||
/************************************************************ | ||
|
@@ -3229,7 +3232,10 @@ if (typeof window.Piwik !== 'object') { | |
configHasConsent = null, // initialized below | ||
|
||
// holds all pending tracking requests that have not been tracked because we need consent | ||
consentRequestsQueue = []; | ||
consentRequestsQueue = [], | ||
|
||
// a unique ID for this tracker during this request | ||
uniqueTrackerId = trackerIdCounter++; | ||
|
||
// Document title | ||
try { | ||
|
@@ -5584,6 +5590,58 @@ if (typeof window.Piwik !== 'object') { | |
|
||
return hookObj; | ||
} | ||
|
||
var requestQueue = { | ||
requests: [], | ||
timeout: null, | ||
sendRequests: function () { | ||
var requestsToTrack = this.requests; | ||
this.requests = []; | ||
if (requestsToTrack.length === 1) { | ||
sendRequest(requestsToTrack[0]); | ||
} else { | ||
sendBulkRequest(requestsToTrack); | ||
} | ||
}, | ||
push: function (requestUrl) { | ||
if (!requestUrl) { | ||
return; | ||
} | ||
if (isPageUnloading) { | ||
// we don't queue as we need to ensure the request will be sent when the page is unloading... | ||
trackerInstance.trackRequest(requestUrl); | ||
return; | ||
} | ||
|
||
this.requests.push(requestUrl); | ||
|
||
if (this.timeout) { | ||
clearTimeout(this.timeout); | ||
this.timeout = null; | ||
} | ||
// we always extend by another 1.75 seconds after receiving a tracking request | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. couldn't decide between 1.5 and 2 seconds :) the earlier we send it, the more likely the request won't fail I suppose. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fyi #13444 will be useful to have to ensure the bulk request won't be too large in case tracking requests keep getting queued There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could make the time configurable. At least that way it won't require a commit to core to experiment with it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to not make it configurable for now to keep it simple but could add it later if needed. For now only premium features will use it anyway I think. Although could maybe also start using it for content impressions eventually. The defined interval shouldn't make too much of a difference. |
||
this.timeout = setTimeout(function () { | ||
requestQueue.timeout = null; | ||
requestQueue.sendRequests(); | ||
}, 1750); | ||
|
||
var trackerQueueId = 'RequestQueue' + uniqueTrackerId; | ||
if (!Object.prototype.hasOwnProperty.call(plugins, trackerQueueId)) { | ||
// we setup one unload handler per tracker... | ||
// Piwik.addPlugin might not be defined at this point, we add the plugin directly also to make | ||
// JSLint happy. | ||
plugins[trackerQueueId] = { | ||
unload: function () { | ||
if (requestQueue.timeout) { | ||
clearTimeout(requestQueue.timeout); | ||
} | ||
requestQueue.sendRequests(); | ||
} | ||
}; | ||
} | ||
} | ||
}; | ||
|
||
/*</DEBUG>*/ | ||
|
||
/************************************************************ | ||
|
@@ -7196,6 +7254,23 @@ if (typeof window.Piwik !== 'object') { | |
}); | ||
}; | ||
|
||
/** | ||
* Won't send the tracking request directly but wait for a short time to possibly send this tracking request | ||
* along with other tracking requests in one go. This can reduce the number of requests send to your server. | ||
* If the page unloads (user navigates to another page or closes the browser), then all remaining queued | ||
* requests will be sent immediately so that no tracking request gets lost. | ||
* Note: Any queued request may not be possible to be replayed in case a POST request is sent. Only queue | ||
* requests that don't have to be replayed. | ||
* | ||
* @param request eg. "param=value¶m2=value2" | ||
*/ | ||
this.queueRequest = function (request) { | ||
trackCallback(function () { | ||
var fullRequest = getRequest(request); | ||
requestQueue.push(fullRequest); | ||
}); | ||
}; | ||
|
||
/** | ||
* If the user has given consent previously and this consent was remembered, it will return the number | ||
* in milliseconds since 1970/01/01 which is the date when the user has given consent. Please note that | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does sending a single request vs bulk request w/ single entry make a big difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't make too much of a big difference. Although... for bulk requests we currently don't fallback to GET requests and always do a POST request compared to usually GET request, and it will be wrapped with
{"requests":["?' + requests.join('","?') + '"]}
so not sure if it can be replayed easily using log analytics etc.I would say it is better when possible to use
sendRequest
when there is only one request and keep differentiating.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see 👍