BZ-44136: feat(observability): Enterprise Langfuse Integration for AI Monitoring - #193
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds optional observability/analytics: Langfuse and OpenTelemetry initialization, tracing, and flushing. Extends TelemetryService, BaseProvider streaming/generation, providers’ stream calls, NeuroLink construction, session bootstrapping, CLI shutdown, and public exports. Updates env/example and dependencies. Introduces instrumentation utilities and observability types without removing existing features. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant BaseProvider
participant Telemetry as TelemetryService
participant Langfuse as Langfuse/OTEL
participant Model as AI Provider
Client->>BaseProvider: stream(...)
BaseProvider->>Telemetry: initializeTelemetryService?
alt telemetry enabled
BaseProvider->>Langfuse: start trace/generation (stream)
BaseProvider->>Model: streamText(..., experimental_telemetry)
Model-->>BaseProvider: chunks
loop for each chunk
BaseProvider->>Langfuse: record chunk event
end
BaseProvider->>Langfuse: complete generation/trace
Telemetry->>Langfuse: flush
else fallback
BaseProvider->>Model: streamText(...)
Model-->>BaseProvider: chunks
end
BaseProvider-->>Client: async iterable (chunks)
sequenceDiagram
autonumber
participant User
participant CLI as CLI Entry
participant Cmd as CLI CommandFactory
participant Inst as instrumentation.ts
User->>CLI: run command
CLI->>CLI: parse/execute
alt on success or error or signal/beforeExit
CLI->>Inst: flushOpenTelemetry()
CLI->>process: exit
end
Cmd->>Inst: flushOpenTelemetry() (pre-exit in paths)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.env.example (1)
426-432: Use standard OpenTelemetry sampler variables
Replace OTEL_SAMPLING_RATIO in.env.examplewith:- OTEL_SAMPLING_RATIO=1.0 + OTEL_TRACES_SAMPLER=traceidratio + OTEL_TRACES_SAMPLER_ARG=1.0Update any remaining OTEL_SAMPLING_RATIO references (e.g. in
memory-bank/otel-langfuse-integration-analysis.md) to use OTEL_TRACES_SAMPLER_ARG.
🧹 Nitpick comments (19)
src/lib/neurolink.ts (1)
798-894: Consider using v4 of the Langfuse SDK.The Langfuse v3.38.5 SDK being used is officially deprecated. According to the retrieved learnings, Langfuse released a full TypeScript SDK v4 in August 2025 with a major rewrite, and the official guidance is to migrate to v4 for new development.
Consider upgrading to Langfuse v4 SDK as per official recommendations:
- "langfuse": "^3.38.5" + "langfuse": "^4.0.0"You may need to review the v4 migration guide for API changes.
Based on learnings
src/lib/services/server/ai/observability/langfusePromptManager.ts (3)
8-8: Update import to use Langfuse v4 SDK.As mentioned in the main file review, the Langfuse v3 SDK is deprecated. The import should be updated to v4 once migrated.
After upgrading the package, update the import according to v4 documentation.
24-43: Consider using the centralized logData from telemetryService.There's a local
logDataimplementation here that duplicates functionality fromsrc/lib/telemetry/telemetryService.ts. Consider importing and using the centralized logging function for consistency across the codebase.-function logData( - component: string, - eventName: string, - severity: "DEBUG" | "INFO" | "WARN" | "ERROR", - data: Record<string, unknown>, -): void { - const logLevel = severity.toLowerCase() as - | "debug" - | "info" - | "warn" - | "error"; - - logger[logLevel](`[${component}] ${eventName}`, { - component, - eventName, - severity, - timestamp: new Date().toISOString(), - ...data, - }); -} +import { logData, LogSeverity } from "../../../../telemetry/telemetryService.js"; // Then use it with the appropriate severity: // logData(componentIdentifier, "initializationSuccess", LogSeverity.INFO, {...})As per coding guidelines
111-163: Add retry logic for transient failures.The prompt fetching doesn't have retry logic for transient network failures. Consider implementing exponential backoff retry logic similar to other external service calls in the codebase.
async getSystemPrompt(): Promise<string> { const promptName = getLangfuseSystemPromptName(); const promptLabel = getLangfuseSystemPromptLabel(); if (!this.isConfigured || !this.langfuse) { logData(componentIdentifier, "promptFetchSkipped", "WARN", { reason: "not_configured", promptName, isConfigured: this.isConfigured, hasClient: !!this.langfuse, }); throw new Error("Langfuse not configured"); } + const maxRetries = 3; + const baseDelay = 1000; + + for (let attempt = 1; attempt <= maxRetries; attempt++) { try { logData(componentIdentifier, "promptFetchStarted", "DEBUG", { promptName, promptLabel, - fetchAttempt: true, + fetchAttempt: attempt, + maxRetries, }); const prompt = await this.langfuse.getPrompt(promptName, undefined, { label: promptLabel, }); if (!prompt?.prompt) { logData(componentIdentifier, "promptFetchEmpty", "ERROR", { promptName, promptLabel, hasPromptObject: !!prompt, promptContentEmpty: !prompt?.prompt, }); throw new Error("Empty prompt received"); } logData(componentIdentifier, "promptFetchSuccess", "INFO", { promptName, promptLabel, promptLength: prompt.prompt.length, promptPreview: prompt.prompt.substring(0, 100) + "...", + attemptNumber: attempt, }); return prompt.prompt; } catch (error) { + const isLastAttempt = attempt === maxRetries; + logData(componentIdentifier, "promptFetchError", "ERROR", { promptName, promptLabel, errorMessage: error instanceof Error ? error.message : String(error), stack: error instanceof Error ? error.stack : undefined, + attempt, + willRetry: !isLastAttempt, }); - throw new Error("Langfuse fetch failed"); + + if (isLastAttempt) { + throw new Error("Langfuse fetch failed after retries"); + } + + // Exponential backoff + await new Promise(resolve => setTimeout(resolve, baseDelay * Math.pow(2, attempt - 1))); } + } + + throw new Error("Langfuse fetch failed"); }src/lib/services/server/config/ai.ts (2)
17-23: Inconsistent environment variable naming for Langfuse base URL.The function checks both
LANGFUSE_BASE_URLandLANGFUSE_BASEURLwhich could lead to confusion. Consider standardizing on one format.export const getLangfuseHost = (): string => { return ( - process.env.LANGFUSE_BASE_URL ?? - process.env.LANGFUSE_BASEURL ?? + process.env.LANGFUSE_BASE_URL ?? "https://cloud.langfuse.com" ).trim(); };Also update line 133 in the validation function to use
LANGFUSE_BASE_URLconsistently.
221-227: Add bounds validation for temperature.Temperature values should typically be between 0 and 2 for most models. Consider adding validation or clamping.
export const getDefaultTemperature = (): number => { - return parseFloat(process.env.DEFAULT_TEMPERATURE || "0.7"); + const temp = parseFloat(process.env.DEFAULT_TEMPERATURE || "0.7"); + if (isNaN(temp)) return 0.7; + // Clamp between 0 and 2 (common range for most models) + return Math.max(0, Math.min(2, temp)); };src/lib/core/baseProvider.ts (2)
2485-2521: Hoist per‑chunk dynamic import out of the loopImporting recordSSEEvent inside the for‑await loop adds overhead per chunk. Import it once before the loop and reuse the function.
Apply this diff to hoist the import:
- // Enhanced stream wrapper with real-time Langfuse chunk tracking - const enhancedStreamWrapper = async function* (this: BaseProvider) { + // Preload SSE event recorder once (if generation exists) + let recordSSEEventFn: + | (( + observation: unknown, + options: { + eventType: string; + message: string; + status: string; + data?: unknown; + level?: "ERROR" | "WARNING" | "DEFAULT"; + }, + ) => void) + | null = null; + if (langfuseGeneration) { + try { + const mod = await import( + "../services/server/ai/observability/langfuseClient.js" + ); + recordSSEEventFn = mod.recordSSEEvent; + } catch { + recordSSEEventFn = null; + } + } + + // Enhanced stream wrapper with real-time Langfuse chunk tracking + const enhancedStreamWrapper = async function* (this: BaseProvider) { let accumulatedContent = ""; try { for await (const chunk of realStreamResult.stream) { // Only yield content chunks, filter out audio chunks if ("content" in chunk && typeof chunk.content === "string") { yield { content: chunk.content }; // Process text content chunks for Langfuse tracking const chunkContent = chunk.content; accumulatedContent += chunkContent; chunkIndex++; // Send individual chunk to Langfuse if available if (langfuseGeneration && chunkContent.length > 0) { try { - const { recordSSEEvent } = await import( - "../services/server/ai/observability/langfuseClient.js" - ); - - recordSSEEvent(langfuseGeneration as never, { + recordSSEEventFn?.(langfuseGeneration as never, { eventType: "stream:chunk", message: chunkContent, status: "streaming", data: { chunkIndex,
2691-2704: Simplify synthetic streaming delaysetTimeout always returns a truthy handle; the failure check is unnecessary. Simplify the promise to reduce code paths.
Apply this diff:
- await new Promise((resolve, reject) => { - const timeoutId = setTimeout( - resolve, - Math.random() * 9 + 1, - ); - // Handle potential timeout issues - if (!timeoutId) { - reject(new Error("Failed to create timeout")); - } - }).catch((err) => { - logger.error("Error in streaming delay:", err); - }); + await new Promise((resolve) => + setTimeout(resolve, Math.random() * 9 + 1), + );memory-bank/otel-langfuse-integration-analysis.md (1)
429-432: Doc update: reflect Langfuse v4 migration path and clarify exporter strategy
- Recommend adding a note that Langfuse v3.x SDK is deprecated in favor of the TS SDK v4 with a migration path. This helps readers plan upgrades.
- Also clarify whether the “OTel → Langfuse exporter” approach is an optional future enhancement vs. in-scope for this PR, to avoid implying it’s already implemented.
Based on learnings
Also applies to: 768-777
src/lib/telemetry/telemetryService.ts (2)
244-263: Set global isNeuroLinkTelemetryInitialized flag after SDK startlogData tries to enrich logs with span context only when this flag is set. Set it once SDK starts to enable correlation.
Apply this diff:
await this.sdk?.start(); logger.debug("[Telemetry] SDK started successfully"); + (globalThis as { isNeuroLinkTelemetryInitialized?: boolean }) + .isNeuroLinkTelemetryInitialized = true;
645-661: Reduce log verbosity for hot pathsNumerous logger.info calls in metrics/tool paths can introduce noise and cost. Consider downgrading to debug or gating under NEUROLINK_DEBUG.
Also applies to: 683-700, 1125-1145, 1153-1172, 1179-1200
src/lib/services/server/ai/observability/langfuseClient.ts (1)
151-169: Plan migration to Langfuse v4 TS SDK
Dependency is pinned at ^3.38.5 (v3, deprecated); continue using v3 for now but schedule an upgrade to the v4 TypeScript SDK..env.example (2)
415-421: Order keys and quote booleans; avoid linter noise and parsing surprises
- Reorder Langfuse keys and quote boolean to satisfy dotenv-linter and avoid accidental truthy parsing.
-# Langfuse Configuration -# Sign up at: https://cloud.langfuse.com or self-host -LANGFUSE_PUBLIC_KEY=pk-lf-your-public-key-here -LANGFUSE_SECRET_KEY=sk-lf-your-secret-key-here -LANGFUSE_BASE_URL=https://cloud.langfuse.com -LANGFUSE_ENABLED=false +# Langfuse Configuration +# Sign up at: https://cloud.langfuse.com or self-host +LANGFUSE_BASE_URL=https://cloud.langfuse.com +LANGFUSE_ENABLED="false" +LANGFUSE_PUBLIC_KEY=pk-lf-your-public-key-here +LANGFUSE_SECRET_KEY=sk-lf-your-secret-key-hereIf you prefer alphabetical sorting for all keys in this section, I can provide a full reordering diff. Also confirm your .env loader doesn’t require unquoted booleans.
423-424: Quote NEUROLINK_TELEMETRY_ENABLEDSome dotenv parsers and linters expect quoted values; “false” is still a string, but quoting prevents lint warnings and accidental whitespace issues.
-NEUROLINK_TELEMETRY_ENABLED=false # Enable comprehensive telemetry +NEUROLINK_TELEMETRY_ENABLED="false" # Enable comprehensive telemetrymemory-bank/systemPatterns.md (5)
8-12: Fix directory tree: ai.ts path is outside observability folderThe tree shows config/ai.ts under observability, but the actual path is src/lib/services/server/config/ai.ts. This can mislead readers.
src/lib/services/server/ai/observability/ ├── langfuseClient.ts # Core Langfuse integration with trace/generation tracking ├── langfusePromptManager.ts # Structured prompt management and versioning -└── config/ai.ts # Centralized AI service configuration +src/lib/services/server/config/ai.ts # Centralized AI service configuration
16-32: Correct TS import extension and clarify conditional wrapper dependencyUse .ts (or no extension) for TS example imports; .js may be confusing. Also consider a short note that processToolEvents is provided by streaming telemetry pattern below.
-const { isLangfuseEnabled } = await import("../services/server/config/ai.js"); +const { isLangfuseEnabled } = await import("../services/server/config/ai");
36-60: Undefined variables in example (startTime, chainInput); make snippet self-containedThe example references startTime and chainInput without definition. Add parameters or define them to avoid copy-paste errors.
export class TelemetryService { - recordToolChain(chainName: string, toolsUsed: string[], context: ExecutionContext): void { - const sessionId = context.sessionId || 'unknown'; - const duration = Date.now() - startTime; + recordToolChain( + chainName: string, + toolsUsed: string[], + context: ExecutionContext, + startTime: number, + chainInput?: unknown + ): void { + const sessionId = context.sessionId || 'unknown'; + const duration = Date.now() - startTime; @@ - hasInput: !!chainInput, + hasInput: !!chainInput, success: true, }); }
62-78: Streaming snippet depends on this.providerName; clarify contextThis is likely within a class; add a short comment that this is inside BaseProvider or pass providerName explicitly to avoid confusion when copying.
110-142: Undefined _analysisSchema in examples; either define or remove to prevent confusionBoth executeStream and executeFakeStreaming receive _analysisSchema, which isn’t defined in the snippet.
- const realStreamResult = await this.executeStream(options, _analysisSchema); + const realStreamResult = await this.executeStream(options); @@ - return await this.executeFakeStreaming(options, _analysisSchema, realStreamError); + return await this.executeFakeStreaming(options, realStreamError);Alternatively, add a placeholder definition above the snippet:
// const _analysisSchema = /* your analysis schema here */;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.env.example(1 hunks)memory-bank/activeContext.md(2 hunks)memory-bank/otel-langfuse-integration-analysis.md(1 hunks)memory-bank/progress.md(1 hunks)memory-bank/systemPatterns.md(1 hunks)package.json(1 hunks)src/lib/core/baseProvider.ts(11 hunks)src/lib/neurolink.ts(3 hunks)src/lib/services/server/ai/observability/langfuseClient.ts(1 hunks)src/lib/services/server/ai/observability/langfusePromptManager.ts(1 hunks)src/lib/services/server/config/ai.ts(1 hunks)src/lib/telemetry/telemetryService.ts(13 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-17T17:55:15.261Z
Learnt from: RajuSudhar
PR: juspay/neurolink#173
File: src/lib/index.ts:16-16
Timestamp: 2025-09-17T17:55:15.261Z
Learning: In src/lib/types/providers.ts, ProviderConfig was renamed to AIModelProviderConfig to deduplicate type names, as there was an existing ProviderConfig type that better suited the "ProviderConfig" name. This was an intentional breaking change for better type organization.
Applied to files:
src/lib/services/server/config/ai.ts
🧬 Code graph analysis (6)
src/lib/services/server/config/ai.ts (2)
src/lib/services/server/ai/observability/langfuseClient.ts (2)
isLangfuseEnabled(322-324)getLangfuseConfig(84-106)src/lib/utils/logger.ts (1)
logger(341-380)
src/lib/services/server/ai/observability/langfusePromptManager.ts (4)
src/lib/telemetry/telemetryService.ts (1)
logData(56-108)src/lib/utils/logger.ts (2)
logger(341-380)error(223-225)src/lib/services/server/config/ai.ts (9)
getLangfuseSecretKey(35-37)getLangfusePublicKey(31-33)getLangfuseProxyUrl(25-29)isLangfuseEnabled(61-64)getLangfuseRequestTimeout(73-75)getLangfuseEnvironment(42-48)getLangfuseRelease(50-56)getLangfuseSystemPromptName(92-96)getLangfuseSystemPromptLabel(98-103)src/lib/services/server/ai/observability/langfuseClient.ts (1)
isLangfuseEnabled(322-324)
src/lib/services/server/ai/observability/langfuseClient.ts (4)
src/lib/telemetry/telemetryService.ts (1)
logData(56-108)src/lib/utils/logger.ts (3)
logger(341-380)debug(190-192)error(223-225)src/lib/services/server/config/ai.ts (11)
getLangfuseSecretKey(35-37)getLangfusePublicKey(31-33)getLangfuseProxyUrl(25-29)getLangfuseEnvironment(42-48)getLangfuseRelease(50-56)getLangfuseRequestTimeout(73-75)getLangfuseSampleRate(77-79)isLangfuseDebugModeEnabled(66-68)getLangfuseFlushAt(81-83)getLangfuseFlushInterval(85-87)isLangfuseEnabled(61-64)src/lib/neurolink.ts (1)
initializeLangfuse(801-894)
src/lib/neurolink.ts (3)
src/lib/services/server/ai/observability/langfuseClient.ts (4)
isLangfuseEnabled(322-324)initializeLangfuse(329-331)getLangfuseHealthStatus(350-352)shutdownLangfuse(336-338)src/lib/services/server/config/ai.ts (1)
isLangfuseEnabled(61-64)src/lib/utils/logger.ts (2)
logger(341-380)error(223-225)
src/lib/core/baseProvider.ts (9)
src/lib/types/typeAliases.ts (1)
ValidationSchema(25-25)src/lib/utils/logger.ts (2)
logger(341-380)error(223-225)src/lib/types/streamTypes.ts (2)
StreamResult(216-257)StreamOptions(143-210)src/lib/index.ts (2)
TextGenerationOptions(214-214)generateText(236-242)src/lib/neurolink.ts (1)
generateText(1855-1871)src/lib/services/server/ai/observability/langfuseClient.ts (8)
isLangfuseEnabled(322-324)createStreamTrace(491-536)createLLMGeneration(541-585)completeGeneration(696-724)completeTrace(729-757)flushLangfuse(343-345)recordSSEEvent(646-691)createToolSpan(590-641)src/lib/services/server/config/ai.ts (1)
isLangfuseEnabled(61-64)src/lib/telemetry/telemetryService.ts (2)
isTelemetryEnabled(153-158)TelemetryService(110-1299)src/lib/core/constants.ts (1)
DEFAULT_MAX_STEPS(10-10)
src/lib/telemetry/telemetryService.ts (5)
src/lib/utils/logger.ts (2)
logger(341-380)error(223-225)src/lib/services/server/ai/observability/langfuseClient.ts (4)
isLangfuseEnabled(322-324)initializeLangfuse(329-331)flushLangfuse(343-345)createLangfuseTrace(358-423)src/lib/services/server/config/ai.ts (1)
isLangfuseEnabled(61-64)src/lib/neurolink.ts (1)
initializeLangfuse(801-894)src/lib/types/streamTypes.ts (1)
AudioChunk(136-141)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 419-419: [UnorderedKey] The LANGFUSE_BASE_URL key should go before the LANGFUSE_PUBLIC_KEY key
(UnorderedKey)
[warning] 420-420: [UnorderedKey] The LANGFUSE_ENABLED key should go before the LANGFUSE_PUBLIC_KEY key
(UnorderedKey)
[warning] 423-423: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
[warning] 431-431: [UnorderedKey] The OTEL_SAMPLING_RATIO key should go before the OTEL_SERVICE_NAME key
(UnorderedKey)
🔇 Additional comments (9)
src/lib/neurolink.ts (2)
1532-1563: LGTM! Well-structured public observability initialization method.The public
initializeLangfuseObservability()method provides a clean API for external initialization with proper error handling that won't break execution if Langfuse fails.
1565-1596: LGTM! Comprehensive graceful shutdown implementation.The
shutdown()method properly handles cleanup for both Langfuse and MCP servers with nested error handling that prevents cascading failures during shutdown.src/lib/services/server/ai/observability/langfusePromptManager.ts (1)
49-63: LGTM! Well-crafted fallback prompt with clear instructions.The fallback system prompt is comprehensive and provides clear guidance for tool usage, parameter handling, and response formatting. This ensures consistent behavior even when Langfuse is unavailable.
src/lib/services/server/config/ai.ts (1)
108-157: LGTM! Comprehensive configuration validation.The validation function provides thorough checks for all Langfuse configuration with clear error and warning messages. This will help operators quickly identify configuration issues.
src/lib/telemetry/telemetryService.ts (1)
387-590: Avoid duplicate Langfuse traces in streaming pathwaystraceAIStreamRequest creates Langfuse traces, while BaseProvider wraps streams with its own Langfuse instrumentation. If both are used, duplicate traces may be emitted.
Please confirm that BaseProvider.stream never calls traceAIStreamRequest (current BaseProvider uses only initializeTelemetryService + internal wrapper). If other call sites use traceAIStreamRequest concurrently for the same operation, add a guard (e.g., a context flag) to prevent double-instrumentation.
src/lib/services/server/ai/observability/langfuseClient.ts (1)
243-258: Lifecycle APIs: ensure compatibility of shutdownAsync/flushAsyncSome langfuse versions use shutdown/flush or async variants. If targeting 3.38.5, verify these APIs exist to avoid runtime errors.
Would you like me to fetch the current v3.38.5 README/API docs to confirm shutdownAsync/flushAsync signatures?
Also applies to: 263-277
memory-bank/progress.md (1)
20-26: Same Langfuse v3 deprecation concern as noted above.memory-bank/systemPatterns.md (2)
185-188: Non-breaking opt-in guard reads wellOptional observability activation via options + isLangfuseEnabled aligns with acceptance criteria.
101-106: Runtime gating includes NEUROLINK_TELEMETRY_ENABLED Verified ininitializeTelemetry(index.ts) and incore/baseProvider.ts; telemetry only initializes when NEUROLINK_TELEMETRY_ENABLED is true, so no further changes required.
3c35a7d to
c7f29a4
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive Langfuse observability integration for enterprise AI monitoring in NeuroLink. The integration adds real-time tracing, performance analytics, and compliance auditing capabilities across all AI operations while maintaining zero overhead when disabled.
Key changes:
- Complete Langfuse client infrastructure with trace/generation/tool tracking
- Enhanced TelemetryService with detailed instrumentation and structured logging
- Streamlined BaseProvider architecture with conditional telemetry integration
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| telemetryService.ts | Enhanced with comprehensive logging, Langfuse integration, and tool/stream tracing capabilities |
| ai.ts | New centralized AI configuration module with Langfuse validation and settings |
| langfusePromptManager.ts | New structured prompt management system with fallback support |
| langfuseClient.ts | New core Langfuse integration with trace/generation/tool tracking |
| neurolink.ts | Added Langfuse initialization and graceful shutdown methods |
| baseProvider.ts | Enhanced streaming logic with telemetry integration and tool event processing |
| package.json | Added langfuse dependency for production observability |
| memory-bank/*.md | Updated documentation with observability patterns and progress |
| .env.example | Added Langfuse and observability configuration examples |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
fe0f9b9 to
a490a06
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
src/lib/telemetry/telemetryService.ts:1
- This line removes the
awaitkeyword from an async method call, which will return a Promise instead of the resolved value. This will break the external tools functionality.
import { NodeSDK } from "@opentelemetry/sdk-node";
src/lib/services/server/ai/observability/langfuseClient.ts:1
- Duplicate global declaration from telemetryService.ts. Global type declarations should be consolidated into a single shared declaration file to avoid duplication and potential conflicts.
/**
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
a490a06 to
261c950
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
48228c0 to
e35d0dc
Compare
e35d0dc to
0ea3646
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 48 out of 49 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
0ea3646 to
6ec3f02
Compare
6ec3f02 to
1155b8a
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
1155b8a to
dded8f3
Compare
dded8f3 to
67eb668
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
67eb668 to
ccf5678
Compare
ccf5678 to
6957c67
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
6957c67 to
498dce4
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/telemetry/telemetryService.ts (3)
259-272: Fix OTel Counter misuse: don’t decrement a Counter; use an UpDownCounterCounters are monotonic;
this.connectionCounter.add(-1, …)is invalid and risks exporter errors. Track active connections with an UpDownCounter and keepconnections_totalas a monotonic counter.Apply these diffs:
Add UpDownCounter type to imports:
-import { - metrics, - trace, - type Meter, - type Tracer, - type Counter, - type Histogram, -} from "@opentelemetry/api"; +import { + metrics, + trace, + type Meter, + type Tracer, + type Counter, + type UpDownCounter, + type Histogram, +} from "@opentelemetry/api";Add an UpDownCounter field:
private mcpToolCalls?: Counter; private connectionCounter?: Counter; + private activeConnectionsUpDownCounter?: UpDownCounter; private responseTimeHistogram?: Histogram;Initialize it:
this.mcpToolCalls = this.meter.createCounter("mcp_tool_calls_total", { description: "Total number of MCP tool calls", }); this.connectionCounter = this.meter.createCounter("connections_total", { description: "Total number of connections", }); + this.activeConnectionsUpDownCounter = this.meter.createUpDownCounter( + "active_connections", + { description: "Current number of active connections" }, + );Use it on connect/close:
this.connectionCounter.add(1, { connection_type: type }); + this.activeConnectionsUpDownCounter?.add(1, { connection_type: type });- // Optionally record disconnection metrics if needed - this.connectionCounter.add(-1, { - connection_type: type, - event: "disconnect", - }); + this.activeConnectionsUpDownCounter?.add(-1, { connection_type: type }); + // Optionally: track disconnects with a separate monotonic counter, e.g. connections_closed_total
300-305: Avoid creating instruments per call; cache custom counters/histogramsCreating meters on every invocation leaks instruments and degrades performance. Cache by name.
Suggested change (outside selected lines): add caches and reuse:
// Fields private customCounters = new Map<string, Counter>(); private customHistograms = new Map<string, Histogram>(); // In recordCustomMetric(...) const key = `custom_${name}`; let counter = this.customCounters.get(key); if (!counter) { counter = this.meter.createCounter(key, { description: `Custom metric: ${name}` }); this.customCounters.set(key, counter); } counter.add(value, labels || {}); // In recordCustomHistogram(...) const key = `custom_${name}_histogram`; let hist = this.customHistograms.get(key); if (!hist) { hist = this.meter.createHistogram(key, { description: `Custom histogram: ${name}` }); this.customHistograms.set(key, hist); } hist.record(value, labels || {});Also applies to: 316-321
225-229: Reduce metric label cardinality and PII riskUsing
error.messageas a metric attribute explodes cardinality and can leak sensitive data. Prefer error class/type only.- this.aiProviderErrors.add(1, { - provider, - error: error.name, - message: error.message.substring(0, 100), // Limit message length - }); + this.aiProviderErrors.add(1, { + provider, + error: error.name, + }); + // Log full message separately if needed via logger at appropriate level
♻️ Duplicate comments (2)
package.json (1)
168-179: Consolidate OpenTelemetry versions per official compatibility matrix
Mismatched experimental (auto-instrumentations-node v0.56.x) and stable modules (core v2.x, sdk-trace v2.x, exporter v0.202.x) cause runtime failures. In package.json replace all @opentelemetry/* except auto-instrumentations-node with:- "@opentelemetry/api": "^1.9.0", - "@opentelemetry/sdk-node": "^0.56.0", - "@opentelemetry/core": "^2.1.0", - "@opentelemetry/exporter-trace-otlp-http": "^0.202.0", - "@opentelemetry/resources": "^2.1.0", - "@opentelemetry/sdk-trace-base": "^2.1.0", - "@opentelemetry/sdk-trace-node": "^2.1.0", - "@opentelemetry/semantic-conventions": "^1.30.1", + "@opentelemetry/api": "^1.29.0", + "@opentelemetry/sdk-node": "^1.29.0", + "@opentelemetry/auto-instrumentations-node": "^0.56.0", + "@opentelemetry/exporter-trace-otlp-http": "^1.29.0",Drop core, resources, sdk-trace-*, semantic-conventions; sdk-node manages them.
src/lib/services/server/ai/observability/instrumentation.ts (1)
44-57: Do not flag telemetry as initialized when setup is skippedWe mark
isInitialized = trueeven when Langfuse is disabled or credentials are missing, so any later call with a valid config bails out early and Langfuse never starts. It also tricksgetLangfuseHealthStatus()into reporting an initialized system when nothing was wired. Drop those assignments (and just cache the config/flags) so a real init can happen once credentials/flags are corrected.if (!config.enabled) { logger.debug(`${LOG_PREFIX} Langfuse disabled, skipping initialization`); - isInitialized = true; + currentConfig = config; return; } if (!config.publicKey || !config.secretKey) { logger.warn( `${LOG_PREFIX} Langfuse enabled but missing credentials, skipping initialization`, ); - isInitialized = true; isCredentialsValid = false; return; }
🧹 Nitpick comments (7)
src/lib/telemetry/telemetryService.ts (1)
171-191: Use SpanStatusCode constants instead of magic numbersImproves clarity and future compatibility.
-import { - metrics, - trace, - type Meter, - type Tracer, - type Counter, - type Histogram, -} from "@opentelemetry/api"; +import { + metrics, + trace, + SpanStatusCode, + type Meter, + type Tracer, + type Counter, + type Histogram, +} from "@opentelemetry/api";- span.setStatus({ code: 1 }); // OK + span.setStatus({ code: SpanStatusCode.OK });- span.setStatus({ - code: 2, + span.setStatus({ + code: SpanStatusCode.ERROR, message: error instanceof Error ? error.message : "Unknown error", });src/lib/core/baseProvider.ts (2)
1816-1833: Rename imported createAnalytics to avoid collision with method nameThe class defines
protected async createAnalytics(...)and also importscreateAnalytics. This shadowing is confusing. Alias the import for stream analytics.-import { nanoid } from "nanoid"; -import { createAnalytics } from "./analytics.js"; +import { nanoid } from "nanoid"; +import { createAnalytics as createStreamAnalyticsFn } from "./analytics.js";- const analytics = createAnalytics( + const analytics = createStreamAnalyticsFn( this.providerName, this.modelName, result, Date.now() - startTime, { requestId: `${this.providerName}-stream-${nanoid()}`, streamingMode: true, ...options.context, }, );
2247-2291: Telemetry config helper is fine; consider adding userId to metadataOptionally include userId from options/context when present for better trace correlation.
- // Add sessionId if available + // Add sessionId / userId if available if ("sessionId" in options && options.sessionId) { const sessionId = options.sessionId; if ( typeof sessionId === "string" || typeof sessionId === "number" || typeof sessionId === "boolean" ) { metadata.sessionId = sessionId; } } + const userId = + (options as { context?: Record<string, unknown> })?.context?.userId ?? + (options as { userId?: unknown })?.userId; + if ( + typeof userId === "string" || + typeof userId === "number" || + typeof userId === "boolean" + ) { + metadata.userId = userId; + }src/cli/index.ts (1)
52-71: Make telemetry cleanup idempotent and resilient; use once-handlers and guard against double flushCleanup is invoked after parse, in catch, SIGINT/SIGTERM, and beforeExit. Without a guard, flushOpenTelemetry may run multiple times or concurrently. Also, process.on allows multiple invocations per signal; beforeExit can re-fire if new work is scheduled. Add:
- A single “once” guard to cleanup()
- process.once for signals
- Timeout-wrapped flush to avoid hangs on exit
Apply:
-// Cleanup on exit -process.on("SIGINT", async () => { - await cleanup(); - process.exit(0); -}); - -process.on("SIGTERM", async () => { - await cleanup(); - process.exit(0); -}); - -process.on("beforeExit", async () => { - await cleanup(); -}); - -async function cleanup() { - const { flushOpenTelemetry } = await import( - "../lib/services/server/ai/observability/instrumentation.js" - ); - await flushOpenTelemetry(); -} +// Cleanup on exit +let cleanupStarted = false; +const FLUSH_TIMEOUT_MS = + Number(process.env.NL_TELEMETRY_FLUSH_TIMEOUT_MS ?? 2000); + +process.once("SIGINT", async () => { + try { + await cleanup(); + } finally { + process.exit(0); + } +}); + +process.once("SIGTERM", async () => { + try { + await cleanup(); + } finally { + process.exit(0); + } +}); + +process.once("beforeExit", async () => { + await cleanup(); +}); + +async function cleanup() { + if (cleanupStarted) return; + cleanupStarted = true; + try { + const { flushOpenTelemetry } = await import( + "../lib/services/server/ai/observability/instrumentation.js" + ); + await Promise.race([ + flushOpenTelemetry(), + new Promise((resolve) => setTimeout(resolve, FLUSH_TIMEOUT_MS)), + ]); + } catch { + // swallow errors on shutdown + } +}src/cli/factories/commandFactory.ts (1)
1389-1391: Avoid process.exit() in handlers; rely on centralized beforeExit cleanup and make flush idempotentCalling process.exit(0) bypasses beforeExit, fragmenting shutdown logic. You already added centralized cleanup in src/cli/index.ts. Prefer setting exitCode and returning; let beforeExit run one consistent flush. If you keep local flush, guard it to run only once.
Replace immediate exits:
- if (!globalSession.getCurrentSessionId()) { - await this.flushLangfuseTraces(); - process.exit(0); - } + if (!globalSession.getCurrentSessionId()) { + process.exitCode = 0; + return; + }Do the same at Lines 1493-1496, 1635-1638, 1983-1986, and 2143-2146.
Additionally, guard flushLangfuseTraces to run once:
/** * Flush Langfuse traces before exit */ private static async flushLangfuseTraces(): Promise<void> { + // prevent duplicate flushes from multiple call sites + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore: attach flag to class + if ((this as unknown as { _flushed?: boolean })._flushed) return; + // @ts-ignore + (this as unknown as { _flushed?: boolean })._flushed = true; try { logger.debug("[CLI] Flushing Langfuse traces before exit..."); const { flushOpenTelemetry } = await import( "../../lib/services/server/ai/observability/instrumentation.js" ); await flushOpenTelemetry(); logger.debug("[CLI] Langfuse traces flushed successfully"); } catch (error) { logger.error("[CLI] Error flushing Langfuse traces", { error }); } }Note: With the centralized beforeExit handler in index.ts, you can also remove these per-command flush calls entirely for simpler, single-path shutdown.
Also applies to: 1493-1496, 1635-1638, 1983-1986, 2143-2146
src/lib/types/observability.ts (1)
10-28: Use a discriminated union for LangfuseConfig to avoid requiring secrets when disabledAs written, consumers must provide publicKey/secretKey even when disabled=false. A discriminated union cleanly models both states and improves type safety.
-export interface LangfuseConfig { - /** Whether Langfuse is enabled */ - enabled: boolean; - /** Langfuse public key */ - publicKey: string; - /** - * Langfuse secret key - * @sensitive - * WARNING: This is a sensitive credential. Handle securely. - * Do NOT log, expose, or share this key. Follow best practices for secret management. - */ - secretKey: string; - /** Langfuse base URL (default: https://cloud.langfuse.com) */ - baseUrl?: string; - /** Environment name (e.g., dev, staging, prod) */ - environment?: string; - /** Release/version identifier */ - release?: string; -} +export type LangfuseConfig = + | { + /** Whether Langfuse is enabled */ + enabled: false; + } + | { + /** Whether Langfuse is enabled */ + enabled: true; + /** Langfuse public key */ + publicKey: string; + /** + * Langfuse secret key + * @sensitive + * WARNING: This is a sensitive credential. Handle securely. + * Do NOT log, expose, or share this key. Follow best practices for secret management. + */ + secretKey: string; + /** Langfuse base URL (default: https://cloud.langfuse.com) */ + baseUrl?: string; + /** Environment name (e.g., dev, staging, prod) */ + environment?: string; + /** Release/version identifier */ + release?: string; + };src/lib/session/globalSessionState.ts (1)
9-40: Broaden env parsing and avoid silent non-activation
- Accept common truthy values for LANGFUSE_ENABLED (true/1/yes/on).
- When LANGFUSE_ENABLED=true but keys are missing, log a debug/warn once to aid diagnostics instead of silently disabling.
Apply:
-function buildObservabilityConfigFromEnv(): ObservabilityConfig | undefined { - const langfuseEnabled = - process.env.LANGFUSE_ENABLED?.trim().toLowerCase() === "true"; +function buildObservabilityConfigFromEnv(): ObservabilityConfig | undefined { + const truthy = (v?: string) => + !!v && ["true", "1", "yes", "on"].includes(v.trim().toLowerCase()); + const langfuseEnabled = truthy(process.env.LANGFUSE_ENABLED); const publicKey = process.env.LANGFUSE_PUBLIC_KEY?.trim(); const secretKey = process.env.LANGFUSE_SECRET_KEY?.trim(); - if (!langfuseEnabled || !publicKey || !secretKey) { + if (!langfuseEnabled || !publicKey || !secretKey) { return undefined; }Optionally, add a debug-level log when enabled is truthy but creds are missing (ensure not to log the secret). For example:
- “[Observability] LANGFUSE_ENABLED set but keys missing; Langfuse disabled.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
.env.example(1 hunks)package.json(1 hunks)src/cli/factories/commandFactory.ts(6 hunks)src/cli/index.ts(1 hunks)src/lib/core/baseProvider.ts(5 hunks)src/lib/index.ts(3 hunks)src/lib/neurolink.ts(5 hunks)src/lib/providers/anthropic.ts(1 hunks)src/lib/providers/azureOpenai.ts(1 hunks)src/lib/providers/googleAiStudio.ts(1 hunks)src/lib/providers/googleVertex.ts(1 hunks)src/lib/providers/openAI.ts(1 hunks)src/lib/services/server/ai/observability/instrumentation.ts(1 hunks)src/lib/session/globalSessionState.ts(3 hunks)src/lib/telemetry/telemetryService.ts(2 hunks)src/lib/types/conversation.ts(1 hunks)src/lib/types/observability.ts(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 673-673: [UnorderedKey] The LANGFUSE_BASE_URL key should go before the LANGFUSE_PUBLIC_KEY key
(UnorderedKey)
[warning] 674-674: [UnorderedKey] The LANGFUSE_ENABLED key should go before the LANGFUSE_PUBLIC_KEY key
(UnorderedKey)
[warning] 679-679: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🔇 Additional comments (7)
src/lib/providers/googleVertex.ts (1)
928-929: Experimental telemetry pass-through supported
Confirmed thatstreamTextacceptsexperimental_telemetrywith the@ai-sdk/google-vertexprovider. No changes required.src/lib/providers/anthropic.ts (1)
233-234: Experimental telemetry pass-through confirmed: Vercel AI SDK’sstreamTextsupports theexperimental_telemetryoption (available since v4.2).src/lib/providers/openAI.ts (1)
436-437: Experimental telemetry pass-through validated
Confirmed thatexperimental_telemetryis supported onstreamTextin the Vercel AI SDK (Telemetry docs [1], streamText reference [2]).src/lib/providers/azureOpenai.ts (1)
212-213: Ensure Vercel AI SDK ≥ 3.3.x for experimental telemetry
experimental_telemetryonstreamTextis supported since SDK v3.3.x; confirmed.src/lib/providers/googleAiStudio.ts (1)
220-221: Confirm @vercel/ai version supports experimental_telemetry
Ensure your project uses @vercel/ai v8.43.0+ (Node/Bun) or v9.29.0+ (Next.js) so thatstreamTextaccepts theexperimental_telemetryoption.src/lib/core/baseProvider.ts (1)
444-447: Ensure Vercel AI SDK ≥3.3 for experimental_telemetry
Theexperimental_telemetryoption is supported on bothgenerateTextandstreamTextstarting in v3.3; confirm your project uses v3.3 or later.src/lib/types/conversation.ts (1)
209-211: LGTM: optional observability config on NeurolinkOptionsType-only import keeps runtime clean and aligns with the new observability surface.
| LANGFUSE_PUBLIC_KEY=pk-lf-your-public-key-here | ||
| LANGFUSE_SECRET_KEY=sk-lf-your-secret-key-here | ||
| LANGFUSE_BASE_URL=https://cloud.langfuse.com | ||
| LANGFUSE_ENABLED=false | ||
|
|
||
| # OpenTelemetry Configuration (Optional) | ||
| # Telemetry is automatically enabled when OTEL_EXPORTER_OTLP_ENDPOINT is set | ||
| # OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 | ||
| # OTEL_SERVICE_NAME=neurolink-ai No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Tidy env keys per linter; add trailing newline
Reorder Langfuse keys (alphabetical) and add a blank line at EOF to satisfy dotenv-linter.
- LANGFUSE_PUBLIC_KEY=pk-lf-your-public-key-here
- LANGFUSE_SECRET_KEY=sk-lf-your-secret-key-here
- LANGFUSE_BASE_URL=https://cloud.langfuse.com
- LANGFUSE_ENABLED=false
+ LANGFUSE_BASE_URL=https://cloud.langfuse.com
+ LANGFUSE_ENABLED=false
+ LANGFUSE_PUBLIC_KEY=pk-lf-your-public-key-here
+ LANGFUSE_SECRET_KEY=sk-lf-your-secret-key-here
@@
-# OTEL_SERVICE_NAME=neurolink-ai
+# OTEL_SERVICE_NAME=neurolink-ai
+Run dotenv-linter to confirm warnings are resolved:
🏁 Script executed:
#!/bin/bash
rg -n '^LANGFUSE_|^OTEL_SERVICE_NAME' .env.example -n
dotenv-linter .env.example || trueLength of output: 11519
Fix all dotenv-linter warnings in .env.example
The file currently has 113 issues (ValueWithoutQuotes, UnorderedKey, SpaceCharacter, missing EOF newline). Run dotenv-linter on .env.example and address every formatting and ordering error.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 673-673: [UnorderedKey] The LANGFUSE_BASE_URL key should go before the LANGFUSE_PUBLIC_KEY key
(UnorderedKey)
[warning] 674-674: [UnorderedKey] The LANGFUSE_ENABLED key should go before the LANGFUSE_PUBLIC_KEY key
(UnorderedKey)
[warning] 679-679: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
In .env.example around lines 671 to 679, dotenv-linter reports 113 issues
including ValueWithoutQuotes, UnorderedKey, SpaceCharacter, and missing EOF
newline; run dotenv-linter locally against .env.example, then fix every reported
issue by quoting values that require quotes (e.g., values containing spaces or
special chars), ordering keys alphabetically where required (or follow repo’s
specified ordering policy), removing stray space characters around `=` and at
line ends, ensuring consistent comment formatting, and add a final newline at
EOF; re-run dotenv-linter until it reports zero warnings and commit the cleaned
.env.example.
| export async function getTelemetryStatus(): Promise<{ | ||
| enabled: boolean; | ||
| initialized: boolean; | ||
| } { | ||
| if (process.env.NEUROLINK_TELEMETRY_ENABLED === "true") { | ||
| return { enabled: true, initialized: false }; | ||
| } | ||
| return { enabled: false, initialized: false }; | ||
| }> { | ||
| const service = TelemetryService.getInstance(); | ||
| return { | ||
| enabled: service.isEnabled(), | ||
| initialized: service.isEnabled(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Bug: getTelemetryStatus.initialized returns enabled state
initialized currently mirrors isEnabled(), which misreports status. Return the actual initialization flag.
Apply:
export async function getTelemetryStatus(): Promise<{
enabled: boolean;
initialized: boolean;
}> {
const service = TelemetryService.getInstance();
return {
enabled: service.isEnabled(),
- initialized: service.isEnabled(),
+ initialized:
+ // Prefer a dedicated API if available
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ (service as any).isInitialized?.() ??
+ // or expose a getter on TelemetryService and use it here
+ false,
};
}If TelemetryService doesn’t yet expose isInitialized(), add it and wire to the underlying instrumentation state.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function getTelemetryStatus(): Promise<{ | |
| enabled: boolean; | |
| initialized: boolean; | |
| } { | |
| if (process.env.NEUROLINK_TELEMETRY_ENABLED === "true") { | |
| return { enabled: true, initialized: false }; | |
| } | |
| return { enabled: false, initialized: false }; | |
| }> { | |
| const service = TelemetryService.getInstance(); | |
| return { | |
| enabled: service.isEnabled(), | |
| initialized: service.isEnabled(), | |
| }; | |
| } | |
| export async function getTelemetryStatus(): Promise<{ | |
| enabled: boolean; | |
| initialized: boolean; | |
| }> { | |
| const service = TelemetryService.getInstance(); | |
| return { | |
| enabled: service.isEnabled(), | |
| initialized: | |
| // Prefer a dedicated API if available | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| (service as any).isInitialized?.() ?? | |
| // or expose a getter on TelemetryService and use it here | |
| false, | |
| }; | |
| } |
🤖 Prompt for AI Agents
In src/lib/index.ts around lines 221 to 230, getTelemetryStatus incorrectly sets
initialized to service.isEnabled(); change it to return the real initialization
flag (initialized: service.isInitialized()). If TelemetryService does not yet
expose isInitialized(), add a public isInitialized(): boolean method on
TelemetryService and implement it to return the underlying instrumentation/SDK
initialization state, then use that method in getTelemetryStatus so initialized
reflects true initialization status rather than the enabled flag.
498dce4 to
03bcfaa
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| release: config.release || "v1.0.0", | ||
| }); | ||
|
|
||
| // Create resource with service metadata (v2.x API) |
There was a problem hiding this comment.
The comment mentions 'v2.x API' but based on the imports and usage, this appears to be using OpenTelemetry v1.x semantic conventions. Update the comment to reflect the correct API version.
| // Create resource with service metadata (v2.x API) | |
| // Create resource with service metadata (v1.x API) |
There was a problem hiding this comment.
but that is using @opentelemetry/resources package with version "@opentelemetry/resources": "^2.1.0"
| protected getStreamTelemetryConfig( | ||
| options: StreamOptions | TextGenerationOptions, | ||
| operationType: "stream" | "generate" = "stream", | ||
| ): | ||
| | { | ||
| isEnabled: boolean; | ||
| functionId?: string; | ||
| metadata?: Record<string, string | number | boolean>; | ||
| } | ||
| | undefined { |
There was a problem hiding this comment.
[nitpick] The return type union with undefined is verbose. Consider defining a named interface TelemetryConfig for the object type to improve readability and maintainability.
|
🎉 This PR is included in version 7.50.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
Technical Implementation
New Observability Services
langfuseClient.ts): Core integration with trace/generation/tool trackinglangfusePromptManager.ts): Structured prompt management and versioningai.ts): Centralized AI service configuration managementEnhanced Components
Key Features
Test plan
Related Issue
Fixes #192
Dependencies
langfuse@^3.38.5for production observability integration🤖 Generated with Claude Code
Summary by CodeRabbit