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

Restrict tracking requests to max 10000 per tracker per page load #16751

Closed
wants to merge 6 commits into from

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Nov 19, 2020

Description:

We limit the tracking requests now to 10000 per page load by default. This is to prevent issues where there is a bug in the website and for example page views are being tracked in a loop by accident due to a bug. You might notice this bug and fix this bug but then there might be still visitor who have a previous version cached or haven't reloaded the page (eg they watch a video or listen to audio or just didn't close the browser window). This way, if a visitor has the page open for quite a long time this automatically stops tracking requests from being sent.

The limit can be customised to your liking.

The counter will not be reset on trackPageView but only on an actual page reload as the counter is kept in memory.

Logs an error message should limit be reached.

Putting this into 4.0 release since it could be seen as a breaking change if someone was to track a LOT of requests during one page load.

Not sure if there's a better name than "page load" but "page view" be confusing as we don't reset the counter on page view because trackPageView might be actually called in a loop and it would prevent the feature from working.

Will update JS API reference once merged

Review

  • Functional review done
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@tsteur tsteur added this to the 4.0.0-RC milestone Nov 19, 2020
@tsteur tsteur changed the title Restrict tracking requests to max 10000 per page load Restrict tracking requests to max 10000 per tracker per page load Nov 19, 2020
@tsteur tsteur added the Needs Review PRs that need a code review label Nov 19, 2020
@tsteur
Copy link
Member Author

tsteur commented Nov 19, 2020

build js

@sgiehl
Copy link
Member

sgiehl commented Nov 19, 2020

Wondering if it maybe would make sense to automatically reset that limit after a day or something like this. There might be other tracking cases for single page applications that are actually almost never be reloaded, like order terminals for anything

@tsteur
Copy link
Member Author

tsteur commented Nov 19, 2020

@sgiehl that's what we're trying to prevent though that they keep sending requests still after 1 day etc when there's no pagereload etc. I think in this case they could maybe set a higher limit or disable this functionality with -1 (added this feature).

I would say we could reset the counter on setVisitorId (this would maybe restore old behaviour in very few cases but not in SPA), or we could reset it when trackPageView is called and the URL is different compared to last time (but this would kind of prevent the feature from working where there is a bug so could almost just as well not merge the feature as it would only add complexity plus this behaviour be hard to document and explain).

I reckon 10K req even for a SPA is quite a bit and not that easily reached. Could also set it to 20K by default if that helps.

@tsteur
Copy link
Member Author

tsteur commented Nov 19, 2020

build js

@sgiehl
Copy link
Member

sgiehl commented Nov 19, 2020

@tsteur that was just a random thought. Guess support for -1 should be enough

@tsteur
Copy link
Member Author

tsteur commented Nov 19, 2020

Actually after thinking for 30 min about it I'm closing this PR for now. If we start resetting counter after certain time or actions things become less predictable and hard to explain/document/reproduce and it's not usable. The only nice way would be to have this in default tracking code where besides trackPageView we also have a setMax...(10000) but this bloats the embed tracking code for maybe a small problem. Generally it be hard to find for some users there is such a limit etc.

@tsteur tsteur closed this Nov 19, 2020
@tsteur tsteur deleted the restrictmaxrequests branch November 19, 2020 20:02
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants