Skip to content
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

chore(wren-ai-service): add correlation ID to langfuse #846

Conversation

grieve54706
Copy link
Contributor

@grieve54706 grieve54706 commented Oct 30, 2024

Add correlation ID from the ibis server to the Langfuse.

The AI team usually provides the issues from the Langfuse, if we want to find the ibis log for the issue, we need to find it by time, and the time is not completely correct.

The correlation ID is from the ibis-server. The AI service gets data and the correlation ID through the web API, and the AI service sends Langfuse logs. So, we must explain the correlation ID and return it to the AI service.

@grieve54706
Copy link
Contributor Author

Hi @onlyjackfrost, @cyyeh. Could you check this PR?

@grieve54706 grieve54706 force-pushed the grieve/feature/add-correlation-id-to-langfuse branch from de44950 to 2b5e265 Compare October 30, 2024 10:28
Copy link
Contributor

@onlyjackfrost onlyjackfrost left a comment

Choose a reason for hiding this comment

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

Could you please explain why the correlationId is being added and returned to the AI service?

@@ -28,8 +28,8 @@ export interface ColumnMetadata {
export interface PreviewDataResponse {
correlationId?: string;
processTime?: string;
columns: ColumnMetadata[];
data: any[][];
columns?: ColumnMetadata[];
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer not to share type definitions here, as preview and dry-run have distinct return formats, and they should be defined separately. Can we define their types individually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

@grieve54706
Copy link
Contributor Author

Could you please explain why the correlationId is being added and returned to the AI service?

Hi @onlyjackfrost. Because the AI team usually provides the issues from the Langfuse, if we want to find the ibis log for the issue, we need to find it by time, and the time is not completely correct.
The correlation ID is from the ibis-server. The AI service gets data and the correlation ID through the web API, and the AI service sends Langfuse logs. So, you must explain the correlation ID and return it to the AI service.

Copy link
Member

@cyyeh cyyeh left a comment

Choose a reason for hiding this comment

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

LGTM

@onlyjackfrost onlyjackfrost merged commit a3b2c38 into Canner:main Nov 1, 2024
4 checks passed
@grieve54706 grieve54706 deleted the grieve/feature/add-correlation-id-to-langfuse branch November 1, 2024 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants