-
Notifications
You must be signed in to change notification settings - Fork 81
RUM-767: Improve reading-errors telemetry for dropped events #3598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,8 @@ import java.util.Locale | |
|
|
||
| internal class TLVBlockFileReader( | ||
| val internalLogger: InternalLogger, | ||
| val fileReaderWriter: FileReaderWriter | ||
| val fileReaderWriter: FileReaderWriter, | ||
| private val maxEntrySize: Int = TLVBlock.MAXIMUM_DATA_SIZE_MB | ||
| ) { | ||
| @WorkerThread | ||
| internal fun read( | ||
|
|
@@ -76,6 +77,7 @@ internal class TLVBlockFileReader( | |
| ) | ||
| } | ||
|
|
||
| @Suppress("ReturnCount") | ||
| private fun readData(inputArray: ByteArray, currentIndex: Int): TLVResult<ByteArray>? { | ||
| val lengthBlockSize = Int.SIZE_BYTES | ||
| var newIndex = currentIndex + lengthBlockSize | ||
|
|
@@ -89,6 +91,11 @@ internal class TLVBlockFileReader( | |
|
|
||
| val lengthData = lengthInBytes.toInt() | ||
|
|
||
| if (lengthData !in 0..maxEntrySize) { | ||
| logBlockSizeExceededError(lengthData) | ||
| return null | ||
| } | ||
|
|
||
| val dataBytes = | ||
| inputArray.copyOfRangeSafe(newIndex, newIndex + lengthData) | ||
|
|
||
|
|
@@ -113,6 +120,16 @@ internal class TLVBlockFileReader( | |
| ) | ||
| } | ||
|
|
||
| private fun logBlockSizeExceededError(lengthData: Int) { | ||
| internalLogger.log( | ||
| level = InternalLogger.Level.ERROR, | ||
| targets = listOf(InternalLogger.Target.USER, InternalLogger.Target.TELEMETRY), | ||
| messageBuilder = { | ||
| CORRUPT_DATA_LENGTH_ERROR.format(Locale.US, maxEntrySize, lengthData) | ||
| } | ||
|
Comment on lines
+126
to
+129
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a feature's DataStore file has an invalid declared TLV length, this new telemetry error is emitted without Useful? React with 👍 / 👎. |
||
| ) | ||
| } | ||
|
|
||
| private fun logFailedToDeserializeError() { | ||
| internalLogger.log( | ||
| target = InternalLogger.Target.MAINTAINER, | ||
|
|
@@ -129,5 +146,10 @@ internal class TLVBlockFileReader( | |
| internal companion object { | ||
| internal const val CORRUPT_TLV_HEADER_TYPE_ERROR = "TLV header corrupt. Invalid type %s" | ||
| internal const val FAILED_TO_DESERIALIZE_ERROR = "Failed to deserialize TLV data length" | ||
|
|
||
| // Read-side counterpart of TLVBlock.BYTE_LENGTH_EXCEEDED_ERROR (write/serialize side). | ||
| // Kept distinct so dropped blocks can be told apart by direction in telemetry. | ||
| internal const val CORRUPT_DATA_LENGTH_ERROR = | ||
| "DataBlock length read from file is invalid or exceeds limit of %s bytes, was %s" | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a batch file is truncated or corrupt and this branch logs
WARNING_NOT_ALL_DATA_READ, the unread bytes represented byremainingare the data that will be lost once the partially read batch is confirmed/deleted. The new write-side oversized-event telemetry includesevent.dropped_bytes, but this read-side drop signal only hasfeature.name, so read corruption remains unqueryable by loss size despite the value being available here.Useful? React with 👍 / 👎.