Skip to content
This repository was archived by the owner on Nov 4, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
71acd57
silence some test logs
mariusandra Dec 3, 2020
c605b8d
admit defeat in the battle with `esModuleInterop`
mariusandra Dec 3, 2020
cf0bc59
add basic typescript piscina workers
mariusandra Dec 3, 2020
cc8b50e
es module fix
mariusandra Dec 3, 2020
6b31621
remove rollup and compile directly with typescript
mariusandra Dec 3, 2020
9185fdb
fix fetch import error
mariusandra Dec 3, 2020
517ddb8
make piscina work in jest, dist and dev... run plugins through it
mariusandra Dec 3, 2020
27fd502
simplify tests
mariusandra Dec 4, 2020
630cf58
store plugin status on the `server`
mariusandra Dec 4, 2020
4456126
config types
mariusandra Dec 4, 2020
2f0a872
not all keys needed
mariusandra Dec 4, 2020
6143d11
add worker concurrency setting
mariusandra Dec 4, 2020
2ed6fee
clean up code
mariusandra Dec 4, 2020
92b2445
fix test worker code
mariusandra Dec 4, 2020
9b85afa
add a basic benchmark test
mariusandra Dec 4, 2020
c04f73d
merge
mariusandra Dec 4, 2020
f4ec9a7
refactor test
mariusandra Dec 4, 2020
45c90e9
refactor test utils
mariusandra Dec 4, 2020
4eec9e6
support more serialized binary formats
mariusandra Dec 4, 2020
90b0913
mock jest behind enemy lines
mariusandra Dec 4, 2020
8436771
less verbose logs in tests
mariusandra Dec 4, 2020
de14d97
add test to make sure adding more CPU cores makes plugins faster!
mariusandra Dec 4, 2020
5d4685c
clone objects
mariusandra Dec 4, 2020
b96f373
add 12
mariusandra Dec 4, 2020
e82f67f
fix linter's worries
mariusandra Dec 4, 2020
6438aad
remove extra word
mariusandra Dec 4, 2020
8f42315
improve display
mariusandra Dec 4, 2020
889cbaf
remove the exact cpu count from the list
mariusandra Dec 4, 2020
ce933a7
add postgres and redis services to github to make tests pass
mariusandra Dec 4, 2020
9886084
add ENV to config
mariusandra Dec 4, 2020
40195c1
add ENV to config
mariusandra Dec 4, 2020
a5cbe9c
process events test wip
mariusandra Dec 4, 2020
b8ead06
add db:init script
mariusandra Dec 5, 2020
63a22ce
add db:init script to CI
mariusandra Dec 5, 2020
9ae210a
add db:init script
mariusandra Dec 5, 2020
629751f
add db:init script to CI
mariusandra Dec 5, 2020
fe0c6cb
No prepublish on yarn install
mariusandra Dec 5, 2020
24baa9e
Merge branch 'master' into worker-threads
mariusandra Dec 5, 2020
b6c0150
Merge branch 'master' into worker-threads
mariusandra Dec 5, 2020
061aa65
Merge branch 'master' into process-events
mariusandra Dec 5, 2020
d298dc1
Merge branch 'worker-threads' into process-events
mariusandra Dec 5, 2020
43bd0e3
processEvents in vm
mariusandra Dec 5, 2020
2b24cb6
run plugins on batches of events
mariusandra Dec 5, 2020
9b3c82d
tests for batching performance
mariusandra Dec 5, 2020
8b8f22e
async processEvent & events
mariusandra Dec 6, 2020
59bc1d8
merge
mariusandra Dec 7, 2020
1d1774d
Merge branch 'master' into process-events
mariusandra Dec 8, 2020
31a264d
lint
mariusandra Dec 8, 2020
1023b6d
Merge branch 'master' into process-events
mariusandra Dec 8, 2020
0f8d7f6
performance test batches
mariusandra Dec 8, 2020
2225f6f
not a dev run
mariusandra Dec 8, 2020
42920e4
prettier for ts
mariusandra Dec 8, 2020
3038e1e
prettier
mariusandra Dec 8, 2020
9ffb68f
remove support for PostHog.capture inside `processEvent` for now. Thi…
mariusandra Dec 8, 2020
29a89e3
less round in tests, increase timeout
mariusandra Dec 8, 2020
9f65f77
refactor
mariusandra Dec 8, 2020
9f0a7ba
slight cleanup
mariusandra Dec 8, 2020
c4c8ac8
rename `processEvents` to `processEventBatch`
mariusandra Dec 8, 2020
539a6d3
only wait in processEventBatch if needed
mariusandra Dec 8, 2020
42dcee6
Merge branch 'master' into process-events
Twixes Dec 9, 2020
05976ba
Order file extensions more readably in lint-staged
Twixes Dec 9, 2020
9193c54
Prettier
Twixes Dec 9, 2020
015e15f
remove time rounding
mariusandra Dec 9, 2020
bdad14c
less lookups
mariusandra Dec 9, 2020
92b8cbc
describe test devmode
mariusandra Dec 9, 2020
0afd70e
add "light"
mariusandra Dec 9, 2020
9dc93ac
Remove redundant Math.round
Twixes Dec 9, 2020
0aaee9d
Run prettier
Twixes Dec 9, 2020
b43bce7
Fix performance measurement
Twixes Dec 9, 2020
fe28b45
Prettier but correctly
Twixes Dec 9, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,6 @@
}
},
"lint-staged": {
"*.{js,css,scss}": "prettier --write"
"*.{js,ts,css,scss}": "prettier --write"
}
}
72 changes: 55 additions & 17 deletions src/plugins.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as path from 'path'
import * as fs from 'fs'
import { createPluginConfigVM, prepareForRun } from './vm'
import { createPluginConfigVM } from './vm'
import { PluginsServer, PluginConfig, PluginJsonConfig, TeamId } from './types'
import { PluginEvent, PluginAttachment } from 'posthog-plugins'
import { clearError, processError } from './error'
Expand Down Expand Up @@ -180,26 +180,21 @@ async function loadPlugin(server: PluginsServer, pluginConfig: PluginConfig): Pr
}

