feat(hog): save function telemetry (#25093)

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
PostHog · Sep 24, 2024 · 44e6709 · 44e6709
1 parent f42697b
commit 44e6709
Show file tree

Hide file tree

Showing 22 changed files with 997 additions and 630 deletions.
diff --git a/frontend/src/scenes/pipeline/hogfunctions/hogFunctionConfigurationLogic.tsx b/frontend/src/scenes/pipeline/hogfunctions/hogFunctionConfigurationLogic.tsx
@@ -18,7 +18,7 @@ import { userLogic } from 'scenes/userLogic'
 import { groupsModel } from '~/models/groupsModel'
 import { performQuery } from '~/queries/query'
 import { EventsNode, EventsQuery, NodeKind, TrendsQuery } from '~/queries/schema'
-import { hogql } from '~/queries/utils'
+import { escapePropertyAsHogQlIdentifier, hogql } from '~/queries/utils'
 import {
     AnyPropertyFilter,
     AvailableFeature,
@@ -642,7 +642,7 @@ export const hogFunctionConfigurationLogic = kea<hogFunctionConfigurationLogicTy
                     orderBy: ['timestamp DESC'],
                 }
                 groupTypes.forEach((groupType) => {
-                    const name = groupType.group_type
+                    const name = escapePropertyAsHogQlIdentifier(groupType.group_type)
                     query.select.push(
                         `tuple(${name}.created_at, ${name}.index, ${name}.key, ${name}.properties, ${name}.updated_at)`
                     )

diff --git a/hogvm/typescript/package.json b/hogvm/typescript/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@posthog/hogvm",
-    "version": "1.0.50",
+    "version": "1.0.52",
     "description": "PostHog Hog Virtual Machine",
     "types": "dist/index.d.ts",
     "source": "src/index.ts",

diff --git a/hogvm/typescript/src/__tests__/execute.test.ts b/hogvm/typescript/src/__tests__/execute.test.ts
@@ -636,6 +636,7 @@ describe('hogvm execute', () => {
                 ops: 3,
                 stack: [],
                 upvalues: [],
+                telemetry: undefined,
                 throwStack: [],
                 syncDuration: expect.any(Number),
             },
@@ -677,10 +678,15 @@ describe('hogvm execute', () => {
         ).toEqual(map({ key: map({ otherKey: 'value' }) }))
 
         // // return {key: 'value'};
-        expect(
-            () => exec(['_h', op.STRING, 'key', op.GET_GLOBAL, 1, op.STRING, 'value', op.DICT, 1, op.RETURN]).result
+        expect(() =>
+            execSync(['_h', op.STRING, 'key', op.GET_GLOBAL, 1, op.STRING, 'value', op.DICT, 1, op.RETURN])
         ).toThrow('Global variable not found: key')
 
+        // // return {key: 'value'};
+        expect(
+            exec(['_h', op.STRING, 'key', op.GET_GLOBAL, 1, op.STRING, 'value', op.DICT, 1, op.RETURN]).error.message
+        ).toEqual('Global variable not found: key')
+
         // var key := 3; return {key: 'value'};
         expect(
             exec(['_h', op.INTEGER, 3, op.GET_LOCAL, 0, op.STRING, 'value', op.DICT, 1, op.RETURN, op.POP]).result
@@ -2105,7 +2111,8 @@ describe('hogvm execute', () => {
             finished: true,
             state: {
                 bytecode: [],
-                stack: [],
+                stack: expect.any(Array),
+                telemetry: undefined,
                 upvalues: [],
                 callStack: [],
                 throwStack: [],
@@ -2443,7 +2450,7 @@ describe('hogvm execute', () => {
             finished: true,
             state: {
                 bytecode: [],
-                stack: [],
+                stack: expect.any(Array),
                 upvalues: [],
                 callStack: [],
                 throwStack: [],
@@ -2455,4 +2462,59 @@ describe('hogvm execute', () => {
             },
         })
     })
+
+    test('logs telemetry', () => {
+        const bytecode = ['_h', op.INTEGER, 1, op.INTEGER, 2, op.PLUS, op.RETURN]
+        const result = exec(bytecode, { telemetry: true })
+        expect(result).toEqual({
+            result: 3,
+            finished: true,
+            state: {
+                bytecode: [],
+                stack: [],
+                upvalues: [],
+                callStack: [],
+                throwStack: [],
+                declaredFunctions: {},
+                ops: 4,
+                asyncSteps: 0,
+                syncDuration: expect.any(Number),
+                maxMemUsed: 16,
+                telemetry: [
+                    [expect.any(Number), 'root', 0, 'START', ''],
+                    [expect.any(Number), '', 1, '33/INTEGER', '1'],
+                    [expect.any(Number), '', 3, '33/INTEGER', '2'],
+                    [expect.any(Number), '', 5, '6/PLUS', ''],
+                    [expect.any(Number), '', 6, '38/RETURN', ''],
+                ],
+            },
+        })
+    })
+
+    test('logs telemetry for calls', () => {
+        const bytecode = ['_h', op.FALSE, op.TRUE, op.CALL_GLOBAL, 'concat', 2]
+        const result = exec(bytecode, { telemetry: true })
+        expect(result).toEqual({
+            result: 'truefalse',
+            finished: true,
+            state: {
+                bytecode: [],
+                stack: [],
+                upvalues: [],
+                callStack: [],
+                throwStack: [],
+                declaredFunctions: {},
+                ops: 3,
+                asyncSteps: 0,
+                syncDuration: expect.any(Number),
+                maxMemUsed: 17,
+                telemetry: [
+                    [expect.any(Number), 'root', 0, 'START', ''],
+                    [expect.any(Number), '', 1, '30/FALSE', ''],
+                    [expect.any(Number), '', 2, '29/TRUE', ''],
+                    [expect.any(Number), '', 3, '2/CALL_GLOBAL', 'concat'],
+                ],
+            },
+        })
+    })
 })
diff --git a/hogvm/typescript/src/__tests__/utils.test.ts b/hogvm/typescript/src/__tests__/utils.test.ts
@@ -1,4 +1,4 @@
-import { calculateCost } from '../utils'
+import { calculateCost, convertJSToHog, convertHogToJS } from '../utils'
 
 const PTR_COST = 8
 
@@ -29,4 +29,26 @@ describe('hogvm utils', () => {
         obj['key'] = obj
         expect(calculateCost(obj)).toBe(PTR_COST * 3 + 3)
     })
+
+    test('convertJSToHog preserves circular references', () => {
+        const obj: any = { a: null, b: true }
+        obj.a = obj
+        const hog = convertJSToHog(obj)
+        expect(hog.get('a') === hog).toBe(true)
+    })
+
+    test('convertHogToJs preserves circular references', () => {
+        const obj: any = { a: null, b: true }
+        obj.a = obj
+        const js = convertHogToJS(obj)
+        expect(js.a === js).toBe(true)
+
+        const map: any = new Map([
+            ['a', null],
+            ['b', true],
+        ])
+        map.set('a', map)
+        const js2 = convertHogToJS(map)
+        expect(js2.a === js2).toBe(true)
+    })
 })