Parcourir la source

fix(otel): add error resilience to yjs metrics observable callback

- Add loggerDiag and wrap addBatchObservableCallback body in try/catch
  to match the pattern used in all other custom-metrics files
- Tighten getDocsCount parameter type from Map to ReadonlyMap (only .size is read)
- Add diag mock and error-propagation test in yjs-metrics.spec.ts

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Yuki Takei il y a 2 semaines
Parent
commit
ad2bf5ca90

+ 25 - 0
apps/app/src/features/opentelemetry/server/custom-metrics/yjs-metrics.spec.ts

@@ -10,7 +10,14 @@ vi.mock('~/utils/logger', () => ({
   }),
 }));
 
+const { mockDiagError } = vi.hoisted(() => ({
+  mockDiagError: vi.fn(),
+}));
+
 vi.mock('@opentelemetry/api', () => ({
+  diag: {
+    createComponentLogger: vi.fn(() => ({ error: mockDiagError })),
+  },
   metrics: {
     getMeter: vi.fn(),
   },
@@ -31,6 +38,7 @@ describe('addYjsMetrics', () => {
   beforeEach(() => {
     vi.clearAllMocks();
     mockDocs.clear();
+    mockDiagError.mockReset();
     vi.mocked(metrics.getMeter).mockReturnValue(mockMeter);
     mockMeter.createObservableGauge.mockReturnValue(mockGauge);
   });
@@ -127,6 +135,23 @@ describe('addYjsMetrics', () => {
       await callback(mockResult);
       expect(mockResult.observe).toHaveBeenLastCalledWith(mockGauge, 2);
     });
+
+    it('does not propagate errors and logs via diag when observation throws', async () => {
+      addYjsMetrics();
+
+      const throwingResult = {
+        observe: vi.fn().mockImplementation(() => {
+          throw new Error('otel observe failed');
+        }),
+      };
+      const callback = mockMeter.addBatchObservableCallback.mock.calls[0][0];
+
+      await expect(async () => callback(throwingResult)).not.toThrow();
+      expect(mockDiagError).toHaveBeenCalledWith(
+        expect.stringContaining('yjs'),
+        expect.objectContaining({ error: expect.any(Error) }),
+      );
+    });
   });
 
   describe('getDocsCount — defensive helper', () => {

+ 10 - 3
apps/app/src/features/opentelemetry/server/custom-metrics/yjs-metrics.ts

@@ -1,16 +1,19 @@
-import { metrics } from '@opentelemetry/api';
+import { diag, metrics } from '@opentelemetry/api';
 import { docs } from 'y-websocket/bin/utils';
 
 import loggerFactory from '~/utils/logger';
 
 const logger = loggerFactory('growi:opentelemetry:custom-metrics:yjs');
+const loggerDiag = diag.createComponentLogger({
+  namespace: 'growi:custom-metrics:yjs',
+});
 
 /**
  * Returns the number of documents in the given map.
  * Returns 0 when the map is undefined or null (y-websocket not yet initialised).
  */
 export function getDocsCount(
-  d: Map<string, unknown> | undefined | null,
+  d: ReadonlyMap<string, unknown> | undefined | null,
 ): number {
   return d?.size ?? 0;
 }
@@ -31,7 +34,11 @@ export function addYjsMetrics(): void {
 
   meter.addBatchObservableCallback(
     (result) => {
-      result.observe(yjsDocsCountGauge, getDocsCount(docs));
+      try {
+        result.observe(yjsDocsCountGauge, getDocsCount(docs));
+      } catch (error) {
+        loggerDiag.error('Failed to collect yjs metrics', { error });
+      }
     },
     [yjsDocsCountGauge],
   );