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 19 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
143 changes: 143 additions & 0 deletions src/__tests__/posthog-core.beforeSend.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import { uuidv7 } from '../uuidv7'
import { defaultPostHog } from './helpers/posthog-instance'
import { logger } from '../utils/logger'
import { CaptureResult, 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 send', () => {
const baseUTCDateTime = new Date(Date.UTC(2020, 0, 1, 0, 0, 0))
const eventName = '$event'

const posthogWith = (configOverride: Pick<Partial<PostHogConfig>, 'before_send'>): 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({
before_send: 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 beforeSend function`
)
})

it('can edit an event', () => {
const posthog = posthogWith({
before_send: 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('can sanitize $set event', () => {
const posthog = posthogWith({
before_send: (cr) => {
cr.$set = { value: 'edited' }
return cr
},
})
;(posthog._send_request as jest.Mock).mockClear()

const capturedData = posthog.capture('$set', {}, { $set: { value: 'provided' } })

expect(capturedData).toHaveProperty(['$set', 'value'], 'edited')
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('warned when making arbitrary event invalid', () => {
const posthog = posthogWith({
before_send: (cr) => {
cr.properties = undefined
return cr
},
})
;(posthog._send_request as jest.Mock).mockClear()

const capturedData = posthog.capture(eventName, { value: 'provided' }, {})

expect(capturedData).not.toHaveProperty(['properties', 'value'], 'provided')
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/',
})
expect(jest.mocked(logger).warn).toHaveBeenCalledWith(
`Event '${eventName}' has no properties after beforeSend function, this is likely an error.`
)
})

it('logs a warning when rejecting an unsafe to edit event', () => {
const posthog = posthogWith({
before_send: 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 beforeSend function. This can cause unexpected behavior.`
)
})
})
64 changes: 64 additions & 0 deletions src/__tests__/utils/before-send-utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { CaptureResult } from '../../types'
import { isNull } from '../../utils/type-utils'
import { sampleByDistinctId, sampleByEvent, sampleBySessionId } from '../../customizations/before-send'

beforeAll(() => {
let fiftyFiftyRandom = true
Math.random = () => {
const val = fiftyFiftyRandom ? 0.48 : 0.51
fiftyFiftyRandom = !fiftyFiftyRandom
return val
}
})

describe('before send 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))

// random is mocked so that it alternates between 0.48 and 0.51
expect(emittedEvents.length).toBe(50)
})

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
69 changes: 69 additions & 0 deletions src/customizations/before-send.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { clampToRange } from '../utils/number-utils'
import { CaptureResult, KnownEventName } from '../types'
import { includes } from '../utils'

function simpleHash(str: string) {
let hash = 0
for (let i = 0; i < str.length; i++) {
hash = (hash << 5) - hash + str.charCodeAt(i) // (hash * 31) + char code
hash |= 0 // Convert to 32bit integer
}
return Math.abs(hash)
}

function sampleOnProperty(prop: string, percent: number): boolean {
return simpleHash(prop) % 100 < clampToRange(percent, 0, 100)
}

/**
* Provides an implementation of sampling that samples based on the distinct ID.
* Using the provided percentage.
* Can be used to create a beforeCapture fn for a PostHog instance.
*
* Causes roughly 50% of distinct ids to have events sent.
* Not 50% of events for each distinct id.
*
* @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
*/
export function sampleByDistinctId(percent: number): (c: CaptureResult) => CaptureResult | null {
return (captureResult: CaptureResult): CaptureResult | null => {
return sampleOnProperty(captureResult.properties.distinct_id, percent) ? captureResult : null
}
}

/**
* Provides an implementation of sampling that samples based on the session ID.
* Using the provided percentage.
* Can be used to create a beforeCapture fn for a PostHog instance.
*
* Causes roughly 50% of sessions to have events sent.
* Not 50% of events for each session.
*
* @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event
*/
export function sampleBySessionId(percent: number): (c: CaptureResult) => CaptureResult | null {
return (captureResult: CaptureResult): CaptureResult | null => {
return sampleOnProperty(captureResult.properties.$session_id, percent) ? captureResult : null
}
}

/**
* Provides an implementation of sampling that samples based on the event name.
* Using the provided percentage.
* Can be used to create a beforeCapture fn for a PostHog instance.
*
* @param eventNames an array of event names to sample, sampling is applied across events not per event name
* @param percent a number from 0 to 100, 100 means never sample, 0 means never send the event
*/
export function sampleByEvent(
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
eventNames: KnownEventName[],
percent: number
): (c: CaptureResult) => CaptureResult | null {
return (captureResult: CaptureResult): CaptureResult | null => {
if (!includes(eventNames, captureResult.event)) {
return captureResult
}

return Math.random() * 100 < clampToRange(percent, 0, 100) ? captureResult : null
}
}
32 changes: 30 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,8 @@ import {
isEmptyObject,
isEmptyString,
isFunction,
isKnownUnsafeEditableEvent,
isNullish,
isNumber,
isObject,
isString,
Expand Down Expand Up @@ -181,6 +184,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
before_send: (x) => x,
})

export const configRenames = (origConfig: Partial<PostHogConfig>): Partial<PostHogConfig> => {
Expand Down Expand Up @@ -787,7 +792,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 +879,25 @@ export class PostHog {
this.setPersonPropertiesForFlags(finalSet)
}

const beforeSendResult = this.config.before_send(data)
pauldambra marked this conversation as resolved.
Show resolved Hide resolved
if (isNullish(beforeSendResult)) {
const logMessage = `Event '${data.event}' was rejected in beforeSend function`
if (isKnownUnsafeEditableEvent(data.event)) {
logger.warn(`${logMessage}. This can cause unexpected behavior.`)
} else {
logger.info(logMessage)
}
// the event is now null so we don't send it
return
} else {
if (!beforeSendResult.properties || isEmptyObject(beforeSendResult.properties)) {
logger.warn(
`Event '${data.event}' has no properties after beforeSend function, this is likely an error.`
)
}
data = beforeSendResult
}

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

const requestOptions: QueuedRequestOptions = {
Expand Down Expand Up @@ -2040,7 +2068,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
Loading
Loading