From 87711fe5993e4c72ad3ec6013ecbf32330f9681c Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 14 Sep 2023 11:47:27 -0700 Subject: [PATCH 1/5] Disable optional rules by default on NuGet package --- .../contentFiles/any/any/instrument.cmd | 1 + .../contentFiles/any/any/instrument.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.cmd b/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.cmd index 75e86d323a..9cf290bae2 100644 --- a/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.cmd +++ b/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.cmd @@ -25,6 +25,7 @@ set DOTNET_STARTUP_HOOKS=%BASE_PATH%OpenTelemetry.AutoInstrumentation.StartupHoo :: Settings for OpenTelemetry set OTEL_DOTNET_AUTO_HOME=%BASE_PATH% +set OTEL_DOTNET_AUTO_RULE_ENGINE_ENABLED=false @echo on %* diff --git a/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.sh b/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.sh index 132f1d009d..0d766c904d 100755 --- a/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.sh +++ b/nuget/OpenTelemetry.AutoInstrumentation/contentFiles/any/any/instrument.sh @@ -11,5 +11,6 @@ export DOTNET_STARTUP_HOOKS=${BASE_PATH}/OpenTelemetry.AutoInstrumentation.Start # Settings for OpenTelemetry export OTEL_DOTNET_AUTO_HOME=${BASE_PATH} +export OTEL_DOTNET_AUTO_RULE_ENGINE_ENABLED=false $@ From cea5e15389c3735f77a0c5412eeee5ec58b25472 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 14 Sep 2023 13:11:47 -0700 Subject: [PATCH 2/5] Delay allocation of optional rules --- .../RulesEngine/RuleEngine.cs | 29 +++++++------------ .../RuleEngineTests.cs | 25 ++++++++++++---- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs index 8b4eb3e3e3..8f93711350 100644 --- a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs +++ b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs @@ -28,23 +28,8 @@ internal class RuleEngine new MinSupportedFrameworkRule() }; - private readonly List _otherRules = new() - { - new OpenTelemetrySdkMinimumVersionRule(), - new DiagnosticSourceRule(), - new InstrumentationAssemblyRule(), - new NativeProfilerDiagnosticsRule() - }; - - internal RuleEngine() - { - } - - // This constructor is used for test purpose. - internal RuleEngine(List rules) - { - _otherRules = rules; - } + // Optional rules are exposed as a property for testing purposes. + internal List? OptionalRules { private get; set; } = null; internal bool ValidateRules() { @@ -65,8 +50,16 @@ internal bool ValidateRules() return result; } + OptionalRules ??= new() + { + new OpenTelemetrySdkMinimumVersionRule(), + new DiagnosticSourceRule(), + new InstrumentationAssemblyRule(), + new NativeProfilerDiagnosticsRule() + }; + // All the rules are validated here. - foreach (var rule in _otherRules) + foreach (var rule in OptionalRules) { if (!EvaluateRule(rule)) { diff --git a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs b/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs index 5e5acf8abd..d144d0033a 100644 --- a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs +++ b/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs @@ -32,7 +32,10 @@ public void RuleEngineValidation_WhenShouldTrackIsTrue() // Arrange SetShouldTrackEnvironmentVariable(true); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine() + { + OptionalRules = new List { testRule } + }; // Act var result = ruleEngine.ValidateRules(); @@ -48,7 +51,10 @@ public void RuleEngineValidation_WhenShouldTrackIsFalse() // Arrange SetShouldTrackEnvironmentVariable(false); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine() + { + OptionalRules = new List { testRule } + }; // Act var result = ruleEngine.ValidateRules(); @@ -64,7 +70,10 @@ public void RuleEngineValidation_WhenShouldTrackIsNull() // Arrange SetShouldTrackEnvironmentVariable(null); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine() + { + OptionalRules = new List { testRule } + }; // Act var result = ruleEngine.ValidateRules(); @@ -79,7 +88,10 @@ public void RuleEngineValidation_WhenShouldTrackIsNotSet() { // Arrange var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine() + { + OptionalRules = new List { testRule } + }; // Act var result = ruleEngine.ValidateRules(); @@ -95,7 +107,10 @@ public void RuleEngineValidation_WhenShouldTrackHasInvalidValue() // Arrange Environment.SetEnvironmentVariable("OTEL_DOTNET_AUTO_RULE_ENGINE_ENABLED", "Invalid"); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine() + { + OptionalRules = new List { testRule } + }; // Act var result = ruleEngine.ValidateRules(); From f87bb1fe4d818d2d9068ffaa0dc7f6be277a4e09 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Thu, 14 Sep 2023 16:42:04 -0700 Subject: [PATCH 3/5] Return to previous dependency injection pattern --- .../RulesEngine/RuleEngine.cs | 17 ++++++++++--- .../RuleEngineTests.cs | 25 ++++--------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs index 8f93711350..cf6524c264 100644 --- a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs +++ b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs @@ -28,8 +28,17 @@ internal class RuleEngine new MinSupportedFrameworkRule() }; - // Optional rules are exposed as a property for testing purposes. - internal List? OptionalRules { private get; set; } = null; + private readonly List? _optionalRules; + + internal RuleEngine() + { + } + + // This constructor is used for test purpose. + internal RuleEngine(List optionalRules) + { + _optionalRules = optionalRules; + } internal bool ValidateRules() { @@ -50,7 +59,7 @@ internal bool ValidateRules() return result; } - OptionalRules ??= new() + var optionalRules = _optionalRules ?? new() { new OpenTelemetrySdkMinimumVersionRule(), new DiagnosticSourceRule(), @@ -59,7 +68,7 @@ internal bool ValidateRules() }; // All the rules are validated here. - foreach (var rule in OptionalRules) + foreach (var rule in optionalRules) { if (!EvaluateRule(rule)) { diff --git a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs b/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs index d144d0033a..5e5acf8abd 100644 --- a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs +++ b/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs @@ -32,10 +32,7 @@ public void RuleEngineValidation_WhenShouldTrackIsTrue() // Arrange SetShouldTrackEnvironmentVariable(true); var testRule = new TestRule(); - var ruleEngine = new RuleEngine() - { - OptionalRules = new List { testRule } - }; + var ruleEngine = new RuleEngine(new List { testRule }); // Act var result = ruleEngine.ValidateRules(); @@ -51,10 +48,7 @@ public void RuleEngineValidation_WhenShouldTrackIsFalse() // Arrange SetShouldTrackEnvironmentVariable(false); var testRule = new TestRule(); - var ruleEngine = new RuleEngine() - { - OptionalRules = new List { testRule } - }; + var ruleEngine = new RuleEngine(new List { testRule }); // Act var result = ruleEngine.ValidateRules(); @@ -70,10 +64,7 @@ public void RuleEngineValidation_WhenShouldTrackIsNull() // Arrange SetShouldTrackEnvironmentVariable(null); var testRule = new TestRule(); - var ruleEngine = new RuleEngine() - { - OptionalRules = new List { testRule } - }; + var ruleEngine = new RuleEngine(new List { testRule }); // Act var result = ruleEngine.ValidateRules(); @@ -88,10 +79,7 @@ public void RuleEngineValidation_WhenShouldTrackIsNotSet() { // Arrange var testRule = new TestRule(); - var ruleEngine = new RuleEngine() - { - OptionalRules = new List { testRule } - }; + var ruleEngine = new RuleEngine(new List { testRule }); // Act var result = ruleEngine.ValidateRules(); @@ -107,10 +95,7 @@ public void RuleEngineValidation_WhenShouldTrackHasInvalidValue() // Arrange Environment.SetEnvironmentVariable("OTEL_DOTNET_AUTO_RULE_ENGINE_ENABLED", "Invalid"); var testRule = new TestRule(); - var ruleEngine = new RuleEngine() - { - OptionalRules = new List { testRule } - }; + var ruleEngine = new RuleEngine(new List { testRule }); // Act var result = ruleEngine.ValidateRules(); From 452ca114176797797bbfac97ffc478b7fc45af60 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Fri, 15 Sep 2023 08:47:53 -0700 Subject: [PATCH 4/5] Fix botched merge --- .../RulesEngine/RuleEngine.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs index cf6524c264..05de83c112 100644 --- a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs +++ b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs @@ -62,8 +62,7 @@ internal bool ValidateRules() var optionalRules = _optionalRules ?? new() { new OpenTelemetrySdkMinimumVersionRule(), - new DiagnosticSourceRule(), - new InstrumentationAssemblyRule(), + new AssemblyFileVersionRule(), new NativeProfilerDiagnosticsRule() }; From 5efcf1e04ddf61b405022472826f386113d35495 Mon Sep 17 00:00:00 2001 From: Paulo Janotti Date: Fri, 15 Sep 2023 10:14:14 -0700 Subject: [PATCH 5/5] Add Lazy and method to create optional rules --- .../RulesEngine/RuleEngine.cs | 24 +++++++++++-------- .../RuleEngineTests.cs | 10 ++++---- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs index 05de83c112..91da865c5c 100644 --- a/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs +++ b/src/OpenTelemetry.AutoInstrumentation.StartupHook/RulesEngine/RuleEngine.cs @@ -28,14 +28,15 @@ internal class RuleEngine new MinSupportedFrameworkRule() }; - private readonly List? _optionalRules; + private readonly Lazy> _optionalRules; internal RuleEngine() + : this(new Lazy>(CreateDefaultOptionalRules)) { } // This constructor is used for test purpose. - internal RuleEngine(List optionalRules) + internal RuleEngine(Lazy> optionalRules) { _optionalRules = optionalRules; } @@ -59,15 +60,8 @@ internal bool ValidateRules() return result; } - var optionalRules = _optionalRules ?? new() - { - new OpenTelemetrySdkMinimumVersionRule(), - new AssemblyFileVersionRule(), - new NativeProfilerDiagnosticsRule() - }; - // All the rules are validated here. - foreach (var rule in optionalRules) + foreach (var rule in _optionalRules.Value) { if (!EvaluateRule(rule)) { @@ -95,4 +89,14 @@ private static bool EvaluateRule(Rule rule) return true; } + + private static List CreateDefaultOptionalRules() + { + return new() + { + new OpenTelemetrySdkMinimumVersionRule(), + new AssemblyFileVersionRule(), + new NativeProfilerDiagnosticsRule() + }; + } } diff --git a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs b/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs index 5e5acf8abd..ab206f223f 100644 --- a/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs +++ b/test/OpenTelemetry.AutoInstrumentation.StartupHook.Tests/RuleEngineTests.cs @@ -32,7 +32,7 @@ public void RuleEngineValidation_WhenShouldTrackIsTrue() // Arrange SetShouldTrackEnvironmentVariable(true); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine(new Lazy>(() => new() { testRule })); // Act var result = ruleEngine.ValidateRules(); @@ -48,7 +48,7 @@ public void RuleEngineValidation_WhenShouldTrackIsFalse() // Arrange SetShouldTrackEnvironmentVariable(false); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine(new Lazy>(() => new() { testRule })); // Act var result = ruleEngine.ValidateRules(); @@ -64,7 +64,7 @@ public void RuleEngineValidation_WhenShouldTrackIsNull() // Arrange SetShouldTrackEnvironmentVariable(null); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine(new Lazy>(() => new() { testRule })); // Act var result = ruleEngine.ValidateRules(); @@ -79,7 +79,7 @@ public void RuleEngineValidation_WhenShouldTrackIsNotSet() { // Arrange var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine(new Lazy>(() => new() { testRule })); // Act var result = ruleEngine.ValidateRules(); @@ -95,7 +95,7 @@ public void RuleEngineValidation_WhenShouldTrackHasInvalidValue() // Arrange Environment.SetEnvironmentVariable("OTEL_DOTNET_AUTO_RULE_ENGINE_ENABLED", "Invalid"); var testRule = new TestRule(); - var ruleEngine = new RuleEngine(new List { testRule }); + var ruleEngine = new RuleEngine(new Lazy>(() => new() { testRule })); // Act var result = ruleEngine.ValidateRules();