Skip to content

[OpenTelemetry] Use config binder source generator - #8100

Merged
martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:gh-4725
Nov 10, 2026
Merged

martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:gh-4725

Conversation

@martincostello

Copy link
Copy Markdown
Member

Fixes #4725

Changes

Use the configuration binding source generator for logging settings when targeting .NET.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, successful build/tests, etc.)
  • Tests added/updated
  • CHANGELOG.md file(s) updated for non-trivial changes
  • Changes in public API reviewed (if applicable)
  • AI usage disclosure:
    • No AI used
    • AI-assisted
    • Bulk AI-generated
  • I have the experience and knowledge necessary to understand, review, and validate all content in this PR1

Footnotes

  1. Yes, I can answer maintainer questions about the content of this PR without using AI. ↩

Use the configuration binding source generator for logging settings when targeting .NET.

Fixes open-telemetry#4725.
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-11-10 11:33 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Copilot AI 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.

🟡 Changes recommended

The changelog still contains #TODO and /pull/TODO placeholders that must be replaced with PR number 8100 to provide the required usable pull-request reference.

1 open issue
What changed in this PR

This PR addresses #4725 by moving SDK logging-option binding to the configuration binding source generator on .NET, improving trimming and Native AOT compatibility.

Changes:

  • Adds a dedicated options configurator while preserving configuration-change notifications.
  • Replaces the reflection-based trimming test with tests for option binding, single registration, and change tokens.
  • Documents the change in the SDK changelog.

[!NOTE]
This review was generated with GitHub Copilot.

File Description
test/​OpenTelemetry.Tests/​Logs/​OpenTelemetryLoggingExtensionsTests.cs Tests configuration binding, internal defaults, registration, and change notifications.
src/​OpenTelemetry/​Logs/​ILogger/​OpenTelemetryLoggingExtensions.cs Registers the new configurator and removes obsolete warning suppressions.
src/​OpenTelemetry/​Logs/​ILogger/​OpenTelemetryLoggerOptionsConfigureOptions.cs Adds the binding call used by the source generator.
src/​OpenTelemetry/​CHANGELOG.md Records the trimming and Native AOT improvement.

Review effort: Balanced

Comment thread src/OpenTelemetry/CHANGELOG.md Resolved
- Fix redundant using.
- Update CHANGELOG.

Copilot AI 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.

🟢 Approval recommended

No active review issues block the source-generated logging configuration update.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@martincostello
martincostello marked this pull request as ready for review November 9, 2026 17:39
@martincostello
martincostello requested a review from a team as a code owner November 9, 2026 17:39
@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.06%. Comparing base (1e88560) to head (5147ccc).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8100      +/-   ##
==========================================
- Coverage   93.09%   93.06%   -0.04%     
==========================================
  Files         397      399       +2     
  Lines       23335    23759     +424     
  Branches     4359     4490     +131     
==========================================
+ Hits        21724    22111     +387     
- Misses       1103     1120      +17     
- Partials      508      528      +20     
Flag Coverage Δ
unittests-Project-Experimental 93.14% <100.00%> (-0.07%) ⬇️
unittests-Project-Stable 93.11% <100.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...gger/OpenTelemetryLoggerOptionsConfigureOptions.cs 100.00% <100.00%> (ø)
...try/Logs/ILogger/OpenTelemetryLoggingExtensions.cs 100.00% <100.00%> (ø)

... and 7 files with indirect coverage changes

@Kielek Kielek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes in general LGTM.

Codex detected one difference:

[P2] Empty boolean settings behave differently on .NET 11. With Logging:OpenTelemetry:IncludeScopes = "", the old binder throws InvalidOperationException; the generated binder skips the value, leaving IncludeScopes at its default. I reproduced this with the new binding call. Preserve the previous validation, or explicitly test and document this change. Comparison output.

@xiang17

xiang17 commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Changes in general LGTM.

Codex detected one difference:

[P2] Empty boolean settings behave differently on .NET 11. With Logging:OpenTelemetry:IncludeScopes = "", the old binder throws InvalidOperationException; the generated binder skips the value, leaving IncludeScopes at its default. I reproduced this with the new binding call. Preserve the previous validation, or explicitly test and document this change. Comparison output.

net11.0 and net472 behave the same way. net10.0 would silently ignore and use default.
Should the SDK defer to .NET Runtime's default behavior or do a consistent check for this case?

- Document binding behaviour change in CHANGELOG.
- Inline binding code registrations.
- Remove redundant "Note:" comments.
- Update tests.
@github-actions github-actions Bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Nov 10, 2026
@martincostello
martincostello added this pull request to the merge queue Nov 10, 2026
Merged via the queue into open-telemetry:main with commit bbf1923 Nov 10, 2026
55 checks passed
@martincostello
martincostello deleted the gh-4725 branch November 10, 2026 11:32
@martincostello martincostello added this to the v1.21.0 milestone Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[AOT] Use Configuration binder source generator in .NET 10+

5 participants