From 90c00b6db9b2f1290ac993c4e7c68cab1a490d67 Mon Sep 17 00:00:00 2001 From: forehalo Date: Fri, 19 Jul 2024 11:31:01 +0800 Subject: [PATCH] Revert "fix(server): wrong usage of optl (#6714)" This reverts commit 4868f6e61100161c3b7487a3a6a1d18ba1e9368e. --- packages/backend/server/package.json | 18 +-- packages/backend/server/scripts/loader.js | 31 +---- packages/backend/server/src/data/index.ts | 4 - .../1703756315970-unamed-account.ts | 3 + .../server/src/fundamentals/metrics/index.ts | 1 - .../fundamentals/metrics/instrumentations.ts | 32 ----- .../src/fundamentals/metrics/opentelemetry.ts | 125 ++++++------------ packages/backend/server/src/index.ts | 2 - tests/affine-cloud/playwright.config.ts | 2 +- .../affine-desktop-cloud/playwright.config.ts | 2 +- yarn.lock | 32 +---- 11 files changed, 70 insertions(+), 182 deletions(-) delete mode 100644 packages/backend/server/src/fundamentals/metrics/instrumentations.ts diff --git a/packages/backend/server/package.json b/packages/backend/server/package.json index 1a39615f02..8648a63400 100644 --- a/packages/backend/server/package.json +++ b/packages/backend/server/package.json @@ -8,16 +8,15 @@ "run-test": "./scripts/run-test.ts" }, "scripts": { - "run:script": "node --import ./scripts/register.js", "build": "tsc", - "start": "yarn run:script ./src/index.ts", + "start": "node --loader ts-node/esm/transpile-only.mjs ./src/index.ts", "dev": "nodemon ./src/index.ts", "test": "ava --concurrency 1 --serial", "test:coverage": "c8 ava --concurrency 1 --serial", "postinstall": "prisma generate", - "data-migration": "yarn run:script ./src/data/index.ts", - "predeploy": "yarn prisma migrate deploy && yarn run:script ./dist/data/index.js run", - "db:upgrade": "yarn prisma migrate deploy && yarn data-migration run" + "data-migration": "node --loader ts-node/esm/transpile-only.mjs ./src/data/index.ts", + "predeploy": "yarn prisma migrate deploy && node --import ./scripts/register.js ./dist/data/index.js run", + "predeploy:ts": "yarn prisma migrate deploy && node --loader ts-node/esm/transpile-only.mjs ./src/data/index.ts run" }, "dependencies": { "@apollo/server": "^4.10.2", @@ -43,8 +42,8 @@ "@node-rs/jsonwebtoken": "^0.5.2", "@opentelemetry/api": "^1.9.0", "@opentelemetry/core": "^1.25.0", - "@opentelemetry/exporter-metrics-otlp-proto": "^0.52.0", - "@opentelemetry/exporter-trace-otlp-proto": "^0.52.0", + "@opentelemetry/exporter-prometheus": "^0.52.0", + "@opentelemetry/exporter-zipkin": "^1.25.0", "@opentelemetry/host-metrics": "^0.35.2", "@opentelemetry/instrumentation": "^0.52.0", "@opentelemetry/instrumentation-graphql": "^0.42.0", @@ -166,8 +165,9 @@ "exec": "node", "script": "./src/index.ts", "nodeArgs": [ - "--import", - "./scripts/register.js" + "--loader", + "ts-node/esm.mjs", + "--es-module-specifier-resolution=node" ], "ignore": [ "**/__tests__/**", diff --git a/packages/backend/server/scripts/loader.js b/packages/backend/server/scripts/loader.js index 83fa58cdd4..ce31fb5096 100644 --- a/packages/backend/server/scripts/loader.js +++ b/packages/backend/server/scripts/loader.js @@ -1,32 +1,11 @@ -import * as otel from '@opentelemetry/instrumentation/hook.mjs'; -import { createEsmHooks, register } from 'ts-node'; +import { create, createEsmHooks } from 'ts-node'; -const service = register({ +const service = create({ experimentalSpecifierResolution: 'node', transpileOnly: true, logError: true, + skipProject: true, }); +const hooks = createEsmHooks(service); -/** - * @type {import('ts-node').NodeLoaderHooksAPI2} - - */ -const ts = createEsmHooks(service); - -/** - * @type {import('ts-node').NodeLoaderHooksAPI2.ResolveHook} - */ -export const resolve = (specifier, context, defaultResolver) => { - return ts.resolve(specifier, context, (s, c) => { - return otel.resolve(s, c, defaultResolver); - }); -}; - -/** - * @type {import('ts-node').NodeLoaderHooksAPI2.LoadHook} - */ -export const load = async (url, context, defaultLoader) => { - return await otel.load(url, context, (u, c) => { - return ts.load(u, c, defaultLoader); - }); -}; +export const resolve = hooks.resolve; diff --git a/packages/backend/server/src/data/index.ts b/packages/backend/server/src/data/index.ts index a2e81f09ef..f9735c6e1a 100644 --- a/packages/backend/server/src/data/index.ts +++ b/packages/backend/server/src/data/index.ts @@ -3,13 +3,9 @@ import '../prelude'; import { Logger } from '@nestjs/common'; import { CommandFactory } from 'nest-commander'; -import { registerInstrumentations } from '../fundamentals/metrics'; - async function bootstrap() { AFFiNE.metrics.enabled = false; AFFiNE.doc.manager.enableUpdateAutoMerging = false; - - registerInstrumentations(); const { CliAppModule } = await import('./app'); await CommandFactory.run(CliAppModule, new Logger()).catch(e => { console.error(e); diff --git a/packages/backend/server/src/data/migrations/1703756315970-unamed-account.ts b/packages/backend/server/src/data/migrations/1703756315970-unamed-account.ts index b2ccbcc35d..59428ccd6e 100644 --- a/packages/backend/server/src/data/migrations/1703756315970-unamed-account.ts +++ b/packages/backend/server/src/data/migrations/1703756315970-unamed-account.ts @@ -9,6 +9,9 @@ export class UnamedAccount1703756315970 { const users = await db.$queryRaw< User[] >`SELECT * FROM users WHERE name ~ E'^[\\s\\u2000-\\u200F]*$';`; + console.log( + `renaming ${users.map(({ email }) => email).join('|')} users` + ); await Promise.all( users.map(({ id, email }) => diff --git a/packages/backend/server/src/fundamentals/metrics/index.ts b/packages/backend/server/src/fundamentals/metrics/index.ts index 26382781f0..ee2d98f292 100644 --- a/packages/backend/server/src/fundamentals/metrics/index.ts +++ b/packages/backend/server/src/fundamentals/metrics/index.ts @@ -50,7 +50,6 @@ export class MetricsModule implements OnModuleInit, OnModuleDestroy { } } -export { registerInstrumentations } from './instrumentations'; export * from './metrics'; export * from './utils'; export { OpentelemetryFactory }; diff --git a/packages/backend/server/src/fundamentals/metrics/instrumentations.ts b/packages/backend/server/src/fundamentals/metrics/instrumentations.ts deleted file mode 100644 index 997331fdcb..0000000000 --- a/packages/backend/server/src/fundamentals/metrics/instrumentations.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { Instrumentation } from '@opentelemetry/instrumentation'; -import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql'; -import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; -import { IORedisInstrumentation } from '@opentelemetry/instrumentation-ioredis'; -import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core'; -import { SocketIoInstrumentation } from '@opentelemetry/instrumentation-socket.io'; -import prismaInstrument from '@prisma/instrumentation'; - -const { PrismaInstrumentation } = prismaInstrument; - -let instrumentations: Instrumentation[] = []; - -export function registerInstrumentations(): void { - if (AFFiNE.metrics.enabled) { - instrumentations = [ - new NestInstrumentation(), - new IORedisInstrumentation(), - new SocketIoInstrumentation({ traceReserved: true }), - new GraphQLInstrumentation({ - mergeItems: true, - ignoreTrivialResolveSpans: true, - depth: 10, - }), - new HttpInstrumentation(), - new PrismaInstrumentation({ middleware: false }), - ]; - } -} - -export function getRegisteredInstrumentations(): Instrumentation[] { - return instrumentations; -} diff --git a/packages/backend/server/src/fundamentals/metrics/opentelemetry.ts b/packages/backend/server/src/fundamentals/metrics/opentelemetry.ts index ad3d9372f0..02e84b8c78 100644 --- a/packages/backend/server/src/fundamentals/metrics/opentelemetry.ts +++ b/packages/backend/server/src/fundamentals/metrics/opentelemetry.ts @@ -1,82 +1,52 @@ -import { Attributes, metrics } from '@opentelemetry/api'; +import { OnModuleDestroy } from '@nestjs/common'; +import { metrics } from '@opentelemetry/api'; import { CompositePropagator, W3CBaggagePropagator, W3CTraceContextPropagator, } from '@opentelemetry/core'; -import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-proto'; -import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-proto'; +import { PrometheusExporter } from '@opentelemetry/exporter-prometheus'; +import { ZipkinExporter } from '@opentelemetry/exporter-zipkin'; import { HostMetrics } from '@opentelemetry/host-metrics'; import { Instrumentation } from '@opentelemetry/instrumentation'; +import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql'; +import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; +import { IORedisInstrumentation } from '@opentelemetry/instrumentation-ioredis'; +import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core'; +import { SocketIoInstrumentation } from '@opentelemetry/instrumentation-socket.io'; import { Resource } from '@opentelemetry/resources'; import type { MeterProvider } from '@opentelemetry/sdk-metrics'; -import { - MetricProducer, - MetricReader, - PeriodicExportingMetricReader, -} from '@opentelemetry/sdk-metrics'; +import { MetricProducer, MetricReader } from '@opentelemetry/sdk-metrics'; import { NodeSDK } from '@opentelemetry/sdk-node'; import { + BatchSpanProcessor, SpanExporter, TraceIdRatioBasedSampler, } from '@opentelemetry/sdk-trace-node'; import { - SEMRESATTRS_K8S_CLUSTER_NAME, SEMRESATTRS_K8S_NAMESPACE_NAME, - SEMRESATTRS_K8S_POD_NAME, + SEMRESATTRS_SERVICE_NAME, SEMRESATTRS_SERVICE_VERSION, } from '@opentelemetry/semantic-conventions'; +import prismaInstrument from '@prisma/instrumentation'; -import { getRegisteredInstrumentations } from './instrumentations'; import { PrismaMetricProducer } from './prisma'; -function withBuiltinAttributesMetricReader( - reader: MetricReader, - attrs: Attributes -) { - const collect = reader.collect; - reader.collect = async options => { - const result = await collect.call(reader, options); - - result.resourceMetrics.scopeMetrics.forEach(metrics => { - metrics.metrics.forEach(metric => { - metric.dataPoints.forEach(dataPoint => { - // @ts-expect-error allow - dataPoint.attributes = Object.assign({}, attrs, dataPoint.attributes); - }); - }); - }); - - return result; - }; - - return reader; -} - -function withBuiltinAttributesSpanExporter( - exporter: SpanExporter, - attrs: Attributes -) { - const exportSpans = exporter.export; - exporter.export = (spans, callback) => { - spans.forEach(span => { - // patch span attributes - // @ts-expect-error allow - span.attributes = Object.assign({}, attrs, span.attributes); - }); - - return exportSpans.call(exporter, spans, callback); - }; - - return exporter; -} +const { PrismaInstrumentation } = prismaInstrument; export abstract class OpentelemetryFactory { abstract getMetricReader(): MetricReader; abstract getSpanExporter(): SpanExporter; getInstractions(): Instrumentation[] { - return getRegisteredInstrumentations(); + return [ + new NestInstrumentation(), + new IORedisInstrumentation(), + new SocketIoInstrumentation({ traceReserved: true }), + new GraphQLInstrumentation({ mergeItems: true }), + new HttpInstrumentation(), + new PrismaInstrumentation(), + ]; } getMetricsProducers(): MetricProducer[] { @@ -85,32 +55,20 @@ export abstract class OpentelemetryFactory { getResource() { return new Resource({ - [SEMRESATTRS_K8S_CLUSTER_NAME]: AFFiNE.flavor.type, [SEMRESATTRS_K8S_NAMESPACE_NAME]: AFFiNE.AFFINE_ENV, - [SEMRESATTRS_K8S_POD_NAME]: process.env.HOSTNAME ?? process.env.HOST, + [SEMRESATTRS_SERVICE_NAME]: AFFiNE.flavor.type, + [SEMRESATTRS_SERVICE_VERSION]: AFFiNE.version, }); } - getBuiltinAttributes(): Attributes { - return { - [SEMRESATTRS_SERVICE_VERSION]: AFFiNE.version, - }; - } - create() { - const builtinAttributes = this.getBuiltinAttributes(); - + const traceExporter = this.getSpanExporter(); return new NodeSDK({ resource: this.getResource(), sampler: new TraceIdRatioBasedSampler(0.1), - traceExporter: withBuiltinAttributesSpanExporter( - this.getSpanExporter(), - builtinAttributes - ), - metricReader: withBuiltinAttributesMetricReader( - this.getMetricReader(), - builtinAttributes - ), + traceExporter, + metricReader: this.getMetricReader(), + spanProcessor: new BatchSpanProcessor(traceExporter), textMapPropagator: new CompositePropagator({ propagators: [ new W3CBaggagePropagator(), @@ -123,19 +81,24 @@ export abstract class OpentelemetryFactory { } } -export class LocalOpentelemetryFactory extends OpentelemetryFactory { - override getMetricReader() { - return new PeriodicExportingMetricReader({ - // requires jeager service running in 'http://localhost:4318' - // with metrics feature enabled. - // see https://www.jaegertracing.io/docs/1.56/spm - exporter: new OTLPMetricExporter(), - }); +export class LocalOpentelemetryFactory + extends OpentelemetryFactory + implements OnModuleDestroy +{ + private readonly metricsExporter = new PrometheusExporter({ + metricProducers: this.getMetricsProducers(), + }); + + async onModuleDestroy() { + await this.metricsExporter.shutdown(); } - override getSpanExporter() { - // requires jeager service running in 'http://localhost:4318' - return new OTLPTraceExporter(); + override getMetricReader(): MetricReader { + return this.metricsExporter; + } + + override getSpanExporter(): SpanExporter { + return new ZipkinExporter(); } } diff --git a/packages/backend/server/src/index.ts b/packages/backend/server/src/index.ts index b62eb587c9..02e7d87d91 100644 --- a/packages/backend/server/src/index.ts +++ b/packages/backend/server/src/index.ts @@ -6,9 +6,7 @@ import { omit } from 'lodash-es'; import { createApp } from './app'; import { URLHelper } from './fundamentals'; -import { registerInstrumentations } from './fundamentals/metrics'; -registerInstrumentations(); const app = await createApp(); const listeningHost = AFFiNE.deploy ? '0.0.0.0' : 'localhost'; await app.listen(AFFiNE.server.port, listeningHost); diff --git a/tests/affine-cloud/playwright.config.ts b/tests/affine-cloud/playwright.config.ts index ed072898d8..0f705382fe 100644 --- a/tests/affine-cloud/playwright.config.ts +++ b/tests/affine-cloud/playwright.config.ts @@ -57,7 +57,7 @@ const config: PlaywrightTestConfig = { DATABASE_URL: process.env.DATABASE_URL ?? 'postgresql://affine:affine@localhost:5432/affine', - NODE_ENV: 'test', + NODE_ENV: 'development', AFFINE_ENV: process.env.AFFINE_ENV ?? 'dev', DEBUG: 'affine:*', FORCE_COLOR: 'true', diff --git a/tests/affine-desktop-cloud/playwright.config.ts b/tests/affine-desktop-cloud/playwright.config.ts index b297292391..d6227be2fc 100644 --- a/tests/affine-desktop-cloud/playwright.config.ts +++ b/tests/affine-desktop-cloud/playwright.config.ts @@ -44,7 +44,7 @@ const config: PlaywrightTestConfig = { DATABASE_URL: process.env.DATABASE_URL ?? 'postgresql://affine:affine@localhost:5432/affine', - NODE_ENV: 'test', + NODE_ENV: 'development', AFFINE_ENV: process.env.AFFINE_ENV ?? 'dev', DEBUG: 'affine:*', FORCE_COLOR: 'true', diff --git a/yarn.lock b/yarn.lock index c054aca192..ac39b912b4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -755,8 +755,8 @@ __metadata: "@node-rs/jsonwebtoken": "npm:^0.5.2" "@opentelemetry/api": "npm:^1.9.0" "@opentelemetry/core": "npm:^1.25.0" - "@opentelemetry/exporter-metrics-otlp-proto": "npm:^0.52.0" - "@opentelemetry/exporter-trace-otlp-proto": "npm:^0.52.0" + "@opentelemetry/exporter-prometheus": "npm:^0.52.0" + "@opentelemetry/exporter-zipkin": "npm:^1.25.0" "@opentelemetry/host-metrics": "npm:^0.35.2" "@opentelemetry/instrumentation": "npm:^0.52.0" "@opentelemetry/instrumentation-graphql": "npm:^0.42.0" @@ -9737,34 +9737,16 @@ __metadata: languageName: node linkType: hard -"@opentelemetry/exporter-metrics-otlp-http@npm:0.52.1": +"@opentelemetry/exporter-prometheus@npm:^0.52.0": version: 0.52.1 - resolution: "@opentelemetry/exporter-metrics-otlp-http@npm:0.52.1" + resolution: "@opentelemetry/exporter-prometheus@npm:0.52.1" dependencies: "@opentelemetry/core": "npm:1.25.1" - "@opentelemetry/otlp-exporter-base": "npm:0.52.1" - "@opentelemetry/otlp-transformer": "npm:0.52.1" "@opentelemetry/resources": "npm:1.25.1" "@opentelemetry/sdk-metrics": "npm:1.25.1" peerDependencies: "@opentelemetry/api": ^1.3.0 - checksum: 10/b2836279dbeb84be8b5b97a02e658b79c3d104b663b033df0bcf9e98d9d4697649c9855ddfa1493f8b89ed83f6a43e41d6c2e71278c64944f2628394dc471c10 - languageName: node - linkType: hard - -"@opentelemetry/exporter-metrics-otlp-proto@npm:^0.52.0": - version: 0.52.1 - resolution: "@opentelemetry/exporter-metrics-otlp-proto@npm:0.52.1" - dependencies: - "@opentelemetry/core": "npm:1.25.1" - "@opentelemetry/exporter-metrics-otlp-http": "npm:0.52.1" - "@opentelemetry/otlp-exporter-base": "npm:0.52.1" - "@opentelemetry/otlp-transformer": "npm:0.52.1" - "@opentelemetry/resources": "npm:1.25.1" - "@opentelemetry/sdk-metrics": "npm:1.25.1" - peerDependencies: - "@opentelemetry/api": ^1.3.0 - checksum: 10/bbce23bf31a9b91ac2836b0b697dd2f853281a3e46bae58b86a0a2b747f995777a9aef77bdbb3885f06ff155d7fd6a8af3364d1348dee3427ce8f87d1ed15675 + checksum: 10/34cb0a1e62ec5e6e53ce70fb390cc2720571c03a45942469f99bc327cdb3ac2a0602f9977917640125ac4ed8b5e6e968577441b65bd712f27e73233fd6378776 languageName: node linkType: hard @@ -9799,7 +9781,7 @@ __metadata: languageName: node linkType: hard -"@opentelemetry/exporter-trace-otlp-proto@npm:0.52.1, @opentelemetry/exporter-trace-otlp-proto@npm:^0.52.0": +"@opentelemetry/exporter-trace-otlp-proto@npm:0.52.1": version: 0.52.1 resolution: "@opentelemetry/exporter-trace-otlp-proto@npm:0.52.1" dependencies: @@ -9814,7 +9796,7 @@ __metadata: languageName: node linkType: hard -"@opentelemetry/exporter-zipkin@npm:1.25.1": +"@opentelemetry/exporter-zipkin@npm:1.25.1, @opentelemetry/exporter-zipkin@npm:^1.25.0": version: 1.25.1 resolution: "@opentelemetry/exporter-zipkin@npm:1.25.1" dependencies: