Skip to content

[CHORE] [V3] Refactor Initialization APIs #1089

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.

Sign up for GitHub

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

Jump to bottom
Open
marco-saia-datadog wants to merge 20 commits into
base: feature/v3
Choose a base branch
from
Open

[CHORE] [V3] Refactor Initialization APIs #1089

marco-saia-datadog wants to merge 20 commits into from

Conversation

@marco-saia-datadog
Copy link
Member

@marco-saia-datadog marco-saia-datadog commented Dec 19, 2025 •
edited
Loading

What does this PR do?

  • Refactored configurations into separate modules

    • Split configuration logic into multiple files and a nested folder structure.
    • This makes it easier to navigate between different supported SDK initialization types.

  • Support for initializing individual features with extra options

    • Feature configurations (core, RUM, logs, trace) now accept all required options as positional constructor arguments.
    • All optional configuration values are passed as the final positional argument as an object, allowing easier extensibility and clearer APIs.

  • Added missing properties in FileBasedConfiguration and AutoInstrumentationConfiguration

    • Fixed multiple configuration properties that were not being picked up from non-standard configs.
    • Updated tests to cover all supported configuration options and ensure correct mapping.

  • Implemented missing JSON converters in FileBasedConfiguration

    • Fixed cases where non-primitive values (e.g. enums) were assigned as raw values, which could lead to runtime issues.
    • All non-primitive types are now correctly parsed from the JSON configuration file.

  • Added missing tests and adjusted behavior

    • Introduced additional tests to cover edge cases and updated existing ones to reflect the new configuration behavior.

  • Removed unrelated cyclic dependency in network-instrumentation layer

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)
  • If this PR is auto-generated, please make sure also to manually update the code related to the change

sbarrio and others added 8 commits December 18, 2025 23:36
@sbarrio
Move resourceTraceSampleRate, nativeLongTaskThreshold, nativeCrashRep…
e053a7d 
…ortingEnabled and useAccesibilityLabel to rumConfiguration
@sbarrio
Fix iOS tests
27a2cdc
@sbarrio
Fix Android tests
7c79ef3
@sbarrio
Expose optional Rum, Logs and Trace configuration objects on DatadogP…
6c79299 
…roviderConfiguration
@sbarrio
Missing changes to sdk init on examples/benchmarks
7bea519
@sbarrio
Configuration constructor now retains default values
2d784e0
@sbarrio
Update snapshots
4c47f03
@marco-saia-datadog
Refactored Initialization APIs and configs
b0f5d5d 
* Refactored configurations and split them in separate modules
* Allow initialization of individual features with extra options
* Added missing properties in FileBasedConfiguration
* Implemented missing JSON converters in FileBasedConfiguration
* Fixed missing properties in various setups
* Added missing tests and adjusted behavior
sbarrio
sbarrio reviewed Jan 2, 2026
View reviewed changes
sbarrio
sbarrio reviewed Jan 2, 2026
View reviewed changes
@sbarrio sbarrio mentioned this pull request Jan 5, 2026
Closed
4 tasks
@sbarrio sbarrio changed the base branch from sbarrio/v3/refactor-configuration-properties to feature/v3 January 5, 2026 09:51
@sbarrio sbarrio changed the base branch from feature/v3 to sbarrio/chore/update-v3-with-latest-from-develop January 7, 2026 11:41
Base automatically changed from sbarrio/chore/update-v3-with-latest-from-develop to feature/v3 January 8, 2026 09:09
@sbarrio sbarrio marked this pull request as ready for review January 8, 2026 14:26
@sbarrio sbarrio requested a review from a team as a code owner January 8, 2026 14:26
@sbarrio sbarrio self-requested a review January 8, 2026 14:31
@sbarrio sbarrio self-assigned this Jan 9, 2026
cdn34dd
cdn34dd reviewed Jan 15, 2026
View reviewed changes
packages/core/src/config/FileBasedConfiguration.ts
...rest
} = jsonConfiguration;

// TODO: Handle undefined clientToken, env or trackingConsent
Copy link
Contributor

@cdn34dd cdn34dd Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Should we handle this before merging ? Or is this something that can go afterwards ? If not we should at create a JIRA ticket for this and add the ticket id here.

cdn34dd
cdn34dd reviewed Jan 15, 2026
View reviewed changes
packages/core/src/config/FileBasedConfiguration.ts
this.rumConfiguration.actionEventMapper =
params?.actionEventMapper ?? RUM_DEFAULTS.actionEventMapper;
} else {
console.warn(
Copy link
Contributor

@cdn34dd cdn34dd Jan 15, 2026

Choose a reason for hiding this comment

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

This seems like it could go unnoticed quite easily, should we not throw an error here instead ?

cdn34dd
cdn34dd reviewed Jan 15, 2026
View reviewed changes
packages/core/src/DdSdkReactNative.tsx
Comment on lines +435 to +440
const nativeLongTaskThresholdMs =
configuration.rumConfiguration?.nativeLongTaskThresholdMs ||
false;
rumConfiguration.nativeLongTaskThresholdMs = adaptLongTaskThreshold(
nativeLongTaskThresholdMs
);
Copy link
Contributor

@cdn34dd cdn34dd Jan 15, 2026

Choose a reason for hiding this comment

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

Why are we defaulting to false here ?

const nativeLongTaskThresholdMs =
                configuration.rumConfiguration?.nativeLongTaskThresholdMs ||
                false;

Seems this should be a number only property from the get go, because on the line after we call this function: adaptLongTaskThreshold, which puts the value to 0 if nativeLongTaskThresholdMs is falsy.

Copy link
Contributor

@cdn34dd cdn34dd Jan 15, 2026

Choose a reason for hiding this comment

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

Also seems that RUM_DEFAULTS has:

{
...
nativeLongTaskThresholdMs: 200,
...
}

So why are we defaulting to false which then gets translated to 0, if the default is 200 ?

cdn34dd
cdn34dd approved these changes Jan 15, 2026
View reviewed changes
Copy link
Contributor

@cdn34dd cdn34dd left a comment

Choose a reason for hiding this comment

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

Overall this looks good, just left a couple of comments regarding some implementation details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdn34dd cdn34dd cdn34dd approved these changes

@sbarrio sbarrio Awaiting requested review from sbarrio

Assignees

@sbarrio sbarrio

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@marco-saia-datadog @sbarrio @cdn34dd