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

feat: allow config of before_send function to edit or reject events #1515

Merged
merged 31 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
79b83fa
first bit of type fangling
pauldambra Nov 8, 2024
5165110
draw the rest of the owl
pauldambra Nov 8, 2024
19abae0
Refactor tests
pauldambra Nov 8, 2024
e95364c
fiddle
pauldambra Nov 8, 2024
08b6741
kind to IE11
pauldambra Nov 8, 2024
54d3031
Add some prebuilt functions
pauldambra Nov 10, 2024
070413d
Refactor
pauldambra Nov 10, 2024
2d7b7ed
mock random
pauldambra Nov 11, 2024
4940a23
more events are uneditable
pauldambra Nov 11, 2024
4bac476
rename the function
pauldambra Nov 11, 2024
0efff91
failing events
pauldambra Nov 11, 2024
31244a1
reorg things
pauldambra Nov 11, 2024
db0228b
test doesn't make sense undefined is valid for
pauldambra Nov 11, 2024
d20d0b7
maybe just a warning
pauldambra Nov 11, 2024
56fb955
some more can be unsafe
pauldambra Nov 12, 2024
ba4f6ca
this is unsafe but editable too
pauldambra Nov 12, 2024
4f5b308
this is unsafe but editable too
pauldambra Nov 12, 2024
4afa6c8
ok, process it all :see-no-evil:
pauldambra Nov 13, 2024
3c018f2
Remove out-of-date tests
pauldambra Nov 13, 2024
938a162
deprecate and stop using _onCapture
pauldambra Nov 13, 2024
0cdbd19
oops
pauldambra Nov 13, 2024
aa6df59
ooops
pauldambra Nov 13, 2024
a395047
merge branch 'main' into feat/before-capture
pauldambra Nov 15, 2024
8c76336
sampling example changes
pauldambra Nov 15, 2024
3a4b136
make before_send config explicit
pauldambra Nov 15, 2024
371f90a
allow array of functions
pauldambra Nov 15, 2024
96b2356
A little comment
pauldambra Nov 15, 2024
a44429e
corrected thresholds tracking
pauldambra Nov 15, 2024
482cfa8
reorganise
pauldambra Nov 15, 2024
3eb6d97
Merge branch 'main' into feat/before-capture
pauldambra Nov 18, 2024
d190358
Merge branch 'main' into feat/before-capture
pauldambra Nov 19, 2024
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
125 changes: 125 additions & 0 deletions src/__tests__/posthog-core.beforeCapture.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { uuidv7 } from '../uuidv7'
import { defaultPostHog } from './helpers/posthog-instance'
import { logger } from '../utils/logger'
import { CaptureResult, knownUnEditableEvent, knownUnsafeEditableEvent, PostHogConfig } from '../types'
import { PostHog } from '../posthog-core'

jest.mock('../utils/logger')

const rejectingEventFn = () => {
return null
}

const editingEventFn = (captureResult: CaptureResult): CaptureResult => {
return {
...captureResult,
properties: {
...captureResult.properties,
edited: true,
},
$set: {
...captureResult.$set,
edited: true,
},
}
}

describe('posthog core - before capture', () => {
const baseUTCDateTime = new Date(Date.UTC(2020, 0, 1, 0, 0, 0))
const eventName = '$event'

const posthogWith = (configOverride: Pick<Partial<PostHogConfig>, 'beforeCapture'>): PostHog => {
const posthog = defaultPostHog().init('testtoken', configOverride, uuidv7())
return Object.assign(posthog, {
_send_request: jest.fn(),
})
}

beforeEach(() => {
jest.useFakeTimers().setSystemTime(baseUTCDateTime)
})

afterEach(() => {
jest.useRealTimers()
})

it('can reject an event', () => {
const posthog = posthogWith({
beforeCapture: rejectingEventFn,
})
;(posthog._send_request as jest.Mock).mockClear()

const capturedData = posthog.capture(eventName, {}, {})

expect(capturedData).toBeUndefined()
expect(posthog._send_request).not.toHaveBeenCalled()
expect(jest.mocked(logger).info).toHaveBeenCalledWith(
`Event '${eventName}' was rejected in beforeCapture function`
)
})

it('can edit an event', () => {
const posthog = posthogWith({
beforeCapture: editingEventFn,
})
;(posthog._send_request as jest.Mock).mockClear()

const capturedData = posthog.capture(eventName, {}, {})

expect(capturedData).toHaveProperty(['properties', 'edited'], true)
expect(capturedData).toHaveProperty(['$set', 'edited'], true)
expect(posthog._send_request).toHaveBeenCalledWith({
batchKey: undefined,
callback: expect.any(Function),
compression: 'best-available',
data: capturedData,
method: 'POST',
url: 'https://us.i.posthog.com/e/',
})
})

it('cannot reject an un-editable event', () => {
const posthog = posthogWith({
beforeCapture: rejectingEventFn,
})
;(posthog._send_request as jest.Mock).mockClear()
// chooses a random string from knownUnEditableEvent
const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)]

const capturedData = posthog.capture(randomUneditableEvent, {}, {})

expect(capturedData).not.toBeUndefined()
expect(posthog._send_request).toHaveBeenCalled()
})

it('cannot edit an un-editable event', () => {
const posthog = posthogWith({
beforeCapture: editingEventFn,
})
;(posthog._send_request as jest.Mock).mockClear()
// chooses a random string from knownUnEditableEvent
const randomUneditableEvent = knownUnEditableEvent[Math.floor(Math.random() * knownUnEditableEvent.length)]

const capturedData = posthog.capture(randomUneditableEvent, {}, {})

expect(capturedData).not.toHaveProperty(['properties', 'edited'])
expect(capturedData).not.toHaveProperty(['$set', 'edited'])
expect(posthog._send_request).toHaveBeenCalled()
})

it('logs a warning when rejecting an unsafe to edit event', () => {
const posthog = posthogWith({
beforeCapture: rejectingEventFn,
})
;(posthog._send_request as jest.Mock).mockClear()
// chooses a random string from knownUnEditableEvent
const randomUnsafeEditableEvent =
knownUnsafeEditableEvent[Math.floor(Math.random() * knownUnsafeEditableEvent.length)]

posthog.capture(randomUnsafeEditableEvent, {}, {})

expect(jest.mocked(logger).warn).toHaveBeenCalledWith(
`Event '${randomUnsafeEditableEvent}' was rejected in beforeCapture function. This can cause unexpected behavior.`
)
})
})
57 changes: 57 additions & 0 deletions src/__tests__/utils/before-capture-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { sampleByDistinctId, sampleByEvent, sampleBySessionId } from '../../utils/before-capture.utils'
import { CaptureResult } from '../../types'
import { isNull } from '../../utils/type-utils'

function expectRoughlyFiftyPercent(emittedEvents: any[]) {
expect(emittedEvents.length).toBeGreaterThanOrEqual(40)
expect(emittedEvents.length).toBeLessThanOrEqual(60)
}

