mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 22:55:08 +08:00
Marc Schier
7d0e5870cb
# Description Migrates the entire code base (Stack, Libraries and Applications) from direct `ILogger.LogInformation/LogError/LogWarning/LogDebug/LogTrace/LogCritical(...)` calls to **source-generated `[LoggerMessage]` logging**, as tracked in #3908. Source-generated logging avoids boxing of value-type arguments, caches the message formatter, and emits an efficient `IsEnabled` check so a disabled log level costs nothing. This is a behavior-preserving refactor: message text, levels and structured placeholders are unchanged; only the call shape changes. ## What changed - **36 projects migrated.** Every static log call now goes through a per-file `internal static partial class <Class>Log` holding `[LoggerMessage]` extension methods on `ILogger`, so call sites keep the natural `logger.SomeEvent(...)` shape. Closely-related files that emit the same messages share one log class (e.g. the encoders/decoders in `Opc.Ua.Types` share `EncodingLog`) to avoid duplication and CS0121 ambiguity. - **Per-assembly event-id classes.** Each project has one `internal static class <AssemblyToken>EventIds` (e.g. `CoreEventIds`, `ClientEventIds`, `PubSubEventIds`) with a reserved `public const int` offset block per log class. The assembly-token prefix avoids CS0436 collisions across `InternalsVisibleTo` boundaries. - **`CA1873` enabled solution-wide** in `.editorconfig` now that the whole code base (including test, sample and tooling projects) is migrated, so any regression back to a direct `ILogger.Log*` call with an expensive argument is flagged. Every occurrence has been guarded with an `IsEnabled` check (or the check merged into an existing condition where the log was the sole body of an `if`/`else`, preserving side effects); the only suppressions are two Moq `Verify` expression-tree sites, which are not executed log calls. - **Dynamic log levels stay hand-written.** A handful of calls whose level is only known at runtime keep the structured `logger.Log(logLevel, ...)` form under an `if (logger.IsEnabled(logLevel))` guard — `[LoggerMessage]` requires a compile-time level. - **netstandard duplicate-generator fix.** Projects that also pull in an R9 package (`Microsoft.Extensions.Http.Resilience`, `.Compliance`, `.Telemetry`) get the `Microsoft.Gen.Logging` generator in addition to the in-box one; on `netstandard` both implement every partial method (CS0757). A single `Directory.Build.targets` target removes the R9 analyzer on `netstandard` only (no-op elsewhere and where it is not referenced). - **Shared/linked source files** that are compiled into more than one project (e.g. sample files linked into test projects) use self-contained literal event ids instead of a per-assembly class the consumer does not compile. - **Docs.** The authoring recipe (event-id convention, log-class convention, parameter rules, the `IsEnabled` guard, shared/linked files, and the netstandard generator interaction) lives in the new [`Docs/DeveloperGuide.md`](Docs/DeveloperGuide.md#add-a-log-message-source-generated); `Docs/Diagnostics.md` keeps the runtime/observability side and links to it. - **Redaction preserved.** Log calls that previously wrapped sensitive values with `Redact.Create(...)` keep passing the `RedactionWrapper<T>` (instead of an eager `.ToString()`), so format-time redaction still applies — this also clears the CodeQL clear-text alerts introduced during the migration. ## Validation - Full `UA.slnx` build passes with **0 errors / 0 warnings**; the multi-target test projects force every referenced library to build on net472/net48/netstandard2.1/net8.0/net9.0/net10.0. - No unguarded static `ILogger.Log*` calls remain anywhere (only the guarded dynamic-level calls); `CA1873` is clean across the full solution on net10.0. ## Related Issues - Fixes #3908 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received.
81 lines
4.6 KiB
XML
81 lines
4.6 KiB
XML
<Project>
|
|
<!--
|
|
Per-project opt-in for the CI per-TFM build matrix: when a `.csproj`
|
|
sets `<RestrictForLegacyTfm>true</RestrictForLegacyTfm>` in its main
|
|
`PropertyGroup`, this Directory.Build.targets makes the project a
|
|
no-op build (empty Library, no compile items, no references) for
|
|
`dotnet build … /p:CustomTestTarget=<legacy>` invocations where
|
|
`<legacy>` is one of `net472` / `net48` / `netstandard2.0` /
|
|
`netstandard2.1`. Use this for projects that are `.NET 8+` only
|
|
(e.g. `PublishAot=true`, `ModelContextProtocol` server, TUnit AOT
|
|
tests) and therefore can't participate in a `UA.slnx` build pinned
|
|
to a legacy TFM via `CustomTestTarget`.
|
|
|
|
The project still loads in the `.slnx` graph so the matrix prep
|
|
script discovers it, but `Compile` / `ProjectReference` /
|
|
`PackageReference` / `AdditionalFiles` / `None` / `Content` are all
|
|
cleared and `OutputType` flips to `Library`, producing an empty
|
|
assembly without dragging the project's `.NET 8+` source through
|
|
the C# compiler against the restricted dependency set.
|
|
|
|
Targets are deliberately NOT overridden here - same-named targets
|
|
in `Directory.Build.targets` ALWAYS replace the SDK definitions
|
|
(the `Condition` only gates execution, not registration), which
|
|
would silently break the SDK's restore in unrestricted builds
|
|
(e.g. the GitHub Actions `dotnet publish` / `dotnet test` flows
|
|
that don't pass `CustomTestTarget`).
|
|
|
|
Projects that don't set `RestrictForLegacyTfm` are completely
|
|
unaffected by this file - every condition below evaluates to
|
|
false and no items / properties / targets are touched.
|
|
-->
|
|
<PropertyGroup>
|
|
<_RestrictedToLegacyTfm
|
|
Condition="'$(RestrictForLegacyTfm)' == 'true' and ('$(CustomTestTarget)' == 'net48' or '$(CustomTestTarget)' == 'net472' or '$(CustomTestTarget)' == 'netstandard2.0' or '$(CustomTestTarget)' == 'netstandard2.1')">true</_RestrictedToLegacyTfm>
|
|
</PropertyGroup>
|
|
<PropertyGroup Condition="'$(_RestrictedToLegacyTfm)' == 'true'">
|
|
<OutputType>Library</OutputType>
|
|
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
|
|
<EnableDefaultEmbeddedResourceItems>false</EnableDefaultEmbeddedResourceItems>
|
|
<EnableDefaultNoneItems>false</EnableDefaultNoneItems>
|
|
<EnableDefaultContentItems>false</EnableDefaultContentItems>
|
|
<IsPackable>false</IsPackable>
|
|
<PackAsTool>false</PackAsTool>
|
|
<PublishAot>false</PublishAot>
|
|
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
|
|
<GenerateTargetFrameworkAttribute>false</GenerateTargetFrameworkAttribute>
|
|
<!-- The no-op shell strips all references, so on netstandard2.x there is no
|
|
assembly providing System.Object; supply an explicit RuntimeMetadataVersion
|
|
so the empty assembly still compiles (net4x get it from the targeting pack). -->
|
|
<RuntimeMetadataVersion Condition="'$(RuntimeMetadataVersion)' == ''">v4.0.30319</RuntimeMetadataVersion>
|
|
</PropertyGroup>
|
|
<ItemGroup Condition="'$(_RestrictedToLegacyTfm)' == 'true'">
|
|
<Compile Remove="@(Compile)" />
|
|
<ProjectReference Remove="@(ProjectReference)" />
|
|
<PackageReference Remove="@(PackageReference)" />
|
|
<AdditionalFiles Remove="@(AdditionalFiles)" />
|
|
<None Remove="@(None)" />
|
|
<Content Remove="@(Content)" />
|
|
</ItemGroup>
|
|
<Target Name="_LogSkippedForLegacyTfm" BeforeTargets="Build" Condition="'$(_RestrictedToLegacyTfm)' == 'true'">
|
|
<Message Importance="high" Text="Skipping $(MSBuildProjectName) for CustomTestTarget=$(CustomTestTarget) (.NET 8+ only)." />
|
|
</Target>
|
|
<!--
|
|
On netstandard target frameworks two [LoggerMessage] source generators can be active at once:
|
|
the in-box generator shipped in Microsoft.Extensions.Logging(.Abstractions) and the R9
|
|
Microsoft.Gen.Logging generator that flows in transitively from R9 packages such as
|
|
Microsoft.Extensions.Http.Resilience / .Compliance / .Telemetry. Both emit an implementation
|
|
for every partial [LoggerMessage] method, producing CS0757 ("a partial method may not have
|
|
multiple implementing declarations"). On .NET Framework and net8.0+ only the R9 generator runs,
|
|
so removing the R9 analyzer on netstandard leaves exactly one generator on every TFM. The
|
|
Analyzer Remove is a no-op for projects that don't reference Microsoft.Gen.Logging.
|
|
-->
|
|
<Target Name="RemoveDuplicateLoggingGenerator" BeforeTargets="CoreCompile"
|
|
Condition="'$(TargetFrameworkIdentifier)' == '.NETStandard'">
|
|
<ItemGroup>
|
|
<Analyzer Remove="@(Analyzer)"
|
|
Condition="$([System.String]::Copy('%(Analyzer.Identity)').Contains('Microsoft.Gen.Logging'))" />
|
|
</ItemGroup>
|
|
</Target>
|
|
</Project>
|