-
-
Couldn't load subscription status.
- Fork 4.5k
feat(llm-detector): Implement LLM-powered issue detection task #102188
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
Conversation
ec7f178 to
2d80d28
Compare
| assert log_extra["num_issues"] == 1 | ||
| assert "trace_id" in log_extra | ||
|
|
||
| def test_detect_llm_issues_no_transactions(self, mock_seer_api): |
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.
Do we need all these tests? there are quite a few validation ones, but im concerned they outnumber ones that actually test behavior which could make this file less readable
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.
but then I wouldn't have 100% coverage 😭
| trace = get_trace_for_transaction(transaction.name, transaction.project_id) | ||
| if trace: | ||
| logger.info("Found trace for LLM issue detection", extra={"trace_id": trace.trace_id}) | ||
| trace: TraceData | None = get_trace_for_transaction( |
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.
do we need the explicit typing here for trace?
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.
I like it - saves me a click or two
| logger.info("Found trace for LLM issue detection", extra={"trace_id": trace.trace_id}) | ||
|
|
||
| seer_request = { | ||
| "telemetry": [{**trace.dict(), "kind": "trace"}], |
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.
might be worth filtering for tags here like we do in the PoC
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.
That is a great suggestion, esp since I can add "kind": "trace" in a better way (I don't really like the way I'm doing it here)
I'm going to make an additional function to clean/format the response from get_trace_for_transaction in a subsequent pr
d382e75 to
c2253e0
Compare
c2253e0 to
5aaab57
Compare
5aaab57 to
32986e6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #102188 +/- ##
============================================
+ Coverage 66.62% 80.95% +14.33%
============================================
Files 8739 8746 +7
Lines 388845 389323 +478
Branches 24706 24706
============================================
+ Hits 259050 315183 +56133
+ Misses 129440 73785 -55655
Partials 355 355 |
Summary
Connects the task to the new Seer endpoint - enables sending performance data to Seer to analyze.
Currently just logging the results, next step is to create Issues from the data returned by Seer.
Changes
detect_llm_issues_for_projecttask implementation/v1/automation/issue-detection/analyzeendpointLLMIssueDetectionErrorThe task is behind feature flags and project allowlists, ready for controlled rollout.