chore(plugin-server): add prometheus metric to instrument()

PostHog · Dec 1, 2023 · 5b7385a · 5b7385a
1 parent e34465e
commit 5b7385a
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 29 deletions.
diff --git a/plugin-server/src/main/graphile-worker/graphile-worker.ts b/plugin-server/src/main/graphile-worker/graphile-worker.ts
@@ -94,10 +94,9 @@ export class GraphileWorker {
         await instrument(
             this.hub.statsd,
             {
-                metricName: 'job_queues_enqueue',
+                metricName: 'job_queues_enqueue_${jobName}',
                 key: instrumentationContext?.key ?? '?',
                 tag: instrumentationContext?.tag ?? '?',
-                tags: { jobName, type: jobType },
                 data: { timestamp: job.timestamp, type: jobType, payload: jobPayload },
             },
             enqueueFn

diff --git a/plugin-server/src/utils/metrics.ts b/plugin-server/src/utils/metrics.ts
@@ -1,5 +1,5 @@
 import { StatsD, Tags } from 'hot-shots'
-import { Histogram } from 'prom-client'
+import { Summary } from 'prom-client'
 
 import { runInSpan } from '../sentry'
 import { UUID } from './utils'
@@ -12,13 +12,7 @@ export function instrumentQuery<T>(
     tag: string | undefined,
     runQuery: () => Promise<T>
 ): Promise<T> {
-    const end = dataStoreQueryDuration
-        .labels({
-            query: metricName,
-            tag: tag ?? 'null',
-        })
-        .startTimer()
-    const result = instrument(
+    return instrument(
         statsd,
         {
             metricName,
@@ -27,8 +21,6 @@ export function instrumentQuery<T>(
         },
         runQuery
     )
-    end()
-    return result
 }
 
 export function instrument<T>(
@@ -37,12 +29,11 @@ export function instrument<T>(
         metricName: string
         key?: string
         tag?: string
-        tags?: Tags
         data?: any
     },
     runQuery: () => Promise<T>
 ): Promise<T> {
-    const tags: Tags | undefined = options.key ? { ...options.tags, [options.key]: options.tag! } : options.tags
+    const tags: Tags = options.key ? { [options.key]: options.tag! } : {}
     return runInSpan(
         {
             op: options.metricName,
@@ -56,6 +47,9 @@ export function instrument<T>(
                 return await runQuery()
             } finally {
                 statsd?.timing(options.metricName, timer, tags)
+                instrumentedFnSummary
+                    .labels(options.metricName, String(options.key ?? 'null'), String(options.tag ?? 'null'))
+                    .observe(Date.now() - timer.getTime())
             }
         }
     )
@@ -76,8 +70,9 @@ export function captureEventLoopMetrics(statsd: StatsD, instanceId: UUID): StopC
     }
 }
 
-export const dataStoreQueryDuration = new Histogram({
-    name: 'data_store_query_duration',
-    help: 'Query latency to data stores, per query and tag',
-    labelNames: ['query', 'tag'],
+const instrumentedFnSummary = new Summary({
+    name: 'instrumented_fn_duration_ms',
+    help: 'Duration of instrumented functions',
+    labelNames: ['metricName', 'key', 'tag'],
+    percentiles: [0.5, 0.9, 0.95, 0.99],
 })
diff --git a/plugin-server/src/utils/retries.ts b/plugin-server/src/utils/retries.ts
@@ -1,5 +1,4 @@
 import { RetryError } from '@posthog/plugin-scaffold'
-import { Counter } from 'prom-client'
 
 import { runInTransaction } from '../sentry'
 import { Hub } from '../types'
@@ -63,12 +62,6 @@ export type RetriableFunctionPayload = RetriableFunctionDefinition &
     Partial<RetryParams> &
     MetricsDefinition & { hub: Hub }
 
-const retryableFnCounter = new Counter({
-    name: 'retryable_fn_status',
-    help: 'Number of times a retriable function status changed',
-    labelNames: ['status', 'name'],
-})
-
 function iterateRetryLoop(retriableFunctionPayload: RetriableFunctionPayload, attempt = 1): Promise<void> {
     const {
         metricName,
@@ -112,8 +105,6 @@ function iterateRetryLoop(retriableFunctionPayload: RetriableFunctionPayload, at
                 }
                 if (error instanceof RetryError && attempt < maxAttempts) {
                     const nextRetryMs = getNextRetryMs(retryBaseMs, retryMultiplier, attempt)
-                    hub.statsd?.increment(`${metricName}.RETRY`)
-                    retryableFnCounter.labels({ name: metricName, status: 'retry' }).inc()
                     nextIterationPromise = new Promise((resolve, reject) =>
                         setTimeout(() => {
                             // This is not awaited directly so that attempts beyond the first one don't stall the payload queue
@@ -126,8 +117,6 @@ function iterateRetryLoop(retriableFunctionPayload: RetriableFunctionPayload, at
                     await hub.promiseManager.awaitPromisesIfNeeded()
                 } else {
                     await catchFn?.(error)
-                    hub.statsd?.increment(`${metricName}.ERROR`)
-                    retryableFnCounter.labels({ name: metricName, status: 'error' }).inc()
                     if (appMetric) {
                         await hub.appMetrics.queueError(
                             {