Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 19 additions & 54 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 10 additions & 7 deletions packages/observability/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,23 @@
"scripts": {
"build": "tsc",
"dev": "tsc --watch",
"lint": "eslint . --max-warnings 0"
"lint": "eslint . --max-warnings 0",
"test": "vitest run"
},
"dependencies": {
"@opentelemetry/api": "^1.7.0",
"@opentelemetry/sdk-node": "^0.45.0",
"@opentelemetry/exporter-trace-otlp-grpc": "^0.45.0",
"@opentelemetry/exporter-metrics-otlp-grpc": "^0.45.0",
"@opentelemetry/exporter-trace-otlp-grpc": "^0.45.0",
"@opentelemetry/instrumentation": "^0.45.0",
"@opentelemetry/resources": "^1.18.0",
"@opentelemetry/sdk-node": "^0.45.0",
"@opentelemetry/semantic-conventions": "^1.18.0"
},
"devDependencies": {
"@types/node": "^20",
"typescript": "^5",
"eslint": "^8"
"@repo/typescript-config": "*",
"@types/node": "^20.19.37",
"eslint": "^8",
"typescript": "^5.9.3",
"vitest": "^4.0.18"
}
}
}
89 changes: 89 additions & 0 deletions packages/observability/src/observability.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { describe, it, expect, beforeEach, vi } from 'vitest';

// Mock the open telemetry modules
vi.mock('@opentelemetry/sdk-node', () => {
return {
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test file is located under src/, and packages/observability/tsconfig.json currently includes all of src without excluding *.test.ts. That means tsc will emit this test into dist/ during npm run build, which is usually undesirable for a library package. Consider moving tests outside src (e.g., tests/), or updating the tsconfig exclude/include patterns to omit test files.

Copilot uses AI. Check for mistakes.
NodeSDK: vi.fn().mockImplementation(() => ({
start: vi.fn(),
shutdown: vi.fn(),
})),
};
});

vi.mock('@opentelemetry/api', async () => {
const actual = await vi.importActual('@opentelemetry/api');
return {
...actual,
trace: {
getTracer: vi.fn().mockReturnValue({
startActiveSpan: vi.fn((name, callback) => {
const mockSpan = {
setAttribute: vi.fn(),
setStatus: vi.fn(),
recordException: vi.fn(),
end: vi.fn(),
};
return callback(mockSpan);
}),
}),
},
metrics: {
getMeter: vi.fn().mockReturnValue({
createCounter: vi.fn().mockReturnValue({ add: vi.fn() }),
createHistogram: vi.fn().mockReturnValue({ record: vi.fn() }),
}),
},
};
});

describe('observability module exports', () => {
const testConfig = {
serviceName: 'test-service',
enabled: false // disabled to prevent actual connections
};

beforeEach(() => {
// Reset vi modules so that each test gets a fresh internal observabilityInstance variable
vi.resetModules();
vi.clearAllMocks();
});

describe('initObservability', () => {
it('should initialize and return an Observability instance', async () => {
// Import freshly to get a new instance
const mod = await import('./observability');
const instance = mod.initObservability(testConfig);

expect(instance).toBeInstanceOf(mod.Observability);
// @ts-ignore - accessing private member for verification
expect(instance.serviceName).toBe('test-service');
// @ts-ignore - accessing private member for verification
expect(instance.enabled).toBe(false);
Comment on lines +60 to +63
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using @ts-ignore to access private members for verification makes tests brittle and bypasses type safety. This goes against the 'strict type hinting' principle from the repository style guide.

A better approach is to test the observable behavior. For example, to test the enabled: false case, you could assert that NodeSDK is not instantiated. To test the serviceName, you could assert that trace.getTracer is called with the correct name in a separate test where observability is enabled.

References
  1. The use of @ts-ignore bypasses TypeScript's type checking, which is contrary to the principle of strict type hinting mentioned in the repository style guide (line 7). (link)

Comment on lines +60 to +63
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assertions rely on @ts-ignore to access private members (serviceName, enabled). That makes the tests brittle against refactors (e.g., switching to #private fields) and tests implementation details rather than behavior. Prefer asserting observable behavior instead (e.g., verify OpenTelemetry SDK setup methods are/aren’t invoked based on config), or add a minimal read-only getter if you really need to validate config.

Copilot uses AI. Check for mistakes.
});

it('should return the same instance when called multiple times', async () => {
const mod = await import('./observability');
const instance1 = mod.initObservability(testConfig);
const instance2 = mod.initObservability({ serviceName: 'other-service' }); // Should ignore this config

expect(instance1).toBe(instance2);
// @ts-ignore
expect(instance2.serviceName).toBe('test-service');
Comment on lines +72 to +73
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

As with the previous comment, it's best to avoid @ts-ignore to maintain strict type safety.

To verify that the singleton instance is not reconfigured on subsequent calls, you could check that the initialization logic (e.g., calls to trace.getTracer()) is only executed once, even when initObservability is called multiple times. This tests the behavior without accessing private implementation details.

References
  1. The use of @ts-ignore bypasses TypeScript's type checking, which is contrary to the principle of strict type hinting mentioned in the repository style guide (line 7). (link)

});
});

describe('getObservability', () => {
it('should throw an error if called before initialization', async () => {
const mod = await import('./observability');
expect(() => mod.getObservability()).toThrow('Observability not initialized. Call initObservability() first.');
});

it('should return the instance if already initialized', async () => {
const mod = await import('./observability');
const initialized = mod.initObservability(testConfig);
const retrieved = mod.getObservability();

expect(retrieved).toBe(initialized);
});
});
});
Loading