Skip to content

✨Standardize RUM integration in Flagging SDK #3634

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

Open
wants to merge 13 commits into
base: flagging-dev
Choose a base branch
from

Conversation

typotter
Copy link
Contributor

@typotter typotter commented Jun 13, 2025

Motivation

FFL-116

Changes

  • new flagging initialization parameters for indicating that the RUM sdk should be integrated and whether to integrate incumbant flag tracking or exposures

Test instructions

  • Integrate this build of the SDK into the sandbox app and update the initialization based on the updated README

Checklist

  • Tested locally
  • Tested on staging
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.

@typotter typotter requested review from a team as code owners June 13, 2025 19:17
@typotter typotter changed the base branch from main to flagging-dev June 13, 2025 19:18
@typotter typotter requested a review from a team as a code owner June 13, 2025 19:18
@typotter typotter requested a review from leoromanovsky June 13, 2025 19:18
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure how the submodule got out of sync...

@@ -31,6 +31,9 @@ describe('evaluate', () => {
expect(result).toEqual({
value: true,
variant: 'variation-124',
flagMetadata: {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plumbing the allocationKey through the metadata so that we have it available when tracking exposures

@typotter typotter marked this pull request as draft June 13, 2025 19:21
@typotter typotter marked this pull request as draft June 13, 2025 19:21
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2025

Codecov Report

Attention: Patch coverage is 38.46154% with 8 lines in your changes missing coverage. Please review.

Project coverage is 92.07%. Comparing base (cbafc82) to head (853379a).

Files with missing lines Patch % Lines
packages/flagging/src/openfeature/provider.ts 38.46% 8 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           flagging-dev    #3634      +/-   ##
================================================
- Coverage         92.13%   92.07%   -0.07%     
================================================
  Files               326      326              
  Lines              8189     8201      +12     
  Branches           1849     1856       +7     
================================================
+ Hits               7545     7551       +6     
- Misses              644      650       +6     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

/**
* The site to use for the Datadog API.
*/
site?: string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

site is the standard name here in the browser sdk

@typotter typotter removed request for a team June 13, 2025 19:32
@typotter typotter marked this pull request as ready for review June 13, 2025 19:33
Copy link
Contributor

@leoromanovsky leoromanovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice enhancement on the integration UX

// Initialize Datadog Browser SDK
datadogRum.init({
...
enableExperimentalFeatures: ["feature_flags"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is no longer needed, that's what @benoit mentioned.

clientToken: 'your-client-token',
rum: {
sdk: datadogRum,
ddFlaggingTracking: true, // Track feature flag evaluations in RUM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice API

Copy link

@petzel petzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the idea behind putting this into the browser-sdk as a RUM action rather than wait for the new EvP track to be available?

Are we committed to cutting users off/requiring them to upgrade once the new track is ready, or do we need to support both the custom RUM action and new EvP track until all customers are upgraded to the new track (which might be never)?

Copy link

cit-pr-commenter bot commented Jun 20, 2025

Bundles Sizes Evolution

📦 Bundle Name Base Size Local Size 𝚫 𝚫% Status
Rum 146.75 KiB 146.75 KiB 0 B 0.00%
Rum Recorder 18.02 KiB 18.02 KiB 0 B 0.00%
Rum Profiler 4.63 KiB 4.63 KiB 0 B 0.00%
Logs 51.68 KiB 51.68 KiB 0 B 0.00%
Flagging 0 B 26.65 KiB 26.65 KiB N/A%
Rum Slim 106.38 KiB 106.38 KiB 0 B 0.00%
Worker 23.59 KiB 23.59 KiB 0 B 0.00%
🚀 CPU Performance
Action Name Base Average Cpu Time (ms) Local Average Cpu Time (ms) 𝚫
addglobalcontext 0.017 0.008 -0.009
addaction 0.061 0.031 -0.030
addtiming 0.015 0.005 -0.009
adderror 0.061 0.024 -0.037
startstopsessionreplayrecording 0.002 0.001 -0.001
startview 0.011 0.008 -0.003
logmessage 0.044 0.028 -0.016
🧠 Memory Performance
Action Name Base Consumption Memory (bytes) Local Consumption Memory (bytes) 𝚫 (bytes)
addglobalcontext 26.08 KiB 29.83 KiB 3.75 KiB
addaction 52.29 KiB 56.87 KiB 4.58 KiB
addtiming 25.08 KiB 26.31 KiB 1.23 KiB
adderror 54.90 KiB 60.80 KiB 5.90 KiB
startstopsessionreplayrecording 24.60 KiB 25.78 KiB 1.18 KiB
startview 428.59 KiB 428.53 KiB -61 B
logmessage 57.25 KiB 57.78 KiB 540 B

🔗 RealWorld

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants