diff --git a/src/internal/monitoring/otel-metrics.ts b/src/internal/monitoring/otel-metrics.ts index 550088fb7..ab396601a 100644 --- a/src/internal/monitoring/otel-metrics.ts +++ b/src/internal/monitoring/otel-metrics.ts @@ -20,8 +20,14 @@ import { FastifyReply, FastifyRequest } from 'fastify' import * as os from 'os' import { getConfig } from '../../config' -const { version, otelMetricsExportIntervalMs, otelMetricsEnabled, otelMetricsTemporality, region } = - getConfig() +const { + version, + otelMetricsExportIntervalMs, + otelMetricsEnabled, + otelMetricsTemporality, + prometheusMetricsEnabled, + region, +} = getConfig() let prometheusExporter: PrometheusExporter | undefined let meterProvider: MeterProvider | undefined @@ -229,12 +235,14 @@ if (otelMetricsEnabled) { ) } - prometheusExporter = new PrometheusExporter({ - prefix: 'storage_api', - preventServerStart: true, - withResourceConstantLabels: /^(region|instance|metric\.version)$/, - }) - readers.push(prometheusExporter) + if (prometheusMetricsEnabled) { + prometheusExporter = new PrometheusExporter({ + prefix: 'storage_api', + preventServerStart: true, + withResourceConstantLabels: /^(region|instance|metric\.version)$/, + }) + readers.push(prometheusExporter) + } meterProvider = new MeterProvider({ resource, diff --git a/src/test/jest.d.ts b/src/test/jest.d.ts new file mode 100644 index 000000000..d0c71be28 --- /dev/null +++ b/src/test/jest.d.ts @@ -0,0 +1,3 @@ +declare namespace jest { + function isolateModulesAsync(fn: () => Promise): Promise +} diff --git a/src/test/otel-metrics.test.ts b/src/test/otel-metrics.test.ts index 1132ac1d4..0f2141f17 100644 --- a/src/test/otel-metrics.test.ts +++ b/src/test/otel-metrics.test.ts @@ -2,7 +2,7 @@ interface OTelGlobalState { __otelMetricsShutdown?: () => Promise } -describe('otel metrics shutdown', () => { +describe('otel metrics', () => { const originalOtelExporterEndpoint = process.env.OTEL_EXPORTER_OTLP_ENDPOINT const originalOtelMetricsEndpoint = process.env.OTEL_EXPORTER_OTLP_METRICS_ENDPOINT const originalOtelMetricsHeaders = process.env.OTEL_EXPORTER_OTLP_METRICS_HEADERS @@ -34,6 +34,7 @@ describe('otel metrics shutdown', () => { } jest.restoreAllMocks() + jest.resetModules() }) test('still shuts down meter provider when unregister throws', async () => { @@ -68,6 +69,7 @@ describe('otel metrics shutdown', () => { otelMetricsExportIntervalMs: 1000, otelMetricsEnabled: true, otelMetricsTemporality: 'CUMULATIVE', + prometheusMetricsEnabled: true, region: 'local', })), })) @@ -135,4 +137,86 @@ describe('otel metrics shutdown', () => { expect.objectContaining({ type: 'otel-metrics', error: unregisterError }) ) }) + + test('does not create a Prometheus reader when Prometheus metrics are disabled', async () => { + delete process.env.OTEL_EXPORTER_OTLP_ENDPOINT + delete process.env.OTEL_EXPORTER_OTLP_METRICS_ENDPOINT + + const registerInstrumentations = jest.fn(() => jest.fn()) + const HostMetrics = jest.fn().mockImplementation(() => ({ + start: jest.fn(), + })) + const MeterProvider = jest.fn().mockImplementation(() => ({ + shutdown: jest.fn().mockResolvedValue(undefined), + })) + const PrometheusExporter = jest.fn().mockImplementation(() => ({ + getMetricsRequestHandler: jest.fn(), + })) + const RuntimeNodeInstrumentation = jest.fn().mockImplementation(() => ({})) + const StorageNodeInstrumentation = jest.fn().mockImplementation(() => ({})) + + jest.doMock('../config', () => ({ + getConfig: jest.fn(() => ({ + version: 'test-version', + otelMetricsExportIntervalMs: 1000, + otelMetricsEnabled: true, + otelMetricsTemporality: 'CUMULATIVE', + prometheusMetricsEnabled: false, + region: 'local', + })), + })) + jest.doMock('@internal/monitoring/logger', () => ({ + logger: { info: jest.fn() }, + logSchema: { error: jest.fn(), info: jest.fn() }, + })) + jest.doMock('@internal/monitoring/system', () => ({ + StorageNodeInstrumentation, + })) + jest.doMock('@opentelemetry/api', () => ({ + metrics: { + setGlobalMeterProvider: jest.fn(), + }, + })) + jest.doMock('@opentelemetry/exporter-metrics-otlp-grpc', () => ({ + OTLPMetricExporter: jest.fn(), + })) + jest.doMock('@opentelemetry/exporter-prometheus', () => ({ + PrometheusExporter, + })) + jest.doMock('@opentelemetry/host-metrics', () => ({ + HostMetrics, + })) + jest.doMock('@opentelemetry/instrumentation', () => ({ + registerInstrumentations, + })) + jest.doMock('@opentelemetry/instrumentation-runtime-node', () => ({ + RuntimeNodeInstrumentation, + })) + jest.doMock('@opentelemetry/resources', () => ({ + resourceFromAttributes: jest.fn(() => ({})), + })) + jest.doMock('@opentelemetry/sdk-metrics', () => ({ + AggregationTemporality: { + CUMULATIVE: 'CUMULATIVE', + DELTA: 'DELTA', + }, + AggregationType: { + DROP: 'DROP', + EXPLICIT_BUCKET_HISTOGRAM: 'EXPLICIT_BUCKET_HISTOGRAM', + }, + MeterProvider, + PeriodicExportingMetricReader: jest.fn(), + })) + + await jest.isolateModulesAsync(async () => { + await import('../internal/monitoring/otel-metrics') + }) + + expect(PrometheusExporter).not.toHaveBeenCalled() + expect(MeterProvider).toHaveBeenCalledWith( + expect.objectContaining({ + readers: [], + }) + ) + }) }) diff --git a/src/test/otel-tracing.test.ts b/src/test/otel-tracing.test.ts index cd95f8609..4b50688d8 100644 --- a/src/test/otel-tracing.test.ts +++ b/src/test/otel-tracing.test.ts @@ -1,21 +1,9 @@ +import { createDeferred } from './utils/promise' + interface OTelGlobalState { __otelTracingShutdown?: () => Promise } -interface Deferred { - promise: Promise - resolve: (value: T) => void -} - -function createDeferred(): Deferred { - let resolve!: (value: T) => void - const promise = new Promise((resolvePromise) => { - resolve = resolvePromise - }) - - return { promise, resolve } -} - describe('otel tracing bootstrap', () => { const originalTracingEnabled = process.env.TRACING_ENABLED const originalTraceEndpoint = process.env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT @@ -188,6 +176,7 @@ describe('otel tracing bootstrap', () => { config, })) const OTLPTraceExporter = jest.fn().mockImplementation(() => ({})) + const classInstrumentationsDeferred = createDeferred() const logSchema = { error: jest.fn(), info: jest.fn(), @@ -214,7 +203,7 @@ describe('otel tracing bootstrap', () => { logSchema, })) jest.doMock('../internal/monitoring/otel-class-instrumentations', () => ({ - loadClassInstrumentations: jest.fn(async () => []), + loadClassInstrumentations: jest.fn(() => classInstrumentationsDeferred.promise), })) let shutdownOtelTracing: (() => Promise) | undefined @@ -225,6 +214,10 @@ describe('otel tracing bootstrap', () => { .__otelTracingShutdown }) + classInstrumentationsDeferred.resolve([]) + await classInstrumentationsDeferred.promise + await Promise.resolve() + await expect(shutdownOtelTracing?.()).resolves.toBeUndefined() expect(unregisterClassInstrumentations).toHaveBeenCalledTimes(1) diff --git a/src/test/progressive-migrations.test.ts b/src/test/progressive-migrations.test.ts index 7b78d7143..fb613c00a 100644 --- a/src/test/progressive-migrations.test.ts +++ b/src/test/progressive-migrations.test.ts @@ -1,3 +1,5 @@ +import { createDeferred } from './utils/promise' + const mockBatchSend = jest.fn() const mockWarning = jest.fn() const mockError = jest.fn() @@ -57,18 +59,6 @@ class TestProgressiveMigrations extends ProgressiveMigrations { } } -function createDeferred() { - let resolve!: (value: T | PromiseLike) => void - let reject!: (reason?: unknown) => void - - const promise = new Promise((res, rej) => { - resolve = res - reject = rej - }) - - return { promise, resolve, reject } -} - const mockGetTenantConfig = jest.mocked(getTenantConfig) const mockAreMigrationsUpToDate = jest.mocked(areMigrationsUpToDate) const mockRunMigrationsBatchSend = jest.mocked(RunMigrationsOnTenants.batchSend) @@ -133,7 +123,7 @@ describe('ProgressiveMigrations', () => { }) it('keeps new tenants queued while a batch is in flight and ignores duplicate adds', async () => { - const deferredBatch = createDeferred() + const deferredBatch = createDeferred() mockRunMigrationsBatchSend.mockReturnValueOnce(deferredBatch.promise as never) const migrations = new TestProgressiveMigrations({ @@ -155,7 +145,7 @@ describe('ProgressiveMigrations', () => { expect(migrations.pending()).toEqual(['tenant-a', 'tenant-b']) - deferredBatch.resolve(undefined) + deferredBatch.resolve() await expect(flushPromise).resolves.toBeUndefined() expect(migrations.pending()).toEqual(['tenant-b']) @@ -163,7 +153,7 @@ describe('ProgressiveMigrations', () => { }) it('serializes drain with an in-flight batch and drains the remaining tenants after it finishes', async () => { - const deferredBatch = createDeferred() + const deferredBatch = createDeferred() mockRunMigrationsBatchSend .mockReturnValueOnce(deferredBatch.promise as never) .mockResolvedValueOnce(undefined as never) @@ -184,7 +174,7 @@ describe('ProgressiveMigrations', () => { expect(mockRunMigrationsBatchSend).toHaveBeenCalledTimes(1) expect(migrations.isEmitting()).toBe(true) - deferredBatch.resolve(undefined) + deferredBatch.resolve() await expect(Promise.all([flushPromise, drainPromise])).resolves.toEqual([undefined, undefined]) diff --git a/src/test/utils/promise.ts b/src/test/utils/promise.ts new file mode 100644 index 000000000..46473c372 --- /dev/null +++ b/src/test/utils/promise.ts @@ -0,0 +1,25 @@ +export interface Deferred { + promise: Promise + resolve(value: T | PromiseLike): void + reject(reason?: unknown): void +} + +export interface VoidDeferred { + promise: Promise + resolve(value?: void | PromiseLike): void + reject(reason?: unknown): void +} + +type DeferredFor = [T] extends [void] ? VoidDeferred : Deferred + +export function createDeferred(): DeferredFor { + let resolve!: (value: T | PromiseLike) => void + let reject!: (reason?: unknown) => void + + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + + return { promise, resolve, reject } as DeferredFor +} diff --git a/src/test/vector-store-manager.test.ts b/src/test/vector-store-manager.test.ts index 73c755181..4f54abae7 100644 --- a/src/test/vector-store-manager.test.ts +++ b/src/test/vector-store-manager.test.ts @@ -18,15 +18,7 @@ import { VectorStore, VectorStoreManager, } from '@storage/protocols/vector' - -function deferred() { - let resolve!: () => void - const promise = new Promise((res) => { - resolve = res - }) - - return { promise, resolve } -} +import { createDeferred } from './utils/promise' function createMockVectorStore(): jest.Mocked { return { @@ -194,8 +186,8 @@ function createDeterministicVectorDb(options: { describe('VectorStoreManager bucket lifecycle', () => { it('serializes concurrent creates for the final bucket slot', async () => { - const releaseFirstCreate = deferred() - const firstCreateStarted = deferred() + const releaseFirstCreate = createDeferred() + const firstCreateStarted = createDeferred() const db = createDeterministicVectorDb({ bucketCount: 1, @@ -250,8 +242,8 @@ describe('VectorStoreManager bucket lifecycle', () => { }) it('shares the bucket-count lock between delete and create so capacity is observed after delete commits', async () => { - const releaseDelete = deferred() - const deleteReachedRemoval = deferred() + const releaseDelete = createDeferred() + const deleteReachedRemoval = createDeferred() const db = createDeterministicVectorDb({ bucketCount: 1, @@ -281,8 +273,8 @@ describe('VectorStoreManager bucket lifecycle', () => { }) it('does not block unrelated creates while delete waits on the target bucket lock', async () => { - const releaseBucketLock = deferred() - const deleteWaitingOnBucketLock = deferred() + const releaseBucketLock = createDeferred() + const deleteWaitingOnBucketLock = createDeferred() const db = createDeterministicVectorDb({ bucketCount: 1,