feat: RemoteConfig don't sync if there are no changes

posthog/models/remote_config.py

@@ -69,8 +69,8 @@ def indent_js(js_content: str, indent: int = 4) -> str:
     return joined
 
 
-def cache_key_for_team_token(team_token: str, suffix: str) -> str:
-    return f"remote_config/{team_token}/{suffix}"
+def cache_key_for_team_token(team_token: str) -> str:
+    return f"remote_config/{team_token}/config"
 
 
 def sanitize_config_for_public_cdn(config: dict, request: Optional[HttpRequest] = None) -> dict:
@@ -291,10 +291,8 @@ def _build_site_apps_js(self):
         return site_apps_js + site_functions_js
 
     @classmethod
-    def _get_via_cache(
-        cls, token: str, suffix: str, fn: Callable[["RemoteConfig"], dict | str], timeout: int = CACHE_TIMEOUT
-    ) -> Any:
-        key = cache_key_for_team_token(token, suffix)
+    def _get_config_via_cache(cls, token: str) -> dict:
+        key = cache_key_for_team_token(token)
 
         data = cache.get(key)
         if data == "404":
@@ -309,25 +307,25 @@ def _get_via_cache(
         try:
             remote_config = cls.objects.select_related("team").get(team__api_token=token)
         except cls.DoesNotExist:
-            cache.set(key, "404", timeout=timeout)
+            cache.set(key, "404", timeout=CACHE_TIMEOUT)
             REMOTE_CONFIG_CACHE_COUNTER.labels(result="miss_but_missing").inc()
             raise
 
-        data = fn(remote_config)
-        cache.set(key, data, timeout=timeout)
+        data = remote_config.build_config()
+        cache.set(key, data, timeout=CACHE_TIMEOUT)
 
         return data
 
     @classmethod
     def get_config_via_token(cls, token: str, request: Optional[HttpRequest] = None) -> dict:
-        config = cls._get_via_cache(token, "config", lambda remote_config: remote_config.build_config())
+        config = cls._get_config_via_cache(token)
         config = sanitize_config_for_public_cdn(config, request=request)
 
         return config
 
     @classmethod
     def get_config_js_via_token(cls, token: str, request: Optional[HttpRequest] = None) -> str:
-        config = cls._get_via_cache(token, "config", lambda remote_config: remote_config.build_config())
+        config = cls._get_config_via_cache(token)
         # Get the site apps JS so we can render it in the JS
         site_apps_js = config.pop("siteAppsJS", None)
         # We don't want to include the minimal site apps content as we have the JS now
@@ -361,16 +359,21 @@ def sync(self):
 
         try:
             config = self.build_config()
-            self.config = config
-
-            cache.set(cache_key_for_team_token(self.team.api_token, "config"), config, timeout=CACHE_TIMEOUT)
 
-            self._purge_cdn()
+            if config == self.config:
+                CELERY_TASK_REMOTE_CONFIG_SYNC.labels(result="no_changes").inc()
+                logger.info(f"RemoteConfig for team {self.team_id} is unchanged")
+                return
 
-            # TODO: Invalidate caches - in particular this will be the Cloudflare CDN cache
+            self.config = config
             self.synced_at = timezone.now()
             self.save()
 
+            # Update the redis cache key for the config
+            cache.set(cache_key_for_team_token(self.team.api_token), config, timeout=CACHE_TIMEOUT)
+            # Invalidate Cloudflare CDN cache
+            self._purge_cdn()
+
             CELERY_TASK_REMOTE_CONFIG_SYNC.labels(result="success").inc()
         except Exception as e:
             capture_exception(e)

posthog/models/test/test_remote_config.py

@@ -280,7 +280,7 @@ def setUp(self):
         super().setUp()
         self.remote_config.refresh_from_db()
         # Clear the cache so we are properly testing each flow
-        assert cache.delete(cache_key_for_team_token(self.team.api_token, "config"))
+        assert cache.delete(cache_key_for_team_token(self.team.api_token))
 
     def _assert_matches_config(self, data):
         assert data == snapshot(
@@ -348,10 +348,15 @@ def test_syncs_if_changes(self):
         self.remote_config.sync()
         assert synced_at < self.remote_config.synced_at  # type: ignore
 
+    def test_does_not_syncs_if_no_changes(self):
+        synced_at = self.remote_config.synced_at
+        self.remote_config.sync()
+        assert synced_at == self.remote_config.synced_at
+
     def test_persists_data_to_redis_on_sync(self):
         self.remote_config.config["surveys"] = True
         self.remote_config.sync()
-        assert cache.get(cache_key_for_team_token(self.team.api_token, "config"))
+        assert cache.get(cache_key_for_team_token(self.team.api_token))
 
     def test_gets_via_redis_cache(self):
         with self.assertNumQueries(CONFIG_REFRESH_QUERY_COUNT):
@@ -453,6 +458,8 @@ def test_purges_cdn_cache_on_sync(self, mock_post):
             REMOTE_CONFIG_CDN_PURGE_TOKEN="MY_TOKEN",
             REMOTE_CONFIG_CDN_PURGE_DOMAINS=["cdn.posthog.com", "https://cdn2.posthog.com"],
         ):
+            # Force a change to the config
+            self.remote_config.config["token"] = "NOT"
             self.remote_config.sync()
             mock_post.assert_called_once_with(
                 "https://api.cloudflare.com/client/v4/zones/MY_ZONE_ID/purge_cache",