diff --git a/CHANGELOG.md b/CHANGELOG.md index 44e7b240c8..ff6aef964e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,9 @@ This component adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.h ### Fixed +- Fixed Rule checking System.Diagnostics.DiagnosticSource version for net7.0 + failing on correct configuration [#2950](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/issues/2950). + ### Security ## [1.0.0](https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/releases/tag/v1.0.0) diff --git a/build/Build.Steps.cs b/build/Build.Steps.cs index cd98d49385..167fa94edf 100644 --- a/build/Build.Steps.cs +++ b/build/Build.Steps.cs @@ -558,18 +558,17 @@ DotNetBuildSettings BuildTestApplication(DotNetBuildSettings x) => .Executes(() => { var netPath = TracerHomeDirectory / "net"; - var ruleEngineJsonFilePath = netPath / "ruleEngine.json"; - var ruleEngineJsonNugetFilePath = RootDirectory / "nuget" / "OpenTelemetry.AutoInstrumentation" / "contentFiles" / "any" / "any" / "RuleEngine.json"; - var fileInfoList = new List(); var files = Directory.GetFiles(netPath); + var fileInfoList = new List(files.Length); foreach (string file in files) { var fileName = Path.GetFileNameWithoutExtension(file); - var fileVersion = FileVersionInfo.GetVersionInfo(file).FileVersion; - if (fileName.StartsWith("OpenTelemetry.") && !fileName.StartsWith("OpenTelemetry.Api") && !fileName.StartsWith("OpenTelemetry.AutoInstrumentation")) + if (fileName == "System.Diagnostics.DiagnosticSource" || + (fileName.StartsWith("OpenTelemetry.") && !fileName.StartsWith("OpenTelemetry.Api") && !fileName.StartsWith("OpenTelemetry.AutoInstrumentation"))) { + var fileVersion = FileVersionInfo.GetVersionInfo(file).FileVersion; fileInfoList.Add(new { FileName = fileName, @@ -580,7 +579,11 @@ DotNetBuildSettings BuildTestApplication(DotNetBuildSettings x) => JsonSerializerOptions options = new JsonSerializerOptions { WriteIndented = true }; string jsonContent = JsonSerializer.Serialize(fileInfoList, options); + + var ruleEngineJsonFilePath = netPath / "ruleEngine.json"; File.WriteAllText(ruleEngineJsonFilePath, jsonContent); + + var ruleEngineJsonNugetFilePath = RootDirectory / "nuget" / "OpenTelemetry.AutoInstrumentation" / "contentFiles" / "any" / "any" / "RuleEngine.json"; File.Delete(ruleEngineJsonNugetFilePath); File.Copy(ruleEngineJsonFilePath, ruleEngineJsonNugetFilePath); }); diff --git a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/InstrumentationAssemblyRule.cs b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/AssemblyFileVersionRule.cs similarity index 80% rename from src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/InstrumentationAssemblyRule.cs rename to src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/AssemblyFileVersionRule.cs index 7f6ca27cbf..f449342ccb 100644 --- a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/InstrumentationAssemblyRule.cs +++ b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/AssemblyFileVersionRule.cs @@ -1,4 +1,4 @@ -// +// // Copyright The OpenTelemetry Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -21,14 +21,14 @@ namespace OpenTelemetry.AutoInstrumentation.RulesEngine; -internal class InstrumentationAssemblyRule : Rule +internal class AssemblyFileVersionRule : Rule { private static readonly IOtelLogger Logger = OtelLogging.GetLogger("StartupHook"); - public InstrumentationAssemblyRule() + public AssemblyFileVersionRule() { - Name = "Instrumentation Assembly Validator"; - Description = "Ensure that the version of the OpenTelemetry Instrumentation libraries is not older than the version used by Automatic Instrumentation."; + Name = "Assembly File Version Validator"; + Description = "Ensure that the version of key assemblies is not older than the version used by Automatic Instrumentation."; } internal override bool Evaluate() @@ -63,11 +63,11 @@ internal override bool Evaluate() if (appInstrumentationFileVersion < autoInstrumentationFileVersion) { result = false; - Logger.Error($"Rule Engine: Application has direct or indirect reference to older version of Instrumentation library {ruleFileInfo.FileName} - {ruleFileInfo.FileVersion}."); + Logger.Error($"Rule Engine: Application has direct or indirect reference to older version of assembly {ruleFileInfo.FileName} - {ruleFileInfo.FileVersion}."); } else { - Logger.Information($"Rule Engine: Application has reference to Instrumentation library {ruleFileInfo.FileName} and loaded successfully."); + Logger.Information($"Rule Engine: Application has reference to assembly {ruleFileInfo.FileName} and loaded successfully."); } } } @@ -75,7 +75,7 @@ internal override bool Evaluate() catch (Exception ex) { // Exception in rule evaluation should not impact the result of the rule. - Logger.Warning($"Rule Engine:Couldn't evaluate OpenTelemetry Instrumentation Evaluation. Exception: {ex}"); + Logger.Warning($"Rule Engine: Couldn't evaluate assembly reference file version. Exception: {ex}"); } return result; diff --git a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/DiagnosticSourceRule.cs b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/DiagnosticSourceRule.cs deleted file mode 100644 index 76ef0ef811..0000000000 --- a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/DiagnosticSourceRule.cs +++ /dev/null @@ -1,104 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System.Diagnostics; -using System.Reflection; -using OpenTelemetry.AutoInstrumentation.Logging; - -namespace OpenTelemetry.AutoInstrumentation.RulesEngine; - -internal class DiagnosticSourceRule : Rule -{ - private static IOtelLogger logger = OtelLogging.GetLogger("StartupHook"); - - public DiagnosticSourceRule() - { - Name = "System.Diagnostics.DiagnosticSource Validator"; - Description = "Ensure that the System.Diagnostics.DiagnosticSource version is not older than the version used by the Automatic Instrumentation"; - } - - // This constructor is used for test purpose. - protected DiagnosticSourceRule(IOtelLogger otelLogger) - : this() - { - logger = otelLogger; - } - - internal override bool Evaluate() - { - string? olderDiagnosticSourcePackageVersion = null; - - try - { - // NuGet package version is not available at runtime. AssemblyVersion is available but it only changes in major version updates. - // Thus using AssemblyFileVersion to compare whether the loaded version is older than that of auto instrumentation. - var loadedDiagnosticSourceFileVersion = GetResolvedVersion(); - if (loadedDiagnosticSourceFileVersion != null) - { - var autoInstrumentationDiagnosticSourceFileVersion = GetVersionFromAutoInstrumentation(); - if (loadedDiagnosticSourceFileVersion < autoInstrumentationDiagnosticSourceFileVersion) - { - olderDiagnosticSourcePackageVersion = loadedDiagnosticSourceFileVersion.ToString(); - } - } - } - catch (Exception ex) - { - // Exception in evaluation should not throw or crash the process. - logger.Warning($"Rule Engine: Couldn't evaluate System.Diagnostics.DiagnosticSource version. Exception: {ex}"); - return false; - } - - if (olderDiagnosticSourcePackageVersion != null) - { - logger.Error($"Rule Engine: Application has direct or indirect reference to older version of System.Diagnostics.DiagnosticSource.dll {olderDiagnosticSourcePackageVersion}."); - return false; - } - - logger.Information("Rule Engine: DiagnosticSourceRule evaluation success."); - return true; - } - - protected virtual Version? GetResolvedVersion() - { - // Look up for type with an assembly name, will load the library. - var diagnosticSourceType = Type.GetType("System.Diagnostics.DiagnosticSource, System.Diagnostics.DiagnosticSource"); - if (diagnosticSourceType != null) - { - var loadedDiagnosticSourceAssembly = Assembly.GetAssembly(diagnosticSourceType); - var loadedDiagnosticSourceAssemblyFileVersionAttribute = loadedDiagnosticSourceAssembly?.GetCustomAttribute(); - var versionString = loadedDiagnosticSourceAssemblyFileVersionAttribute?.Version; - if (versionString != null) - { - return new Version(versionString); - } - } - - return null; - } - - protected virtual Version GetVersionFromAutoInstrumentation() - { - var autoInstrumentationDiagnosticSourceLocation = Path.Combine(StartupHook.LoaderAssemblyLocation ?? string.Empty, "System.Diagnostics.DiagnosticSource.dll"); - var fileVersionInfo = FileVersionInfo.GetVersionInfo(autoInstrumentationDiagnosticSourceLocation); - var autoInstrumentationDiagnosticSourceFileVersion = new Version( - fileVersionInfo.FileMajorPart, - fileVersionInfo.FileMinorPart, - fileVersionInfo.FileBuildPart, - fileVersionInfo.FilePrivatePart); - return autoInstrumentationDiagnosticSourceFileVersion; - } -} diff --git a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs index 8b4eb3e3e3..b4da092982 100644 --- a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs +++ b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs @@ -31,8 +31,7 @@ internal class RuleEngine private readonly List _otherRules = new() { new OpenTelemetrySdkMinimumVersionRule(), - new DiagnosticSourceRule(), - new InstrumentationAssemblyRule(), + new AssemblyFileVersionRule(), new NativeProfilerDiagnosticsRule() }; diff --git a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/DiagnosticSourceRuleTests.cs b/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/DiagnosticSourceRuleTests.cs deleted file mode 100644 index 1ea5e93f52..0000000000 --- a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/DiagnosticSourceRuleTests.cs +++ /dev/null @@ -1,72 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using OpenTelemetry.AutoInstrumentation.Logging; -using OpenTelemetry.AutoInstrumentation.RulesEngine; -using Xunit; - -namespace OpenTelemetry.AutoInstrumentation.StartupHook.Tests; -public class DiagnosticSourceRuleTests -{ -#pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type. - public static IEnumerable RuleTestData => - new List - { - new object[] { "6.0.0.0", "7.0.0.0", "Rule Engine: Application has direct or indirect reference to older version of System.Diagnostics.DiagnosticSource.dll 6.0.0.0.", false }, - new object[] { "8.0.0.0", "7.0.0.0", "Rule Engine: DiagnosticSourceRule evaluation success.", true }, - new object[] { "7.0.0.0", "7.0.0.0", "Rule Engine: DiagnosticSourceRule evaluation success.", true }, - new object[] { null, "7.0.0.0", "Rule Engine: DiagnosticSourceRule evaluation success.", true }, - }; -#pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. - - [Theory] - [MemberData(nameof(RuleTestData))] - public void DiagnosticSourceVersion(string appVersion, string autoInstrumentationVersion, string logMessage, bool result) - { - var logger = new TestLogger(); - var rule = new TestableDiagnosticSourceRule(appVersion, autoInstrumentationVersion, logger); - Assert.Equal(result, rule.Evaluate()); - Assert.Equal(logMessage, logger.LogRecords[0].Message); - } -} - -internal class TestableDiagnosticSourceRule : DiagnosticSourceRule -{ - private readonly string appVersion; - private readonly string autoInstrumentationVersion; - - public TestableDiagnosticSourceRule(string appVersion, string autoInstrumentationVersion, IOtelLogger otelLogger) - : base(otelLogger) - { - this.appVersion = appVersion; - this.autoInstrumentationVersion = autoInstrumentationVersion; - } - - protected override Version? GetResolvedVersion() - { - if (appVersion == null) - { - return null; - } - - return new Version(appVersion); - } - - protected override Version GetVersionFromAutoInstrumentation() - { - return new Version(autoInstrumentationVersion); - } -}