[sdk] Nullable annotations for the internal folder + some misc cleanup (

#4841)
open-telemetry · Sep 11, 2023 · c97a233 · c97a233
1 parent 185d622
commit c97a233
Show file tree

Hide file tree

Showing 13 changed files with 68 additions and 41 deletions.
diff --git a/...emetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj b/...emetry.Exporter.OpenTelemetryProtocol/OpenTelemetry.Exporter.OpenTelemetryProtocol.csproj
@@ -38,7 +38,7 @@
     <Compile Include="$(RepoRoot)\src\Shared\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" RequiresExposedExperimentalFeatures="true" />
     <Compile Include="$(RepoRoot)\src\Shared\StatusHelper.cs" Link="Includes\StatusHelper.cs" RequiresExposedExperimentalFeatures="true" />
-    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" RequiresExposedExperimentalFeatures="true" />
+    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" RequiresExposedExperimentalFeatures="true" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj b/src/OpenTelemetry.Instrumentation.AspNetCore/OpenTelemetry.Instrumentation.AspNetCore.csproj
@@ -17,7 +17,7 @@
     <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" Condition="'$(TargetFramework)' == 'netstandard2.0' OR '$(TargetFramework)' == 'netstandard2.1'" />
     <Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
-    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(RunningDotNetPack)' == '' OR '$(RunningDotNetPack)' == 'false'">

diff --git a/...elemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj b/...elemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj
@@ -16,7 +16,7 @@
     <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
     <Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
-    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(RunningDotNetPack)' == '' OR '$(RunningDotNetPack)' == 'false'">

diff --git a/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj b/src/OpenTelemetry.Instrumentation.Http/OpenTelemetry.Instrumentation.Http.csproj
@@ -15,7 +15,7 @@
     <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
     <Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
-    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(RunningDotNetPack)' == '' OR '$(RunningDotNetPack)' == 'false'">

diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj b/src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj
@@ -15,7 +15,7 @@
     <Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
     <Compile Include="$(RepoRoot)\src\Shared\HttpSemanticConventionHelper.cs" Link="Includes\HttpSemanticConventionHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
-    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
   </ItemGroup>
 
   <ItemGroup Condition="'$(RunningDotNetPack)' == '' OR '$(RunningDotNetPack)' == 'false'">

diff --git a/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs b/src/OpenTelemetry/Internal/OpenTelemetrySdkEventSource.cs
@@ -15,6 +15,7 @@
 // </copyright>
 
 #nullable enable
+
 using System.Diagnostics;
 #if NET6_0_OR_GREATER
 using System.Diagnostics.CodeAnalysis;

diff --git a/src/OpenTelemetry/Internal/SelfDiagnostics.cs b/src/OpenTelemetry/Internal/SelfDiagnostics.cs
@@ -14,6 +14,8 @@
 // limitations under the License.
 // </copyright>
 
+#nullable enable
+
 namespace OpenTelemetry.Internal;
 
 /// <summary>

diff --git a/src/OpenTelemetry/Internal/SelfDiagnosticsConfigParser.cs b/src/OpenTelemetry/Internal/SelfDiagnosticsConfigParser.cs
@@ -14,6 +14,9 @@
 // limitations under the License.
 // </copyright>
 
+#nullable enable
+
+using System.Diagnostics.CodeAnalysis;
 using System.Diagnostics.Tracing;
 using System.Text;
 using System.Text.RegularExpressions;
@@ -43,9 +46,12 @@ internal sealed class SelfDiagnosticsConfigParser
     // This class is called in SelfDiagnosticsConfigRefresher.UpdateMemoryMappedFileFromConfiguration
     // in both main thread and the worker thread.
     // In theory the variable won't be access at the same time because worker thread first Task.Delay for a few seconds.
-    private byte[] configBuffer;
+    private byte[]? configBuffer;
 
-    public bool TryGetConfiguration(out string logDirectory, out int fileSizeInKB, out EventLevel logLevel)
+    public bool TryGetConfiguration(
+        [NotNullWhen(true)] out string? logDirectory,
+        out int fileSizeInKB,
+        out EventLevel logLevel)
     {
         logDirectory = null;
         fileSizeInKB = 0;
@@ -67,6 +73,7 @@ public bool TryGetConfiguration(out string logDirectory, out int fileSizeInKB, o
             }
 
             using FileStream file = File.Open(configFilePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete);
+
             var buffer = this.configBuffer;
             if (buffer == null)
             {
@@ -76,6 +83,7 @@ public bool TryGetConfiguration(out string logDirectory, out int fileSizeInKB, o
 
             file.Read(buffer, 0, buffer.Length);
             string configJson = Encoding.UTF8.GetString(buffer);
+
             if (!TryParseLogDirectory(configJson, out logDirectory))
             {
                 return false;
@@ -101,18 +109,19 @@ public bool TryGetConfiguration(out string logDirectory, out int fileSizeInKB, o
                 return false;
             }
 
-            logLevel = (EventLevel)Enum.Parse(typeof(EventLevel), logLevelString);
-            return true;
+            return Enum.TryParse(logLevelString, out logLevel);
         }
         catch (Exception)
         {
             // do nothing on failure to open/read/parse config file
+            return false;
         }
-
-        return false;
     }
 
-    internal static bool TryParseLogDirectory(string configJson, out string logDirectory)
+    internal static bool TryParseLogDirectory(
+        string configJson,
+        [NotNullWhen(true)]
+        out string logDirectory)
     {
         var logDirectoryResult = LogDirectoryRegex.Match(configJson);
         logDirectory = logDirectoryResult.Groups["LogDirectory"].Value;
@@ -126,7 +135,10 @@ internal static bool TryParseFileSize(string configJson, out int fileSizeInKB)
         return fileSizeResult.Success && int.TryParse(fileSizeResult.Groups["FileSize"].Value, out fileSizeInKB);
     }
 
-    internal static bool TryParseLogLevel(string configJson, out string logLevel)
+    internal static bool TryParseLogLevel(
+        string configJson,
+        [NotNullWhen(true)]
+        out string? logLevel)
     {
         var logLevelResult = LogLevelRegex.Match(configJson);
         logLevel = logLevelResult.Groups["LogLevel"].Value;

diff --git a/src/OpenTelemetry/Internal/SelfDiagnosticsConfigRefresher.cs b/src/OpenTelemetry/Internal/SelfDiagnosticsConfigRefresher.cs
@@ -14,7 +14,10 @@
 // limitations under the License.
 // </copyright>
 
+#nullable enable
+
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Diagnostics.Tracing;
 using System.IO.MemoryMappedFiles;
 using System.Text;
@@ -47,10 +50,10 @@ internal class SelfDiagnosticsConfigRefresher : IDisposable
     private bool disposedValue;
 
     // Once the configuration file is valid, an eventListener object will be created.
-    private SelfDiagnosticsEventListener eventListener;
-    private volatile FileStream underlyingFileStreamForMemoryMappedFile;
-    private volatile MemoryMappedFile memoryMappedFile;
-    private string logDirectory;  // Log directory for log files
+    private SelfDiagnosticsEventListener? eventListener;
+    private volatile FileStream? underlyingFileStreamForMemoryMappedFile;
+    private volatile MemoryMappedFile? memoryMappedFile;
+    private string? logDirectory;  // Log directory for log files
     private int logFileSize;  // Log file size in bytes
     private long logFilePosition;  // The logger will write into the byte at this position
     private EventLevel logEventLevel = (EventLevel)(-1);
@@ -77,7 +80,11 @@ public void Dispose()
     /// <param name="stream">When this method returns, contains the Stream object where `byteCount` of bytes can be written.</param>
     /// <param name="availableByteCount">The number of bytes that is remaining until the end of the stream.</param>
     /// <returns>Whether the logger should log in the stream.</returns>
-    public virtual bool TryGetLogStream(int byteCount, out Stream stream, out int availableByteCount)
+    public virtual bool TryGetLogStream(
+        int byteCount,
+        [NotNullWhen(true)]
+        out Stream? stream,
+        out int availableByteCount)
     {
         if (this.memoryMappedFile == null)
         {
@@ -145,7 +152,7 @@ private async Task Worker(CancellationToken cancellationToken)
 
     private void UpdateMemoryMappedFileFromConfiguration()
     {
-        if (this.configParser.TryGetConfiguration(out string newLogDirectory, out int fileSizeInKB, out EventLevel newEventLevel))
+        if (this.configParser.TryGetConfiguration(out string? newLogDirectory, out int fileSizeInKB, out EventLevel newEventLevel))
         {
             int newFileSize = fileSizeInKB * 1024;
             if (!newLogDirectory.Equals(this.logDirectory) || this.logFileSize != newFileSize)
@@ -173,7 +180,7 @@ private void UpdateMemoryMappedFileFromConfiguration()
 
     private void CloseLogFile()
     {
-        MemoryMappedFile mmf = Interlocked.CompareExchange(ref this.memoryMappedFile, null, this.memoryMappedFile);
+        MemoryMappedFile? mmf = Interlocked.CompareExchange(ref this.memoryMappedFile, null, this.memoryMappedFile);
         if (mmf != null)
         {
             // Each thread has its own MemoryMappedViewStream created from the only one MemoryMappedFile.
@@ -190,7 +197,7 @@ private void CloseLogFile()
             mmf.Dispose();
         }
 
-        FileStream fs = Interlocked.CompareExchange(
+        FileStream? fs = Interlocked.CompareExchange(
             ref this.underlyingFileStreamForMemoryMappedFile,
             null,
             this.underlyingFileStreamForMemoryMappedFile);
@@ -202,7 +209,7 @@ private void OpenLogFile(string newLogDirectory, int newFileSize)
         try
         {
             Directory.CreateDirectory(newLogDirectory);
-            var fileName = Path.GetFileName(Process.GetCurrentProcess().MainModule.FileName) + "."
+            var fileName = Path.GetFileName(Process.GetCurrentProcess().MainModule?.FileName ?? "OpenTelemetrySdk") + "."
                 + Process.GetCurrentProcess().Id + ".log";
             var filePath = Path.Combine(newLogDirectory, fileName);
 

diff --git a/src/OpenTelemetry/Internal/SelfDiagnosticsEventListener.cs b/src/OpenTelemetry/Internal/SelfDiagnosticsEventListener.cs
@@ -14,6 +14,8 @@
 // limitations under the License.
 // </copyright>
 
+#nullable enable
+
 using System.Collections.ObjectModel;
 using System.Diagnostics.Tracing;
 using System.Text;
@@ -32,8 +34,8 @@ internal sealed class SelfDiagnosticsEventListener : EventListener
     private readonly object lockObj = new();
     private readonly EventLevel logLevel;
     private readonly SelfDiagnosticsConfigRefresher configRefresher;
-    private readonly ThreadLocal<byte[]> writeBuffer = new(() => null);
-    private readonly List<EventSource> eventSourcesBeforeConstructor = new();
+    private readonly ThreadLocal<byte[]?> writeBuffer = new(() => null);
+    private readonly List<EventSource>? eventSourcesBeforeConstructor = new();
 
     private bool disposedValue = false;
 
@@ -78,14 +80,14 @@ public override void Dispose()
     /// <param name="buffer">The byte array to contain the resulting sequence of bytes.</param>
     /// <param name="position">The position at which to start writing the resulting sequence of bytes.</param>
     /// <returns>The position of the buffer after the last byte of the resulting sequence.</returns>
-    internal static int EncodeInBuffer(string str, bool isParameter, byte[] buffer, int position)
+    internal static int EncodeInBuffer(string? str, bool isParameter, byte[] buffer, int position)
     {
         if (string.IsNullOrEmpty(str))
         {
             return position;
         }
 
-        int charCount = str.Length;
+        int charCount = str!.Length;
         int ellipses = isParameter ? "{...}\n".Length : "...\n".Length;
 
         // Ensure there is space for "{...}\n" or "...\n".
@@ -124,7 +126,7 @@ internal static int EncodeInBuffer(string str, bool isParameter, byte[] buffer,
         return position;
     }
 
-    internal void WriteEvent(string eventMessage, ReadOnlyCollection<object> payload)
+    internal void WriteEvent(string? eventMessage, ReadOnlyCollection<object?>? payload)
     {
         try
         {
@@ -143,10 +145,10 @@ internal void WriteEvent(string eventMessage, ReadOnlyCollection<object> payload
                 // Not using foreach because it can cause allocations
                 for (int i = 0; i < payload.Count; ++i)
                 {
-                    object obj = payload[i];
+                    object? obj = payload[i];
                     if (obj != null)
                     {
-                        pos = EncodeInBuffer(obj.ToString(), true, buffer, pos);
+                        pos = EncodeInBuffer(obj.ToString() ?? "null", true, buffer, pos);
                     }
                     else
                     {
@@ -157,7 +159,7 @@ internal void WriteEvent(string eventMessage, ReadOnlyCollection<object> payload
 
             buffer[pos++] = (byte)'\n';
             int byteCount = pos - 0;
-            if (this.configRefresher.TryGetLogStream(byteCount, out Stream stream, out int availableByteCount))
+            if (this.configRefresher.TryGetLogStream(byteCount, out Stream? stream, out int availableByteCount))
             {
                 if (availableByteCount >= byteCount)
                 {

diff --git a/src/OpenTelemetry/Internal/WildcardHelper.cs b/src/OpenTelemetry/Internal/WildcardHelper.cs
@@ -14,13 +14,19 @@
 // limitations under the License.
 // </copyright>
 
+#nullable enable
+
+using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Text.RegularExpressions;
 
 namespace OpenTelemetry;
 
 internal static class WildcardHelper
 {
-    public static bool ContainsWildcard(string value)
+    public static bool ContainsWildcard(
+        [NotNullWhen(true)]
+        string? value)
     {
         if (value == null)
         {
@@ -30,16 +36,14 @@ public static bool ContainsWildcard(string value)
         return value.Contains('*') || value.Contains('?');
     }
 
-    public static Regex GetWildcardRegex(IEnumerable<string> patterns = default)
+    public static Regex GetWildcardRegex(IEnumerable<string> patterns)
     {
-        if (patterns == null)
-        {
-            return null;
-        }
+        Debug.Assert(patterns?.Any() == true, "patterns was null or empty");
 
         var convertedPattern = string.Join(
             "|",
             from p in patterns select "(?:" + Regex.Escape(p).Replace("\\*", ".*").Replace("\\?", ".") + ')');
+
         return new Regex("^(?:" + convertedPattern + ")$", RegexOptions.Compiled | RegexOptions.IgnoreCase);
     }
 }
diff --git a/src/OpenTelemetry/OpenTelemetry.csproj b/src/OpenTelemetry/OpenTelemetry.csproj
@@ -21,7 +21,7 @@
     <Compile Include="$(RepoRoot)\src\Shared\MathHelper.cs" Link="Includes\MathHelper.cs" />
     <Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
     <Compile Include="$(RepoRoot)\src\Shared\ResourceSemanticConventions.cs" Link="Includes\ResourceSemanticConventions.cs" />
-    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
+    <Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
   </ItemGroup>
 
 </Project>
diff --git a/src/OpenTelemetry/Trace/TracerProviderSdk.cs b/src/OpenTelemetry/Trace/TracerProviderSdk.cs
@@ -19,6 +19,7 @@
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
 using System.Text;
+using System.Text.RegularExpressions;
 using Microsoft.Extensions.DependencyInjection;
 using OpenTelemetry.Internal;
 using OpenTelemetry.Resources;
@@ -78,13 +79,11 @@ internal TracerProviderSdk(
 
         this.supportLegacyActivity = state.LegacyActivityOperationNames.Count > 0;
 
-        bool legacyActivityWildcardMode = false;
-        var legacyActivityWildcardModeRegex = WildcardHelper.GetWildcardRegex();
+        Regex? legacyActivityWildcardModeRegex = null;
         foreach (var legacyName in state.LegacyActivityOperationNames)
         {
             if (WildcardHelper.ContainsWildcard(legacyName))
             {
-                legacyActivityWildcardMode = true;
                 legacyActivityWildcardModeRegex = WildcardHelper.GetWildcardRegex(state.LegacyActivityOperationNames);
                 break;
             }
@@ -121,7 +120,7 @@ internal TracerProviderSdk(
         if (this.supportLegacyActivity)
         {
             Func<Activity, bool>? legacyActivityPredicate = null;
-            if (legacyActivityWildcardMode)
+            if (legacyActivityWildcardModeRegex != null)
             {
                 legacyActivityPredicate = activity => legacyActivityWildcardModeRegex.IsMatch(activity.OperationName);
             }