fix: drop tenant id from metrics to reduce allocations (#1121)

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>

Author: ferhatelmas

src/config.ts

@@ -187,7 +187,6 @@ type StorageConfigType = {
     upload: boolean
   }
   prometheusMetricsEnabled: boolean
-  prometheusMetricsIncludeTenantId: boolean
   tenantPoolCacheTtlMs: number
   tenantPoolCacheHitLogSampleRate: number
   tenantPoolCacheMissLogSampleRate: number
@@ -491,8 +490,6 @@ export function getConfig(options?: { reload?: boolean }): StorageConfigType {
 
     // OpenTelemetry Metrics
     prometheusMetricsEnabled: getOptionalConfigFromEnv('PROMETHEUS_METRICS_ENABLED') === 'true',
-    prometheusMetricsIncludeTenantId:
-      getOptionalConfigFromEnv('PROMETHEUS_METRICS_INCLUDE_TENANT') === 'true',
     otelMetricsEnabled: getOptionalConfigFromEnv('OTEL_METRICS_ENABLED') === 'true',
     otelMetricsTemporality: getOptionalConfigFromEnv('OTEL_METRICS_TEMPORALITY') || 'CUMULATIVE',
     otelMetricsExportIntervalMs: parseInt(

src/http/plugins/metrics.test.ts

@@ -0,0 +1,52 @@
+import {
+  httpRequestDuration,
+  httpRequestSizeBytes,
+  httpResponseSizeBytes,
+} from '@internal/monitoring/metrics'
+import Fastify from 'fastify'
+import { httpMetrics } from './metrics'
+
+describe('httpMetrics plugin', () => {
+  it('records HTTP metric attributes without tenant id labels', async () => {
+    const app = Fastify()
+    const durationSpy = vi.spyOn(httpRequestDuration, 'record')
+    const requestSizeSpy = vi.spyOn(httpRequestSizeBytes, 'add')
+    const responseSizeSpy = vi.spyOn(httpResponseSizeBytes, 'add')
+
+    app.decorateRequest('tenantId', 'tenant-a')
+    await app.register(httpMetrics())
+    app.post('/objects/:bucket', async (_request, reply) => {
+      reply.header('content-length', '2')
+      return 'ok'
+    })
+
+    try {
+      const response = await app.inject({
+        method: 'POST',
+        url: '/objects/bucket-a',
+        headers: {
+          'content-length': '7',
+          'content-type': 'text/plain',
+        },
+        payload: 'payload',
+      })
+
+      expect(response.statusCode).toBe(200)
+
+      const expectedAttributes = {
+        method: 'POST',
+        operation: 'unknown',
+        status_code: '2xx',
+      }
+
+      expect(durationSpy).toHaveBeenCalledWith(expect.any(Number), expectedAttributes)
+      expect(requestSizeSpy).toHaveBeenCalledWith(7, expectedAttributes)
+      expect(responseSizeSpy).toHaveBeenCalledWith(2, expectedAttributes)
+    } finally {
+      durationSpy.mockRestore()
+      requestSizeSpy.mockRestore()
+      responseSizeSpy.mockRestore()
+      await app.close()
+    }
+  })
+})

src/http/plugins/metrics.ts

@@ -93,7 +93,6 @@ export const httpMetrics = (options: HttpMetricsOptions = {}) =>
           operation:
             request.operation?.type || request.routeOptions?.config?.operation?.type || 'unknown',
           status_code: statusCode,
-          tenantId: request.tenantId || '',
         }
 
         // Record duration (histogram count replaces httpRequestsTotal)

src/internal/monitoring/metrics.ts

@@ -1,7 +1,4 @@
-import { Attributes, metrics } from '@opentelemetry/api'
-import { getConfig } from '../../config'
-
-const { prometheusMetricsIncludeTenantId } = getConfig()
+import { metrics } from '@opentelemetry/api'
 
 // ============================================================================
 // Metric Registry — tracks all metrics for admin API
@@ -48,35 +45,12 @@ export function isMetricEnabled(name: string): boolean {
 // ============================================================================
 export const meter = metrics.getMeter('storage-api')
 
-function stripTenantAttrs(attrs: Attributes): Attributes {
-  const { tenantId, tenant_id, ...rest } = attrs as Record<string, unknown>
-  return rest as Attributes
-}
-
 /**
- * Registers a metric in the admin registry and wraps .record()/.add()
- * to automatically strip tenant attributes when prometheusMetricsIncludeTenantId is false.
+ * Registers a metric in the admin registry.
  */
 export function registerMetric<T>(name: string, type: MetricType, factory: () => T): T {
   metricsRegistry.set(name, { name, type, enabled: !disabledMetrics.has(name) })
-  const instrument = factory()
-
-  if (prometheusMetricsIncludeTenantId) return instrument
-
-  // biome-ignore lint/suspicious/noExplicitAny: wrapping OTel instrument methods
-  const inst = instrument as any
-  if (typeof inst.record === 'function') {
-    const original = inst.record.bind(inst)
-    inst.record = (value: number, attrs?: Attributes) =>
-      original(value, attrs ? stripTenantAttrs(attrs) : attrs)
-  }
-  if (typeof inst.add === 'function') {
-    const original = inst.add.bind(inst)
-    inst.add = (value: number, attrs?: Attributes) =>
-      original(value, attrs ? stripTenantAttrs(attrs) : attrs)
-  }
-
-  return instrument
+  return factory()
 }
 
 // ============================================================================

src/internal/monitoring/otel-metrics.ts

@@ -110,7 +110,6 @@ const gcHistogramAggregation = {
 const dropAggregation = { type: AggregationType.DROP } as const
 
 // Views — custom histogram buckets + drop auto-instrumentation duplicates.
-// Tenant attribute stripping is handled by registerMetric() in metrics.ts.
 const views = [
   {
     meterName: 'storage-api',

src/storage/database/knex.test.ts

@@ -1,6 +1,13 @@
 import { TenantConnection } from '../../internal/database/connection'
+import { dbQueryPerformance } from '../../internal/monitoring/metrics'
 import { escapeLike, StorageKnexDB } from './knex'
 
+class TestStorageKnexDB extends StorageKnexDB {
+  runTestQuery() {
+    return this.runQuery('TestQuery', async () => 'allowed')
+  }
+}
+
 describe('escapeLike', () => {
   test('escapes SQL wildcard characters', () => {
     expect(escapeLike('%_abc')).toBe('\\%\\_abc')
@@ -23,15 +30,15 @@ function createStorageKnexTestHarness() {
     getAbortSignal: vi.fn().mockReturnValue(undefined),
   } as unknown as TenantConnection
 
-  const db = new StorageKnexDB(connection, {
+  const db = new TestStorageKnexDB(connection, {
     tenantId: 'test-tenant',
     host: 'localhost',
   })
 
   return { db, connection, transaction }
 }
 
-describe('StorageKnexDB.testPermission', () => {
+describe('StorageKnexDB', () => {
   it('returns the callback result after rolling back the transaction', async () => {
     const { db, connection, transaction } = createStorageKnexTestHarness()
 
@@ -61,4 +68,19 @@ describe('StorageKnexDB.testPermission', () => {
     expect(transaction.rollback).toHaveBeenCalledTimes(1)
     expect(transaction.commit).not.toHaveBeenCalled()
   })
+
+  it('records query performance attributes without tenant id labels', async () => {
+    const { db } = createStorageKnexTestHarness()
+    const recordSpy = vi.spyOn(dbQueryPerformance, 'record')
+
+    try {
+      await db.runTestQuery()
+
+      expect(recordSpy).toHaveBeenCalledWith(expect.any(Number), {
+        name: 'TestQuery',
+      })
+    } finally {
+      recordSpy.mockRestore()
+    }
+  })
 })

src/storage/database/knex.ts

@@ -1084,7 +1084,6 @@ export class StorageKnexDB implements Database {
       const duration = Number(process.hrtime.bigint() - startTime) / 1e9
       dbQueryPerformance.record(duration, {
         name: queryName,
-        tenantId: this.tenantId,
       })
     }
 

src/storage/uploader.ts

@@ -106,7 +106,6 @@ export class Uploader {
     await this.canUpload(options)
     fileUploadStarted.add(1, {
       uploadType: options.uploadType,
-      tenantId: this.db.tenantId,
     })
 
     return randomUUID()
@@ -284,7 +283,6 @@ export class Uploader {
 
         fileUploadedSuccess.add(1, {
           uploadType,
-          tenantId: this.db.tenantId,
         })
 
         return { obj: newObject, isNew, metadata: objectMetadata }

src/test/uploader.test.ts

@@ -2,7 +2,8 @@ import { once } from 'events'
 import { FastifyRequest } from 'fastify'
 import { PassThrough, Readable } from 'stream'
 import { ErrorCode, isStorageError, StorageBackendError } from '../internal/errors'
-import { ObjectAdminDelete } from '../storage/events'
+import { fileUploadedSuccess, fileUploadStarted } from '../internal/monitoring/metrics'
+import { ObjectAdminDelete, ObjectCreatedPostEvent } from '../storage/events'
 import { TenantLocation } from '../storage/locator'
 import { fileUploadFromRequest, Uploader } from '../storage/uploader'
 
@@ -21,6 +22,21 @@ function createUploader(
     new TenantLocation('test-bucket')
   )
 }
+
+function createUploaderDb(overrides: Partial<UploaderDatabase> = {}) {
+  const db = {
+    tenantId: 'stub-tenant',
+    reqId: 'req-1',
+    sbReqId: 'sb-req-1',
+    tenant: () => ({ ref: 'stub-tenant', host: 'stub-tenant.local' }),
+    testPermission: vi.fn(async () => undefined),
+    ...overrides,
+  } as Partial<UploaderDatabase> &
+    Pick<UploaderDatabase, 'tenantId' | 'reqId' | 'tenant' | 'testPermission'>
+
+  return db
+}
+
 describe('fileUploadFromRequest', () => {
   test('keeps multipart/form-data file size undefined even when the request content-length exceeds 5GB', async () => {
     const file = Readable.from(['payload']) as Readable & { truncated: boolean }
@@ -380,3 +396,83 @@ describe('fileUploadFromRequest', () => {
     completeUploadSpy.mockRestore()
   })
 })
+
+describe('Uploader metrics', () => {
+  test('prepareUpload records upload start attributes without tenant id labels', async () => {
+    const addSpy = vi.spyOn(fileUploadStarted, 'add')
+    const uploader = createUploader(
+      {
+        uploadObject: vi.fn(),
+      },
+      createUploaderDb()
+    )
+
+    try {
+      await uploader.prepareUpload({
+        bucketId: 'bucket',
+        objectName: 'test.txt',
+        owner: undefined,
+        isUpsert: false,
+        userMetadata: undefined,
+        metadata: undefined,
+        uploadType: 'standard',
+      })
+
+      expect(addSpy).toHaveBeenCalledWith(1, { uploadType: 'standard' })
+    } finally {
+      addSpy.mockRestore()
+    }
+  })
+
+  test('completeUpload records upload success attributes without tenant id labels', async () => {
+    const addSpy = vi.spyOn(fileUploadedSuccess, 'add')
+    const sendWebhookSpy = vi
+      .spyOn(ObjectCreatedPostEvent, 'sendWebhook')
+      .mockResolvedValue(undefined)
+    const transactionDb = {
+      waitObjectLock: vi.fn().mockResolvedValue(undefined),
+      findObject: vi.fn().mockResolvedValue(undefined),
+      upsertObject: vi.fn().mockResolvedValue({ id: 'object-id' }),
+    }
+    const db = createUploaderDb({
+      asSuperUser: vi.fn().mockReturnValue({
+        connection: {
+          setAbortSignal: vi.fn(),
+        },
+        withTransaction: vi.fn(async (fn) => fn(transactionDb)),
+      }),
+    })
+    const uploader = createUploader(
+      {
+        uploadObject: vi.fn(),
+      },
+      db
+    )
+
+    try {
+      await uploader.completeUpload({
+        version: 'version-1',
+        bucketId: 'bucket',
+        objectName: 'test.txt',
+        owner: undefined,
+        objectMetadata: {
+          eTag: '"etag"',
+          mimetype: 'text/plain',
+          cacheControl: 'max-age=3600',
+          lastModified: new Date(),
+          contentLength: 7,
+          httpStatusCode: 200,
+          size: 7,
+        },
+        uploadType: 'standard',
+        isUpsert: false,
+        userMetadata: undefined,
+      })
+
+      expect(addSpy).toHaveBeenCalledWith(1, { uploadType: 'standard' })
+    } finally {
+      addSpy.mockRestore()
+      sendWebhookSpy.mockRestore()
+    }
+  })
+})