[feat] Add user tracking and enhance telemetry with execution context #286
+176
−95
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@toyamarinyon is attempting to deploy a commit to the Edge Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
I would like to ask @Rindrics to review the code as well! |
Rindrics
reviewed
Jan 6, 2025
tsconfig.json
Outdated
| @@ -19,7 +19,11 @@ | |||
| } | |||
| ], | |||
| "paths": { | |||
| "@/*": ["./*"] | |||
| "@/*": ["./*"], | |||
| "@giselles-ai/*": ["./packages/*"], | |||
| import { PropertiesPanelProvider } from "@giselles-ai/contexts/properties-panel"; | ||
| import { ToastProvider } from "@giselles-ai/contexts/toast"; | ||
| import { ToolbarContextProvider } from "@giselles-ai/contexts/toolbar"; | ||
| import { persistGraph } from "@giselles-ai/giselle-provider/graph/persist"; |
| @@ -347,7 +305,7 @@ export default async function Page({ | |||
| <DeveloperModeProvider developerMode={developerMode}> | |||
| <GraphContextProvider | |||
| defaultGraph={graph} | |||
| onPersistAction={persistGraph} | |||
| onPersistAction={persistGraphAction} | |||
| @@ -335,6 +322,11 @@ async function performFlowExecution( | |||
| ), | |||
| topP, | |||
| temperature, | |||
| experimental_telemetry: { | |||
There was a problem hiding this comment.
|
|
||
| registerOTel({ | ||
| serviceName: "giselle", | ||
| spanProcessors: [noopSpanProcessor], | ||
| metricReader, | ||
| logRecordProcessor, | ||
| traceExporter: new LangfuseExporter({ |
There was a problem hiding this comment.
I can't expect where the telemetry sent because this initialization does not contain target endpoint.
But reading the Langfuse documant, this configuration seems ok.
Let's merge and check that new fields are included in the telemetry arrived to our Langfuse account
There was a problem hiding this comment.
I think having "staging" environment is good if we accept pull requests from forked repo: we can check new functionalities before merging them into main
There was a problem hiding this comment.
@Rindrics It deployed to staging. please verify!
- Update background image import to use shared component - Import `waitForLangfuseFlush` and implement it in node actions - Add LangfuseExporter for OpenTelemetry integration - Refactor execution telemetry logic to use experimental telemetry - Enhance Tailwind config paths to include packages - Upgrade langfuse dependencies in package.json These changes introduce telemetry integration via Langfuse, enhancing observability. LangfuseExporter is configured with a flushing mechanism to ensure timely data export. Refactoring improves performance by reducing overhead from previous Langfuse traces within execution logic.
- Moved persistGraph logic from page.tsx to a new module at giselle-provider/graph/persist.ts for better modularity - Replaced inline persistGraph logic with import from new module - Simplified graph handling in page.tsx by outsourcing blob deletion and graph URL update functionality This refactor should make the codebase more maintainable by adhering to the single responsibility principle, without introducing breaking changes.
- Change waitForLangfuseFlush from a promise to a function that returns a promise in utils.ts for enhanced laziness - Remove redundant import of after from page.tsx - Call waitForLangfuseFlush using `after` in execution.ts to ensure proper scheduling of post-execution logic This update aims to address more efficient handling of post-execution scheduling and resolve potential memory overhead by improving how flush timing is managed. No breaking changes introduced.
- Introduce userId to the execution context for enhanced telemetry data, allowing better user-specific tracking and debugging - Modify performFlowExecution to include userId in experimental telemetry metadata - Implement parseExecutionContextToTelemetryMetadata to structure metadata, ensuring consistent formatting across different telemetry actions This change aims to improve telemetry insights by associating user identifiers with execution flows, thereby assisting in user behavior analysis and debugging. No breaking changes introduced.
- Add userId to execution parameters for executeStep and retryStep functions to capture user-specific telemetry data - Modify page functions to pass userId from the user context for more granular tracking and user-driven analytics - Ensure all execution paths maintain consistent userId propagation for future scalability in user data insights This enhancement improves the ability to trace executions back to specific users, enabling better analytics and debugging capabilities with no breaking changes introduced.
toyamarinyon
force-pushed
the
use-telemetry-integration
branch
from
c13463e to
5725481
Compare
shige
requested changes
Jan 8, 2025
There was a problem hiding this comment.
Please check the errors occurring in Sentry, and SigNoz.
Route /p/[agentId] used "cookies" inside "after(...)". This is not supported. If you need this data inside an "after" callback, use "cookies" outside of the callback. See more info here: https://nextjs.org/docs/canary/app/api-reference/functions/after
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Stacks
This repository stacked on #285