Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2

### :boom: Breaking Changes

* fix(configuration)!: stop removing `null` values from parsed config object [#6679](https://github.com/open-telemetry/opentelemetry-js/pull/6679) @trentm
* It is now the responsibility of the user of a parsed declarative config object, typically just the `sdk-node` package, to handle `null` values.

### :rocket: Features

### :bug: Bug Fixes
Expand Down
55 changes: 5 additions & 50 deletions experimental/packages/configuration/scripts/generate-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,51 +218,6 @@ function rtrimDescription(node) {
}
rtrimDescription(schema);

// Avoid unnecessary `| null` in TypeScript types.
//
// opentelemetry-configuration intentionally adds an optional "null" type to
// most fields to express optional/nullable values, e.g.:
// "ConsoleMetricExporter": {
// "type": [
// "object",
// "null"
// ],
// See explanation at: https://github.com/open-telemetry/opentelemetry-configuration/blob/main/CONTRIBUTING.md#required-and-null-properties
//
// In TypeScript, `?` expresses a property being optional, e.g.:
// endpoint?: string;
// Leaving "null" in the JSON Schema results in TypeScript like this:
// endpoint?: string | null;
//
// That null option has a downstream blast radius. E.g. a downstream API that
// takes that `endpoint` value, now needs to handle `null`.
//
// By preprocessing the schema to remove those we get nicer typescript types.
function stripNullTypeFallback(obj) {
if (Array.isArray(obj.type)) {
if (obj.type.length === 2 && obj.type[1] === 'null') {
obj.type.pop();
}
} else if (Array.isArray(obj.oneOf)) {
// Handle the more complex case of AttributeNameValue#value, which uses:
// "value": { "oneOf": [{"type":"string"}, {"type":"null"}, ...] }
if (obj.oneOf.length > 1) {
obj.oneOf = obj.oneOf.filter(entry => {
return !(entry?.type === 'null' && Object.keys(entry).length === 1);
});
}
}
if (typeof obj.properties === 'object' && obj.properties != null) {
for (const prop of Object.values(obj.properties)) {
stripNullTypeFallback(prop);
}
}
}
stripNullTypeFallback(schema);
for (const def of Object.values(schema.$defs)) {
stripNullTypeFallback(def);
}

compile(schema, 'OpenTelemetryConfiguration', {
bannerComment,
unknownAny: false,
Expand All @@ -288,7 +243,7 @@ compile(schema, 'OpenTelemetryConfiguration', {
// of the codebase and other OTel SDKs.
ts = ts.replace(/\bOpenTelemetryConfiguration\b/g, 'ConfigurationModel');

// Change the TypeScript representation for interfaces where
// Refine the TypeScript representation for interfaces where
// "additionalProperties" is allowed.
//
// The configuration JSON schema uses the following for types that have
Expand All @@ -301,17 +256,17 @@ compile(schema, 'OpenTelemetryConfiguration', {
// [k: string]: {} | null
//
// However, we want:
// [k: string]: object | undefined
// [k: string]: object | null | undefined
//
// - `object` instead of `{}`, because JSON schema "object" means a thing
// with keys and values (https://json-schema.org/understanding-json-schema/reference/object)
// and TypeScript "object" means non-Primitive values (https://stackoverflow.com/a/49465172)
// which is closest. `{}` allows too much (e.g. 42 matches `{}`).
// - `undefined` rather than `null` because we want to express that the
// property can be unspecified.
// - `null` to allow an empty value in the YAML
// - `undefined` to allow the property to not be specified in the YAML
ts = ts.replace(
/\[k: string\]: \{\} \| null;/g,
'[k: string]: object | undefined;'
'[k: string]: object | null | undefined;'
);
// Similarly for schema types with the following (e.g. `Distribution`):
// "additionalProperties": {
Expand Down
118 changes: 0 additions & 118 deletions experimental/packages/configuration/src/FileConfigFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ export function parseConfigFile(): ConfigurationModel {
);
}

// Normalize YAML null type-tag values to {} before validation
normalizeYamlNulls(processed);

const valid = validateConfig(processed);
if (!valid) {
let detail: string;
Expand All @@ -76,14 +73,6 @@ export function parseConfigFile(): ConfigurationModel {
// Strip file_format from output — it's a meta-field, not a config value
delete (data as Record<string, unknown>)['file_format'];

// The generated TypeScript types omit `| null` from unions (see
// generate-config.js) because consumers expect `T | undefined`, not
// `T | null`. To match, we delete any properties still set to `null`
// after YAML parsing so that accessing them returns `undefined`.
// Runs after normalizeYamlNulls, which has already converted type-tag
// nulls (e.g. `console:`) to `{}`.
stripNulls(processed);

applyConfigDefaults(data);
mergeAttributesList(data);
mergeCompositeList(data);
Expand Down Expand Up @@ -237,113 +226,6 @@ function applyConfigDefaults(data: ConfigurationModel): void {
}
}

/**
* Recursively delete object properties whose value is `null`.
*
* YAML `key:` (no value) parses to `{ key: null }`. The generated TypeScript
* types use `?: T` (not `T | null`) because downstream consumers like sdk-node
* assign config values to interfaces that only accept `T | undefined`. The
* codegen script (generate-config.js) strips `| null` from type unions; this
* function makes the runtime data match by deleting null-valued keys so that
* property access returns `undefined` instead of `null`.
*
* Must run AFTER normalizeYamlNulls, which converts type-tag nulls
* (e.g. `console: null` → `console: {}`) — those are already `{}` by the
* time this function runs and won't be affected.
*/
function stripNulls(value: unknown): void {
if (value == null || typeof value !== 'object') return;
if (Array.isArray(value)) {
for (const item of value) {
stripNulls(item);
}
} else {
for (const [key, val] of Object.entries(value as Record<string, unknown>)) {
if (val === null) {
delete (value as Record<string, unknown>)[key];
} else {
stripNulls(val);
}
}
}
}

/**
* Object type-tag keys that may appear as nested (non-array-element) properties.
* YAML `console:` (no value) parses to `{ console: null }`, but the SDK checks
* `if (exporter.console)` — truthy — so null must become {}.
*
* We exclude keys that are also used as nullable primitives in other contexts
* (e.g. `host` is a string field in prometheus config; detectors use it as a
* type-tag but only as array elements, which are already handled below).
*/
const NESTED_OBJECT_TYPE_TAGS = new Set([
// Exporter type discriminators (nested inside simple/batch processor or reader)
'console',
'otlp_http',
'otlp_grpc',
// Sampler type discriminators (nested inside tracer_provider.sampler)
'always_on',
'always_off',
'trace_id_ratio_based',
'parent_based',
'jaeger_remote',
// Propagator type discriminators (array elements, but covered here for completeness)
'tracecontext',
'baggage',
'b3',
'b3multi',
'xray',
// Telemetry producer type discriminators
'opencensus',
]);

/**
* Normalizes YAML null values to empty objects for discriminated-union type-tag fields.
*
* YAML `key:` (no value) parses to `{ key: null }`, but the SDK checks `if (obj.key)`
* to determine which type to instantiate — null fails that check. Two cases:
*
* 1. Array element properties: convert ALL null-valued keys to {} (handles detectors,
* composite propagators, and most exporter/processor discriminators).
* 2. Nested NESTED_OBJECT_TYPE_TAGS: convert known type-discriminator keys to {} when
* encountered anywhere in the tree (handles sampler types, console exporter nested
* inside simple/batch, etc.).
*
* Top-level nullable primitive fields (disabled, log_level, endpoint, ca_file, etc.)
* are NOT in NESTED_OBJECT_TYPE_TAGS and are left as-is; applyConfigDefaults()
* handles the boolean/string ones that need defaults.
*/
function normalizeYamlNulls(value: unknown): void {
if (value == null || typeof value !== 'object') return;
if (Array.isArray(value)) {
for (const item of value) {
if (item != null && typeof item === 'object' && !Array.isArray(item)) {
// All null-valued properties of array elements are type-tag discriminators.
for (const key of Object.keys(item as object)) {
if ((item as Record<string, unknown>)[key] === null) {
(item as Record<string, unknown>)[key] = {};
}
}
}
normalizeYamlNulls(item);
}
} else {
for (const [key, val] of Object.entries(value as Record<string, unknown>)) {
if (
val === null &&
// Known type-tag keys, or slash-qualified experimental type-tags
// (e.g. otlp_file/development, prometheus/development)
(NESTED_OBJECT_TYPE_TAGS.has(key) || key.includes('/'))
) {
(value as Record<string, unknown>)[key] = {};
} else {
normalizeYamlNulls(val);
}
}
}
}

const ENV_VAR_PATTERN = /\$\{[^}]+\}/;

function substituteEnvVars(obj: unknown): unknown {
Expand Down
Loading
Loading