Skip to content

[configoptional] Add support for setting an 'enabled' field under a feature gate#13995

Merged
mx-psi merged 10 commits intoopen-telemetry:mainfrom
mx-psi:mx-psi/configoptional-enabled-impl
Oct 23, 2025
Merged

[configoptional] Add support for setting an 'enabled' field under a feature gate#13995
mx-psi merged 10 commits intoopen-telemetry:mainfrom
mx-psi:mx-psi/configoptional-enabled-impl

Conversation

@mx-psi
Copy link
Copy Markdown
Member

@mx-psi mx-psi commented Oct 10, 2025

Description

Adds support for disabling or enabling optional fields through an enabled key under an alpha feature gate, configoptional.AddEnabledField. For example, the following configuration becomes valid:

receivers:
  otlp:
    protocols:
      grpc:
        enabled: true

exporters:
  nop:

service:
  pipelines:
    traces:
      receivers: [otlp]
      exporters: [nop]

and is equivalent to:

receivers:
  otlp:
    protocols:
      grpc:

exporters:
  nop:

service:
  pipelines:
    traces:
      receivers: [otlp]
      exporters: [nop]

Link to tracking issue

Fixes #13894
Updates #14021

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.65%. Comparing base (c480657) to head (8fab976).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #13995      +/-   ##
==========================================
+ Coverage   91.63%   91.65%   +0.01%     
==========================================
  Files         656      656              
  Lines       42801    42813      +12     
==========================================
+ Hits        39221    39240      +19     
+ Misses       2760     2755       -5     
+ Partials      820      818       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@lahsivjar lahsivjar left a comment

Choose a reason for hiding this comment

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

Nice! LGTM!

@mx-psi mx-psi marked this pull request as ready for review October 14, 2025 15:49
@mx-psi mx-psi requested a review from a team as a code owner October 14, 2025 15:49
@mx-psi mx-psi requested a review from TylerHelmuth October 14, 2025 15:49
@mx-psi mx-psi changed the title [configoptional] Add support for setting an 'enabled' field [configoptional] Add support for setting an 'enabled' field under a feature gate Oct 14, 2025
Copy link
Copy Markdown
Contributor

@jade-guiton-dd jade-guiton-dd left a comment

Choose a reason for hiding this comment

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

Looks mostly good to me, although I think we'll want community feedback from end user and component author perspectives. Additionally, while I'm ambivalent about enabled vs. disabled, others may have strong opinions.

Comment thread .chloggen/mx-psi_configoptional-enabled-impl.yaml Outdated
Comment thread config/configoptional/optional.go Outdated
Comment thread .chloggen/mx-psi_configoptional-enabled-impl.yaml Outdated
Copy link
Copy Markdown
Contributor

@jmacd jmacd left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

👍

@mx-psi
Copy link
Copy Markdown
Member Author

mx-psi commented Oct 17, 2025

I am going to bring this PR up on the next spec meeting given the discussion on open-telemetry/opentelemetry-specification/issues/4344 has not been resolved yet and is relevant for this PR

@mx-psi
Copy link
Copy Markdown
Member Author

mx-psi commented Oct 22, 2025

I am going to bring this PR up on the next spec meeting given the discussion on open-telemetry/opentelemetry-specification/issues/4344 has not been resolved yet and is relevant for this PR

After spending some time earlier this week writing open-telemetry/opentelemetry-specification#4344 (comment) down and looking at the discussion I think we should go ahead with enabled.

We can revisit the discussion before the feature gate hits beta in case I manage to get some traction at the spec, but given the widespread usage of enabled in the Collector already, I don't feel like this PR makes it significantly worse.

@mx-psi mx-psi requested a review from jade-guiton-dd October 22, 2025 11:01
@mx-psi
Copy link
Copy Markdown
Member Author

mx-psi commented Oct 22, 2025

I am going to merge this once I get Jade's approval so we can continue validating this. Since this is in alpha we can still change our minds along the way

@mx-psi mx-psi added this pull request to the merge queue Oct 23, 2025
Merged via the queue into open-telemetry:main with commit 5060166 Oct 23, 2025
60 checks passed
@mx-psi mx-psi deleted the mx-psi/configoptional-enabled-impl branch October 23, 2025 14:19
@github-actions github-actions Bot added this to the next release milestone Oct 23, 2025
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.

[exporterhelper] Enable batching by default in a component

7 participants