♻️ [RUM-249] update worker protocol #2346
Conversation
BenoitZugmeyer
force-pushed
the
benoit/update-worker-protocol
branch
from
8598c9e
to
b5a5071
Compare
Codecov Report
@@ Coverage Diff @@
## main #2346 +/- ##
=======================================
Coverage 94.17% 94.18%
=======================================
Files 206 207 +1
Lines 6134 6176 +42
Branches 1358 1367 +9
=======================================
+ Hits 5777 5817 +40
- Misses 357 359 +2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
BenoitZugmeyer
force-pushed
the
benoit/update-worker-protocol
branch
from
b5a5071
to
f7b4829
Compare
BenoitZugmeyer
force-pushed
the
benoit/update-worker-protocol
branch
from
f7b4829
to
d024234
Compare
packages/rum/src/domain/segmentCollection/startDeflateWorker.spec.ts
Outdated
Show resolved
Hide resolved
| } | ||
| } | ||
|
|
||
| function concatBuffers(buffers: Uint8Array[]) { |
There was a problem hiding this comment.
💭 thought: this function is needed in both the worker and RUM. I duplicated it, it's not great, but it avoids adding a dependency between the worker code and main packages. Maybe the worker can expose some utility function like that, but we'd need to ensure that it is tree-shakable to a pulling too much of the worker code in main packages.
There was a problem hiding this comment.
Would it be simpler if the utility code was exposed from core?
There was a problem hiding this comment.
Let's try it. I put it in core, so now the worker depends on core. I was hesitating about it, because in the future core might very well depend on the worker. We'll see how it goes!
BenoitZugmeyer
force-pushed
the
benoit/update-worker-protocol
branch
from
b27c517
to
e326704
Compare
This makes tests impler as they are now synchronous, and will allow to test more scenarios
|
/to-staging |
|
🚂 Branch Integration: starting soon, merge in < 0s commit 093e058d0d will soon be integrated into staging-30. This build is going to start soon! (estimated merge in less than 0s) you can cancel this operation by commenting your pull request with |
|
🚂 Branch Integration: this commit was successfully integrated Commit 093e058d0d has been merged into staging-30 in merge commit b8fa9dfdec. Check out the triggered pipeline on Gitlab 🦊 |
| /** | ||
| * Creates a buffer of bytes to append to the end of the Zlib stream to finish it. It is composed of | ||
| * two parts: | ||
| * * an empty deflate block as specified in https://www.rfc-editor.org/rfc/rfc1951.html#page-13 , | ||
| * which happens to be always 3, 0 | ||
| * * an adler32 checksum as specified in https://www.rfc-editor.org/rfc/rfc1950.html#page-4 | ||
| * | ||
| * This is essentially what pako writes to the stream when invoking `deflate.push('', | ||
| * constants.Z_FINISH)` operation after some data has been pushed with "Z_SYNC_FLUSH", but doing so | ||
| * ends the stream and no more data can be pushed into it. | ||
| * | ||
| * Since we want to let the main thread end the stream synchronously at any point without needing to | ||
| * send a message to the worker to flush it, we send back a trailer in each "wrote" response so the | ||
| * main thread can just append it to the compressed data to end the stream. | ||
| * | ||
| * Beside creating a valid zlib stream, those 6 bits are expected to be here so the Datadog backend | ||
| * can merge streams together (see internal doc). | ||
| */ |
BenoitZugmeyer
force-pushed
the
benoit/update-worker-protocol
branch
from
c45b77f
to
dd5c779
Compare
Motivation
Adjust the Worker protocol to prepare for RUM/Logs data compression
Changes
Lots of changes discussed in the RFC. Commits are self-contained, please review commit by commit.
Note: changes in
segment.tswill be extracted to aDeflateWorkerin a next PRTesting
I have gone over the contributing documentation.