-
Notifications
You must be signed in to change notification settings - Fork 63
@W-20481777 Setup Generic Telemetry interface #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5770f43
e672b12
31d23ae
0e95fb0
3b981b8
936736d
e4dcea9
7bec3f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| import { beforeEach, describe, expect, it, Mock, vi } from 'vitest'; | ||
|
|
||
| // Mock the config module | ||
| vi.mock('../config.js', () => ({ | ||
| getConfig: vi.fn(), | ||
| })); | ||
|
|
||
| // Mock the provider modules | ||
| vi.mock('./moncloud.js', () => ({ | ||
| MonCloudTelemetryProvider: vi.fn().mockImplementation(() => ({ | ||
| initialize: vi.fn(), | ||
| addAttributes: vi.fn(), | ||
| })), | ||
| })); | ||
|
|
||
| vi.mock('./noop.js', () => ({ | ||
| NoOpTelemetryProvider: vi.fn().mockImplementation(() => ({ | ||
| initialize: vi.fn(), | ||
| addAttributes: vi.fn(), | ||
| })), | ||
| })); | ||
|
|
||
| import { getConfig } from '../config.js'; | ||
| import { initializeTelemetry } from './init.js'; | ||
| import { MonCloudTelemetryProvider } from './moncloud.js'; | ||
| import { NoOpTelemetryProvider } from './noop.js'; | ||
| import { TelemetryConfig } from './types.js'; | ||
|
|
||
| describe('initializeTelemetry', () => { | ||
| const mockGetConfig = getConfig as Mock; | ||
|
|
||
| const defaultTelemetryConfig: TelemetryConfig = { | ||
| enabled: true, | ||
| provider: 'noop', | ||
| }; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| // MonCloud tests | ||
| it('returns MonCloudTelemetryProvider when provider is "moncloud"', () => { | ||
| mockGetConfig.mockReturnValue({ | ||
| telemetry: { ...defaultTelemetryConfig, provider: 'moncloud' }, | ||
| }); | ||
|
|
||
| initializeTelemetry(); | ||
|
|
||
| expect(MonCloudTelemetryProvider).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| // NoOp tests | ||
| it('returns NoOpTelemetryProvider when telemetry is disabled', () => { | ||
| mockGetConfig.mockReturnValue({ | ||
| telemetry: { ...defaultTelemetryConfig, enabled: false }, | ||
| }); | ||
|
|
||
| const provider = initializeTelemetry(); | ||
|
|
||
| expect(NoOpTelemetryProvider).toHaveBeenCalled(); | ||
| expect(provider.initialize).toBeDefined(); | ||
| expect(provider.addAttributes).toBeDefined(); | ||
| }); | ||
|
|
||
| it('returns NoOpTelemetryProvider when provider is "noop"', () => { | ||
| mockGetConfig.mockReturnValue({ | ||
| telemetry: { ...defaultTelemetryConfig, provider: 'noop' }, | ||
| }); | ||
|
|
||
| initializeTelemetry(); | ||
|
|
||
| expect(NoOpTelemetryProvider).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('returns NoOpTelemetryProvider for unknown provider with warning', () => { | ||
| mockGetConfig.mockReturnValue({ | ||
| telemetry: { ...defaultTelemetryConfig, provider: 'unknown-provider' }, | ||
| }); | ||
|
|
||
| initializeTelemetry(); | ||
|
|
||
| expect(NoOpTelemetryProvider).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('falls back to NoOpTelemetryProvider on initialization error', () => { | ||
| const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); | ||
| const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); | ||
|
|
||
| // Make MonCloudTelemetryProvider throw during initialization | ||
| (MonCloudTelemetryProvider as Mock).mockImplementationOnce(() => ({ | ||
| initialize: vi.fn().mockImplementation(() => { | ||
| throw new Error('Init failed'); | ||
| }), | ||
| addAttributes: vi.fn(), | ||
| })); | ||
|
|
||
| mockGetConfig.mockReturnValue({ | ||
| telemetry: { ...defaultTelemetryConfig, provider: 'moncloud' }, | ||
| }); | ||
|
|
||
| const provider = initializeTelemetry(); | ||
|
|
||
| expect(consoleErrorSpy).toHaveBeenCalledWith( | ||
| 'Failed to initialize telemetry provider:', | ||
| expect.any(Error), | ||
| ); | ||
| expect(consoleWarnSpy).toHaveBeenCalledWith('Falling back to NoOp telemetry provider'); | ||
| expect(provider).toBeDefined(); | ||
|
|
||
| consoleErrorSpy.mockRestore(); | ||
| consoleWarnSpy.mockRestore(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| /** | ||
| * Telemetry initialization and provider factory | ||
| */ | ||
|
|
||
| import { resolve } from 'path'; | ||
|
|
||
| import { getConfig } from '../config.js'; | ||
| import { MonCloudTelemetryProvider } from './moncloud.js'; | ||
| import { NoOpTelemetryProvider } from './noop.js'; | ||
| import { TelemetryProvider } from './types.js'; | ||
|
|
||
| /** | ||
| * Initialize the telemetry provider based on configuration. | ||
| * | ||
| * This function should be called early in application startup, before any | ||
| * HTTP requests or other instrumented operations occur. | ||
| * | ||
| * @returns A configured telemetry provider | ||
| * | ||
| * @example | ||
| * ```typescript | ||
| * function main() { | ||
| * // Initialize telemetry first | ||
| * const telemetry = initializeTelemetry(); | ||
| * | ||
| * // Add global attributes | ||
| * telemetry.addAttributes({ | ||
| * 'tableau.server': config.server, | ||
| * 'mcp.version': '1.0.0', | ||
| * }); | ||
| * | ||
| * // Start application... | ||
| * } | ||
| * ``` | ||
| */ | ||
| export function initializeTelemetry(): TelemetryProvider { | ||
| const config = getConfig(); | ||
|
|
||
| // If telemetry is disabled, use NoOp provider | ||
| if (!config.telemetry.enabled) { | ||
| const provider = new NoOpTelemetryProvider(); | ||
| provider.initialize(); | ||
| return provider; | ||
| } | ||
|
|
||
| let provider: TelemetryProvider; | ||
|
|
||
| try { | ||
| // Select provider based on configuration | ||
| switch (config.telemetry.provider) { | ||
| case 'moncloud': | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think having separate cases for moncloud vs custom will become even more apparent when i need to write custom metrics for the tools using telemetry.addAttributes(), as each underlying telemetry provider has different apis for doing that |
||
| provider = new MonCloudTelemetryProvider(); | ||
| break; | ||
|
|
||
| case 'custom': | ||
| // Load custom provider from user's filesystem | ||
| provider = loadCustomProvider(config.telemetry.providerConfig); | ||
| break; | ||
|
|
||
| case 'noop': | ||
| default: | ||
| if (config.telemetry.provider !== 'noop') { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The type of |
||
| console.warn( | ||
| `Unknown telemetry provider: ${config.telemetry.provider}. Using NoOp provider.`, | ||
| ); | ||
| } | ||
| provider = new NoOpTelemetryProvider(); | ||
| } | ||
|
|
||
| // Initialize the provider | ||
| provider.initialize(); | ||
| return provider; | ||
| } catch (error) { | ||
| console.error('Failed to initialize telemetry provider:', error); | ||
| console.warn('Falling back to NoOp telemetry provider'); | ||
|
|
||
| // Fallback to NoOp on error - telemetry failures shouldn't break the application | ||
| const fallbackProvider = new NoOpTelemetryProvider(); | ||
| fallbackProvider.initialize(); | ||
| return fallbackProvider; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Load a custom telemetry provider from user's filesystem or npm package. | ||
| * | ||
| * The custom provider module should export a default class that implements TelemetryProvider. | ||
| * | ||
| * @param config - Provider configuration containing the module path | ||
| * @returns A configured custom telemetry provider | ||
| * | ||
| * @example Custom provider from file | ||
| * ```bash | ||
| * TELEMETRY_PROVIDER=custom | ||
| * TELEMETRY_PROVIDER_CONFIG='{"module":"./my-telemetry.js"}' | ||
| * ``` | ||
| * | ||
| * @example Custom provider from npm package | ||
| * ```bash | ||
| * TELEMETRY_PROVIDER=custom | ||
| * TELEMETRY_PROVIDER_CONFIG='{"module":"my-company-telemetry"}' | ||
| * ``` | ||
| */ | ||
| function loadCustomProvider(config?: Record<string, unknown>): TelemetryProvider { | ||
| if (!config?.module) { | ||
| throw new Error( | ||
| 'Custom telemetry provider requires "module" in providerConfig. ' + | ||
| 'Example: TELEMETRY_PROVIDER_CONFIG=\'{"module":"./my-telemetry.js"}\'', | ||
| ); | ||
| } | ||
|
|
||
| const modulePath = config.module as string; | ||
|
|
||
| // Determine if it's a file path or npm package name | ||
| let resolvedPath: string; | ||
|
|
||
| if (modulePath.startsWith('.') || modulePath.startsWith('/')) { | ||
| // File path - resolve relative to process working directory (user's project root) | ||
| resolvedPath = resolve(process.cwd(), modulePath); | ||
| } else { | ||
| // npm package name - require as-is | ||
| resolvedPath = modulePath; | ||
| } | ||
|
|
||
| try { | ||
| // eslint-disable-next-line @typescript-eslint/no-require-imports -- Sync load for preload script | ||
| const module = require(resolvedPath); | ||
|
|
||
| // Look for default export or named export "TelemetryProvider" | ||
| const ProviderClass = module.default || module.TelemetryProvider; | ||
|
|
||
| if (!ProviderClass) { | ||
| throw new Error( | ||
| `Module ${modulePath} must export a default class or named export "TelemetryProvider" ` + | ||
| 'that implements the TelemetryProvider interface', | ||
| ); | ||
| } | ||
|
|
||
| // Instantiate the provider with the full config | ||
| return new ProviderClass(config); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before returning the instance, let's verify the class implements the interface. I did something similar here: https://github.com/tableau/tableau-mcp/blob/anyoung/session-store/src/server/storage/storeFactory.ts#L157 |
||
| } catch (error) { | ||
| // Provide helpful error message with common issues | ||
| let errorMessage = `Failed to load custom telemetry provider from "${modulePath}". `; | ||
|
|
||
| if ((error as any).code === 'MODULE_NOT_FOUND') { | ||
| errorMessage += | ||
| 'Module not found. ' + | ||
| 'If using a file path, ensure the file exists and the path is correct. ' + | ||
| 'If using an npm package, ensure it is installed.'; | ||
| } else { | ||
| errorMessage += `Error: ${error}`; | ||
| } | ||
|
|
||
| throw new Error(errorMessage); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a type guard for the telemtryProvider, similar to how
this.authis set below. Currently the runtime value could be anything.Something like: