Source build version from shared metadata.version#326
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
eb80c25 to
a800ced
Compare
Expose the configured `liveDebugger.version` in the injected runtime bootstrap so the Browser Debugger SDK can default its runtime version from build metadata. Move the bootstrap logic into a dedicated helper and document the new behavior.
The deployed build identifier is a cross-plugin concern rather than a Live Debugger one. Move it from `liveDebugger.version` onto the shared `metadata.version` field, alongside the existing `metadata.name`. Type-validation of the shared `metadata` block is centralized in the factory's `validateOptions`, so consumers don't each re-implement it. Only `metadata.version` is validated today: a TODO in factory/validate.ts flags that `metadata.name` should also be tightened in the next major, since the root README has long documented its default as `null`, and adding the check now would break users who took that literally. The Live-Debugger-specific invariant — `metadata.version` must match `errorTracking.sourcemaps.releaseVersion` when both are configured — stays in the live-debugger validator, since that's the plugin enforcing it.
a800ced to
eeba682
Compare
Have `errorTracking.sourcemaps.releaseVersion` and `rum.sourceCodeContext.version` fall back to the shared `metadata.version` when no plugin-specific value is configured. Users who declare the build version once at the top level no longer need to repeat it under each plugin namespace. To support the fallback, `SourcemapsOptions.releaseVersion` is loosened from required to optional. The existing "is required" check still fires when neither `sourcemaps.releaseVersion` nor `metadata.version` resolves, with an updated message that mentions both options. Document `metadata.version` as a top-level option in the root README so it sits alongside `metadata.name`, and update the auto-generated configuration block template in `@dd/tools` accordingly.
yoannmoinet
left a comment
There was a problem hiding this comment.
I'm not sure I understand why the errorTracking concern is handled by the liveDebugger plugin.
I don't think these two plugins should know anything about each others.
| ``` | ||
|
|
||
| Optional. When set, use an immutable deployed browser build identifier. This value should match: | ||
| If both `metadata.version` and an explicit `errorTracking.sourcemaps.releaseVersion` are configured and disagree, this plugin surfaces the mismatch as a build error. |
There was a problem hiding this comment.
Not sure this is super relevant for the Live Debugger plugin's documentation.
| if (metadataVersion && sourcemapReleaseVersion && metadataVersion !== sourcemapReleaseVersion) { | ||
| errors.push( | ||
| `${red('version')} must match ${red('errorTracking.sourcemaps.releaseVersion')} when both Live Debugger and sourcemap upload are configured`, | ||
| `${red('metadata.version')} must match ${red('errorTracking.sourcemaps.releaseVersion')} when both Live Debugger and sourcemap upload are configured`, |
There was a problem hiding this comment.
Does it make sense to have this validation in the live debugger plugin?
I would probably see this more in the error tracking plugin instead.
| const makeConfig = (liveDebugger?: unknown, errorTracking?: unknown, metadata?: unknown): Options => | ||
| ({ liveDebugger, errorTracking, metadata }) as unknown as Options; |
There was a problem hiding this comment.
Nit: not a fan of all these unknown casting.
All the types are accessible and we have a bunch of mocks to alleviate the burden of mocking these out.
| // Build the resolved config only when `releaseVersion` actually | ||
| // resolves; otherwise an error has been recorded and the caller will | ||
| // throw before the config is read. | ||
| if (releaseVersion) { |
There was a problem hiding this comment.
Not sure this if () {} wrapping is really necessary.
It brings more confusion when reading the code than anything.
Even the comment tends to explain that it's actually unnecessary since it would throw anyways.
|
|
||
| export type LiveDebuggerOptions = { | ||
| enable?: boolean; | ||
| version?: string; |
There was a problem hiding this comment.
I believe it's acceptable to remove it from the public API since the plugin didn't actually release publicly yet, but just to confirm it's not been missed.

What and why?
This PR makes
metadata.versionthe canonical place to declare the deployed build identifier across the Datadog build plugins. Three plugins now read from it:versionduringinit()— keeping browser build lookup and source-code resolution working without extra SDK wiring.errorTracking.sourcemaps.releaseVersion) falls back to it when no plugin-specific value is set.rum.sourceCodeContext.version) falls back to it when no plugin-specific value is set.Users who declare the build version once at the top level no longer need to repeat it under each plugin namespace. The change is additive: existing per-plugin configurations keep working unchanged.
This builds on #325, which introduced build-time validation of the version but stopped short of using it at runtime.
How?
BuildMetadata(in@dd/core/types) gains an optionalversion?: string, alongside the existingname. The root README documents it as a top-level option, and the auto-generated configuration block template in@dd/toolsis updated to include it.metadata.*is centralized in the factory'svalidateOptions. Today it strictly type-checksmetadata.version. A TODO flagsmetadata.nameto be tightened in the next major — its documented default is currentlynull, so adding the check now would be a breaking change.metadata.versionmust matcherrorTracking.sourcemaps.releaseVersionwhen both are explicitly configured.runtime-bootstraphelper. The bootstrap installs the minimal$dd_probesno-op stub, and whenmetadata.versionis set it also exposes build metadata on a runtime-visible global so the Browser Debugger SDK can default itsversionfrom there duringinit().SourcemapsOptions.releaseVersionis loosened from required to optional. The existing "is required" check still fires when neithersourcemaps.releaseVersionnormetadata.versionresolves, with an updated message that mentions both options.