Add comments and remove dead code

Author: MattKotsenas

src/OpenTelemetry.Instrumentation.Kusto/Implementation/ActivityExtensions.cs

@@ -5,8 +5,26 @@
 
 namespace OpenTelemetry.Instrumentation.Kusto.Implementation;
 
+/// <summary>
+/// Extensions on for <see cref="Activity"/>.
+/// </summary>
 internal static class ActivityExtensions
 {
+    /// <summary>
+    /// Adds the specified tags to the activity if they do not already exist.
+    /// </summary>
+    /// <remarks>
+    /// This method does not overwrite existing tags on the activity. Only tags with keys not already
+    /// present are added.
+    /// </remarks>
+    /// <param name="activity">The activity to which tags will be added.</param>
+    /// <param name="tags">
+    /// The collection of tags to add to the activity. Each tag is added only if its key does not already exist on the
+    /// activity.
+    /// </param>
+    /// <returns>
+    /// The activity instance with the new tags added, or unchanged if all tag keys already exist.
+    /// </returns>
     public static Activity AddTags(this Activity activity, TagList tags)
     {
         foreach (var tag in tags)

src/OpenTelemetry.Instrumentation.Kusto/Implementation/InstrumentationHandleManagerExtensions.cs

@@ -3,13 +3,30 @@
 
 namespace OpenTelemetry.Instrumentation.Kusto.Implementation;
 
+/// <summary>
+/// Provides extension methods for <see cref="InstrumentationHandleManager" />.
+/// </summary>
 internal static class InstrumentationHandleManagerExtensions
 {
+    /// <summary>
+    /// Returns <see langword="true"/> if tracing is active (i.e., there is at least one tracing handle); otherwise, <see langword="false"/>.
+    /// </summary>
+    /// <param name="handleManager">
+    /// The <see cref="InstrumentationHandleManager"/> to check for active tracing handles.
+    /// </param>
+    /// <returns><see langword="true"/> if tracing is active; otherwise, <see langword="false"/>.</returns>
     public static bool IsTracingActive(this InstrumentationHandleManager handleManager)
     {
         return handleManager.TracingHandles > 0;
     }
 
+    /// <summary>
+    /// Returns <see langword="true"/> if metrics is active (i.e., there is at least one metrics handle); otherwise, <see langword="false"/>.
+    /// </summary>
+    /// <param name="handleManager">
+    /// The <see cref="InstrumentationHandleManager"/> to check for active metrics handles.
+    /// </param>
+    /// <returns><see langword="true"/> if metrics is active; otherwise, <see langword="false"/>.</returns>
     public static bool IsMetricsActive(this InstrumentationHandleManager handleManager)
     {
         return handleManager.MetricHandles > 0;

src/OpenTelemetry.Instrumentation.Kusto/Implementation/KustoInstrumentation.cs

@@ -5,6 +5,9 @@
 
 namespace OpenTelemetry.Instrumentation.Kusto.Implementation;
 
+/// <summary>
+/// Class to hold the singleton instances used for Kusto instrumentation.
+/// </summary>
 internal static class KustoInstrumentation
 {
     private static readonly Lazy<ITraceListener> Listener = new(() =>
@@ -17,8 +20,14 @@ internal static class KustoInstrumentation
         return listener;
     });
 
+    /// <summary>
+    /// Gets the post-configured options for Kusto instrumentation.
+    /// </summary>
     public static KustoInstrumentationOptions Options { get; } = new KustoInstrumentationOptions();
 
+    /// <summary>
+    /// Gets the <see cref="InstrumentationHandleManager"/> that tracks if there are any active listeners for <see cref="KustoTraceRecordListener"/>.
+    /// </summary>
     public static InstrumentationHandleManager HandleManager { get; } = new InstrumentationHandleManager();
 
     public static void Initialize() => _ = Listener.Value;

src/OpenTelemetry.Instrumentation.Kusto/Implementation/KustoProcessor.cs

@@ -8,8 +8,12 @@
 
 namespace OpenTelemetry.Instrumentation.Kusto.Implementation;
 
+/// <summary>
+/// Use the Kusto query language services to process Kusto queries for summarization and sanitization.
+/// </summary>
 internal static class KustoProcessor
 {
+    // Because we're not doing full semantic analysis for queries, we can reuse the default global state (which includes all built-in functions and types)
     private static readonly GlobalState KustoParserGlobalState = GlobalState.Default.WithCache();
 
     private enum ReplacementKind
@@ -18,14 +22,39 @@ private enum ReplacementKind
         Remove,
     }
 
+    /// <summary>
+    /// Processes the specified Kusto query and optionally generates a summary and/or a sanitized version based on the
+    /// provided options.
+    /// </summary>
+    /// <remarks>
+    /// If both summarization and sanitization are requested, the query is parsed only once for
+    /// efficiency. The returned <see cref="KustoStatementInfo"/> will have null values for summary or sanitized output
+    /// if the corresponding option is not enabled.
+    /// </remarks>
+    /// <param name="shouldSummarize">
+    /// Indicates whether to generate a summary of the query. If <see langword="true"/>, the returned object will
+    /// include a summarized representation.
+    /// </param>
+    /// <param name="shouldSanitize">
+    /// Indicates whether to generate a sanitized version of the query. If <see langword="true"/>, the returned object
+    /// will include a sanitized representation.
+    /// </param>
+    /// <param name="query">
+    /// The Kusto query to process.
+    /// </param>
+    /// <returns>
+    /// A <see cref="KustoStatementInfo"/> containing the summary and/or sanitized version of the query, depending on
+    /// the options specified.
+    /// </returns>
     public static KustoStatementInfo Process(bool shouldSummarize, bool shouldSanitize, string query)
     {
         string? summarized = null;
         string? sanitized = null;
 
         KustoCode? code = null;
 
-        // Note that order matters here as summarization requires semantic analysis, but we want to avoid parsing twice if both are requested.
+        // Note that order matters here as summarization requires semantic analysis to find potential table references,
+        // but we want to avoid parsing twice if both are requested.
         if (shouldSummarize)
         {
             code ??= KustoCode.ParseAndAnalyze(query, KustoParserGlobalState);
@@ -75,6 +104,9 @@ private static string Summarize(KustoCode code)
 
     private static TextEdit CreateRemoval(SyntaxElement node) => TextEdit.Deletion(node.TextStart, node.Width);
 
+    /// <summary>
+    /// Visitor that traverses the KQL looking for literal values to replace with the PLACEHOLDER value.
+    /// </summary>
     private sealed class SanitizerVisitor : DefaultSyntaxVisitor
     {
         private readonly List<TextEdit> edits = [];
@@ -119,6 +151,9 @@ private void VisitChildren(SyntaxNode node)
         }
     }
 
+    /// <summary>
+    /// Visitor that traverses the KQL to produce a summarized representation of the query.
+    /// </summary>
     private sealed class SummarizerVisitor : DefaultSyntaxVisitor, IDisposable
     {
         private readonly TruncatingStringBuilder builder = new();

src/OpenTelemetry.Instrumentation.Kusto/Implementation/KustoTraceRecordListener.cs

@@ -8,6 +8,13 @@
 
 namespace OpenTelemetry.Instrumentation.Kusto.Implementation;
 
+/// <summary>
+/// Class that is registered with the Kusto client library to receive trace records.
+/// </summary>
+/// <remarks>
+/// The Kusto client library uses its own tracing infrastructure. Many types share names with common diagnostic types
+/// (e.g. Activity, ITraceListener, etc.) but in the Kusto.Cloud.Platform.Utils namespace.
+/// </remarks>
 internal sealed class KustoTraceRecordListener : KustoUtils.ITraceListener
 {
     // The client's async machinery may not call us back using the same AsyncLocal context, so we must manually track

src/OpenTelemetry.Instrumentation.Kusto/Implementation/SpanExtensions.cs

@@ -3,17 +3,26 @@
 
 namespace OpenTelemetry.Instrumentation.Kusto.Implementation;
 
+/// <summary>
+/// Extension methods for <see cref="ReadOnlySpan{T}"/>.
+/// </summary>
 internal static class SpanExtensions
 {
+    /// <summary>
+    /// Slices the span after the first occurrence of the <paramref name="needle"/>.
+    /// </summary>
+    /// <param name="span">
+    /// The span to slice.
+    /// </param>
+    /// <param name="needle">
+    /// The value to search for.
+    /// </param>
+    /// <returns>
+    /// A <see cref="ReadOnlySpan{T}"/> that is a slice of the original span after the first occurrence of the <paramref name="needle"/>.
+    /// </returns>
     public static ReadOnlySpan<char> SliceAfter(this ReadOnlySpan<char> span, ReadOnlySpan<char> needle)
     {
         var idx = span.IndexOf(needle);
         return idx >= 0 ? span.Slice(idx + needle.Length) : [];
     }
-
-    public static ReadOnlySpan<char> SliceBefore(this ReadOnlySpan<char> span, ReadOnlySpan<char> needle)
-    {
-        var idx = span.IndexOf(needle);
-        return idx >= 0 ? span.Slice(0, idx) : [];
-    }
 }

src/OpenTelemetry.Instrumentation.Kusto/Implementation/StringBuilderExtensions.cs

@@ -7,6 +7,9 @@
 
 namespace OpenTelemetry.Instrumentation.Kusto.Implementation;
 
+/// <summary>
+/// Class that parses the delimited messages in <see cref="global::Kusto.Cloud.Platform.Utils.TraceRecord"/> instances.
+/// </summary>
 internal class TraceRecordParser
 {
 #if NET9_0_OR_GREATER
@@ -25,7 +28,7 @@ public static ParsedRequestStart ParseRequestStart(ReadOnlySpan<char> message)
         // so we can just take everything after "text="
         var queryText = message.SliceAfter("text=");
 
-        return new ParsedRequestStart(uri, parsed?.Host, parsed?.Port, database, queryText);
+        return new ParsedRequestStart(parsed?.Host, parsed?.Port, database, queryText);
     }
 
     public static ParsedActivityComplete ParseActivityComplete(ReadOnlySpan<char> message)
@@ -59,15 +62,13 @@ private static ReadOnlySpan<char> ExtractValueBetween(ReadOnlySpan<char> haystac
 
     internal readonly ref struct ParsedRequestStart
     {
-        public readonly string Uri;
         public readonly string? ServerAddress;
         public readonly int? ServerPort;
         public readonly ReadOnlySpan<char> Database;
         public readonly ReadOnlySpan<char> QueryText;
 
-        public ParsedRequestStart(string uri, string? serverAddress, int? serverPort, ReadOnlySpan<char> database, ReadOnlySpan<char> queryText)
+        public ParsedRequestStart(string? serverAddress, int? serverPort, ReadOnlySpan<char> database, ReadOnlySpan<char> queryText)
         {
-            this.Uri = uri;
             this.ServerAddress = serverAddress;
             this.ServerPort = serverPort;
             this.Database = database;

src/OpenTelemetry.Instrumentation.Kusto/Implementation/TraceRecordParser.cs

@@ -14,7 +14,6 @@ public void ParseRequestStartSuccess()
         const string message = "$$HTTPREQUEST[RestClient2]: Verb=POST, Uri=http://127.0.0.1:49902/v1/rest/message, DatabaseName=NetDefaultDB, App=testhost, User=REDMOND\\mattkot, ClientVersion=Kusto.Dotnet.Client:{14.0.2+b2d66614da1a4ff4561c5037c48e5be7002d66d4}|Runtime:{.NET_10.0.0/CLRv10.0.0/10.0.0-rtm.25523.111}, ClientRequestId=SW52YWxpZFRhYmxlIHwgdGFrZSAxMCB8IHdoZXJlIENvbDEgPSA3, text=InvalidTable | take 10 | where Col1=7 | summarize by Date, Time";
         var result = TraceRecordParser.ParseRequestStart(message);
 
-        Assert.Equal("http://127.0.0.1:49902/v1/rest/message", result.Uri);
         Assert.Equal("127.0.0.1", result.ServerAddress);
         Assert.Equal("49902", result.ServerPort.ToString());
         Assert.Equal("NetDefaultDB", result.Database.ToString());
@@ -27,14 +26,14 @@ public void ParseRequestStartFailure()
         const string message = "$$HTTPREQUEST[RestClient2]: Verb=POST, Uri=http://";
         var result = TraceRecordParser.ParseRequestStart(message);
 
-        Assert.Equal("http://", result.Uri);
         Assert.Null(result.ServerAddress);
         Assert.Equal(string.Empty, result.ServerPort.ToString());
         Assert.Equal(string.Empty, result.Database.ToString());
         Assert.Equal(string.Empty, result.QueryText.ToString());
     }
 
     [Theory]
+    [InlineData("$$HTTPREQUEST[RestClient2]: Verb=POST, Uri=https://clustername.kusto.windows.net/v1/rest/query, DatabaseName=TestDB, text=print 1", "clustername.kusto.windows.net", 443)]
     [InlineData("$$HTTPREQUEST[RestClient2]: Verb=POST, Uri=http://localhost/v1/rest/query, DatabaseName=TestDB, text=print 1", "localhost", 80)]
     [InlineData("$$HTTPREQUEST[RestClient2]: Verb=POST, Uri=http://[2001:db8::1]:8080/v1/rest/query, DatabaseName=TestDB, text=print 1", "[2001:db8::1]", 8080)]
     [InlineData("$$HTTPREQUEST[RestClient2]: Verb=POST, Uri=https://[2001:db8::1]/v1/rest/query, DatabaseName=TestDB, text=print 1", "[2001:db8::1]", 443)]