describe('before capture utils', () => {
it('can sample by event name', () => {
const sampleFn = sampleByEvent(['$autocapture'], 50)
const results = []
Array.from({ length: 100 }).forEach(() => {
const captureResult = { event: '$autocapture' } as unknown as CaptureResult
results.push(sampleFn(captureResult))
})
const emittedEvents = results.filter((r) => !isNull(r))
expectRoughlyFiftyPercent(emittedEvents)
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
})

it('can sample by distinct id', () => {
const sampleFn = sampleByDistinctId(50)
const results = []
const distinct_id_one = 'user-1'
const distinct_id_two = 'user-that-hashes-to-no-events'
Array.from({ length: 100 }).forEach(() => {
;[distinct_id_one, distinct_id_two].forEach((distinct_id) => {
const captureResult = { properties: { distinct_id } } as unknown as CaptureResult
results.push(sampleFn(captureResult))
})
})
const distinctIdOneEvents = results.filter((r) => !isNull(r) && r.properties.distinct_id === distinct_id_one)
const distinctIdTwoEvents = results.filter((r) => !isNull(r) && r.properties.distinct_id === distinct_id_two)

expect(distinctIdOneEvents.length).toBe(100)
expect(distinctIdTwoEvents.length).toBe(0)
})

it('can sample by session id', () => {
const sampleFn = sampleBySessionId(50)
const results = []
const session_id_one = 'a-session-id'
const session_id_two = 'id-that-hashes-to-not-sending-events'
Array.from({ length: 100 }).forEach(() => {
;[session_id_one, session_id_two].forEach((session_id) => {
const captureResult = { properties: { $session_id: session_id } } as unknown as CaptureResult
results.push(sampleFn(captureResult))
})
})
const sessionIdOneEvents = results.filter((r) => !isNull(r) && r.properties.$session_id === session_id_one)
const sessionIdTwoEvents = results.filter((r) => !isNull(r) && r.properties.$session_id === session_id_two)

expect(sessionIdOneEvents.length).toBe(100)
expect(sessionIdTwoEvents.length).toBe(0)
})
})
6 changes: 2 additions & 4 deletions src/autocapture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
splitClassString,
} from './autocapture-utils'
import RageClick from './extensions/rageclick'
import { AutocaptureConfig, DecideResponse, Properties } from './types'
import { AutocaptureConfig, COPY_AUTOCAPTURE_EVENT, DecideResponse, EventName, Properties } from './types'
import { PostHog } from './posthog-core'
import { AUTOCAPTURE_DISABLED_SERVER_SIDE } from './constants'

Expand All @@ -25,8 +25,6 @@ import { document, window } from './utils/globals'
import { convertToURL } from './utils/request-utils'
import { isDocumentFragment, isElementNode, isTag, isTextNode } from './utils/element-utils'

const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture'

