RUM-767: Improve reading-errors telemetry for dropped events - #3598
RUM-767: Improve reading-errors telemetry for dropped events#3598satween wants to merge 1 commit into
Conversation
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR improves internal telemetry for persistence-layer drops and read/write errors by introducing consistent attribute keys (e.g., feature.name, event.dropped_bytes) and threading feature context through the persistence stack so telemetry is easier to query across features and code paths.
Changes:
- Added
TelemetryAttributes(internal module) and updated API surface files accordingly. - Propagated
featureName/size context through core persistence components (FileEventBatchWriter,BatchFileReaderWriter/PlainBatchFileReaderWriter,SdkFeature,CoreFeature) and updated unit tests. - Added a max-item-size drop path in
RumDataWriterwith telemetry/user logging and expanded RUM unit tests for size-exceeded scenarios.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| features/dd-sdk-android-rum/src/test/kotlin/com/datadog/android/rum/internal/domain/RumDataWriterTest.kt | Adds unit coverage for RUM events dropped when serialized payload exceeds maxItemSize. |
| features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt | Wires storageConfiguration.maxItemSize into RumDataWriter. |
| features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/domain/RumDataWriter.kt | Adds size-limit drop handling with user+telemetry logging and dropped-bytes attribute. |
| dd-sdk-android-internal/src/main/java/com/datadog/android/internal/telemetry/TelemetryAttributes.kt | Introduces shared telemetry attribute keys for persistence diagnostics. |
| dd-sdk-android-internal/api/dd-sdk-android-internal.api | API dump updated for the new TelemetryAttributes public type. |
| dd-sdk-android-internal/api/apiSurface | API surface updated to include TelemetryAttributes. |
| dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockTest.kt | Updates expectations for TLV serialization oversize logging targets/level. |
| dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReaderTest.kt | Adds/updates tests for invalid TLV declared length handling and logging. |
| dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriterTest.kt | Updates tests for new telemetry targets and added dropped-bytes/feature attributes. |
| dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriterTest.kt | Updates tests for new constructor signature and feature-name telemetry attributes. |
| dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriterTest.kt | Updates tests for create(..., featureName) signature. |
| dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/CoreFeatureTest.kt | Updates usage of BatchFileReaderWriter.create to include feature name. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/SdkFeature.kt | Threads featureName into persistence reader/writer creation. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReader.kt | Adds declared-length validation with telemetry/user error logging. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlock.kt | Changes oversize TLV serialization log to ERROR and USER+TELEMETRY; exposes size constant internally. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/FileEventBatchWriter.kt | Adds featureName and attaches feature.name + event.dropped_bytes on oversize drops. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/PlainBatchFileReaderWriter.kt | Adds featureName and attaches it to read-side telemetry errors/warnings. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/file/batch/BatchFileReaderWriter.kt | Extends factory to require featureName and passes it down. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/ConsentAwareStorage.kt | Passes featureName into FileEventBatchWriter. |
| dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt | Ensures core-created batch reader/writers include Feature.RUM_FEATURE_NAME. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: cfc0a6226a
ℹ️ 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".
9307351 to
c7c311f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/persistence/tlvformat/TLVBlockFileReader.kt:102
readData()usescopyOfRangeSafe(newIndex, newIndex + lengthData)but never checks that the requested range is actually present ininputArray. If the file is truncated,copyOfRangeSafereturns an empty array and the reader will still advancenewIndexand return a block, which can later crash (e.g.ByteArray.toInt()on empty data inDatastoreFileReader). Add an explicit bounds check and treat this as a deserialize failure.
val dataBytes = inputArray.copyOfRangeSafe(newIndex, newIndex + lengthData)
newIndex += lengthData
return TLVResult(
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7c311f9e0
ℹ️ 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".
a326ebc to
ad3f057
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad3f05750b
ℹ️ 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".
ad3f057 to
8a88098
Compare
8a88098 to
973960e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 973960e048
ℹ️ 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".
973960e to
e44eaf1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e44eaf10ee
ℹ️ 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".
e44eaf1 to
d40d544
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d40d54444d
ℹ️ 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".
What does this PR do?
Improves internal telemetry for events dropped during file read/write on the persistence layer:
TelemetryAttributes(feature.name,event.dropped_bytes) so dropped-event error telemetry carries consistent, queryable attributes across features (RUM, logs, tracing) and across read/write paths.RumDataWriter,FileEventBatchWriter,BatchFileReaderWriter/PlainBatchFileReaderWriter,TLVBlock/TLVBlockFileReader,CoreFeature, andSdkFeatureso telemetry errors report which feature and how many bytes were involved when an event is dropped.Motivation
When events are dropped during persistence (e.g. oversized batches), the existing telemetry lacked enough context to diagnose which feature was affected and how large the dropped payload was. RUM-767 tracks improving this visibility.
Additional Notes
TLVBlockFileReader,RumDataWriter,FileEventBatchWriter,BatchFileReaderWriter/PlainBatchFileReaderWriter,TLVBlock.dd-sdk-android-internalpublic API surface updated (apiSurface/.apifiles) for the newTelemetryAttributesobject.Review checklist (to be filled by reviewers)