Merge feature/timeseries to develop - #3085
Conversation
697fe71 to
5762412
Compare
![datadog-official[bot]](https://avatars.githubusercontent.com/in/1358509?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The timeseries collector lifecycle and event serialization paths show no reproducible behavioral regression in the reviewed diff. Focused iOS execution was unavailable because this environment has no xcodebuild or simulator; Swift syntax parsing passed, and no additional tests are recommended because no production-only input pattern was available to lock in.
🤖 Datadog Autotest · Commit 4fa762a · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fa762a789
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
4fa762a to
92db64f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92db64fbf9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
maxep
left a comment
maxep
left a comment
There was a problem hiding this comment.
Everything was already reviewed 👍
simaoseica-dd
left a comment
simaoseica-dd
left a comment
There was a problem hiding this comment.
Left some comments.
My main point is architectural. This initiative could have pushed for detaching timeseries from views instead of working around a required singular view field in the event. The series of edge cases surfaced by codex is the signal that it will be complex to address.
|
We've decided to first do a broader timeseries cleanup, the mandatory-view question and the related review comments here are being addressed as part of that effort, split across a few follow-up PRs (view field made optional, plus the collector/session-lifecycle fixes now in #3118). |
5de4626 to
16fc4cc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 258b6fa7a4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… the view at flush time
…tiveContextReader
…ionActivityReader
…d drop collector's session-scope dependency
af46298 to
2b5f03e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ba76c1d3f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
4 participants
What and why?
The review of this feature has been split into multiple PRs:
The only change made by this PR is that
enableTimeseriesis marked experimental and I removed pod artifacts.Warning
This API is marked experimental. The frontend support for visualizing Timeseries data is not yet available, so customers are advised not to enable this feature.
Review checklist
make api-surfacewhen adding new APIs