feat(llmobs): add LLM Observability SDK #3938
Conversation
BenchmarksBenchmark execution time: 2025-09-05 12:32:55 Comparing candidate commit 00310d9 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 24 metrics, 0 unstable metrics. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
llmobs/internal/dne_client.go
Outdated
| seconds = int(time.Until(time.Unix(resetTime, 0)).Seconds()) | ||
| } else { | ||
| // Assume it's a duration in seconds | ||
| seconds = int(resetTime) |
Check failure
Code scanning / CodeQL
Incorrect conversion between integer types High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 days ago
To fix the issue, we need to ensure that converting resetTime from int64 to int will not overflow or wrap around unexpectedly. The best approach is to add bounds checks when converting. Since resetTime comes from external input, we should check that its value is within acceptable bounds for the platform's int. In Go, this can be done using constants math.MaxInt32 or math.MaxInt64 depending on platform, but for maximum safety and compatibility, use math.MinInt32, math.MaxInt32 and restrict values accordingly. Add a check before converting to int, and fallback gracefully if out of bounds (e.g., use a default backoff value of 1 second).
Required changes:
- Add
import "math"(if not present). - Before converting to
int, check ifresetTimeis within[0, math.MaxInt32]range (since negative sleep seconds make no sense and int32 is the minimum required for cross-platform code); use only positive values. - If value falls outside the range, use a default, e.g.,
1second.
Update the block around lines 440–448 to implement these bounds checks.
| @@ -16,7 +16,7 @@ | ||
| "net/url" | ||
| "strconv" | ||
| "time" | ||
|
|
||
| "math" | ||
| "github.com/DataDog/dd-trace-go/v2/internal" | ||
| "github.com/cenkalti/backoff/v5" | ||
| ) | ||
| @@ -441,10 +441,15 @@ | ||
| seconds := 0 | ||
| if resetTime > time.Now().Unix() { | ||
| // Assume it's a Unix timestamp | ||
| seconds = int(time.Until(time.Unix(resetTime, 0)).Seconds()) | ||
| sec := time.Until(time.Unix(resetTime, 0)).Seconds() | ||
| if sec > 0 && sec <= float64(math.MaxInt32) { | ||
| seconds = int(sec) | ||
| } | ||
| } else { | ||
| // Assume it's a duration in seconds | ||
| seconds = int(resetTime) | ||
| if resetTime > 0 && resetTime <= int64(math.MaxInt32) { | ||
| seconds = int(resetTime) | ||
| } | ||
| } | ||
| if seconds > 0 { | ||
| waitSeconds = seconds |
llmobs/internal/errors/errors.go
Outdated
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2025 Datadog, Inc. | ||
|
|
||
| package errors |
There was a problem hiding this comment.
We have a similar package that we merged a bit ago. Could it cover the use cases we have here, instead of creating a new package for it? We can add more functionality to the other package as needed.
There was a problem hiding this comment.
yes! I didn't know this existed. I already refactored the code to use this package, will push the changes shortly 🙏
What does this PR do?
Motivation
Reviewer's Checklist
./scripts/lint.shlocally.Unsure? Have a question? Request a review!