✨⚗️ [RUM-4469] introduce a plugin system #2809

BenoitZugmeyer · 2024-06-13T16:19:36Z

Motivation

Experiment with a plugin system to provide custom integrations.

This is experimental: the plugin interface might change in any version, be warned!

Changes

Testing

Local
Staging
Unit
End to end

I have gone over the contributing documentation.

codecov-commenter · 2024-06-13T16:22:01Z

Codecov Report

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

Project coverage is 93.66%. Comparing base (9b21a36) to head (5c5e461).
Report is 1 commits behind head on main.

Files	Patch %	Lines
packages/rum-core/src/domain/plugins.ts	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2809      +/-   ##
==========================================
- Coverage   93.66%   93.66%   -0.01%     
==========================================
  Files         244      245       +1     
  Lines        7168     7181      +13     
  Branches     1605     1611       +6     
==========================================
+ Hits         6714     6726      +12     
- Misses        454      455       +1

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

cit-pr-commenter · 2024-06-14T08:23:48Z

Bundles Sizes Evolution

📦 Bundle Name	Base Size	Local Size	𝚫	𝚫%	Status
Rum	160.07 KiB	160.44 KiB	379 B	+0.23%	✅
Logs	58.07 KiB	58.09 KiB	20 B	+0.03%	✅
Rum Slim	108.65 KiB	109.02 KiB	379 B	+0.34%	✅
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.000
addaction	0.035	0.038	0.002
adderror	0.038	0.033	-0.006
addtiming	0.001	0.001	-0.000
startview	1.354	1.069	-0.285
startstopsessionreplayrecording	0.989	1.106	0.117
logmessage	0.020	0.023	0.003

🧠 Memory Performance

Action Name	Base Consumption Memory (bytes)	Local Consumption Memory (bytes)	𝚫 (bytes)
addglobalcontext	20.64 KiB	18.83 KiB	-1855 B
addaction	69.28 KiB	69.23 KiB	-50 B
adderror	84.99 KiB	84.92 KiB	-64 B
addtiming	17.99 KiB	15.84 KiB	-2201 B
startview	314.13 KiB	320.54 KiB	6.41 KiB
startstopsessionreplayrecording	13.31 KiB	13.57 KiB	262 B
logmessage	69.38 KiB	67.67 KiB	-1748 B

🔗 RealWorld

thomas-lebeau · 2024-06-14T08:58:04Z

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

+  if (!plugins) {
+    return
+  }


💭 thought: ‏ What about also checking for a new experimental feature like plugins? That would be a clear sign for customers that this is not stable, especially in the case a 3rd party releases a plugin, they would have to instruct people to add enableExperimentalFeatures: ['plugins'] to their init config.

Sounds good, let's do it for now.

thomas-lebeau · 2024-06-14T09:04:20Z

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

