fix: versions are not numbers, when will I ever learn (#21228)

* fix: parsing versions but not stupid * update tests * unknown for safety
PostHog · Mar 28, 2024 · 3b67989 · 3b67989
1 parent d929ca9
commit 3b67989
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 8 deletions.
diff --git a/plugin-server/src/main/ingestion-queues/session-recording/utils.ts b/plugin-server/src/main/ingestion-queues/session-recording/utils.ts
@@ -143,7 +143,12 @@ function readLibVersionFromHeaders(headers: MessageHeader[] | undefined): string
     return typeof libVersionHeader === 'string' ? libVersionHeader : libVersionHeader?.toString()
 }
 
-function majorAndMinorVersionFrom(libVersion: string | undefined): number | undefined {
+interface LibVersion {
+    major: number
+    minor: number
+}
+
+function parseVersion(libVersion: string | undefined): LibVersion | undefined {
     try {
         let majorString: string | undefined = undefined
         let minorString: string | undefined = undefined
@@ -157,7 +162,12 @@ function majorAndMinorVersionFrom(libVersion: string | undefined): number | unde
         }
         const validMajor = majorString && !isNaN(parseInt(majorString))
         const validMinor = minorString && !isNaN(parseInt(minorString))
-        return validMajor && validMinor ? parseFloat(`${majorString}.${minorString}`) : undefined
+        return validMajor && validMinor
+            ? {
+                  major: parseInt(majorString as string),
+                  minor: parseInt(minorString as string),
+              }
+            : undefined
     } catch (e) {
         status.warn('⚠️', 'could_not_read_minor_lib_version', { libVersion })
         return undefined
@@ -210,14 +220,14 @@ export const parseKafkaMessage = async (
     // this has to be ahead of the payload parsing in case we start dropping traffic from older versions
     if (!!ingestionWarningProducer && !!teamIdWithConfig.teamId) {
         const libVersion = readLibVersionFromHeaders(message.headers)
-        const parsedVersion = majorAndMinorVersionFrom(libVersion)
+        const parsedVersion = parseVersion(libVersion)
         /**
          * We introduced SVG mutation throttling in version 1.74.0 fix: Recording throttling for SVG-like things (#758)
          * and improvements like jitter on retry and better batching in session recording in earlier versions
          * So, versions older than 1.75.0 can cause ingestion pressure or incidents
          * because they send much more information and more messages for the same recording
          */
-        if (parsedVersion && parsedVersion <= 1.74) {
+        if (parsedVersion && parsedVersion.major === 1 && parsedVersion.minor < 75) {
             counterLibVersionWarning.inc()
 
             await captureIngestionWarning(
@@ -228,7 +238,7 @@ export const parseKafkaMessage = async (
                     libVersion,
                     parsedVersion,
                 },
-                { key: libVersion || parsedVersion.toString() }
+                { key: libVersion || 'unknown' }
             )
         }
     }

diff --git a/plugin-server/tests/main/ingestion-queues/session-recording/utils.test.ts b/plugin-server/tests/main/ingestion-queues/session-recording/utils.test.ts
@@ -203,7 +203,7 @@ describe('session-recording utils', () => {
             ],
             [
                 'three-part lib version that is recent enough means no call to capture ingestion warning',
-                [{ lib_version: '1.75.0' }],
+                [{ lib_version: '1.116.0' }],
                 [],
             ],
             [
@@ -216,7 +216,7 @@ describe('session-recording utils', () => {
                                 messages: [
                                     expectedIngestionWarningMessage({
                                         libVersion: '1.74.0',
-                                        parsedVersion: 1.74,
+                                        parsedVersion: { major: 1, minor: 74 },
                                     }),
                                 ],
                                 topic: 'clickhouse_ingestion_warnings_test',
@@ -236,7 +236,7 @@ describe('session-recording utils', () => {
                                 messages: [
                                     expectedIngestionWarningMessage({
                                         libVersion: '1.32.0',
-                                        parsedVersion: 1.32,
+                                        parsedVersion: { major: 1, minor: 32 },
                                     }),
                                 ],
                                 topic: 'clickhouse_ingestion_warnings_test',