⚗️[RUM-4780] Remote configuration #2799

amortemousque · 2024-06-07T15:39:13Z

Motivation

Add remote configuration capability

Changes

Add new init parameter remoteConfigurationId
Wait for the remote configuration in the preStart module
Overrides the static init configuration with the remote configuration values

How to test

window.DD_RUM.init({
    remoteConfigurationId: "50d8ba4b-2ac1-481f-b16e-f110699ecf66", // only config available
    enableExperimentalFeatures: ['remote_configuration'],
    ...
})

Testing

Local
Staging
Unit
End to end

I have gone over the contributing documentation.

amortemousque · 2024-06-07T15:45:55Z

packages/rum-core/src/boot/preStartRum.spec.ts

@@ -452,6 +502,22 @@ describe('preStartRum', () => {

      expect(strategy.initConfiguration).toEqual(initConfiguration)
    })
+
+    it('returns the initConfiguration with the remote configuration when a remoteConfigurationId is provided', (done) => {


I'm not sure about this behaviour. Basically DD_RUM.getInitConfiguration() will also have the remote configuration values. It has the advantage to make the browser extension reflect the remote config without any change.
Wdyt?

💭 thought: ‏ Currently DD_RUM.getInitConfiguration() returns exactly what the customer called init() with. I wouldn't mind it to return what the SDK was actually initialised with, this means the combination of what the customer called init() with, the remote config if applicable and the defaults.

I think it would be useful information to debug rum, both for customers, for us through telemetry and even for support.

I'm not sure if it is safe to do that or if this will be a breaking change.

We could review our usages of getInitConfiguration https://github.com/search?q=repo%3ADataDog%2Fsynthetics-worker%20getInitConfiguration&type=code

To me it doesn't matter much, but we'll maybe need a onInitConfigurationReady(cb) API in the future if we have use-cases where we need to access the resolved config.

From what I identifyed in Datadog repos, we use the API for

Browser SDK extension: to display the conf

synthetics-worker: Use it to get the applicationId

rum-react-integration-examples (old not maintained repo) uses it to get trackViewsManually

dd-profile-browser: it is used to collected profiles with service, env, and version tags

Clearly if we get one of fields applicationId, service, env or version, from the RC we will have some issues. But luckily for now they won't be available in RC :). So the question is, should we anticipate by introducing a new API right away and defining the getInitConfiguration() has only returning whats passed to the init() API
I'll create an RFC

Seen with @BenoitZugmeyer, since the feature is under FF, lets keep the behaviour as is and revisite later when needed.

perfect! Sounds good!

amortemousque · 2024-06-07T15:48:55Z

packages/rum-core/src/domain/configuration/remoteConfiguration.ts

+import { display, addEventListener, assign } from '@datadog/browser-core'
+import type { RumInitConfiguration } from './configuration'
+
+export const REMOTE_CONFIGURATION_URL = 'https://www.datad0g-browser-agent.com/configuration'


For testing purposes, I hosted the RC in the same CDN as the bundles but it will change once the Remote config provides their CDN

cit-pr-commenter · 2024-06-10T07:47:20Z

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	159.49 KiB	160.06 KiB	578 B	+0.35%	✅
Logs	58.02 KiB	58.07 KiB	46 B	+0.08%	✅
Rum Slim	108.07 KiB	108.64 KiB	578 B	+0.52%	✅
Worker	25.21 KiB	25.21 KiB	0 B	0.00%	✅

🚀 CPU Performance

Action Name	Base Average Cpu Time (ms)	Local Average Cpu Time (ms)	𝚫
addglobalcontext	0.002	0.002	0.001
addaction	0.050	0.053	0.004
adderror	0.052	0.041	-0.011
addtiming	0.001	0.001	-0.000
startview	1.427	1.185	-0.242
startstopsessionreplayrecording	1.077	1.090	0.013
logmessage	0.007	0.023	0.016

🧠 Memory Performance

Action Name	Base Consumption Memory (bytes)	Local Consumption Memory (bytes)	𝚫 (bytes)
addglobalcontext	19.58 KiB	19.47 KiB	-116 B
addaction	68.97 KiB	68.50 KiB	-486 B
adderror	83.18 KiB	83.93 KiB	762 B
addtiming	17.24 KiB	17.73 KiB	501 B
startview	313.58 KiB	320.50 KiB	6.92 KiB
startstopsessionreplayrecording	11.51 KiB	11.99 KiB	499 B
logmessage	64.11 KiB	66.83 KiB	2.72 KiB

packages/rum-core/src/domain/configuration/remoteConfiguration.ts

packages/core/src/browser/addEventListener.ts

packages/core/src/transport/eventBridge.ts

thomas-lebeau · 2024-06-10T08:46:27Z

packages/rum-core/src/boot/preStartRum.spec.ts

@@ -452,6 +502,22 @@ describe('preStartRum', () => {

      expect(strategy.initConfiguration).toEqual(initConfiguration)
    })
+
+    it('returns the initConfiguration with the remote configuration when a remoteConfigurationId is provided', (done) => {


💭 thought: ‏ Currently DD_RUM.getInitConfiguration() returns exactly what the customer called init() with. I wouldn't mind it to return what the SDK was actually initialised with, this means the combination of what the customer called init() with, the remote config if applicable and the defaults.

I think it would be useful information to debug rum, both for customers, for us through telemetry and even for support.

I'm not sure if it is safe to do that or if this will be a breaking change.

packages/rum-core/src/domain/configuration/configuration.spec.ts

codecov-commenter · 2024-06-10T10:05:37Z

Codecov Report

Attention: Patch coverage is 97.91667% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.66%. Comparing base (b839f8c) to head (3ad4673).

Files	Patch %	Lines
...re/src/domain/configuration/remoteConfiguration.ts	94.44%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2799      +/-   ##
==========================================
+ Coverage   93.65%   93.66%   +0.01%     
==========================================
  Files         243      244       +1     
  Lines        7140     7168      +28     
  Branches     1599     1603       +4     
==========================================
+ Hits         6687     6714      +27     
- Misses        453      454       +1

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

amortemousque · 2024-06-11T07:22:26Z

/to-staging

dd-devflow · 2024-06-11T07:22:32Z

🚂 Branch Integration: starting soon, median merge time is 10m

Commit 10bc671246 will soon be integrated into staging-24.

Use /to-staging -c to cancel this operation!

Integrated commit sha: 10bc671 Co-authored-by: Aymeric Mortemousque <[email protected]>

dd-devflow · 2024-06-11T07:32:48Z

🚂 Branch Integration: This commit was successfully integrated

Commit 10bc671246 has been merged into staging-24 in merge commit 4e239e6834.

Check out the triggered pipeline on Gitlab 🦊

packages/rum-core/src/boot/preStartRum.spec.ts

BenoitZugmeyer · 2024-06-11T09:50:48Z

packages/rum-core/src/boot/preStartRum.spec.ts

@@ -452,6 +502,22 @@ describe('preStartRum', () => {

      expect(strategy.initConfiguration).toEqual(initConfiguration)
    })
+
+    it('returns the initConfiguration with the remote configuration when a remoteConfigurationId is provided', (done) => {


We could review our usages of getInitConfiguration https://github.com/search?q=repo%3ADataDog%2Fsynthetics-worker%20getInitConfiguration&type=code

To me it doesn't matter much, but we'll maybe need a onInitConfigurationReady(cb) API in the future if we have use-cases where we need to access the resolved config.

packages/rum-core/src/domain/configuration/configuration.ts

) into staging-24 pm_trace_id: 36652155 feature_branch_pipeline_id: 36652155 source: to-staging * commit '040eb5094590ebb77f1f26c154eefd69b1d81306': Improve callback naming Remove `custom` field. ⚗️[RUM-4780] Remote configuration (#2799) Use assign polyfill

amortemousque added 2 commits June 7, 2024 17:25

Move the configuration in a dedicated folder

153e1e7

Add the remote configuration

f5b9b06

amortemousque changed the title ~~Aymeric/remote configuration~~ ⚗️[RUM-4780] Remote configuration Jun 7, 2024

amortemousque commented Jun 7, 2024

View reviewed changes

amortemousque marked this pull request as ready for review June 7, 2024 15:50

amortemousque requested a review from a team as a code owner June 7, 2024 15:50

thomas-lebeau reviewed Jun 10, 2024

View reviewed changes

Fix type

10bc671

dd-mergequeue bot added a commit that referenced this pull request Jun 11, 2024

Integrate aymeric/remote-configuration (#2799) into staging-24

4e239e6

Integrated commit sha: 10bc671 Co-authored-by: Aymeric Mortemousque <[email protected]>

dd-devflow bot added the staging-24 label Jun 11, 2024

Use the internalAnalyticsSubdomain

3e83fd1

BenoitZugmeyer reviewed Jun 12, 2024

View reviewed changes

amortemousque added 2 commits June 12, 2024 15:26

Use mockExperimentalFeatures

80bd637

Remove unnecessary storeContextsAcrossPages

94fa08e

BenoitZugmeyer approved these changes Jun 12, 2024

View reviewed changes

thomas-lebeau approved these changes Jun 13, 2024

View reviewed changes

Merge branch 'main' into aymeric/remote-configuration

3ad4673

amortemousque merged commit e13a109 into main Jun 13, 2024
20 checks passed

amortemousque deleted the aymeric/remote-configuration branch June 13, 2024 07:54

amortemousque mentioned this pull request Dec 20, 2024

🔉[RUM-7840] Update remote configuration #3239

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚗️[RUM-4780] Remote configuration #2799

⚗️[RUM-4780] Remote configuration #2799

amortemousque commented Jun 7, 2024 •

edited

Loading

amortemousque Jun 7, 2024

thomas-lebeau Jun 10, 2024

BenoitZugmeyer Jun 11, 2024

amortemousque Jun 12, 2024

amortemousque Jun 12, 2024

thomas-lebeau Jun 13, 2024

amortemousque Jun 7, 2024

cit-pr-commenter bot commented Jun 10, 2024 •

edited

Loading

thomas-lebeau Jun 10, 2024

codecov-commenter commented Jun 10, 2024 •

edited

Loading

amortemousque commented Jun 11, 2024

dd-devflow bot commented Jun 11, 2024

dd-devflow bot commented Jun 11, 2024

BenoitZugmeyer Jun 11, 2024

⚗️[RUM-4780] Remote configuration #2799

⚗️[RUM-4780] Remote configuration #2799

Conversation

amortemousque commented Jun 7, 2024 • edited Loading

Motivation

Changes

How to test

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cit-pr-commenter bot commented Jun 10, 2024 • edited Loading

Bundles Sizes Evolution

Choose a reason for hiding this comment

codecov-commenter commented Jun 10, 2024 • edited Loading

Codecov Report

amortemousque commented Jun 11, 2024

dd-devflow bot commented Jun 11, 2024

dd-devflow bot commented Jun 11, 2024

Choose a reason for hiding this comment

amortemousque commented Jun 7, 2024 •

edited

Loading

cit-pr-commenter bot commented Jun 10, 2024 •

edited

Loading

codecov-commenter commented Jun 10, 2024 •

edited

Loading