From 53b600d37e49a8bae8e0e1bab77a0d2f3387fdde Mon Sep 17 00:00:00 2001 From: David Newell Date: Tue, 19 Nov 2024 16:04:18 +0000 Subject: [PATCH] chore: plugin-server updates for exception handling (#26276) --- .../enrichExceptionEventStep.ts | 83 --------- ...roduceExceptionSymbolificationEventStep.ts | 2 +- .../worker/ingestion/event-pipeline/runner.ts | 9 +- .../__snapshots__/runner.test.ts.snap | 16 -- .../enrichExceptionEventStep.test.ts | 161 ------------------ .../ingestion/event-pipeline/runner.test.ts | 5 +- 6 files changed, 3 insertions(+), 273 deletions(-) delete mode 100644 plugin-server/src/worker/ingestion/event-pipeline/enrichExceptionEventStep.ts delete mode 100644 plugin-server/tests/worker/ingestion/event-pipeline/enrichExceptionEventStep.test.ts diff --git a/plugin-server/src/worker/ingestion/event-pipeline/enrichExceptionEventStep.ts b/plugin-server/src/worker/ingestion/event-pipeline/enrichExceptionEventStep.ts deleted file mode 100644 index 909e3f291bd4e..0000000000000 --- a/plugin-server/src/worker/ingestion/event-pipeline/enrichExceptionEventStep.ts +++ /dev/null @@ -1,83 +0,0 @@ -import { captureException } from '@sentry/node' -import { Counter } from 'prom-client' - -import { PreIngestionEvent } from '../../../types' -import { EventPipelineRunner } from './runner' - -const EXTERNAL_FINGERPRINT_COUNTER = new Counter({ - name: 'enrich_exception_events_external_fingerprint', - help: 'Counter for exceptions that already have a fingerprint', -}) - -const COULD_NOT_PARSE_STACK_TRACE_COUNTER = new Counter({ - name: 'enrich_exception_events_could_not_parse_stack_trace', - help: 'Counter for exceptions where the stack trace could not be parsed', -}) - -const COULD_NOT_PREPARE_FOR_FINGERPRINTING_COUNTER = new Counter({ - name: 'enrich_exception_events_could_not_prepare_for_fingerprinting', - help: 'Counter for exceptions where the event could not be prepared for fingerprinting', -}) - -const EXCEPTIONS_ENRICHED_COUNTER = new Counter({ - name: 'enrich_exception_events_enriched', - help: 'Counter for exceptions that have been enriched', -}) - -export function enrichExceptionEventStep( - _runner: EventPipelineRunner, - event: PreIngestionEvent -): Promise { - if (event.event !== '$exception') { - return Promise.resolve(event) - } - - let type: string | null = null - let message: string | null = null - let firstFunction: string | null = null - let exceptionList: any[] | null = null - - try { - exceptionList = event.properties['$exception_list'] - const fingerPrint = event.properties['$exception_fingerprint'] - type = event.properties['$exception_type'] - message = event.properties['$exception_message'] - - if (!type && exceptionList && exceptionList.length > 0) { - type = exceptionList[0].type - } - if (!message && exceptionList && exceptionList.length > 0) { - message = exceptionList[0].value - } - - if (fingerPrint) { - EXTERNAL_FINGERPRINT_COUNTER.inc() - return Promise.resolve(event) - } - } catch (e) { - captureException(e) - COULD_NOT_PREPARE_FOR_FINGERPRINTING_COUNTER.inc() - } - - try { - if (exceptionList && exceptionList.length > 0) { - const firstException = exceptionList[0] - if (firstException.stacktrace) { - // TODO: Should this be the last function instead?, or first in app function? - firstFunction = firstException.stacktrace.frames[0].function - } - } - } catch (e) { - captureException(e) - COULD_NOT_PARSE_STACK_TRACE_COUNTER.inc() - } - - const fingerprint = [type, message, firstFunction].filter(Boolean) - event.properties['$exception_fingerprint'] = fingerprint.length ? fingerprint : undefined - - if (event.properties['$exception_fingerprint']) { - EXCEPTIONS_ENRICHED_COUNTER.inc() - } - - return Promise.resolve(event) -} diff --git a/plugin-server/src/worker/ingestion/event-pipeline/produceExceptionSymbolificationEventStep.ts b/plugin-server/src/worker/ingestion/event-pipeline/produceExceptionSymbolificationEventStep.ts index d0f3712d8aeb0..7d2b6682c5808 100644 --- a/plugin-server/src/worker/ingestion/event-pipeline/produceExceptionSymbolificationEventStep.ts +++ b/plugin-server/src/worker/ingestion/event-pipeline/produceExceptionSymbolificationEventStep.ts @@ -9,7 +9,7 @@ export function produceExceptionSymbolificationEventStep( const ack = runner.hub.kafkaProducer .produce({ topic: runner.hub.EXCEPTIONS_SYMBOLIFICATION_KAFKA_TOPIC, - key: event.uuid, + key: String(event.team_id), value: Buffer.from(JSON.stringify(event)), waitForAck: true, }) diff --git a/plugin-server/src/worker/ingestion/event-pipeline/runner.ts b/plugin-server/src/worker/ingestion/event-pipeline/runner.ts index 157c5fc252d4d..25b3d77d128b3 100644 --- a/plugin-server/src/worker/ingestion/event-pipeline/runner.ts +++ b/plugin-server/src/worker/ingestion/event-pipeline/runner.ts @@ -12,7 +12,6 @@ import { EventsProcessor } from '../process-event' import { captureIngestionWarning, generateEventDeadLetterQueueMessage } from '../utils' import { createEventStep } from './createEventStep' import { emitEventStep } from './emitEventStep' -import { enrichExceptionEventStep } from './enrichExceptionEventStep' import { extractHeatmapDataStep } from './extractHeatmapDataStep' import { eventProcessedAndIngestedCounter, @@ -254,15 +253,9 @@ export class EventPipelineRunner { heatmapKafkaAcks.forEach((ack) => kafkaAcks.push(ack)) } - const enrichedIfErrorEvent = await this.runStep( - enrichExceptionEventStep, - [this, preparedEventWithoutHeatmaps], - event.team_id - ) - const rawEvent = await this.runStep( createEventStep, - [this, enrichedIfErrorEvent, person, processPerson], + [this, preparedEventWithoutHeatmaps, person, processPerson], event.team_id ) diff --git a/plugin-server/tests/worker/ingestion/event-pipeline/__snapshots__/runner.test.ts.snap b/plugin-server/tests/worker/ingestion/event-pipeline/__snapshots__/runner.test.ts.snap index 37a9ace98cd74..f4dc48882a622 100644 --- a/plugin-server/tests/worker/ingestion/event-pipeline/__snapshots__/runner.test.ts.snap +++ b/plugin-server/tests/worker/ingestion/event-pipeline/__snapshots__/runner.test.ts.snap @@ -96,22 +96,6 @@ Array [ }, ], ], - Array [ - "enrichExceptionEventStep", - Array [ - Object { - "distinctId": "my_id", - "elementsList": Array [], - "event": "$pageview", - "eventUuid": "uuid1", - "ip": "127.0.0.1", - "projectId": 1, - "properties": Object {}, - "teamId": 2, - "timestamp": "2020-02-23T02:15:00.000Z", - }, - ], - ], Array [ "createEventStep", Array [ diff --git a/plugin-server/tests/worker/ingestion/event-pipeline/enrichExceptionEventStep.test.ts b/plugin-server/tests/worker/ingestion/event-pipeline/enrichExceptionEventStep.test.ts deleted file mode 100644 index 1f007b1aff57c..0000000000000 --- a/plugin-server/tests/worker/ingestion/event-pipeline/enrichExceptionEventStep.test.ts +++ /dev/null @@ -1,161 +0,0 @@ -import { ISOTimestamp, PreIngestionEvent } from '../../../../src/types' -import { cloneObject } from '../../../../src/utils/utils' -import { enrichExceptionEventStep } from '../../../../src/worker/ingestion/event-pipeline/enrichExceptionEventStep' - -jest.mock('../../../../src/worker/plugins/run') - -const DEFAULT_EXCEPTION_LIST = [ - { - mechanism: { - handled: true, - type: 'generic', - synthetic: false, - }, - stacktrace: { - frames: [ - { - colno: 220, - filename: 'https://app-static-prod.posthog.com/static/chunk-UFQKIDIH.js', - function: 'submitZendeskTicket', - in_app: true, - lineno: 25, - }, - ], - }, - type: 'Error', - value: 'There was an error creating the support ticket with zendesk.', - }, -] - -const preIngestionEvent: PreIngestionEvent = { - eventUuid: '018eebf3-cb48-750b-bfad-36409ea6f2b2', - event: '$exception', - distinctId: '018eebf3-79b1-7082-a7c6-eeb56a36002f', - properties: { - $current_url: 'http://localhost:3000/', - $host: 'localhost:3000', - $pathname: '/', - $viewport_height: 1328, - $viewport_width: 1071, - $device_id: '018eebf3-79b1-7082-a7c6-eeb56a36002f', - $session_id: '018eebf3-79cd-70da-895f-b6cf352bd688', - $window_id: '018eebf3-79cd-70da-895f-b6d09add936a', - }, - timestamp: '2024-04-17T12:06:46.861Z' as ISOTimestamp, - teamId: 1, - projectId: 1, -} - -describe('enrichExceptionEvent()', () => { - let runner: any - let event: PreIngestionEvent - - beforeEach(() => { - event = cloneObject(preIngestionEvent) - runner = { - hub: { - kafkaProducer: { - produce: jest.fn((e) => Promise.resolve(e)), - }, - }, - } - }) - - it('ignores non-exception events - even if they have a stack trace', async () => { - event.event = 'not_exception' - event.properties['$exception_list'] = DEFAULT_EXCEPTION_LIST - expect(event.properties['$exception_fingerprint']).toBeUndefined() - - const response = await enrichExceptionEventStep(runner, event) - expect(response).toBe(event) - }) - - it('use a fingerprint if it is present', async () => { - event.event = '$exception' - event.properties['$exception_list'] = DEFAULT_EXCEPTION_LIST - - event.properties['$exception_fingerprint'] = 'some-fingerprint' - - const response = await enrichExceptionEventStep(runner, event) - - expect(response.properties['$exception_fingerprint']).toBe('some-fingerprint') - }) - - it('uses the message and stack trace as the simplest grouping', async () => { - event.event = '$exception' - event.properties['$exception_message'] = 'some-message' - event.properties['$exception_list'] = DEFAULT_EXCEPTION_LIST - - const response = await enrichExceptionEventStep(runner, event) - - expect(response.properties['$exception_fingerprint']).toStrictEqual([ - 'Error', - 'some-message', - 'submitZendeskTicket', - ]) - }) - - it('includes type in stack grouping when present', async () => { - event.event = '$exception' - event.properties['$exception_message'] = 'some-message' - event.properties['$exception_list'] = DEFAULT_EXCEPTION_LIST - event.properties['$exception_type'] = 'UnhandledRejection' - - const response = await enrichExceptionEventStep(runner, event) - - expect(response.properties['$exception_fingerprint']).toStrictEqual([ - 'UnhandledRejection', - 'some-message', - 'submitZendeskTicket', - ]) - }) - - it('falls back to message and type when no stack trace', async () => { - event.event = '$exception' - event.properties['$exception_message'] = 'some-message' - event.properties['$exception_list'] = null - event.properties['$exception_type'] = 'UnhandledRejection' - - const response = await enrichExceptionEventStep(runner, event) - - expect(response.properties['$exception_fingerprint']).toStrictEqual(['UnhandledRejection', 'some-message']) - }) - - it('adds no fingerprint if no qualifying properties', async () => { - event.event = '$exception' - event.properties['$exception_message'] = null - event.properties['$exception_list'] = null - event.properties['$exception_type'] = null - - const response = await enrichExceptionEventStep(runner, event) - - expect(response.properties['$exception_fingerprint']).toBeUndefined() - }) - - it('uses exception_list to generate message, type, and fingerprint when not present', async () => { - event.event = '$exception' - event.properties['$exception_list'] = DEFAULT_EXCEPTION_LIST - - const response = await enrichExceptionEventStep(runner, event) - - expect(response.properties['$exception_fingerprint']).toStrictEqual([ - 'Error', - 'There was an error creating the support ticket with zendesk.', - 'submitZendeskTicket', - ]) - }) - - it('exception_type overrides exception_list to generate fingerprint when present', async () => { - event.event = '$exception' - event.properties['$exception_list'] = DEFAULT_EXCEPTION_LIST - event.properties['$exception_type'] = 'UnhandledRejection' - - const response = await enrichExceptionEventStep(runner, event) - - expect(response.properties['$exception_fingerprint']).toStrictEqual([ - 'UnhandledRejection', - 'There was an error creating the support ticket with zendesk.', - 'submitZendeskTicket', - ]) - }) -}) diff --git a/plugin-server/tests/worker/ingestion/event-pipeline/runner.test.ts b/plugin-server/tests/worker/ingestion/event-pipeline/runner.test.ts index 9a8576e540982..2fd02a018be5a 100644 --- a/plugin-server/tests/worker/ingestion/event-pipeline/runner.test.ts +++ b/plugin-server/tests/worker/ingestion/event-pipeline/runner.test.ts @@ -145,7 +145,6 @@ describe('EventPipelineRunner', () => { 'processPersonsStep', 'prepareEventStep', 'extractHeatmapDataStep', - 'enrichExceptionEventStep', 'createEventStep', 'emitEventStep', ]) @@ -175,7 +174,6 @@ describe('EventPipelineRunner', () => { 'processPersonsStep', 'prepareEventStep', 'extractHeatmapDataStep', - 'enrichExceptionEventStep', 'createEventStep', 'emitEventStep', ]) @@ -199,7 +197,7 @@ describe('EventPipelineRunner', () => { const result = await runner.runEventPipeline(pipelineEvent) expect(result.error).toBeUndefined() - expect(pipelineStepMsSummarySpy).toHaveBeenCalledTimes(9) + expect(pipelineStepMsSummarySpy).toHaveBeenCalledTimes(8) expect(pipelineLastStepCounterSpy).toHaveBeenCalledTimes(1) expect(eventProcessedAndIngestedCounterSpy).toHaveBeenCalledTimes(1) expect(pipelineStepMsSummarySpy).toHaveBeenCalledWith('emitEventStep') @@ -398,7 +396,6 @@ describe('EventPipelineRunner', () => { 'processPersonsStep', 'prepareEventStep', 'extractHeatmapDataStep', - 'enrichExceptionEventStep', 'createEventStep', 'emitEventStep', ])