-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add gzip compression on reaper report #242
base: main
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 |
---|---|---|
|
@@ -23,12 +23,14 @@ import okhttp3.Request | |
import okhttp3.RequestBody.Companion.toRequestBody | ||
import okhttp3.Response | ||
import okhttp3.internal.closeQuietly | ||
import java.io.ByteArrayOutputStream | ||
import java.io.DataInputStream | ||
import java.io.EOFException | ||
import java.io.File | ||
import java.io.FileInputStream | ||
import java.io.FileOutputStream | ||
import java.io.IOException | ||
import java.util.zip.GZIPOutputStream | ||
import kotlin.coroutines.resumeWithException | ||
|
||
internal class ReaperReportUploadWorker( | ||
|
@@ -175,11 +177,14 @@ internal class ReaperReportUploadWorker( | |
stream.write(reportString.encodeToByteArray()) | ||
} | ||
|
||
val compressedReport = gzipCompressReport(reportString) | ||
|
||
val url = "$baseUrl/report" | ||
val request = Request.Builder().apply { | ||
header("Authorization", "Bearer $apiKey") | ||
header("Content-Encoding", "gzip") | ||
url(url) | ||
post(reportString.toRequestBody("application/json; charset=utf-8".toMediaTypeOrNull())) | ||
post(compressedReport.toRequestBody("application/json; charset=utf-8".toMediaTypeOrNull())) | ||
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. I think okhttp has a method for this: https://square.github.io/okhttp/5.x/okhttp/okhttp3/-request-body/-companion/gzip.html Something like:
I think 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. Appears to only be in the 5.0 snapshot, which we're on 4.11. I'll see if 4.x has one, as I'd prefer to not leverage a snapshot release. 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. Follow up, did some reading and despite 5.x-snapshot state, it sounds like it actually is quite stable/no breaking changes. We need to land backend support for this, but I can follow up with updating OkHTTP to 5.x and leverage the helper function. |
||
}.build() | ||
|
||
client.newCall(request).executeAsync().use { response -> | ||
|
@@ -196,6 +201,14 @@ internal class ReaperReportUploadWorker( | |
} | ||
} | ||
|
||
private fun gzipCompressReport(reportString: String): ByteArray { | ||
val byteArrayOutputStream = ByteArrayOutputStream() | ||
GZIPOutputStream(byteArrayOutputStream).use { gzip -> | ||
gzip.write(reportString.toByteArray()) | ||
} | ||
return byteArrayOutputStream.toByteArray() | ||
} | ||
|
||
@OptIn(ExperimentalCoroutinesApi::class) | ||
private suspend fun Call.executeAsync(): Response = suspendCancellableCoroutine { continuation -> | ||
continuation.invokeOnCancellation { | ||
|
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.
I thought sorting the records might help with compression, and it does, but not as much as I had hoped:
(Number is compression ratio)
So maybe not worth it.