Skip to content

fix: apply spec-defined BLRP schedule_delay default in sdk-node, default log_level in env config#6788

Merged
pichlermarc merged 2 commits into
open-telemetry:mainfrom
honeycombio:mike/sdk-node-blrp-default-and-env-log-level
Jun 15, 2026
Merged

fix: apply spec-defined BLRP schedule_delay default in sdk-node, default log_level in env config#6788
pichlermarc merged 2 commits into
open-telemetry:mainfrom
honeycombio:mike/sdk-node-blrp-default-and-env-log-level

Conversation

@MikeGoldsmith

Copy link
Copy Markdown
Member

Which problem is this PR solving?

Two small fixes for spec-conformance gaps, split out from #6717 (now closed) per Trent's preferred direction in #6765: defaults should be applied at the consumer boundary (sdk-node), not in the parse step of the configuration package.

1. BatchLogRecordProcessor schedule_delay

The OTel config spec defines schedule_delay for BatchLogRecordProcessor as 1000ms. The JS SDK's BatchLogRecordProcessorBase defaults it to 5000ms (same as BSP), which is an SDK-level divergence from spec. Apply the correct 1000ms default at the consumer boundary in getLogRecordProcessorsFromConfiguration.

2. log_level default for env config

FileConfigFactory.parseConfigFile() produces a model with log_level: 'info' when the field is absent from the YAML. EnvironmentConfigFactory did not have a matching default, so env-based configs produced models without log_level set unless OTEL_LOG_LEVEL was provided. Adding the default in initializeDefaultConfiguration() makes env and file config consistent.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

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

…og_level in env config

Two small fixes split out from open-telemetry#6717:

1. BatchLogRecordProcessor: apply spec-defined schedule_delay default
   of 1000ms when building from declarative config. The SDK's
   BatchLogRecordProcessorBase defaults to 5000ms (same as BSP), but
   the OTel config spec says BLRP schedule_delay defaults to 1000ms.
   Apply the correct default at the consumer boundary in sdk-node.

2. initializeDefaultConfiguration: default log_level to 'info' so the
   env-based config has the same default as file-based config (where
   the spec-defined default of INFO is applied in the same place).

Assisted-by: Claude Opus 4.6
@MikeGoldsmith
MikeGoldsmith requested a review from a team as a code owner June 5, 2026 13:52
@codecov

codecov Bot commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.92%. Comparing base (9532e64) to head (eab1ebc).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6788   +/-   ##
=======================================
  Coverage   94.92%   94.92%           
=======================================
  Files         377      377           
  Lines       12832    12832           
  Branches     2928     2928           
=======================================
  Hits        12181    12181           
  Misses        651      651           
Files with missing lines Coverage Δ
experimental/packages/configuration/src/utils.ts 98.03% <ø> (ø)
...ental/packages/opentelemetry-sdk-node/src/utils.ts 97.01% <100.00%> (ø)
🚀 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.

@pichlermarc
pichlermarc added this pull request to the merge queue Jun 12, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 12, 2026
@pichlermarc
pichlermarc added this pull request to the merge queue Jun 15, 2026
Merged via the queue into open-telemetry:main with commit 55606c3 Jun 15, 2026
31 checks passed
@trentm

trentm commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

The JS SDK's BatchLogRecordProcessorBase defaults it to 5000ms (same as BSP), which is an SDK-level divergence from spec. Apply the correct 1000ms default at the consumer boundary in getLogRecordProcessorsFromConfiguration.

I think we should consider it a bug in sdk-logs that it defaults BatchLogRecordProcessor's scheduleDelayMillis to 5000, rather than 1000. The spec says it should default to 1000. (open-telemetry/opentelemetry-specification#3002 includes a comment with a discussion on why this default differs from the equivalent for BatchSpanProcessor).

So if we update the default value in sdk-logs, then we no longer need a separate application of a default value in the configuration handling in sdk-node.

Update: I've opened #6796 for this.

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