Skip to content

feat(sdk-node): wire up id_generator from declarative config#6782

Merged
trentm merged 12 commits into
open-telemetry:mainfrom
honeycombio:mike/idgenerator-config
Jun 23, 2026
Merged

feat(sdk-node): wire up id_generator from declarative config#6782
trentm merged 12 commits into
open-telemetry:mainfrom
honeycombio:mike/idgenerator-config

Conversation

@MikeGoldsmith

@MikeGoldsmith MikeGoldsmith commented Jun 3, 2026

Copy link
Copy Markdown
Member

Which problem is this PR solving?

Wire up id_generator from the declarative config to the tracer provider. The config schema added id_generator support in v1.1.0 (open-telemetry/opentelemetry-configuration#624).

Closes #6616.

Stacked on top of #6781

This PR depends on #6781 which bumps the config schema pin from v1.0.0 to v1.1.0 (where id_generator lives). Please review and merge #6781 first so this PR's diff is clean.

Short description of the changes

  • Add getIdGeneratorFromConfiguration helper in sdk-node that maps the config model's tracer_provider.id_generator to an SDK IdGenerator instance
  • Currently only random is supported per the spec; unknown types warn and fall back to the SDK default (RandomIdGenerator)
  • Export IdGenerator as IdGeneratorConfigModel from the configuration package so downstream consumers can reference the type
  • Pass the resolved idGenerator to BasicTracerProvider

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • 82 configuration package tests pass
  • 197 sdk-node tests pass
  • Full lint passes

Bump opentelemetry-configuration schema from v1.0.0 to a SHA on main
that includes upcoming v1.1.0 changes. Tracking issue for v1.1.0:
open-telemetry/opentelemetry-configuration#643.

Pulls in the Prometheus exporter property rename (open-telemetry#612 upstream):
- `without_scope_info` -> `scope_info_enabled` (inverted semantics)
- `without_target_info/development` -> `target_info_enabled/development` (inverted semantics)
- `with_resource_constant_labels` -> `resource_constant_labels`

Updates EnvironmentConfigFactory hardcoded defaults and all test
fixtures/assertions to match the new schema. Regenerated types and
validator.

The schema pin should be reverted to a tagged release (v1.1.0) once
upstream cuts the release.

Assisted-by: Claude Opus 4.6
@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.53%. Comparing base (248759a) to head (b35e108).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6782   +/-   ##
=======================================
  Coverage   95.52%   95.53%           
=======================================
  Files         384      384           
  Lines       12974    12986   +12     
  Branches     2976     2979    +3     
=======================================
+ Hits        12394    12406   +12     
  Misses        580      580           
Files with missing lines Coverage Δ
...ental/packages/opentelemetry-sdk-node/src/start.ts 97.02% <100.00%> (+0.02%) ⬆️
...ental/packages/opentelemetry-sdk-node/src/utils.ts 96.47% <100.00%> (+0.06%) ⬆️
🚀 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.

@MikeGoldsmith

Copy link
Copy Markdown
Member Author

The CI failure is from a docs link check - please could someone re-run it for me?

@MikeGoldsmith
MikeGoldsmith marked this pull request as ready for review June 4, 2026 14:57
@MikeGoldsmith
MikeGoldsmith requested a review from a team as a code owner June 4, 2026 14:57
@MikeGoldsmith
MikeGoldsmith marked this pull request as draft June 5, 2026 11:03
v1.1.0 of opentelemetry-configuration was released on 2026-06-05,
which includes the IdGenerator definition and the Prometheus property
rename. Switch the codegen pin from the temporary main SHA back to a
tagged release.

Regenerated types and validator.

Assisted-by: Claude Opus 4.6
@MikeGoldsmith
MikeGoldsmith force-pushed the mike/idgenerator-config branch from 8ac0cbd to 8d94e03 Compare June 5, 2026 16:48
…omments

Trent's review feedback on open-telemetry#6781: the eslint config v10 update
(open-telemetry#6382) accidentally added the generated config files to the ignore
list, so npm run lint:fix in the codegen no longer ran on them. This
produced formatting noise in generated types.ts.

- Remove the experimental/packages/configuration/src/generated/**
  ignore from eslint.config.mjs so lint:fix runs on these files
- Drop the now-redundant /* eslint-disable */ header in validator.js
  and validator.d.ts (lint:fix handles formatting; the AUTO-GENERATED
  comment signals intent)
- Regenerate types and validator with the corrected lint

Assisted-by: Claude Opus 4.6
Add getIdGeneratorFromConfiguration helper in sdk-node that maps
the config model's tracer_provider.id_generator to an SDK IdGenerator
instance. Currently only `random` is supported per the spec; unknown
types warn and fall back to the SDK default (RandomIdGenerator).

Export IdGenerator as IdGeneratorConfigModel from the configuration
package so downstream consumers can reference the type.

Based on mike/bump-config-schema-prometheus-rename which bumps the
schema to a SHA on main that includes IdGenerator (open-telemetry#624 upstream).

Closes open-telemetry#6616

Assisted-by: Claude Opus 4.6
Cover the four branches: no tracer_provider, no id_generator, random
id_generator, and unsupported (warn + undefined).

Assisted-by: Claude Opus 4.6
@MikeGoldsmith
MikeGoldsmith force-pushed the mike/idgenerator-config branch from 8d94e03 to f87db0f Compare June 8, 2026 10:42
MikeGoldsmith and others added 2 commits June 18, 2026 14:05
…nfig

# Conflicts:
#	experimental/CHANGELOG.md
#	experimental/packages/opentelemetry-sdk-node/src/start.ts

@trentm trentm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Still draft, because #6781 just merged.
However, after merging from main, this LGTM (modulo the one point on warn-and-default vs fail-fast below).

@MikeGoldsmith I'll let you respond and pull this out of draft.

Comment on lines +973 to +974
// Any other key is a third-party / custom id_generator type which we
// don't currently support. Warn and fall back to SDK default.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool for now.

However, I think we should discuss and agree on behaviour for unknown keys.
My thoughts here: #6107 (comment)

Note (for comparison), in #6785 I made Create handling throw on unrecognized keys, e.g. https://github.com/open-telemetry/opentelemetry-js/pull/6785/changes#diff-a6c0e622127e4d2eef8ea3011fbea3abc886a8d8e4adec2a765f47127e739180R731

…nfig

# Conflicts:
#	experimental/CHANGELOG.md
…to mike/idgenerator-config

# Conflicts:
#	experimental/CHANGELOG.md
@trentm
trentm enabled auto-merge June 19, 2026 21:24
@trentm
trentm disabled auto-merge June 19, 2026 21:24
@trentm

trentm commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

@MikeGoldsmith Please apply this patch to your PR to fix the build. In my attempt to merge from main in the GitHub UI I fixed some conflicts, but accidentally left this function in (it was moved to createSpanLimitsFromConfig in an earlier PR):

diff --git a/experimental/packages/opentelemetry-sdk-node/src/utils.ts b/experimental/packages/opentelemetry-sdk-node/src/utils.ts
index 06ad90e7b..5363eadd8 100644
--- a/experimental/packages/opentelemetry-sdk-node/src/utils.ts
+++ b/experimental/packages/opentelemetry-sdk-node/src/utils.ts
@@ -982,30 +982,6 @@ export function getIdGeneratorFromConfiguration(
   return undefined;
 }

-export function getSpanLimitsFromConfiguration(
-  config: ConfigurationModel
-): SpanLimits | undefined {
-  if (config.tracer_provider?.limits) {
-    const limitsConfig = config.tracer_provider.limits;
-    const spanLimits: SpanLimits = {};
-    spanLimits.attributeCountLimit = limitsConfig.attribute_count_limit ?? 128;
-    spanLimits.eventCountLimit = limitsConfig.event_count_limit ?? 128;
-    spanLimits.linkCountLimit = limitsConfig.link_count_limit ?? 128;
-    spanLimits.attributePerLinkCountLimit =
-      limitsConfig.link_attribute_count_limit ?? 128;
-    spanLimits.attributePerEventCountLimit =
-      limitsConfig.event_attribute_count_limit ?? 128;
-
-    if (limitsConfig.attribute_value_length_limit != null) {
-      spanLimits.attributeValueLengthLimit =
-        limitsConfig.attribute_value_length_limit;
-    }
-
-    return spanLimits;
-  }
-  return undefined;
-}
-
 export function getMeterReadersFromConfiguration(
   config: ConfigurationModel
 ): IMetricReader[] | undefined {

Once that is in, we can merge.

…nfig

# Conflicts:
#	experimental/packages/opentelemetry-sdk-node/src/start.ts
#	experimental/packages/opentelemetry-sdk-node/src/utils.ts
#	experimental/packages/opentelemetry-sdk-node/test/utils.test.ts
…to mike/idgenerator-config

# Conflicts:
#	experimental/packages/opentelemetry-sdk-node/src/utils.ts
@trentm
trentm added this pull request to the merge queue Jun 23, 2026
Merged via the queue into open-telemetry:main with commit d273b03 Jun 23, 2026
29 of 30 checks passed
@otelbot-js otelbot-js Bot mentioned this pull request Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

[configuration] Pass IdGenerator

3 participants