[v3] Minor updates for subsequent work (#5065)

* Add nullability attributes to public interfaces * Reduce some duplication in TestModule and TestSession to make migration to V3 easier * Add fix for incorrect WAF compatibility check * Fix bug in compare throughput
DataDog · Jul 2, 2024 · 8ba9997 · 8ba9997
1 parent 8be9542
commit 8ba9997
Show file tree

Hide file tree

Showing 11 changed files with 39 additions and 112 deletions.
diff --git a/tracer/build/_build/Build.GitHub.cs b/tracer/build/_build/Build.GitHub.cs
@@ -979,7 +979,7 @@ await client.Issue.Milestone.Update(
                  // current (not released version)
                  var version = new Version(Version);
                  var versionsToCheck = 3;
-                 while (versionsToCheck > 0)
+                 while (versionsToCheck > 0 && version.Minor > 0)
                  {
                      // only looking back across minor releases (ignoring patch etc)
                      versionsToCheck--;

diff --git a/tracer/missing-nullability-files.csv b/tracer/missing-nullability-files.csv
@@ -11,12 +11,8 @@ src/Datadog.Trace/HttpHeaderNames.cs
 src/Datadog.Trace/IDatadogOpenTracingTracer.cs
 src/Datadog.Trace/IDatadogTracer.cs
 src/Datadog.Trace/ILockedTracer.cs
-src/Datadog.Trace/IScope.cs
 src/Datadog.Trace/IScopeManager.cs
 src/Datadog.Trace/IScopeRawAccess.cs
-src/Datadog.Trace/ISpan.cs
-src/Datadog.Trace/ISpanContext.cs
-src/Datadog.Trace/ITracer.cs
 src/Datadog.Trace/Metrics.cs
 src/Datadog.Trace/NativeLoader.cs
 src/Datadog.Trace/OSPlatformName.cs
@@ -28,7 +24,6 @@ src/Datadog.Trace/Scope.IScope.cs
 src/Datadog.Trace/Span.cs
 src/Datadog.Trace/Span.ISpan.cs
 src/Datadog.Trace/SpanContext.cs
-src/Datadog.Trace/SpanCreationSettings.cs
 src/Datadog.Trace/SpanExtensions.Core.cs
 src/Datadog.Trace/SpanExtensions.cs
 src/Datadog.Trace/SpanExtensions.Framework.cs

diff --git a/tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/WafLibraryInvoker.cs b/tracer/src/Datadog.Trace/AppSec/Waf/NativeBindings/WafLibraryInvoker.cs
@@ -213,10 +213,10 @@ private static bool CheckVersionCompatibility(WafLibraryInvoker wafLibraryInvoke
             }
 
             // tracer >= 2.34.0 needs waf >= 1.11 cause it passes a ddwafobject for diagnostics instead of a ruleset info struct which causes unpredictable unmanaged crashes
-            if ((tracerVersion is { Minor: >= 34, Major: >= 2 } && wafMajor == 1 && wafMinor <= 10) ||
-                (tracerVersion is { Minor: >= 38, Major: >= 2 } && wafMajor == 1 && wafMinor < 13) ||
-                (tracerVersion is { Minor: >= 44, Major: >= 2 } && wafMajor == 1 && wafMinor < 15) ||
-                (tracerVersion is { Minor: >= 51, Major: >= 2 } && wafMajor == 1 && wafMinor < 17))
+            if ((tracerVersion is { Minor: >= 34, Major: 2 } or { Major: > 2 } && wafMajor == 1 && wafMinor <= 10) ||
+                (tracerVersion is { Minor: >= 38, Major: 2 } or { Major: > 2 } && wafMajor == 1 && wafMinor < 13) ||
+                (tracerVersion is { Minor: >= 44, Major: 2 } or { Major: > 2 } && wafMajor == 1 && wafMinor < 15) ||
+                (tracerVersion is { Minor: >= 51, Major: 2 } or { Major: > 2 } && wafMajor == 1 && wafMinor < 17))
             {
                 Log.Warning("Waf version {WafVersion} is not compatible with tracer version {TracerVersion}", versionWaf, tracerVersion);
                 return false;

diff --git a/tracer/src/Datadog.Trace/Ci/TestModule.cs b/tracer/src/Datadog.Trace/Ci/TestModule.cs
@@ -248,17 +248,7 @@ internal static IReadOnlyCollection<TestModule> ActiveTestModules
     public static TestModule Create(string name)
     {
         TelemetryFactory.Metrics.RecordCountCIVisibilityManualApiEvent(MetricTags.CIVisibilityTestingEventType.Module);
-        return InternalCreate(name);
-    }
-
-    /// <summary>
-    /// Create a new Test Module
-    /// </summary>
-    /// <param name="name">Test module name</param>
-    /// <returns>New test module instance</returns>
-    internal static TestModule InternalCreate(string name)
-    {
-        return new TestModule(name, null, null, null);
+        return InternalCreate(name, framework: null, frameworkVersion: null, startDate: null);
     }
 
     /// <summary>
@@ -272,7 +262,7 @@ internal static TestModule InternalCreate(string name)
     public static TestModule Create(string name, string framework, string frameworkVersion)
     {
         TelemetryFactory.Metrics.RecordCountCIVisibilityManualApiEvent(MetricTags.CIVisibilityTestingEventType.Module);
-        return InternalCreate(name, framework, frameworkVersion);
+        return InternalCreate(name, framework, frameworkVersion, startDate: null);
     }
 
     /// <summary>
@@ -281,10 +271,13 @@ public static TestModule Create(string name, string framework, string frameworkV
     /// <param name="name">Test module name</param>
     /// <param name="framework">Testing framework name</param>
     /// <param name="frameworkVersion">Testing framework version</param>
+    /// <param name="startDate">Test session start date</param>
     /// <returns>New test module instance</returns>
-    internal static TestModule InternalCreate(string name, string framework, string frameworkVersion)
+    [PublicApi]
+    public static TestModule Create(string name, string framework, string frameworkVersion, DateTimeOffset startDate)
     {
-        return new TestModule(name, framework, frameworkVersion, null);
+        TelemetryFactory.Metrics.RecordCountCIVisibilityManualApiEvent(MetricTags.CIVisibilityTestingEventType.Module);
+        return InternalCreate(name, framework, frameworkVersion, startDate);
     }
 
     /// <summary>
@@ -293,14 +286,9 @@ internal static TestModule InternalCreate(string name, string framework, string
     /// <param name="name">Test module name</param>
     /// <param name="framework">Testing framework name</param>
     /// <param name="frameworkVersion">Testing framework version</param>
-    /// <param name="startDate">Test session start date</param>
     /// <returns>New test module instance</returns>
-    [PublicApi]
-    public static TestModule Create(string name, string framework, string frameworkVersion, DateTimeOffset startDate)
-    {
-        TelemetryFactory.Metrics.RecordCountCIVisibilityManualApiEvent(MetricTags.CIVisibilityTestingEventType.Module);
-        return InternalCreate(name, framework, frameworkVersion, startDate);
-    }
+    internal static TestModule InternalCreate(string name, string? framework, string? frameworkVersion)
+        => InternalCreate(name, framework, frameworkVersion, null);
 
     /// <summary>
     /// Create a new Test Module
@@ -310,7 +298,7 @@ public static TestModule Create(string name, string framework, string frameworkV
     /// <param name="frameworkVersion">Testing framework version</param>
     /// <param name="startDate">Test session start date</param>
     /// <returns>New test module instance</returns>
-    internal static TestModule InternalCreate(string name, string framework, string frameworkVersion, DateTimeOffset startDate)
+    internal static TestModule InternalCreate(string name, string? framework, string? frameworkVersion, DateTimeOffset? startDate)
     {
         return new TestModule(name, framework, frameworkVersion, startDate);
     }

diff --git a/tracer/src/Datadog.Trace/Ci/TestSession.cs b/tracer/src/Datadog.Trace/Ci/TestSession.cs
@@ -164,22 +164,7 @@ internal static IReadOnlyCollection<TestSession> ActiveTestSessions
     public static TestSession GetOrCreate(string command)
     {
         TelemetryFactory.Metrics.RecordCountCIVisibilityManualApiEvent(MetricTags.CIVisibilityTestingEventType.Session);
-        return InternalGetOrCreate(command);
-    }
-
-    /// <summary>
-    /// Get or create a new Test Session
-    /// </summary>
-    /// <param name="command">Test session command</param>
-    /// <returns>New test session instance</returns>
-    internal static TestSession InternalGetOrCreate(string command)
-    {
-        if (Current is { } current)
-        {
-            return current;
-        }
-
-        return new TestSession(command, null, null, null, false);
+        return InternalGetOrCreate(command, workingDirectory: null, framework: null, startDate: null);
     }
 
     /// <summary>
@@ -192,23 +177,7 @@ internal static TestSession InternalGetOrCreate(string command)
     public static TestSession GetOrCreate(string command, string workingDirectory)
     {
         TelemetryFactory.Metrics.RecordCountCIVisibilityManualApiEvent(MetricTags.CIVisibilityTestingEventType.Session);
-        return InternalGetOrCreate(command, workingDirectory);
-    }
-
-    /// <summary>
-    /// Get or create a new Test Session
-    /// </summary>
-    /// <param name="command">Test session command</param>
-    /// <param name="workingDirectory">Test session working directory</param>
-    /// <returns>New test session instance</returns>
-    internal static TestSession InternalGetOrCreate(string command, string workingDirectory)
-    {
-        if (Current is { } current)
-        {
-            return current;
-        }
-
-        return new TestSession(command, workingDirectory, null, null, false);
+        return InternalGetOrCreate(command, workingDirectory, framework: null, startDate: null);
     }
 
     /// <summary>
@@ -222,24 +191,7 @@ internal static TestSession InternalGetOrCreate(string command, string workingDi
     public static TestSession GetOrCreate(string command, string workingDirectory, string framework)
     {
         TelemetryFactory.Metrics.RecordCountCIVisibilityManualApiEvent(MetricTags.CIVisibilityTestingEventType.Session);
-        return InternalGetOrCreate(command, workingDirectory, framework);
-    }
-
-    /// <summary>
-    /// Get or create a new Test Session
-    /// </summary>
-    /// <param name="command">Test session command</param>
-    /// <param name="workingDirectory">Test session working directory</param>
-    /// <param name="framework">Testing framework name</param>
-    /// <returns>New test session instance</returns>
-    internal static TestSession InternalGetOrCreate(string command, string workingDirectory, string framework)
-    {
-        if (Current is { } current)
-        {
-            return current;
-        }
-
-        return new TestSession(command, workingDirectory, framework, null, false);
+        return InternalGetOrCreate(command, workingDirectory, framework, startDate: null);
     }
 
     /// <summary>
@@ -257,24 +209,6 @@ public static TestSession GetOrCreate(string command, string workingDirectory, s
         return InternalGetOrCreate(command, workingDirectory, framework, startDate);
     }
 
-    /// <summary>
-    /// Get or create a new Test Session
-    /// </summary>
-    /// <param name="command">Test session command</param>
-    /// <param name="workingDirectory">Test session working directory</param>
-    /// <param name="framework">Testing framework name</param>
-    /// <param name="startDate">Test session start date</param>
-    /// <returns>New test session instance</returns>
-    internal static TestSession InternalGetOrCreate(string command, string workingDirectory, string framework, DateTimeOffset startDate)
-    {
-        if (Current is { } current)
-        {
-            return current;
-        }
-
-        return new TestSession(command, workingDirectory, framework, startDate, false);
-    }
-
     /// <summary>
     /// Get or create a new Test Session
     /// </summary>
@@ -300,7 +234,7 @@ public static TestSession GetOrCreate(string command, string? workingDirectory,
     /// <param name="startDate">Test session start date</param>
     /// <param name="propagateEnvironmentVariables">Propagate session data through environment variables (out of proc session)</param>
     /// <returns>New test session instance</returns>
-    internal static TestSession InternalGetOrCreate(string command, string? workingDirectory, string? framework, DateTimeOffset? startDate, bool propagateEnvironmentVariables)
+    internal static TestSession InternalGetOrCreate(string command, string? workingDirectory, string? framework, DateTimeOffset? startDate, bool propagateEnvironmentVariables = false)
     {
         if (Current is { } current)
         {

diff --git a/tracer/src/Datadog.Trace/IScope.cs b/tracer/src/Datadog.Trace/IScope.cs
@@ -3,6 +3,8 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
 
+#nullable enable
+
 using System;
 
 namespace Datadog.Trace

diff --git a/tracer/src/Datadog.Trace/ISpan.cs b/tracer/src/Datadog.Trace/ISpan.cs
@@ -3,6 +3,8 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
 
+#nullable enable
+
 using System;
 
 namespace Datadog.Trace
@@ -63,7 +65,7 @@ public interface ISpan : IDisposable
         /// <param name="key">The tag's key.</param>
         /// <param name="value">The tag's value.</param>
         /// <returns>This span to allow method chaining.</returns>
-        ISpan SetTag(string key, string value);
+        ISpan SetTag(string key, string? value);
 
         /// <summary>
         /// Record the end time of the span and flushes it to the backend.
@@ -89,6 +91,6 @@ public interface ISpan : IDisposable
         /// </summary>
         /// <param name="key">The tag's key</param>
         /// <returns> The value for the tag with the key specified, or null if the tag does not exist</returns>
-        string GetTag(string key);
+        string? GetTag(string key);
     }
 }
diff --git a/tracer/src/Datadog.Trace/ISpanContext.cs b/tracer/src/Datadog.Trace/ISpanContext.cs
@@ -3,6 +3,8 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
 
+#nullable enable
+
 namespace Datadog.Trace
 {
     /// <summary>
@@ -23,6 +25,6 @@ public interface ISpanContext
         /// <summary>
         /// Gets the service name to propagate to child spans.
         /// </summary>
-        string ServiceName { get; }
+        string? ServiceName { get; }
     }
 }
diff --git a/tracer/src/Datadog.Trace/ITracer.cs b/tracer/src/Datadog.Trace/ITracer.cs
@@ -3,6 +3,8 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
 
+#nullable enable
+
 using Datadog.Trace.Configuration;
 using Datadog.Trace.SourceGenerators;
 
@@ -16,7 +18,7 @@ public interface ITracer
         /// <summary>
         /// Gets the active scope
         /// </summary>
-        IScope ActiveScope { get; }
+        IScope? ActiveScope { get; }
 
         /// <summary>
         /// Gets this tracer's settings.

diff --git a/tracer/src/Datadog.Trace/SpanCreationSettings.cs b/tracer/src/Datadog.Trace/SpanCreationSettings.cs
@@ -3,6 +3,8 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
 // </copyright>
 
+#nullable enable
+
 using System;
 
 namespace Datadog.Trace
@@ -22,7 +24,7 @@ public struct SpanCreationSettings
         /// set to <see cref="SpanContext.None"/>. If not set, defaults to <c>null</c> and
         /// the currently active span (if any) is used as the parent.
         /// </summary>
-        public ISpanContext Parent { get; set; }
+        public ISpanContext? Parent { get; set; }
 
         /// <summary>
         /// Gets or sets whether closing the new scope will close the contained span.

diff --git a/...og.Trace.Tests/Snapshots/PublicApiTests.Datadog.Trace.PublicApiHasNotChanged.verified.txt b/...og.Trace.Tests/Snapshots/PublicApiTests.Datadog.Trace.PublicApiHasNotChanged.verified.txt
@@ -395,13 +395,13 @@ namespace Datadog.Trace
         string Type { get; set; }
         void Finish();
         void Finish(System.DateTimeOffset finishTimestamp);
-        string GetTag(string key);
+        string? GetTag(string key);
         void SetException(System.Exception exception);
-        Datadog.Trace.ISpan SetTag(string key, string value);
+        Datadog.Trace.ISpan SetTag(string key, string? value);
     }
     public interface ISpanContext
     {
-        string ServiceName { get; }
+        string? ServiceName { get; }
         ulong SpanId { get; }
         ulong TraceId { get; }
     }
@@ -411,7 +411,7 @@ namespace Datadog.Trace
     }
     public interface ITracer
     {
-        Datadog.Trace.IScope ActiveScope { get; }
+        Datadog.Trace.IScope? ActiveScope { get; }
         Datadog.Trace.Configuration.ImmutableTracerSettings Settings { get; }
         Datadog.Trace.IScope StartActive(string operationName);
         Datadog.Trace.IScope StartActive(string operationName, Datadog.Trace.SpanCreationSettings settings);
@@ -446,7 +446,7 @@ namespace Datadog.Trace
     public struct SpanCreationSettings
     {
         public bool? FinishOnClose { get; set; }
-        public Datadog.Trace.ISpanContext Parent { get; set; }
+        public Datadog.Trace.ISpanContext? Parent { get; set; }
         public System.DateTimeOffset? StartTime { get; set; }
     }
     public static class SpanExtensions