⚗️✨ [RUM-253] compress RUM data #2400

BenoitZugmeyer · 2023-08-30T14:20:47Z

Motivation

To reduce the bandwidth impact when using the RUM Browser SDK, we want to compress data sent to the
backend.

Changes

This PR implement data compression for the "rum" package/bundle behind an experimental feature flag.
This will stay like this during dogfooding, then we'll introduce a proper initialization parameter +
support it on "rum-slim" and "logs" as well.

Testing

Local
Stagin
Unit
End to end (see ✅ [RUM-253] add E2E tests related to compression #2416)

I have gone over the contributing documentation.

BenoitZugmeyer · 2023-09-05T10:45:55Z

Current dependencies on/for this PR:

main:
- PR 🔊 [RUM-253] customize deflate worker failure logs #2414
  - PR ♻️ [RUM-253] adapt transport to send encoded data #2415
    - PR ⚗️✨ [RUM-253] compress RUM data #2400 👈
      - PR ✅ [RUM-253] add E2E tests related to compression #2416

This comment was autogenerated by Freephite.

packages/rum/src/domain/deflate/deflateEncoder.ts

packages/rum/src/domain/deflate/deflateEncoder.spec.ts

packages/rum-core/src/boot/rumPublicApi.ts

amortemousque · 2023-09-07T16:04:30Z

packages/core/src/transport/startBatchWithReplica.ts

    return new Batch(
-      createHttpRequest(configuration, endpointBuilder, configuration.batchBytesLimit, reportError),
+      encoder || createIdentityEncoder(),


💭 thought: ‏I had a hard time understanding in which case the encoder was undefined and I'm wondering if it could me simplified. FMU we have to look at:

Identify that createDeflateEncoder can be undefined :

browser-sdk/packages/rum-core/src/transport/startRumBatch.ts

Line 36 in 9862d64

encoder: createDeflateEncoder && createDeflateEncoder(DeflateEncoderStreamId.RUM),

Because deflateWorker && createDeflateEncoder can be undefined :

browser-sdk/packages/rum-core/src/boot/rumPublicApi.ts

Lines 193 to 195 in 9862d64

deflateWorker &&

createDeflateEncoder &&

((streamId) => createDeflateEncoder(configuration, deflateWorker!, streamId))

Then deflateWorker can be null because of:

browser-sdk/packages/rum-core/src/boot/rumPublicApi.ts

Lines 152 to 161 in 9862d64

if (

isExperimentalFeatureEnabled(ExperimentalFeature.COMPRESS_BATCH) &&

!eventBridgeAvailable &&

startDeflateWorker

) {

deflateWorker = startDeflateWorker(configuration, 'Datadog RUM')

if (!deflateWorker) {

return

}

}

Personally I prefer to have a rumPublicApi as dump as possible and make the instantiation checks in startRum.
Also, It could be nice to create the encoder in startRum() to remove the logic from startRumBatch and startBatchWithReplica.
I don't know all the intricacies of the implement so it might no be feasible 🙂 Let me know what you think.

As seen IRL, I changed a bit how encoders are created so the condition on whether we use an "Identity" or "Deflate" encoder is clearer.

codecov-commenter · 2023-09-08T16:49:10Z

Codecov Report

Attention: Patch coverage is 97.77778% with 4 lines in your changes missing coverage. Please review.

Project coverage is 93.86%. Comparing base (5d5935c) to head (e4853e1).
Report is 412 commits behind head on main.

❗ Current head e4853e1 differs from pull request most recent head ebb9a24

Please upload reports for the commit ebb9a24 to get more accurate results.

Files with missing lines	Patch %	Lines
packages/rum/src/domain/deflate/deflateEncoder.ts	95.12%	2 Missing ⚠️
packages/core/src/transport/batch.ts	97.22%	1 Missing ⚠️
packages/core/src/transport/httpRequest.ts	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2400      +/-   ##
==========================================
+ Coverage   93.26%   93.86%   +0.60%     
==========================================
  Files         221      222       +1     
  Lines        6580     6459     -121     
  Branches     1443     1431      -12     
==========================================
- Hits         6137     6063      -74     
+ Misses        443      396      -47

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bcaudan · 2023-09-15T08:13:17Z

packages/rum/src/domain/deflate/deflateEncoder.ts

@@ -29,19 +29,21 @@ export function createDeflateEncoder(
    configuration,
    worker,
    'message',
-    ({ data: workerResponse }: MessageEvent<DeflateWorkerResponse>) => {
-      if (workerResponse.type !== 'wrote' || workerResponse.streamId !== streamId) {
+    ({ data }: MessageEvent<DeflateWorkerResponse>) => {


❓ question: ‏you have reverted the previous commit changes, is it intended?

Ah, no, must have been a conflict resolution error.

This new interface is slightly different than the current deflate encoder implementation. This is because it has been adjusted to take the Batch constraints into account. The deflate encoder implementation will be changed in a future commit.

This commit is not pretty. To avoid having inflated packages while dogfooding, we pass some functions from rum to rum-core when creating the RUM public API. In the future, those functions will be moved to the core package, and rum-core will be able to import them directly.

BenoitZugmeyer force-pushed the benoit/compress--rum-data branch from 51ca845 to 1cd7901 Compare September 5, 2023 10:26

BenoitZugmeyer changed the base branch from main to benoit/compress--adapt-transport September 5, 2023 10:45

BenoitZugmeyer mentioned this pull request Sep 5, 2023

✅ [RUM-253] add E2E tests related to compression #2416

Merged

4 tasks

BenoitZugmeyer changed the title ~~⚗️✨ compress RUM data~~ ⚗️✨ [RUM-235] compress RUM data Sep 5, 2023

BenoitZugmeyer marked this pull request as ready for review September 5, 2023 10:49

BenoitZugmeyer requested review from a team as code owners September 5, 2023 10:49

This was referenced Sep 5, 2023

🔊 [RUM-253] customize deflate worker failure logs #2414

Merged

♻️ [RUM-253] adapt transport to send encoded data #2415

Merged

BenoitZugmeyer changed the title ~~⚗️✨ [RUM-235] compress RUM data~~ ⚗️✨ [RUM-253] compress RUM data Sep 5, 2023

bcaudan reviewed Sep 6, 2023

View reviewed changes

packages/rum/src/domain/deflate/deflateEncoder.ts Outdated Show resolved Hide resolved

packages/rum/src/domain/deflate/deflateEncoder.spec.ts Outdated Show resolved Hide resolved

packages/rum-core/src/boot/rumPublicApi.ts Outdated Show resolved Hide resolved

amortemousque reviewed Sep 7, 2023

View reviewed changes

BenoitZugmeyer force-pushed the benoit/compress--rum-data branch 3 times, most recently from 7a494ca to de29ea4 Compare September 8, 2023 16:46

BenoitZugmeyer force-pushed the benoit/compress--rum-data branch from 8329f48 to 399820b Compare September 8, 2023 17:12

BenoitZugmeyer force-pushed the benoit/compress--adapt-transport branch from ec86af6 to 5466106 Compare September 11, 2023 14:12

BenoitZugmeyer force-pushed the benoit/compress--rum-data branch from 399820b to 3a7384d Compare September 11, 2023 14:12

amortemousque approved these changes Sep 11, 2023

View reviewed changes

bcaudan approved these changes Sep 15, 2023

View reviewed changes

Base automatically changed from benoit/compress--adapt-transport to main September 15, 2023 13:55

BenoitZugmeyer added 7 commits October 30, 2023 14:41

✨ [RUM-253] use an Encoder in Batch

9c291f7

♻️ [RUM-253] adjust DeflateEncoder implementation

19a9128

🚚 [RUM-253] move some deflate types to core

fa009b7

[RUM-253] allow providing an encoder when creating batches

461e29d

🐛 fix support for IE

2a1213a

BenoitZugmeyer added 5 commits October 30, 2023 14:43

👌 clearer variable name

21d3ad8

👌 cancel finish callbacks when running finishSync()

abd4471

👌 use the Worker instance created from RUM in the recorder

95c68f8

👌 simplify encoding creation condition

8bbf305

[RUM-253] increase a bit the estimated deflate ratio + add justification

ebb9a24

BenoitZugmeyer force-pushed the benoit/compress--rum-data branch from e4853e1 to ebb9a24 Compare October 30, 2023 13:48

BenoitZugmeyer merged commit 0013510 into main Oct 31, 2023
2 checks passed

BenoitZugmeyer deleted the benoit/compress--rum-data branch October 31, 2023 09:06

BenoitZugmeyer mentioned this pull request Nov 7, 2023

✨ [RUM-253] enable RUM request compression via the compressIntakeRequests parameter #2500

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚗️✨ [RUM-253] compress RUM data #2400

⚗️✨ [RUM-253] compress RUM data #2400

BenoitZugmeyer commented Aug 30, 2023 •

edited

Loading

BenoitZugmeyer commented Sep 5, 2023 •

edited

Loading

amortemousque Sep 7, 2023

BenoitZugmeyer Sep 8, 2023

codecov-commenter commented Sep 8, 2023 •

edited

Loading

bcaudan Sep 15, 2023

BenoitZugmeyer Sep 15, 2023

	deflateWorker &&
	createDeflateEncoder &&
	((streamId) => createDeflateEncoder(configuration, deflateWorker!, streamId))

	if (
	isExperimentalFeatureEnabled(ExperimentalFeature.COMPRESS_BATCH) &&
	!eventBridgeAvailable &&
	startDeflateWorker
	) {
	deflateWorker = startDeflateWorker(configuration, 'Datadog RUM')
	if (!deflateWorker) {
	return
	}
	}

⚗️✨ [RUM-253] compress RUM data #2400

⚗️✨ [RUM-253] compress RUM data #2400

Conversation

BenoitZugmeyer commented Aug 30, 2023 • edited Loading

Motivation

Changes

Testing

BenoitZugmeyer commented Sep 5, 2023 • edited Loading

amortemousque Sep 7, 2023

Choose a reason for hiding this comment

BenoitZugmeyer Sep 8, 2023

Choose a reason for hiding this comment

codecov-commenter commented Sep 8, 2023 • edited Loading

Codecov Report

bcaudan Sep 15, 2023

Choose a reason for hiding this comment

BenoitZugmeyer Sep 15, 2023

Choose a reason for hiding this comment

BenoitZugmeyer commented Aug 30, 2023 •

edited

Loading

BenoitZugmeyer commented Sep 5, 2023 •

edited

Loading

codecov-commenter commented Sep 8, 2023 •

edited

Loading