function limitText(length: number, text: string): string {
if (text.length > length) {
return text.slice(0, length) + '...'
Expand Down Expand Up @@ -343,7 +341,7 @@ export class Autocapture {
return !disabledClient && !disabledServer
}

private _captureEvent(e: Event, eventName = '$autocapture'): boolean | void {
private _captureEvent(e: Event, eventName: EventName = '$autocapture'): boolean | void {
if (!this.isEnabled) {
return
}
Expand Down
29 changes: 27 additions & 2 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
Compression,
DecideResponse,
EarlyAccessFeatureCallback,
EventName,
IsFeatureEnabledOptions,
JsonType,
PostHogConfig,
Expand All @@ -58,6 +59,9 @@ import {
isEmptyObject,
isEmptyString,
isFunction,
isKnownUnEditableEvent,
isKnownUnsafeEditableEvent,
isNullish,
isNumber,
isObject,
isString,
Expand Down Expand Up @@ -181,6 +185,8 @@ export const defaultConfig = (): PostHogConfig => ({
session_idle_timeout_seconds: 30 * 60, // 30 minutes
person_profiles: 'identified_only',
__add_tracing_headers: false,
// by default the before capture fn is the identity function
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
beforeCapture: (x) => x,
})

export const configRenames = (origConfig: Partial<PostHogConfig>): Partial<PostHogConfig> => {
Expand Down Expand Up @@ -787,7 +793,11 @@ export class PostHog {
* @param {String} [config.transport] Transport method for network request ('XHR' or 'sendBeacon').
* @param {Date} [config.timestamp] Timestamp is a Date object. If not set, it'll automatically be set to the current time.
*/
capture(event_name: string, properties?: Properties | null, options?: CaptureOptions): CaptureResult | undefined {
capture(
event_name: EventName,
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
properties?: Properties | null,
options?: CaptureOptions
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
): CaptureResult | undefined {
// While developing, a developer might purposefully _not_ call init(),
// in this case, we would like capture to be a noop.
if (!this.__loaded || !this.persistence || !this.sessionPersistence || !this._requestQueue) {
Expand Down Expand Up @@ -870,6 +880,21 @@ export class PostHog {
this.setPersonPropertiesForFlags(finalSet)
}

if (!isKnownUnEditableEvent(data.event)) {
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
const beforeCaptureResult = this.config.beforeCapture(data)
if (isNullish(beforeCaptureResult)) {
const logMessage = `Event '${data.event}' was rejected in beforeCapture function`
if (isKnownUnsafeEditableEvent(data.event)) {
logger.warn(`${logMessage}. This can cause unexpected behavior.`)
} else {
logger.info(logMessage)
}
return
} else {
data = beforeCaptureResult
}
}

this._internalEventEmitter.emit('eventCaptured', data)

const requestOptions: QueuedRequestOptions = {
Expand Down Expand Up @@ -2040,7 +2065,7 @@ export class PostHog {
* @param {Object} [config.capture_properties] Set of properties to be captured along with the opt-in action
*/
opt_in_capturing(options?: {
captureEventName?: string | null | false /** event name to be used for capturing the opt-in action */
captureEventName?: EventName | null | false /** event name to be used for capturing the opt-in action */
captureProperties?: Properties /** set of properties to be captured along with the opt-in action */
}): void {
this.consent.optInOut(true)
Expand Down
65 changes: 64 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,68 @@ import type { SegmentAnalytics } from './extensions/segment-integration'
export type Property = any
export type Properties = Record<string, Property>

export const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture'

export const knownUnEditableEvent = [
'$web_experiment_applied',
'$$client_ingestion_warning',
'$feature_enrollment_update',
'$feature_flag_called',
'survey dismissed',
'survey sent',
'survey shown',
'$snapshot',
] as const

/**
* These events are not processed by the `beforeCapture` function
*/
export type KnownUnEditableEvent = typeof knownUnEditableEvent[number]

export const knownUnsafeEditableEvent = [
'$pageview',
'$pageleave',
'$identify',
'$set',
'$groupidentify',
'$create_alias',
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
] as const

/**
* These events can be processed by the `beforeCapture` function
* but can cause unexpected confusion in data.
*
* Some features of PostHog rely on receiving 100% of these events
*/
export type KnownUnsafeEditableEvent = typeof knownUnsafeEditableEvent[number]

/**
* These are known events PostHog events that can be processed by the `beforeCapture` function
* That means PostHog functionality does not rely on receiving 100% of these for calculations
* So, it is safe to sample them to reduce the volume of events sent to PostHog
*/
export type KnownEventName =
| '$heatmaps_data'
| '$opt_in'
| '$exception'
| '$$heatmap'
| '$web_vitals'
| '$dead_click'
| '$autocapture'
| typeof COPY_AUTOCAPTURE_EVENT
| '$rageclick'

export type EventName =
| KnownUnEditableEvent
| KnownUnsafeEditableEvent
| KnownEventName
// magic value so that the type of event name is a set of known strings or any other strings
// which means you get autocomplete for known strings
| (string & {})

export interface CaptureResult {
uuid: string
event: string
event: EventName
properties: Properties
$set?: Properties
$set_once?: Properties
Expand Down Expand Up @@ -214,7 +273,11 @@ export interface PostHogConfig {
feature_flag_request_timeout_ms: number
get_device_id: (uuid: string) => string
name: string
/**
* this is a read-only function that can be used to react to event capture
*/
_onCapture: (eventName: string, eventData: CaptureResult) => void
beforeCapture: (cr: CaptureResult) => CaptureResult | null
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
capture_performance?: boolean | PerformanceCaptureConfig
// Should only be used for testing. Could negatively impact performance.
disable_compression: boolean
Expand Down
Loading
Loading