fix(llmobs): include missing tags and make SubmitEvaluationFromSpan more flexible#4050
Conversation
|
sabrenner
left a comment
There was a problem hiding this comment.
nice lgtm! just a couple noob Go questions 😄
| } | ||
| if cfg.AgentlessEnabled != nil { | ||
| if !*cfg.AgentlessEnabled && !agentSupportsLLMObs { | ||
| log.Warn("llmobs: DD_LLMOBS_AGENTLESS_ENABLED has been configured to false and the agent is not available or does not support llmobs") |
There was a problem hiding this comment.
should we return an error in this case? i don't think llmobs will be able to submit data in the scenario where there's no agent running and agentlessEnabled is explicitly set to false, and maybe we wanna surface that to the user harsher than a warning log, but not sure of the protocols for doing that in the go tracer!
There was a problem hiding this comment.
hmm yes! this totally makes sense, I will change it 👍
…ode is explicitly setup
|
/merge |
|
View all feedbacks in Devflow UI.
This merge request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
devflow unqueued this merge request: It did not become mergeable within the expected time |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What does this PR do?
Fix some minor issues in the llmobs SDK highlighted by the shared testing suite:
session_idin tags. It was currently only being included in the root field of the span event payload, but it should be present in both places.ddtrace.versionin evaluation metrics.Motivation
Correct/consistent implementation of the Go LLMObs SDK.
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!