[configoptional] Allow wrapping scalar values by evan-bradley · Pull Request #15175 · open-telemetry/opentelemetry-collector

evan-bradley · 2026-04-22T15:41:40Z

Description

Allow configoptional.Optional to wrap scalar values using new interfaces designed specifically to handle similar wrappers around scalar values.

Follow up to #13524.

evan-bradley · 2026-04-22T16:16:45Z

Leaving in draft while I resolve a bunch of rebase issues and prepare follow-up PRs to show this in use.

jade-guiton-dd

Some first reactions

codspeed-hq · 2026-04-23T17:38:49Z

Merging this PR will not alter performance

⚠️

Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks
⏩ 76 skipped benchmarks¹

_{Comparing evan-bradley:configoptional-scalars-revived (2048001) with main (495f2c5)}

76 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

codecov · 2026-04-23T18:22:09Z

Codecov Report

❌ Patch coverage is 87.20930% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.19%. Comparing base (91b32ef) to head (2048001).
⚠️ Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
confmap/internal/scalar.go	88.88%	3 Missing and 2 partials ⚠️
config/configoptional/optional.go	90.00%	1 Missing and 1 partial ⚠️
confmap/internal/marshaloption.go	66.66%	1 Missing and 1 partial ⚠️
confmap/internal/unmarshaloption.go	66.66%	1 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (87.20%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15175      +/-   ##
==========================================
- Coverage   91.23%   91.19%   -0.04%     
==========================================
  Files         703      705       +2     
  Lines       45902    45998      +96     
==========================================
+ Hits        41878    41950      +72     
- Misses       2820     2837      +17     
- Partials     1204     1211       +7

☔ 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.

…ars-revived

mx-psi

This looks reasonable to me and I think we have enough leeway to modify things that I feel comfortable moving forward with it now

…ars-revived

evan-bradley · 2026-05-08T20:05:22Z

I looked this over again and I think it's ready for a review now.

jade-guiton-dd

The approach seems good overall

Co-authored-by: Jade Guiton <[email protected]>

jade-guiton-dd

Almost good, mostly just nits, just one concern about the behavior of scalarValue.Marshal(nil)

Co-authored-by: Jade Guiton <[email protected]>

jade-guiton-dd

Sorry for all the nitpicks, good work!

evan-bradley · 2026-05-12T18:16:54Z

Thank you! And really appreciate your careful reviews, your points led to a lot of meaningful improvements.

mx-psi

Re-approving after reading the convo above, thanks Jade for the careful review ❤️

#### Description Allow `configoptional.Optional` to wrap scalar values using new interfaces designed specifically to handle similar wrappers around scalar values. Follow up to open-telemetry#13524. --------- Co-authored-by: Jade Guiton <[email protected]>

evan-bradley requested a review from jade-guiton-dd April 22, 2026 15:41

Allow configoptional to wrap scalar values

495f28d

evan-bradley force-pushed the configoptional-scalars-revived branch from 521f5fd to 495f28d Compare April 22, 2026 15:49

jade-guiton-dd reviewed Apr 22, 2026

View reviewed changes

Comment thread confmap/xconfmap/scalarunmarshaler.go Outdated

Comment thread confmap/xconfmap/scalarunmarshaler.go Outdated

Fixes and address PR feedback

76a0b4c

evan-bradley commented Apr 23, 2026

View reviewed changes

Comment thread config/configoptional/optional.go Outdated

Comment thread confmap/internal/decoder.go Outdated

Fix merge, other CI fixes

2a9ae95

evan-bradley added 2 commits April 23, 2026 14:09

Revert unintentional change

1b64c73

Revise Marshaler, allow passing options

b5ec6fd

evan-bradley added 3 commits April 23, 2026 14:23

Fix lint

96b20e1

Undo commenting out tests

ae8d617

Merge remote-tracking branch 'upstream/main' into configoptional-scal…

875bad9

…ars-revived

evan-bradley mentioned this pull request Apr 23, 2026

[exporterhelper] Use configoptional for storage field #15184

Closed

mx-psi approved these changes Apr 29, 2026

View reviewed changes

evan-bradley added 2 commits May 8, 2026 15:16

Merge remote-tracking branch 'upstream/main' into configoptional-scal…

b5b6f0e

…ars-revived

Remove likely unnecessary test

3c2a5ee

evan-bradley marked this pull request as ready for review May 8, 2026 20:04

evan-bradley requested a review from a team as a code owner May 8, 2026 20:04

evan-bradley requested a review from atoulme May 8, 2026 20:04

Formatting

7934cd2

jade-guiton-dd reviewed May 11, 2026

View reviewed changes

evan-bradley and others added 3 commits May 11, 2026 10:05

Apply suggestions from code review

efbafbd

Co-authored-by: Jade Guiton <[email protected]>

Refactor hook

1da5267

Get rid of scalar (un)marshaling options

cae76d7

evan-bradley force-pushed the configoptional-scalars-revived branch 2 times, most recently from 433357b to f58565d Compare May 11, 2026 19:31

Remove interfaces from xconfmap

1faf1b6

evan-bradley force-pushed the configoptional-scalars-revived branch from f58565d to 1faf1b6 Compare May 11, 2026 20:14

jade-guiton-dd reviewed May 12, 2026

View reviewed changes

evan-bradley and others added 6 commits May 12, 2026 08:04

Update confmap/internal/scalar.go

f55ef80

Co-authored-by: Jade Guiton <[email protected]>

Update confmap/internal/scalar.go

7d875a8

Co-authored-by: Jade Guiton <[email protected]>

Update confmap/internal/decoder.go

29a1a2c

Co-authored-by: Jade Guiton <[email protected]>

PR review

c821575

Add scalar handling notes to non-scalar methods

0fc42d6

Remove special casing in multiple locations

193c61a

jade-guiton-dd approved these changes May 12, 2026

View reviewed changes

evan-bradley added 2 commits May 12, 2026 14:06

Clean up tests

8468925

Move textmarshaler hook to before scalarmarshaler hook

5d07f1d

evan-bradley force-pushed the configoptional-scalars-revived branch from 05b8d8a to ca2cb4c Compare May 12, 2026 18:16

Undo formatting change

2048001

evan-bradley force-pushed the configoptional-scalars-revived branch from ca2cb4c to 2048001 Compare May 12, 2026 18:18

mx-psi approved these changes May 15, 2026

View reviewed changes

mx-psi linked an issue May 19, 2026 that may be closed by this pull request

[configoptional] Define direction in scalar values support #13421

Closed

evan-bradley added this pull request to the merge queue May 19, 2026

Merged via the queue into open-telemetry:main with commit 6fe6256 May 19, 2026
65 of 66 checks passed

evan-bradley deleted the configoptional-scalars-revived branch May 19, 2026 14:24

mx-psi mentioned this pull request Jun 2, 2026

Use configoptional.Optional for queuebatch.Config.StorageID #13822

Open

Uh oh!

Conversation

evan-bradley commented Apr 22, 2026

Description

Uh oh!

evan-bradley commented Apr 22, 2026

Uh oh!

jade-guiton-dd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codspeed-hq Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

codecov Bot commented Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mx-psi left a comment

Choose a reason for hiding this comment

Uh oh!

evan-bradley commented May 8, 2026

Uh oh!

jade-guiton-dd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jade-guiton-dd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jade-guiton-dd left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

evan-bradley commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mx-psi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codspeed-hq Bot commented Apr 23, 2026 •

edited

Loading

codecov Bot commented Apr 23, 2026 •

edited

Loading

jade-guiton-dd left a comment •

edited

Loading

evan-bradley commented May 12, 2026 •

edited

Loading