Skip to content

fix(llmobs/dataset): add pagination and big datasets support#4039

Merged
dd-mergequeue[bot] merged 1 commit into
mainfrom
rarguelloF/add-dataset-pagination
Oct 9, 2025
Merged

fix(llmobs/dataset): add pagination and big datasets support#4039
dd-mergequeue[bot] merged 1 commit into
mainfrom
rarguelloF/add-dataset-pagination

Conversation

@rarguelloF

@rarguelloF rarguelloF commented Oct 8, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Implements pagination when pulling dataset records. This is necessary for big datasets to work.
  • Uses an alternative BulkUploadDataset endpoint for datasets >5MB
  • Increases timeouts for these specific endpoints and removes global http client timeouts

Motivation

Reviewer's Checklist

  • Changed code has unit tests for its functionality at or near 100% coverage.
  • System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
  • There is a benchmark for any new code, or changes to existing code.
  • If this interacts with the agent in a new way, a system test has been added.
  • New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
  • Add an appropriate team label so this PR gets put in the right place for the release notes.
  • Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

@rarguelloF
rarguelloF requested a review from a team as a code owner October 8, 2025 14:58
@rarguelloF rarguelloF mentioned this pull request Oct 8, 2025
Closed
7 tasks
@pr-commenter

pr-commenter Bot commented Oct 8, 2025 •
edited
Loading

Copy link
Copy Markdown

Benchmarks

Benchmark execution time: 2025-10-08 15:02:08

Comparing candidate commit d03ba07 in PR branch rarguelloF/add-dataset-pagination with baseline commit 5603e37 in branch main.

Found 0 performance improvements and 1 performance regressions! Performance is the same for 5 metrics, 0 unstable metrics.

scenario:BenchmarkConfig/scenario_WithStartSpanConfig-24

  • 🟥 execution_time [+2.252ns; +2.968ns] or [+2.326%; +3.067%]

genesor
genesor reviewed Oct 8, 2025
View reviewed changes
Comment thread internal/llmobs/transport/transport.go
defaultSite = "datadoghq.com"
defaultMaxRetries uint = 3

defaultTimeout time.Duration = 5 * time.Second

@genesor genesor Oct 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want the timeouts to be configurable somehow or using only hardcoded values is acceptable ?

@gary-huang gary-huang Oct 9, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

imo it's fine to leave it for now, we can make it configurable in the future

gary-huang
gary-huang approved these changes Oct 9, 2025
View reviewed changes

@gary-huang gary-huang left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔥

darccio
darccio approved these changes Oct 9, 2025
View reviewed changes

@darccio darccio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@rarguelloF

rarguelloF commented Oct 9, 2025

Copy link
Copy Markdown
Contributor Author

/merge

@dd-devflow-routing-codex

dd-devflow-routing-codex Bot commented Oct 9, 2025 •
edited
Loading

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2025-10-09 08:23:16 UTC ℹ️ Start processing command /merge

2025-10-09 08:23:22 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 19m (p90).

2025-10-09 08:40:24 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue
dd-mergequeue Bot merged commit a2caaee into main Oct 9, 2025
243 checks passed
@dd-mergequeue
dd-mergequeue Bot deleted the rarguelloF/add-dataset-pagination branch October 9, 2025 08:40
hannahkm pushed a commit that referenced this pull request Oct 9, 2025
@rarguelloF @hannahkm
fix(llmobs/dataset): add pagination and big datasets support (#4039)
ec72bfb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@genesor genesor genesor left review comments
@darccio darccio darccio approved these changes
+1 more reviewer
@gary-huang gary-huang gary-huang approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

mergequeue-status: done

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rarguelloF @darccio @genesor @gary-huang