diff --git a/.changeset/diagnostics-provider-conflicts.md b/.changeset/diagnostics-provider-conflicts.md new file mode 100644 index 000000000..6b1875f05 --- /dev/null +++ b/.changeset/diagnostics-provider-conflicts.md @@ -0,0 +1,5 @@ +--- +"@croco/diagnostics-core": patch +--- + +Reject duplicate diagnostics provider names instead of silently overwriting an existing provider. diff --git a/docs/troubleshooting/diagnostics.md b/docs/troubleshooting/diagnostics.md index 7e8532184..9d7851d3b 100644 --- a/docs/troubleshooting/diagnostics.md +++ b/docs/troubleshooting/diagnostics.md @@ -99,6 +99,9 @@ const app = createApp({ 각 `DiagnosticsProvider`는 다음과 같은 정보를 수집합니다. +Provider `name`은 collector 안에서 유일해야 합니다. 같은 인스턴스를 다시 등록하는 것은 no-op이지만, +같은 `name`을 가진 다른 provider를 등록하면 `DuplicateDiagnosticsProviderProblem`이 발생합니다. + ### TelemetryDiagnosticsProvider - **수집 정보**: OTel SDK 초기화 여부(`isInitialized`), 현재 샘플링 확률(`probability`) diff --git a/packages/diagnostics-core/package.json b/packages/diagnostics-core/package.json index 0f7e38968..bce2ed9e0 100644 --- a/packages/diagnostics-core/package.json +++ b/packages/diagnostics-core/package.json @@ -26,6 +26,8 @@ "test": "vitest run", "typecheck": "tsc --noEmit" }, - "dependencies": {}, + "dependencies": { + "@croco/problems-core": "workspace:*" + }, "devDependencies": {} } diff --git a/packages/diagnostics-core/src/index.ts b/packages/diagnostics-core/src/index.ts index b7608abfc..41ead2c32 100644 --- a/packages/diagnostics-core/src/index.ts +++ b/packages/diagnostics-core/src/index.ts @@ -2,6 +2,7 @@ export { DiagnosticsCollector } from "./libs/DiagnosticsCollector"; export { ErrorHistoryRingBuffer } from "./libs/ErrorHistoryRingBuffer"; +export { DuplicateDiagnosticsProviderProblem } from "./libs/problems/DiagnosticsProblems"; export type { DiagnosticsProvider, HealthStatus, diff --git a/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts b/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts index 586b36e82..33f3335e7 100644 --- a/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts +++ b/packages/diagnostics-core/src/libs/DiagnosticsCollector.ts @@ -1,5 +1,6 @@ import type { DiagnosticsProvider, HealthStatus, ErrorRecord, DiagnosticsReport } from "./types"; import { ErrorHistoryRingBuffer } from "./ErrorHistoryRingBuffer"; +import { DuplicateDiagnosticsProviderProblem } from "./problems/DiagnosticsProblems"; function capMessage(message: string, maxLength: number): string { if (message.length <= maxLength) { @@ -26,6 +27,16 @@ export class DiagnosticsCollector { private readonly errors = new ErrorHistoryRingBuffer(); registerProvider(provider: DiagnosticsProvider): void { + const existingProvider = this.providers.get(provider.name); + + if (existingProvider !== undefined) { + if (existingProvider === provider) { + return; + } + + throw new DuplicateDiagnosticsProviderProblem(provider.name); + } + this.providers.set(provider.name, provider); } diff --git a/packages/diagnostics-core/src/libs/problems/DiagnosticsProblems.ts b/packages/diagnostics-core/src/libs/problems/DiagnosticsProblems.ts new file mode 100644 index 000000000..f11f0d009 --- /dev/null +++ b/packages/diagnostics-core/src/libs/problems/DiagnosticsProblems.ts @@ -0,0 +1,18 @@ +import { Problem, ProblemCategory } from "@croco/problems-core"; + +/** + * 같은 이름으로 서로 다른 diagnostics provider가 등록될 때 발생하는 Problem입니다. + */ +export class DuplicateDiagnosticsProviderProblem extends Problem { + readonly code = "diagnostics-core/duplicate-provider"; + readonly category = ProblemCategory.InternalServerError; + + constructor(providerName: string) { + super(undefined, undefined, `Diagnostics provider '${providerName}' is already registered`, { + extensions: { + providerName, + retryable: false, + }, + }); + } +} diff --git a/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts b/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts index e41ccf753..746482a75 100644 --- a/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts +++ b/packages/diagnostics-core/src/tests/DiagnosticsIntegration.spec.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it } from "vitest"; import { DiagnosticsCollector } from "../libs/DiagnosticsCollector"; +import { DuplicateDiagnosticsProviderProblem } from "../libs/problems/DiagnosticsProblems"; import type { DiagnosticsProvider, HealthStatus, ErrorRecord } from "../libs/types"; class MockDiagnosticsProvider implements DiagnosticsProvider { @@ -132,6 +133,45 @@ describe("DiagnosticsCollector Integration", () => { expect(providers[1].name).toBe("cache"); }); + it("should keep the original provider when a different provider uses the same name", async () => { + const provider1 = new MockDiagnosticsProvider("db", { + status: "healthy", + component: "db", + lastChecked: new Date().toISOString(), + }); + const provider2 = new MockDiagnosticsProvider("db", { + status: "unhealthy", + component: "db", + lastChecked: new Date().toISOString(), + }); + + collector.registerProvider(provider1); + + expect(() => collector.registerProvider(provider2)).toThrow( + DuplicateDiagnosticsProviderProblem, + ); + + const providers = collector.getProviders(); + const report = await collector.getReport(); + + expect(providers).toEqual([provider1]); + expect(report.components).toHaveLength(1); + expect(report.components[0].status).toBe("healthy"); + }); + + it("should treat registering the same provider instance as idempotent", () => { + const provider = new MockDiagnosticsProvider("db", { + status: "healthy", + component: "db", + lastChecked: new Date().toISOString(), + }); + + collector.registerProvider(provider); + collector.registerProvider(provider); + + expect(collector.getProviders()).toEqual([provider]); + }); + it("should handle provider that throws in getHealth()", async () => { const failingProvider = new MockDiagnosticsProvider("failing-service", { status: "healthy", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d6cf2c2d0..8bd737988 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -648,7 +648,11 @@ importers: specifier: 4.0.16 version: 4.0.16(@opentelemetry/api@1.9.0)(@types/node@25.2.0)(jiti@2.6.1)(tsx@4.21.0)(yaml@2.8.3) - packages/diagnostics-core: {} + packages/diagnostics-core: + dependencies: + '@croco/problems-core': + specifier: workspace:* + version: link:../problems-core packages/docs: dependencies: