Skip to content

Obsolete legacy TraceConfiguration in favor of ITelemetryContext - #4090

Merged
marcschier merged 3 commits into
master378from
copilot/update-applicationconfiguration-traceconfiguration
Jul 26, 2026
Merged

Obsolete legacy TraceConfiguration in favor of ITelemetryContext#4090
marcschier merged 3 commits into
master378from
copilot/update-applicationconfiguration-traceconfiguration

Conversation

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

ApplicationConfiguration.TraceConfiguration still existed as a live configuration surface after ITelemetryContext was introduced, which made the migration path unclear. This change makes that legacy surface explicitly discoverable as transitional API and points consumers to the telemetry-based replacement.

  • Public API guidance

    • Marks ApplicationConfiguration.TraceConfiguration as obsolete with a message directing consumers to ITelemetryContext and ILogger-based diagnostics.
    • Keeps the property functional so existing configuration loading and legacy trace application continue to work during migration.
  • Internal compatibility

    • Adds targeted CS0618 suppressions at the remaining internal legacy call sites that intentionally preserve TraceConfiguration behavior.
    • Avoids broad warning suppression; only the compatibility path is annotated.
  • Coverage

    • Adds a focused reflection-based test that verifies the property is marked obsolete and that the migration message stays stable.
[Obsolete("Use ITelemetryContext and ILogger-based diagnostics instead.")]
public TraceConfiguration TraceConfiguration { get; set; }

Related Issues

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply. You can also fill these out after creating the PR.

  • Bugfix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds functionality)
  • Test enhancement (non-breaking change to increase test coverage)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
  • Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc.
  • I have signed the CLA.
  • I ran tests locally with my changes, all passed.
  • I fixed all failing tests in the CI pipelines.
  • I fixed all introduced issues with CodeQL and LGTM.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added necessary documentation (if appropriate).
  • Any dependent changes have been merged and published in downstream modules.

Further comments

This is intentionally narrow in scope: it clarifies the migration path without removing the legacy trace pipeline yet. The change is limited to surfacing the deprecation at the public configuration boundary and isolating the remaining intentional legacy usages.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@marcschier marcschier added the 1.5.378 Only affects 1.5.378 (pre 2.0) label Jul 25, 2026
Copilot AI changed the title [WIP] Clarify relevance of ApplicationConfiguration.TraceConfiguration now that ITelemetryContext exists Obsolete legacy TraceConfiguration in favor of ITelemetryContext Jul 25, 2026
Copilot AI requested a review from marcschier July 25, 2026 05:53

@marcschier marcschier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot please fix feedback

Comment thread Tests/Opc.Ua.Core.Tests/Stack/Schema/ApplicationConfigurationObsoleteTests.cs Outdated
Comment thread Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs
Comment thread Tests/Opc.Ua.Gds.Tests/GlobalDiscoveryTestClient.cs Outdated
Comment thread Tests/Opc.Ua.Gds.Tests/ServerConfigurationPushTestClient.cs Outdated
@marcschier
marcschier marked this pull request as ready for review July 25, 2026 06:40
Copilot AI review requested due to automatic review settings July 25, 2026 06:40

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.

Pull request overview

This PR clarifies the migration path away from the legacy ApplicationConfiguration.TraceConfiguration surface by marking it obsolete (with guidance toward ITelemetryContext/ILogger) while preserving existing behavior via narrowly-scoped CS0618 suppressions, and adding a regression test to keep the obsolete marker/message stable.

Changes:

  • Mark ApplicationConfiguration.TraceConfiguration as [Obsolete("Use ITelemetryContext and ILogger-based diagnostics instead.")].
  • Add targeted #pragma warning disable/restore CS0618 at intentional internal/compat call sites that still use TraceConfiguration.
  • Add a reflection-based unit test verifying the property’s obsolete attribute and message.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Tests/Opc.Ua.Gds.Tests/ServerConfigurationPushTestClient.cs Suppress CS0618 where legacy trace output file path is still used by tests.
Tests/Opc.Ua.Gds.Tests/GlobalDiscoveryTestClient.cs Suppress CS0618 where legacy trace output file path is still used by tests.
Tests/Opc.Ua.Core.Tests/Stack/Schema/ApplicationConfigurationObsoleteTests.cs New test asserting TraceConfiguration is obsolete and message is stable.
Stack/Opc.Ua.Core/Stack/Configuration/ApplicationConfiguration.cs Suppress CS0618 around legacy TraceConfiguration.ApplySettings() compatibility path during load.
Stack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs Add [Obsolete] to the TraceConfiguration property; suppress CS0618 in the copy ctor.
Libraries/Opc.Ua.Server/Server/StandardServer.cs Suppress CS0618 around legacy trace configuration update/apply logic.
Libraries/Opc.Ua.Configuration/ApplicationInstance.cs Suppress CS0618 where a default TraceConfiguration is created/applied for compatibility.
Libraries/Opc.Ua.Configuration/ApplicationConfigurationBuilder.cs Suppress CS0618 in builder methods that still configure legacy trace settings.
Applications/ConsoleReferenceServer/Program.cs Suppress CS0618 where trace output file path is used for shadow config behavior.
Applications/ConsoleReferenceServer/ConsoleUtils.cs Suppress CS0618 where legacy trace masks/output file path inform logging configuration.
Applications/ConsoleReferenceClient/Program.cs Suppress CS0618 where legacy trace output file path is overridden/applied.

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please fix feedback

Addressed in 361dd8f.

Copilot AI requested a review from marcschier July 25, 2026 06:54
@codecov

codecov Bot commented Jul 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.14%. Comparing base (7e8f9e5) to head (361dd8f).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##           master378    #4090      +/-   ##
=============================================
- Coverage      60.17%   60.14%   -0.03%     
=============================================
  Files            378      378              
  Lines          79070    79066       -4     
  Branches       13837    13837              
=============================================
- Hits           47577    47558      -19     
- Misses         27080    27094      +14     
- Partials        4413     4414       +1     
Files with missing lines Coverage Δ
...a.Configuration/ApplicationConfigurationBuilder.cs 73.55% <ø> (+0.06%) ⬆️
...raries/Opc.Ua.Configuration/ApplicationInstance.cs 62.64% <100.00%> (+0.14%) ⬆️
Libraries/Opc.Ua.Server/Server/StandardServer.cs 70.98% <ø> (ø)
...ack/Opc.Ua.Core/Schema/ApplicationConfiguration.cs 63.72% <ø> (ø)
...re/Stack/Configuration/ApplicationConfiguration.cs 44.30% <ø> (ø)

... and 12 files with indirect coverage changes

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

@marcschier
marcschier merged commit 23063af into master378 Jul 26, 2026
91 of 92 checks passed
@marcschier
marcschier deleted the copilot/update-applicationconfiguration-traceconfiguration branch July 26, 2026 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.5.378 Only affects 1.5.378 (pre 2.0)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify relevance of ApplicationConfiguration.TraceConfiguration now that ITelemetryContext exists

5 participants