Introduce an analytics sampling rate for VIP sites #48

ingeniumed · 2023-09-04T06:26:45Z

Description

This PR will introduce a sampling algorithm for the analytics that sent, when this plugin is used on a VIP site. The sampling algorithm is as follows:

Sample every 10s, or
Sample every 10m.

This will ensure that we catch burst usage, or regular usage throughout the day without losing the core purpose behind the analytics.

Fixes #45

Steps to Test

Spin up a local dev using vip dev-env
Adjust the is_wpvip_site() to give back true
Pop a breakpoint inside is_it_sampling_time.
Call http://<slug>.vipdev.lndo.site/wp-json/vip-block-data-api/v1/posts/1/blocks, while keeping an eye on the time
Every 10m or 10s it should trigger the breakpoint, otherwise it shouldn't be triggered.

ingeniumed · 2023-09-04T06:34:08Z

tests/rest/test-rest-api.php

@@ -107,7 +107,7 @@ public function test_rest_api_returns_blocks_for_post() {
 					'mediaId'           => 6,
 					'mediaLink'         => 'https://gutenberg-block-data-api-test.go-vip.net/?attachment_id=6',
 					'mediaType'         => 'image',
-					'align'             => 'wide',
+					'align'             => 'none',


From my local testing, it looks like the default value for this has changed so this keeps it in line with that. Wonder if it's a 6.3 change?

Good catch! Looks like this was changed 6 months ago in Gutenberg, which was later merged into WordPress 6.3 like you said.

alecgeatches · 2023-09-04T16:45:24Z

src/analytics/analytics.php

+		$minutes = $current_timestamp->format( 'i' );
+
+		// Get the seconds from the date.
+		$seconds = $current_timestamp->format( 's' );


format() produces strings, so we should probably intval() these or expressions like 0 !== $seconds (i.e. 0 !== "0") won't work as expected.

Actually, thinking about this a bit more we may not need checks if seconds/minutes is 0. If I'm doing my math right, the code (with proper types) would be skipping 1/6th of the sampling times:

00:00:00 -> Skip, seconds is 0 00:00:10 -> Send analytics 00:00:20 -> Send analytics 00:00:30 -> Send analytics 00:00:40 -> Send analytics 00:00:50 -> Send analytics 00:01:00 -> Skip, seconds is 0

Yip, updated that! Good catch 👍🏾

alecgeatches · 2023-09-04T16:47:59Z

src/analytics/analytics.php

+	 * 
+	 * Current sampling algorithm is that every 10s or 10m, we send analytics.
+	 * 
+	 * Max calls possible based on 1 call every 10s: 8640.


@ingeniumed Please correct me if I'm wrong, but does this math work? We could have several requests come through in the same second which would result in multiple analytics calls in the same second. I think that's fine for sampling purposes, but it doesn't necessarily give us an upper limit on analytics requests.

You are right, I added that in mainly for reference. But I don't want to make us think that we can use this to determine actual usage easily. I've taken it out

alecgeatches · 2023-09-04T16:56:46Z

src/analytics/analytics.php

+		$seconds = $current_timestamp->format( 's' );
+
+		// Only send analytics every 10 minutes or 10 seconds.
+		if ( ( 0 !== $seconds && 0 === $seconds % WPCOMVIP__BLOCK_DATA_API__STAT_SAMPLING_RATE_SEC ) || ( 0 !== $minutes && 0 === $minutes % WPCOMVIP__BLOCK_DATA_API__STAT_SAMPLING_RATE_MIN ) ) {


@ingeniumed Can you explain a bit why we're doing one sample every 10 seconds and another every 10 minutes? It seems like the sampling math is easier to estimate real usage if we stick with just one of these. Even just doing seconds would reduce analytics by 90% but still give us relatively good coverage.

Discussed, and I have dropped the 10m leaving just 10s in place.

alecgeatches

Sampling is a great way to reduce our analytics overhead with no extra db/caching cost, I love it. I have a few questions above about the implementation.

smithjw1 · 2023-09-05T23:26:46Z

README.md

@@ -942,6 +942,8 @@ The plugin records two data points for analytics, on VIP sites:

 Both of these data points are a counter that is incremented, and do not contain any other telemetry or sensitive data. You can see what's being [collected in code here][repo-analytics], and WPVIP's privacy policy [here](https://wpvip.com/privacy/).

+In addition, the analytics are sent every 10 seconds only.


We could say "analytics are sampled" that way we don't have to update the README if we change the sampling rate.

While I do see what you mean, I would lean towards being open about the sampling rate as well as the method with which we do that in the README. That way, we are being transparent about it and when we do change it we also have to update the README to reflect that.

smithjw1 · 2023-09-05T23:28:22Z

src/analytics/analytics.php

+		$seconds = intval( $current_timestamp->format( 's' ) );
+
+		// Only send analytics every 10 seconds.
+		if ( 0 === ( $seconds % WPCOMVIP__BLOCK_DATA_API__STAT_SAMPLING_RATE_SEC ) ) {


Someday we may want a hook so that customers can sent their own sampling rate no lower than X, but for a different day.

I do see the value of that, especially if they are at a scale where even the tiniest change could have a large impact. We would need to figure out a good floor that we can use, that would still make the data useful to us. I'll add this to the backlog.

smithjw1

Does what it says on the tin!

ingeniumed added 9 commits September 4, 2023 15:01

Add the timestamp retrieval in place

b1bdd7e

Add in sampling, along with a note for what the upscaling factor is

b75bbf8

Remove the test for the bucket

8cb1db9

Tweak the phpdoc explanation for the algorithm

9d22006

Adjust the doc again

58d0c00

Add the total calls

a22e5c4

Shift the constants around

a8fa17b

Update docs

1d59e6c

Tweak the logic to be actually right, and update the readme

136b081

ingeniumed requested a review from a team as a code owner September 4, 2023 06:26

Adjust test response

612dc40

ingeniumed commented Sep 4, 2023

View reviewed changes

alecgeatches reviewed Sep 4, 2023

View reviewed changes

alecgeatches requested changes Sep 4, 2023

View reviewed changes

ingeniumed added 4 commits September 5, 2023 08:58

Drop the minute sampling, and the comment about max calls

6b01df7

Drop the extra 10s

5217f85

BEDMAS

05134af

Drop the dot, just dot

aaf7fba

alecgeatches approved these changes Sep 4, 2023

View reviewed changes

smithjw1 reviewed Sep 5, 2023

View reviewed changes

smithjw1 approved these changes Sep 5, 2023

View reviewed changes

Update plugin version

939c422

ingeniumed merged commit 54ceb30 into trunk Sep 6, 2023
2 checks passed

ingeniumed deleted the modify/reduce-analytics-sampling-rate branch September 6, 2023 22:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce an analytics sampling rate for VIP sites #48

Introduce an analytics sampling rate for VIP sites #48

ingeniumed commented Sep 4, 2023

ingeniumed Sep 4, 2023

alecgeatches Sep 4, 2023

alecgeatches Sep 4, 2023

alecgeatches Sep 4, 2023

ingeniumed Sep 4, 2023 •

edited

Loading

alecgeatches Sep 4, 2023

ingeniumed Sep 4, 2023

alecgeatches Sep 4, 2023

ingeniumed Sep 4, 2023

alecgeatches left a comment

smithjw1 Sep 5, 2023

ingeniumed Sep 6, 2023

smithjw1 Sep 5, 2023

ingeniumed Sep 6, 2023

smithjw1 left a comment

		@@ -942,6 +942,8 @@ The plugin records two data points for analytics, on VIP sites:

		Both of these data points are a counter that is incremented, and do not contain any other telemetry or sensitive data. You can see what's being [collected in code here][repo-analytics], and WPVIP's privacy policy [here](https://wpvip.com/privacy/).

		In addition, the analytics are sent every 10 seconds only.

Introduce an analytics sampling rate for VIP sites #48

Introduce an analytics sampling rate for VIP sites #48

Conversation

ingeniumed commented Sep 4, 2023

Description

Steps to Test

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ingeniumed Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecgeatches left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smithjw1 left a comment

Choose a reason for hiding this comment

ingeniumed Sep 4, 2023 •

edited

Loading