@@ -296,6 +305,7 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration) {
      track_user_interactions: configuration.trackUserInteractions,
      track_resources: configuration.trackResources,
      track_long_task: configuration.trackLongTasks,
+      plugins: configuration.plugins?.map((plugin) => assign({ name: plugin.name }, plugin.serializeConfiguration?.())),


💭 thought: ‏ What about doing the serialization within rum instead of the plugin? This way we can control how the serialization is done and might avoid duplicating serialization logic across multiple plugins

Hum, 'serialization' might not be the best term for it. We don't serialize anything per-se, we transform the configuration before sending it as telemetry event. We need to ensure that we don't leak PII (ex: we avoid sending URLs specified in the configuration, ex: configuration.proxy = 'https://...' becomes use_proxy: true). It's not possible to define a generic way to do it.

Maybe we could rename serializeConfiguration() to something like getConfigurationTelemetry()?

oh, I didn't understood it was for telemetry. getConfigurationTelemetry sounds good, even though it's a bit weird to expose this to public plugins, but I guess that's fine for now and we can revisit if needed when we go GA with plugins

it's a bit weird to expose this to public plugins

There is no plan to support public plugins any time soon though. We'll go GA with the react plugin, but this doesn't imply supporting third-party plugins.

amortemousque · 2024-06-14T09:10:59Z

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

+
+export interface RumPlugin {
+  name: string
+  serializeConfiguration?(): Record<string, unknown>


❓ question: ‏What is the serializeConfiguration meant for? Is it only to be passed in the telemetry config?

Yes, but I agree the name isn't great, see #2809 (comment)

amortemousque · 2024-06-14T09:27:39Z

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

@@ -140,6 +141,8 @@ export function createPreStartStrategy(
        return
      }

+      callHook(initConfiguration.plugins, 'onInit', { initConfiguration, publicApi })


💬 suggestion: ‏To anticipate a bit future hooks, it could be nice to have

Suggested change

callHook(initConfiguration.plugins, 'onInit', { initConfiguration, publicApi })

registerPlugins(initConfiguration.plugins) // At some point we could have some checks like ensuring no duplicated plugins

callHook('onInit', { initConfiguration, publicApi })

What registerPlugins would do? I don't want to anticipate too much, because we don't know what we'll need in future plugins :) even callHook is a bit too much. Maybe I could remove this 'hook' concept and just move the code here:

initConfiguration.plugins?.forEach(plugin => plugin?.onInit({ initConfiguration, publicApi }))

Make sense, let's keep it like this for now :)

so we don't need to introduce a new concept

nulrich · 2024-06-14T10:19:25Z

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

+import type { RumInitConfiguration } from './configuration'
+
+export interface RumPlugin {
+  name: string


💭 thought: ‏Maybe adding the plugin version, for telemetry and maybe checking compatibility with the SDK.

For now installing a plugin with a different version than the SDK won't be supported. Plugins and SDK need to be updated at the same time.

) into staging-25 pm_trace_id: 37003110 feature_branch_pipeline_id: 37003110 source: to-staging * commit '50503639a46a42efa486a4f72a30b9fc76e12ab5': fix type Add files parameter in karma local Fix event format 👷Skip safari and ie for extension unit tests ✨⚗️ [RUM-4469] introduce a plugin system (#2809)

✨⚗️ [RUM-4469] introduce a plugin system

8a1d0d9

🏷️ update rum-events-format

08465c3

BenoitZugmeyer force-pushed the benoit/react-plugin--1-plugin-concept branch from d61e27d to 08465c3 Compare June 13, 2024 16:34

BenoitZugmeyer marked this pull request as ready for review June 13, 2024 16:37

BenoitZugmeyer requested a review from a team as a code owner June 13, 2024 16:37

📝 add plugin comment warning

8065f7a

thomas-lebeau reviewed Jun 14, 2024

View reviewed changes

amortemousque reviewed Jun 14, 2024

View reviewed changes

amortemousque approved these changes Jun 14, 2024

View reviewed changes

BenoitZugmeyer added 3 commits June 14, 2024 12:09

put plugins behind a feature flag

e48a05b

rename serializeConfiguration to getConfigurationTelemetry

3b383e0

rename hook to method

5c5e461

so we don't need to introduce a new concept

BenoitZugmeyer force-pushed the benoit/react-plugin--1-plugin-concept branch from 3b14f17 to 5c5e461 Compare June 14, 2024 10:16

nulrich reviewed Jun 14, 2024

View reviewed changes

thomas-lebeau approved these changes Jun 14, 2024

View reviewed changes

nulrich approved these changes Jun 14, 2024

View reviewed changes

N-Boutaib approved these changes Jun 14, 2024

View reviewed changes

BenoitZugmeyer merged commit c1a4aa2 into main Jun 17, 2024
20 checks passed

BenoitZugmeyer deleted the benoit/react-plugin--1-plugin-concept branch June 17, 2024 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨⚗️ [RUM-4469] introduce a plugin system #2809

✨⚗️ [RUM-4469] introduce a plugin system #2809

BenoitZugmeyer commented Jun 13, 2024

codecov-commenter commented Jun 13, 2024 •

edited

Loading

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

edited

Loading

thomas-lebeau Jun 14, 2024

BenoitZugmeyer Jun 14, 2024

thomas-lebeau Jun 14, 2024

BenoitZugmeyer Jun 14, 2024

thomas-lebeau Jun 14, 2024

BenoitZugmeyer Jun 14, 2024

amortemousque Jun 14, 2024

BenoitZugmeyer Jun 14, 2024

amortemousque Jun 14, 2024 •

edited

Loading

BenoitZugmeyer Jun 14, 2024 •

edited

Loading

amortemousque Jun 14, 2024 •

edited

Loading

nulrich Jun 14, 2024

BenoitZugmeyer Jun 14, 2024

	callHook(initConfiguration.plugins, 'onInit', { initConfiguration, publicApi })
	registerPlugins(initConfiguration.plugins) // At some point we could have some checks like ensuring no duplicated plugins
	callHook('onInit', { initConfiguration, publicApi })

✨⚗️ [RUM-4469] introduce a plugin system #2809

✨⚗️ [RUM-4469] introduce a plugin system #2809

Conversation

BenoitZugmeyer commented Jun 13, 2024

Motivation

Changes

Testing

codecov-commenter commented Jun 13, 2024 • edited Loading

Codecov Report

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

Bundles Sizes Evolution

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

Choose a reason for hiding this comment

amortemousque Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

BenoitZugmeyer Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

amortemousque Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 13, 2024 •

edited

Loading

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

edited

Loading

amortemousque Jun 14, 2024 •

edited

Loading

BenoitZugmeyer Jun 14, 2024 •

edited

Loading

amortemousque Jun 14, 2024 •

edited

Loading