Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: move UUID validation up in pipeline #18317

Merged
merged 18 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
70ef252
Move UUID validation from `process` step to `prepare` step
davemurphysf Oct 31, 2023
1d69564
Move uuid validation all the way up to `populateTeamData`
davemurphysf Nov 2, 2023
b3553dc
Update tests to use `processEvent` instead of `eventsProcessor.proces…
davemurphysf Nov 2, 2023
96e815d
Add temporary vscode config to just run relavent tests
davemurphysf Nov 2, 2023
097ea27
Tests are fixed; was looking for an event that won't be produced
davemurphysf Nov 2, 2023
58d6465
Remove temp vscode debug config
davemurphysf Nov 2, 2023
a0b8aa1
nit: remove added newline
davemurphysf Nov 2, 2023
26ab733
Fix another test that wasn't using a proper UUID
davemurphysf Nov 2, 2023
977ba01
Move UUID validation from `process` step to `prepare` step
davemurphysf Oct 31, 2023
197a5da
Move uuid validation all the way up to `populateTeamData`
davemurphysf Nov 2, 2023
f5492fc
Update tests to use `processEvent` instead of `eventsProcessor.proces…
davemurphysf Nov 2, 2023
45b94d2
Add temporary vscode config to just run relavent tests
davemurphysf Nov 2, 2023
c9c5449
Tests are fixed; was looking for an event that won't be produced
davemurphysf Nov 2, 2023
5d6f91f
Remove temp vscode debug config
davemurphysf Nov 2, 2023
591fa2b
nit: remove added newline
davemurphysf Nov 2, 2023
62766d6
Fix another test that wasn't using a proper UUID
davemurphysf Nov 2, 2023
dccd0f5
Merge branch 'dave/move-uuid-check' of https://github.com/PostHog/pos…
Nov 7, 2023
ac34983
Add missing import after git stupidity
Nov 7, 2023
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
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
import { PluginEvent } from '@posthog/plugin-scaffold'
import { PreIngestionEvent } from 'types'

import { UUID } from '../../../utils/utils'
import { parseEventTimestamp } from '../timestamps'
import { captureIngestionWarning } from '../utils'
import { EventPipelineRunner } from './runner'

export async function prepareEventStep(runner: EventPipelineRunner, event: PluginEvent): Promise<PreIngestionEvent> {
const { team_id, uuid } = event
const { db } = runner.hub
davemurphysf marked this conversation as resolved.
Show resolved Hide resolved

if (!UUID.validateString(uuid, false)) {
await captureIngestionWarning(db, team_id, 'skipping_event_invalid_uuid', {
eventUuid: JSON.stringify(uuid),
})
throw new Error(`Not a valid UUID: "${uuid}"`)
}

const tsParsingIngestionWarnings: Promise<void>[] = []
const invalidTimestampCallback = function (type: string, details: Record<string, any>) {
// TODO: make that metric name more generic when transitionning to prometheus
Expand Down
9 changes: 1 addition & 8 deletions plugin-server/src/worker/ingestion/process-event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ import { elementsToString, extractElements } from '../../utils/db/elements-chain
import { KafkaProducerWrapper } from '../../utils/db/kafka-producer-wrapper'
import { safeClickhouseString, sanitizeEventName, timeoutGuard } from '../../utils/db/utils'
import { status } from '../../utils/status'
import { castTimestampOrNow, UUID } from '../../utils/utils'
import { castTimestampOrNow } from '../../utils/utils'
import { GroupTypeManager } from './group-type-manager'
import { addGroupProperties } from './groups'
import { upsertGroup } from './properties-updater'
import { PropertyDefinitionsManager } from './property-definitions-manager'
import { TeamManager } from './team-manager'
import { captureIngestionWarning } from './utils'

export class EventsProcessor {
pluginsServer: Hub
Expand Down Expand Up @@ -66,12 +65,6 @@ export class EventsProcessor {
timestamp: DateTime,
eventUuid: string
): Promise<PreIngestionEvent> {
if (!UUID.validateString(eventUuid, false)) {
await captureIngestionWarning(this.db, teamId, 'skipping_event_invalid_uuid', {
eventUuid: JSON.stringify(eventUuid),
})
throw new Error(`Not a valid UUID: "${eventUuid}"`)
}
const singleSaveTimer = new Date()
const timeout = timeoutGuard('Still inside "EventsProcessor.processEvent". Timeout warning after 30 sec!', {
event: JSON.stringify(data),
Expand Down
Loading