From ed456fb9ff9eb36c0619d8df37196d412faa61cf Mon Sep 17 00:00:00 2001 From: dudik Date: Tue, 5 Nov 2024 17:14:54 +0100 Subject: [PATCH] PR comments and fix tests --- .../Debugger/Expressions/ProbeProcessor.cs | 2 +- .../SpanCodeOrigin/SpanCodeOriginManager.cs | 20 +------- .../Debugger/SpanCodeOriginTests.cs | 48 ++++++++++++------- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs b/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs index 0909277701b7..96aacaae9aad 100644 --- a/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs +++ b/tracer/src/Datadog.Trace/Debugger/Expressions/ProbeProcessor.cs @@ -393,7 +393,7 @@ private void SetSpanDecoration(DebuggerSnapshotCreator snapshotCreator, ref bool { Tracer.Instance.ScopeManager.Active.Root.Span.SetTag(evaluationErrorTag, string.Join(";", decoration.Errors)); } - else if (Tracer.Instance.ScopeManager.Active.Root.Span.GetTag(evaluationErrorTag) != null) + else if (Tracer.Instance.ScopeManager.Active.Span.GetTag(evaluationErrorTag) != null) { Tracer.Instance.ScopeManager.Active.Root.Span.SetTag(evaluationErrorTag, null); } diff --git a/tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs b/tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs index 167ca1ca80ba..1a2fd7ccb24f 100644 --- a/tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs +++ b/tracer/src/Datadog.Trace/Debugger/SpanCodeOrigin/SpanCodeOriginManager.cs @@ -6,12 +6,9 @@ using System; using System.Diagnostics; -using System.Runtime.CompilerServices; -using System.Threading; using Datadog.Trace.Debugger.Symbols; using Datadog.Trace.Logging; using Datadog.Trace.VendoredMicrosoftCode.System.Buffers; -using Datadog.Trace.VendoredMicrosoftCode.System.Collections.Immutable; namespace Datadog.Trace.Debugger.SpanCodeOrigin { @@ -23,26 +20,13 @@ internal class SpanCodeOriginManager private static readonly IDatadogLogger Log = DatadogLogging.GetLoggerFor(typeof(SpanCodeOriginManager)); - private static object _globalInstanceLock = new(); - - private static bool _globalInstanceInitialized; - -#pragma warning disable CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable. - private static SpanCodeOriginManager _instance; -#pragma warning restore CS8618 // Non-nullable field must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring as nullable. - private readonly DebuggerSettings _settings = LiveDebugger.Instance?.Settings ?? DebuggerSettings.FromDefaultSource(); - internal static SpanCodeOriginManager Instance => - LazyInitializer.EnsureInitialized( - ref _instance, - ref _globalInstanceInitialized, - ref _globalInstanceLock); + internal static SpanCodeOriginManager Instance { get; } = new(); - [MethodImpl(MethodImplOptions.AggressiveInlining)] internal void SetCodeOrigin(Span? span) { - if (span == null || !this._settings.CodeOriginForSpansEnabled) + if (span == null || !_settings.CodeOriginForSpansEnabled) { return; } diff --git a/tracer/test/Datadog.Trace.Tests/Debugger/SpanCodeOriginTests.cs b/tracer/test/Datadog.Trace.Tests/Debugger/SpanCodeOriginTests.cs index 4c59ebf6055b..8237c11fc11f 100644 --- a/tracer/test/Datadog.Trace.Tests/Debugger/SpanCodeOriginTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Debugger/SpanCodeOriginTests.cs @@ -5,13 +5,15 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using Datadog.Trace.Configuration; using Datadog.Trace.Configuration.Telemetry; using Datadog.Trace.Debugger; using Datadog.Trace.Debugger.SpanCodeOrigin; -using Datadog.Trace.VendoredMicrosoftCode.System.Collections.Immutable; +using FluentAssertions; using Xunit; namespace Datadog.Trace.Tests.Debugger @@ -39,7 +41,7 @@ public void SetCodeOrigin_WhenDisabled_DoesNotSetTags() SpanCodeOriginManager.Instance.SetCodeOrigin(span); // Assert - Assert.Null(span.Tags.GetTag(CodeOriginTag + ".type")); + span.Tags.GetTag(CodeOriginTag + ".type").Should().BeNull(); } [Fact] @@ -54,13 +56,18 @@ public void SetCodeOrigin_WhenEnabled_SetsCorrectTags() TestMethod(span); // Assert - Assert.NotNull(span.Tags.GetTag($"{CodeOriginTag}.type")); - Assert.Equal("exit", span.Tags.GetTag($"{CodeOriginTag}.type")); - - Assert.NotNull(span.Tags.GetTag($"{CodeOriginTag}.frames.0.method")); - Assert.Equal(nameof(TestMethod), span.Tags.GetTag($"{CodeOriginTag}.frames.0.method")); - Assert.NotNull(span.Tags.GetTag($"{CodeOriginTag}.frames.0.type")); - Assert.Contains(nameof(SpanCodeOriginTests), span.Tags.GetTag($"{CodeOriginTag}.frames.0.type")); + var codeOriginType = span.Tags.GetTag($"{CodeOriginTag}.type"); + codeOriginType.Should().Be("exit"); + var frame0Method = span.Tags.GetTag($"{CodeOriginTag}.frames.0.method"); + frame0Method.Should().Be(nameof(TestMethod)); + var frame0Type = span.Tags.GetTag($"{CodeOriginTag}.frames.0.type"); + frame0Type.Should().Be(GetType().FullName); + var file = span.Tags.GetTag($"{CodeOriginTag}.frames.0.file"); + file.Should().EndWith($"{nameof(SpanCodeOriginTests)}.cs"); + var line = span.Tags.GetTag($"{CodeOriginTag}.frames.0.line"); + line.Should().NotBeNullOrEmpty(); + var column = span.Tags.GetTag($"{CodeOriginTag}.frames.0.column"); + column.Should().NotBeNullOrEmpty(); } [Fact] @@ -76,40 +83,45 @@ public void SetCodeOrigin_WithMaxFramesLimit_RespectsLimit() // Assert var tags = ((List>)(typeof(Datadog.Trace.Tagging.TagsList).GetField("_tags", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(span.Tags))).Select(i => i.Key).ToList(); - Assert.Contains(tags, s => s.StartsWith($"{CodeOriginTag}.frames.0")); - Assert.Contains(tags, s => s.StartsWith($"{CodeOriginTag}.frames.1")); - Assert.DoesNotContain(tags, s => s.StartsWith($"{CodeOriginTag}.frames.2")); + tags.Should().Contain(s => s.StartsWith($"{CodeOriginTag}.frames.0")); + tags.Should().Contain(s => s.StartsWith($"{CodeOriginTag}.frames.1")); + tags.Should().NotContain(s => s.StartsWith($"{CodeOriginTag}.frames.2")); } private static void CreateCodeOriginManager(bool isEnable = false, int numberOfFrames = 8, string excludeFromFilter = "Datadog.Trace.Tests") { var overrideSettings = DebuggerSettings.FromSource( - new NameValueConfigurationSource(new() - { - { ConfigurationKeys.Debugger.CodeOriginForSpansEnabled, isEnable.ToString() }, - { ConfigurationKeys.Debugger.CodeOriginMaxUserFrames, numberOfFrames.ToString() }, - { ConfigurationKeys.Debugger.ThirdPartyDetectionExcludes, excludeFromFilter } - }), + new NameValueConfigurationSource( + new NameValueCollection + { + { ConfigurationKeys.Debugger.CodeOriginForSpansEnabled, isEnable.ToString() }, + { ConfigurationKeys.Debugger.CodeOriginMaxUserFrames, numberOfFrames.ToString() }, + { ConfigurationKeys.Debugger.ThirdPartyDetectionExcludes, excludeFromFilter } + }), NullConfigurationTelemetry.Instance); var instance = SpanCodeOriginManager.Instance; instance.GetType().GetField("_settings", BindingFlags.NonPublic | BindingFlags.Instance).SetValue(instance, overrideSettings); } + [MethodImpl(MethodImplOptions.NoInlining)] private void TestMethod(Span span) { SpanCodeOriginManager.Instance.SetCodeOrigin(span); } + [MethodImpl(MethodImplOptions.NoInlining)] private void DeepTestMethod1(Span span) { DeepTestMethod2(span); } + [MethodImpl(MethodImplOptions.NoInlining)] private void DeepTestMethod2(Span span) { DeepTestMethod3(span); } + [MethodImpl(MethodImplOptions.NoInlining)] private void DeepTestMethod3(Span span) { SpanCodeOriginManager.Instance.SetCodeOrigin(span);