timeseries [5/6] RUM-13949 Wire timeseries collection into RUM session lifecycle - #3435
Conversation
timeseries RUM-13949 Wire timeseries collection into RUM session lifecycle
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feature/timeseries #3435 +/- ##
======================================================
+ Coverage 72.39% 72.40% +0.01%
======================================================
Files 977 977
Lines 35820 35895 +75
Branches 5970 5983 +13
======================================================
+ Hits 25930 25987 +57
- Misses 8268 8278 +10
- Partials 1622 1630 +8
🚀 New features to boost your workflow:
|
1632f2f to
cdcacee
Compare
f630b6d to
3b05b0c
Compare
cdcacee to
9c62b82
Compare
3b05b0c to
9d01d39
Compare
9c62b82 to
509d75e
Compare
9d01d39 to
8ea9614
Compare
509d75e to
db0263b
Compare
8ea9614 to
9a49aaa
Compare
This comment has been minimized.
This comment has been minimized.
db0263b to
ec3a571
Compare
0db4915 to
91bf158
Compare
ec3a571 to
62b2166
Compare
91bf158 to
7298723
Compare
4ee1870 to
4c08c9b
Compare
7298723 to
cd319d9
Compare
timeseries RUM-13949 Wire timeseries collection into RUM session lifecycletimeseries [5/6] RUM-13949 Wire timeseries collection into RUM session lifecycle
4c08c9b to
acebd04
Compare
cd319d9 to
ac48ae9
Compare
cf947b6 to
527f8b5
Compare
eb04d67 to
3bc2f67
Compare
527f8b5 to
d8bd478
Compare
3bc2f67 to
8c1fee9
Compare
d8bd478 to
3615ce2
Compare
8c1fee9 to
27ffe71
Compare
3615ce2 to
b30c63a
Compare
d4fea36 to
e46fbe0
Compare
e46fbe0 to
8e019ac
Compare
3e7c2dc to
d0b680c
Compare
89cae98 to
98ef2c6
Compare
98ef2c6 to
65d113c
Compare
Adds `enableTimeseries()`/`disableTimeseries()` builder methods on `RumConfiguration.Builder`. Wires the `Timeseries.Factory` through `RumFeature` → `DatadogRumMonitor` → `RumApplicationScope` → `RumSessionScope`, so that a new `RumSessionScopeTimeseries` collector is created and started for each sampled session and stopped on session end or SDK teardown. Also enables timeseries in the Kotlin sample app. Connects the standalone pipeline and public configuration API to the existing RUM session lifecycle, completing the end-to-end data flow from configuration through to event emission. Existing scope and monitor test scaffolding is updated to pass `timeseriesFactory = null` / a mock factory where required. The `RumFeature.onStop()` ordering test verifies the writer is still live when the final flush fires.
- Restore closing `}` for `createDataWriter` accidentally dropped when `createTimeseriesCollectingFactory` was inserted after it - Add `@OptIn(ExperimentalRumApi::class)` to `createTimeseries` test helper to satisfy -Werror on the experimental API usage - Rename `RumSessionScopeFactoryTest.kt` → `RumSessionScopeTimeseriesFactoryTest.kt` to match contained class (ktlint filename rule) Ref: RUM-13949
cde192e to
af31b3d
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af31b3d917
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
all notes will be fixed in sepatate branch |
| ), | ||
| dataWriter = dataWriter | ||
| ), | ||
| if (totalRamBytes > 0L) { |
There was a problem hiding this comment.
tbh, I find it hard to believe that there will be a situation that the total available RAM will be 0.
Should we just simplify and forget about this edge case? What will break?
There was a problem hiding this comment.
if by some reason totalRam return 0 - it would lead to ArithmeticException. Timeserices's RAM collector would be trying to collect some values, then would fail on percentage calculation so basically would do some useless job
| } | ||
|
|
||
| private fun initializeFPSVitalMonitor(frequency: VitalsUpdateFrequency): FPSVitalListener? { | ||
| if (frequency == VitalsUpdateFrequency.NEVER) return null |
There was a problem hiding this comment.
Could you explain how is this change related to the topic of this PR? is FPSVitalListener used in time series somehow?
There was a problem hiding this comment.
FPSVitalListener is enabled when vital frequency != NEVER. In the old logic a vitalScheduler was created iff frequency != NEVER. But with timeseries we also using vitalScheduler so it should be created if frequency != NEVER or timeseriesConfiguration != null. So why this changes was made

What does this PR do?
Connects the standalone pipeline and public configuration API to the existing RUM session lifecycle, completing the end-to-end data flow from configuration through to event emission.
Review checklist (to be filled by reviewers)