export async function runPlugins(server: PluginsServer, event: PluginEvent): Promise<PluginEvent | null> {
const pluginsToRun = server.pluginConfigsPerTeam.get(event.team_id) || server.defaultConfigs

const pluginsToRun = getPluginsForTeam(server, event.team_id)
let returnedEvent: PluginEvent | null = event

for (const pluginConfig of pluginsToRun.reverse()) {
if (pluginConfig.vm) {
const processEvent = prepareForRun(server, event.team_id, pluginConfig, 'processEvent', event)

if (processEvent) {
const startTime = performance.now()
try {
returnedEvent = (await processEvent(returnedEvent)) || null
const ms = Math.round((performance.now() - startTime) * 1000) / 1000
logTime(pluginConfig.plugin?.name || 'noname', ms)
} catch (error) {
await processError(server, pluginConfig, error, returnedEvent)
const ms = Math.round((performance.now() - startTime) * 1000) / 1000
logTime(pluginConfig.plugin?.name || 'noname', ms, true)
}
if (pluginConfig.vm?.methods?.processEvent) {
let errored = false
const { processEvent } = pluginConfig.vm.methods
const startTime = performance.now()
try {
returnedEvent = (await processEvent(returnedEvent)) || null
} catch (error) {
errored = true
await processError(server, pluginConfig, error, returnedEvent)
}
logTime(pluginConfig.plugin?.name || 'noname', performance.now() - startTime, errored)

if (!returnedEvent) {
return null
Expand All @@ -209,3 +204,46 @@ export async function runPlugins(server: PluginsServer, event: PluginEvent): Pro

return returnedEvent
}

export async function runPluginsOnBatch(server: PluginsServer, batch: PluginEvent[]): Promise<PluginEvent[]> {
const eventsByTeam = new Map<number, PluginEvent[]>()

for (const event of batch) {
if (eventsByTeam.has(event.team_id)) {
eventsByTeam.get(event.team_id)!.push(event)
} else {
eventsByTeam.set(event.team_id, [event])
}
}

let allReturnedEvents: PluginEvent[] = []

for (const [teamId, teamEvents] of eventsByTeam.entries()) {
const pluginsToRun = getPluginsForTeam(server, teamId)

let returnedEvents: PluginEvent[] = teamEvents

for (const pluginConfig of pluginsToRun.reverse()) {
const { processEventBatch } = pluginConfig.vm?.methods || {}
if (processEventBatch && returnedEvents.length > 0) {
const startTime = performance.now()
let errored = false
try {
returnedEvents = (await processEventBatch(returnedEvents)) || []
} catch (error) {
errored = true
await processError(server, pluginConfig, error, returnedEvents[0])
}
logTime(pluginConfig.plugin?.name || 'noname', performance.now() - startTime, errored)
}
}

allReturnedEvents = allReturnedEvents.concat(returnedEvents)
}

return allReturnedEvents
}

function getPluginsForTeam(server: PluginsServer, teamId: number): PluginConfig[] {
return server.pluginConfigsPerTeam.get(teamId) || server.defaultConfigs
}
2 changes: 1 addition & 1 deletion src/stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const histories = new Map<string, Array<number>>()
const historyIndex = new Map<string, number>()

export function logTime(name: string, time: number, error?: boolean): void {
const ms = Math.round(time * 1000) / 1000
// const ms = Math.round(time * 1000) / 1000
// TODO: add this back with better dev logging. Disabling since this trashes performance tests.
// console.log(`Running plugin ${name}: ${error ? 'ERROR IN ' : ''}${ms}ms`)
if (!histories.has(name)) {
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,5 +113,6 @@ export interface PluginConfigVMReponse {
vm: VM
methods: {
processEvent: (event: PluginEvent) => Promise<PluginEvent>
processEventBatch: (batch: PluginEvent[]) => Promise<PluginEvent[]>
}
}
55 changes: 24 additions & 31 deletions src/vm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,32 @@ export function createPluginConfigVM(
// run the plugin setup script, if present
__callWithMeta('setupPlugin');

// we have processEvent, but not processEventBatch
if (!__getExported('processEventBatch') && __getExported('processEvent')) {
exports.processEventBatch = async function processEventBatch (batch, meta) {
const processEvent = __getExported('processEvent');
let waitFor = false
const processedEvents = batch.map(event => {
const e = processEvent(event, meta)
if (e && typeof e.then !== 'undefined') {
waitFor = true
}
return e
})
const response = waitFor ? (await Promise.all(processedEvents)) : processedEvents;
return response.filter(r => r)
}
// we have processEventBatch, but not processEvent
Comment on lines +68 to +83

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we handle the case when there are both processEvent and processEventBatch?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user handled it for us in that case. I think it's fine for the user to give both functions (e.g. making custom batching that only works with max 10 events), yet if they want to give just one of the two, the system should still work. Also if they give zero, the system should work... and then just do the plugin differently.

} else if (!__getExported('processEvent') && __getExported('processEventBatch')) {
exports.processEvent = async function processEvent (event, meta) {
return (await (__getExported('processEventBatch'))([event], meta))?.[0]
}
}

// export various functions
const __methods = {
processEvent: __bindMeta('processEvent')
processEvent: __bindMeta('processEvent'),
processEventBatch: __bindMeta('processEventBatch')
};
`
)
Expand All @@ -77,33 +100,3 @@ export function createPluginConfigVM(
methods: vm.run('__methods'),
}
}

export function prepareForRun(
server: PluginsServer,
teamId: number,
pluginConfig: PluginConfig, // might have team_id=0
method: 'processEvent',
event?: PluginEvent
): null | ((event: PluginEvent) => Promise<PluginEvent>) | (() => Promise<void>) {
if (!pluginConfig.vm?.methods[method]) {
return null
}

const { vm } = pluginConfig.vm

if (event?.properties?.token) {
// TODO: this should be nicer... and it's not optimised for batch processing
const posthog = createInternalPostHogInstance(
event.properties.token,
{ apiHost: event.site_url, fetch },
{
performance: performance,
}
)
vm.freeze(posthog, 'posthog')
} else {
vm.freeze(null, 'posthog')
}

return pluginConfig.vm.methods[method]
}
7 changes: 6 additions & 1 deletion src/worker/worker.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { runPlugins, runPluginsOnBatch, setupPlugins } from '../plugins'
import { cloneObject, setLogLevel } from '../utils'
import { runPlugins, setupPlugins } from '../plugins'
import { createServer } from '../server'
import { PluginsServerConfig } from '../types'

Expand Down Expand Up @@ -29,5 +29,10 @@ export async function createWorker(config: PluginsServerConfig): Promise<TaskWor
// must clone the object, as we may get from VM2 something like { ..., properties: Proxy {} }
return cloneObject(processedEvent as Record<string, any>)
}
if (task === 'processEventBatch') {
const processedEvents = await runPluginsOnBatch(server, args.batch)
// must clone the object, as we may get from VM2 something like { ..., properties: Proxy {} }
return cloneObject(processedEvents as any[])
}
}
}
2 changes: 1 addition & 1 deletion tests/plugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ test('setupPlugins and runPlugins', async () => {
},
})
expect(pluginConfig.vm).toBeDefined()
expect(Object.keys(pluginConfig.vm!.methods)).toEqual(['processEvent'])
expect(Object.keys(pluginConfig.vm!.methods)).toEqual(['processEvent', 'processEventBatch'])

expect(setError).toHaveBeenCalled()
expect(setError.mock.calls[0][0]).toEqual(mockServer)
Expand Down
Loading