Skip to content

Conversation

@YangJonghun
Copy link
Contributor

@YangJonghun YangJonghun commented Jul 19, 2025

Which problem is this PR solving?

  • fixes incorrect configuration priority where environment variables were overriding programmatic configuration
  • aligns with OpenTelemetry JS SDK convention: programmatic config > environment variables
  • resolves issue where users cannot override environment variables with explicit code configuration
  • enables users to programmatically enable default-excluded instrumentations (like fs) regardless of environment variable settings

Short description of the changes

  • Establish correct configuration precedence: programmatic config > environment variables > defaults
  • Fix priority order: userConfig.enabled is now checked first, before any environment variable logic
  • Enable flexible overrides: users can now override any environment variable setting with explicit programmatic configuration
  • Maintain consistency: follows the same pattern as other OpenTelemetry JS SDK packages

Configuration Priority (after changes):

  1. Programmatic config (userConfig.enabled: true/false) - highest priority, always respected
  2. Environment variables (OTEL_NODE_DISABLED_INSTRUMENTATIONS, OTEL_NODE_ENABLED_INSTRUMENTATIONS) - used when programmatic config is not set
  3. Default exclusions (defaultExcludedInstrumentations) - applied when neither programmatic config nor env vars are set

@YangJonghun YangJonghun requested a review from a team as a code owner July 19, 2025 07:03
@YangJonghun YangJonghun changed the title Fix/instrumentation config priority fix(auto-instrumentations-node): enable programmatic config when environment variables are unset Jul 19, 2025
@YangJonghun YangJonghun changed the title fix(auto-instrumentations-node): enable programmatic config when environment variables are unset fix: enable programmatic config when environment variables are unset Jul 19, 2025
@dyladan dyladan added bug Something isn't working priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect and removed priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization labels Aug 6, 2025
disabledInstrumentationsFromEnv.includes(name)
) {
diag.debug(`Disabling instrumentation for ${name}`);
// Configuration priority: Environment variables > programmatic config
Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't a clear guideline of what takes precedent (env or programmatic), but in the JS SDK we have been using
programmatic > env
so to keep consistency across the SDK, I would suggest you make the changes accordingly here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maryliag
Thank you for pointing this out! I've updated the PR to follow the programmatic > env convention as you suggested. (e1e4e4b)
I'd appreciate it if you could review the changes again. Thanks!

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

Thank you for the changes!

@maryliag maryliag merged commit cf80246 into open-telemetry:main Nov 25, 2025
21 checks passed
@otelbot
Copy link
Contributor

otelbot bot commented Nov 25, 2025

Thank you for your contribution @YangJonghun! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

@YangJonghun YangJonghun deleted the fix/instrumentation-config-priority branch November 27, 2025 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working pkg:auto-instrumentations-node priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants