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

feat(autofix): Add pr event tags to langfuse traces #823

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jennmueng
Copy link
Member

@jennmueng jennmueng commented Jun 27, 2024

Logs PR events as langfuse trace tags for visibility. Will need to have sentry call this endpoint.

TODO: Log to sentry LLM monitoring too

Tested locally w/ dev version of langfuse.

@jennmueng jennmueng requested a review from a team June 27, 2024 15:52
@jennmueng jennmueng marked this pull request as ready for review June 27, 2024 15:52
Copy link
Member

@roaga roaga left a comment

Choose a reason for hiding this comment

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

LGTM code-wise (though minor merge conflict needs to be fixed).

Though this won't actually do anything without changes in Sentry, right? Is there a PR for that?

I'm also not fully clear on how this endpoint can be used in an Autofix run besides PR creation. I would imagine a lot of the PR actions this endpoint says it can log will happen long after the Autofix run is completed and the Sentry web app closed.

How will Sentry know if a user closed or merged a PR created by Autofix? What if the user merges the PR while the Sentry web app is closed? Does it call this endpoint when the user re-opens the issue and the GitHub status is updated? Are we using webhooks (#454)?

(this is more about Sentry than Seer, but I'm wondering how this endpoint can be used as intended)

Also some questions on other PR events and related data we want to track below.

@roaga roaga requested review from roaga and removed request for roaga July 22, 2024 16:29
Comment on lines +204 to +205
action_tag_map = {"opened": "pr:opened", "closed": "pr:closed", "merged": "pr:merged"}

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to support other actions as well? Like commenting, editing, approving, requesting changes, etc.?

Copy link
Member

Choose a reason for hiding this comment

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

Are we differentiating between who takes an action? Autofix agent vs. human?

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.

2 participants