Skip to content

Commit

Permalink
chore: env for webhooks timeouts (#17779)
Browse files Browse the repository at this point in the history
  • Loading branch information
tiina303 authored Oct 5, 2023
1 parent 5256e51 commit 0508730
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 9 deletions.
1 change: 1 addition & 0 deletions plugin-server/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ export function getDefaultConfig(): PluginsServerConfig {
MAX_TEAM_ID_TO_BUFFER_ANONYMOUS_EVENTS_FOR: 0,
USE_KAFKA_FOR_SCHEDULED_TASKS: true,
CLOUD_DEPLOYMENT: null,
EXTERNAL_REQUEST_TIMEOUT_MS: 10 * 1000, // 10 seconds

STARTUP_PROFILE_DURATION_SECONDS: 300, // 5 minutes
STARTUP_PROFILE_CPU: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ export const startAsyncWebhooksHandlerConsumer = async ({
organizationManager,
new Set(serverConfig.FETCH_HOSTNAME_GUARD_TEAMS.split(',').filter(String).map(Number)),
appMetrics,
statsd
statsd,
serverConfig.EXTERNAL_REQUEST_TIMEOUT_MS
)
const concurrency = serverConfig.TASKS_PER_WORKER || 20

Expand Down
1 change: 1 addition & 0 deletions plugin-server/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ export interface PluginsServerConfig {
EVENT_OVERFLOW_BUCKET_REPLENISH_RATE: number
/** Label of the PostHog Cloud environment. Null if not running PostHog Cloud. @example 'US' */
CLOUD_DEPLOYMENT: string | null
EXTERNAL_REQUEST_TIMEOUT_MS: number

// dump profiles to disk, covering the first N seconds of runtime
STARTUP_PROFILE_DURATION_SECONDS: number
Expand Down
6 changes: 4 additions & 2 deletions plugin-server/src/worker/ingestion/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,15 +263,16 @@ export class HookCommander {
fetchHostnameGuardTeams: Set<number> | null

/** Hook request timeout in ms. */
EXTERNAL_REQUEST_TIMEOUT = 10 * 1000
EXTERNAL_REQUEST_TIMEOUT: number

constructor(
postgres: PostgresRouter,
teamManager: TeamManager,
organizationManager: OrganizationManager,
fetchHostnameGuardTeams: Set<number> | null = new Set(),
appMetrics: AppMetrics,
statsd: StatsD | undefined
statsd: StatsD | undefined,
timeout: number
) {
this.postgres = postgres
this.teamManager = teamManager
Expand All @@ -285,6 +286,7 @@ export class HookCommander {
}
this.statsd = statsd
this.appMetrics = appMetrics
this.EXTERNAL_REQUEST_TIMEOUT = timeout
}

public async findAndFireHooks(event: PostIngestionEvent, actionMatches: Action[]): Promise<void> {
Expand Down
6 changes: 5 additions & 1 deletion plugin-server/tests/main/ingestion-queues/each-batch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,11 @@ describe('eachBatchX', () => {
const hookCannon = new HookCommander(
queue.pluginsServer.postgres,
queue.pluginsServer.teamManager,
queue.pluginsServer.organizationManager
queue.pluginsServer.organizationManager,
new Set(),
queue.pluginsServer.appMetrics,
undefined,
queue.pluginsServer.EXTERNAL_REQUEST_TIMEOUT_MS
)
const matchSpy = jest.spyOn(actionMatcher, 'match')
// mock hasWebhooks to return true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,15 @@ describe('Event Pipeline integration test', () => {
actionManager = new ActionManager(hub.db.postgres)
await actionManager.prepare()
actionMatcher = new ActionMatcher(hub.db.postgres, actionManager)
hookCannon = new HookCommander(hub.db.postgres, hub.teamManager, hub.organizationManager)
hookCannon = new HookCommander(
hub.db.postgres,
hub.teamManager,
hub.organizationManager,
new Set(hub.FETCH_HOSTNAME_GUARD_TEAMS.split(',').filter(String).map(Number)),
hub.appMetrics,
undefined,
hub.EXTERNAL_REQUEST_TIMEOUT_MS
)

jest.spyOn(hub.db, 'fetchPerson')
jest.spyOn(hub.db, 'createPerson')
Expand Down
9 changes: 5 additions & 4 deletions plugin-server/tests/worker/ingestion/hooks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,9 @@ describe('hooks', () => {
{} as any,
{} as any,
new Set([hook.team_id]), // Hostname guard enabled
// mock object with queueError function as no-op
{ queueError: () => Promise.resolve(), queueMetric: () => Promise.resolve() } as AppMetrics
{ queueError: () => Promise.resolve(), queueMetric: () => Promise.resolve() } as unknown as AppMetrics,
undefined,
20000
)
})

Expand All @@ -517,7 +518,7 @@ describe('hooks', () => {
),
headers: { 'Content-Type': 'application/json' },
method: 'POST',
timeout: 10000,
timeout: 20000,
})
})

Expand Down Expand Up @@ -556,7 +557,7 @@ describe('hooks', () => {
),
headers: { 'Content-Type': 'application/json' },
method: 'POST',
timeout: 10000,
timeout: 20000,
})
})

Expand Down

0 comments on commit 0508730

Please sign in to comment.