-
-
Notifications
You must be signed in to change notification settings - Fork 118
[WIP] Feat: opentelemetry support #259
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?
Conversation
π WalkthroughWalkthroughAdds telemetry types and an optional telemetry field to multiple activity option interfaces, extends BaseEventContext with telemetry, threads telemetry through event payloads across text, image, video, speech, transcription, and summarization activities, and updates adapter export shapes and a changeset. Changes
Estimated code review effortπ― 3 (Moderate) | β±οΈ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
π₯ Pre-merge checks | β 2 | β 1β Failed checks (1 inconclusive)
β Passed checks (2 passed)
βοΈ Tip: You can configure your own custom pre-merge checks in the settings. β¨ Finishing touches
π§ͺ Generate unit tests (beta)
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 |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
packages/typescript/ai/src/activities/generateVideo/index.ts (1)
168-181:β οΈ Potential issue | π‘ MinorAdd telemetry events to
generateVideofunction to match other activities.The
generateVideofunction (lines 168-181) doesn't emitvideo:request:startedandvideo:request:completedevents, unlikegenerateImageandsummarizewhich emit telemetry for tracking. Additionally, the function accepts atelemetryparameter viaVideoActivityBaseOptionsbut doesn't use it. Emit telemetry events and pass the telemetry data to maintain consistency with other activities.
π§Ή Nitpick comments (1)
packages/typescript/ai/src/types.ts (1)
3-8: Consider adding JSDoc for expected metadata keys.The
Telemetryinterface usesRecord<string, any>which provides flexibility but offers no guidance on expected keys. Since this is for billing/token tracking, consider documenting common metadata keys users should provide.π Suggested documentation improvement
/** * Telemetry data for tracking and debugging. + * + * `@example` + * ```ts + * telemetry: { + * metadata: { + * userId: 'user-123', + * sessionId: 'session-456', + * billingAccount: 'acct-789' + * } + * } + * ``` */ export interface Telemetry { + /** + * Arbitrary key-value pairs for tracking context. + * Common keys: userId, sessionId, billingAccount, environment, etc. + */ metadata: Record<string, any> }
|
View your CI Pipeline Execution β for commit 08d1fef
βοΈ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
π― Changes
I've encountered a problem with tracking user token usage for billing, so I've added a telemetry field for every activity and all events.
β Checklist
pnpm run test:pr.π Release Impact
Summary by CodeRabbit