From e265c8382c5976603c86c66311faf72a499c4e9f Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Wed, 6 Nov 2024 16:31:04 -0800 Subject: [PATCH 1/2] Make SqlClientInstrumentation a singleton --- .../SqlClientDiagnosticListener.cs | 28 +++++++++------- .../SqlEventSourceListener.netfx.cs | 26 +++++++++------ .../SqlClientInstrumentation.cs | 32 ++++++++++++++++--- .../TracerProviderBuilderExtensions.cs | 4 +-- 4 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs index 027a6dc749..f10fb831ec 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs @@ -34,18 +34,22 @@ internal sealed class SqlClientDiagnosticListener : ListenerHandler private readonly PropertyFetcher commandTextFetcher = new("CommandText"); private readonly PropertyFetcher exceptionFetcher = new("Exception"); private readonly PropertyFetcher exceptionNumberFetcher = new("Number"); - private readonly SqlClientTraceInstrumentationOptions options; - public SqlClientDiagnosticListener(string sourceName, SqlClientTraceInstrumentationOptions? options) + public SqlClientDiagnosticListener(string sourceName) : base(sourceName) { - this.options = options ?? new SqlClientTraceInstrumentationOptions(); } public override bool SupportsNullActivity => true; public override void OnEventWritten(string name, object? payload) { + if (SqlClientInstrumentation.TracingHandles == 0) + { + return; + } + + var options = SqlClientInstrumentation.TracingOptions; var activity = Activity.Current; switch (name) { @@ -63,7 +67,7 @@ public override void OnEventWritten(string name, object? payload) _ = this.databaseFetcher.TryFetch(connection, out var databaseName); _ = this.dataSourceFetcher.TryFetch(connection, out var dataSource); - var startTags = SqlActivitySourceHelper.GetTagListFromConnectionInfo(dataSource, databaseName, this.options, out var activityName); + var startTags = SqlActivitySourceHelper.GetTagListFromConnectionInfo(dataSource, databaseName, options, out var activityName); activity = SqlActivitySourceHelper.ActivitySource.StartActivity( activityName, ActivityKind.Client, @@ -80,7 +84,7 @@ public override void OnEventWritten(string name, object? payload) { try { - if (this.options.Filter?.Invoke(command) == false) + if (options.Filter?.Invoke(command) == false) { SqlClientInstrumentationEventSource.Log.CommandIsFilteredOut(activity.OperationName); activity.IsAllDataRequested = false; @@ -103,12 +107,12 @@ public override void OnEventWritten(string name, object? payload) switch (commandType) { case CommandType.StoredProcedure: - if (this.options.EmitOldAttributes) + if (options.EmitOldAttributes) { activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); } - if (this.options.EmitNewAttributes) + if (options.EmitNewAttributes) { activity.SetTag(SemanticConventions.AttributeDbOperationName, "EXECUTE"); activity.SetTag(SemanticConventions.AttributeDbCollectionName, commandText); @@ -118,14 +122,14 @@ public override void OnEventWritten(string name, object? payload) break; case CommandType.Text: - if (this.options.SetDbStatementForText) + if (options.SetDbStatementForText) { - if (this.options.EmitOldAttributes) + if (options.EmitOldAttributes) { activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); } - if (this.options.EmitNewAttributes) + if (options.EmitNewAttributes) { activity.SetTag(SemanticConventions.AttributeDbQueryText, commandText); } @@ -142,7 +146,7 @@ public override void OnEventWritten(string name, object? payload) try { - this.options.Enrich?.Invoke(activity, "OnCustom", command); + options.Enrich?.Invoke(activity, "OnCustom", command); } catch (Exception ex) { @@ -199,7 +203,7 @@ public override void OnEventWritten(string name, object? payload) activity.SetStatus(ActivityStatusCode.Error, exception.Message); - if (this.options.RecordException) + if (options.RecordException) { activity.RecordException(exception); } diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs index 45f1800db2..329aa8e184 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs @@ -29,15 +29,9 @@ internal sealed class SqlEventSourceListener : EventListener internal const int BeginExecuteEventId = 1; internal const int EndExecuteEventId = 2; - private readonly SqlClientTraceInstrumentationOptions options; private EventSource? adoNetEventSource; private EventSource? mdsEventSource; - public SqlEventSourceListener(SqlClientTraceInstrumentationOptions? options = null) - { - this.options = options ?? new SqlClientTraceInstrumentationOptions(); - } - public override void Dispose() { if (this.adoNetEventSource != null) @@ -106,6 +100,13 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) (https://github.com/dotnet/SqlClient/blob/f4568ce68da21db3fe88c0e72e1287368aaa1dc8/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs#L6641) */ + if (SqlClientInstrumentation.TracingHandles == 0) + { + return; + } + + var options = SqlClientInstrumentation.TracingOptions; + if (eventData.Payload.Count < 4) { SqlClientInstrumentationEventSource.Log.InvalidPayload(nameof(SqlEventSourceListener), nameof(this.OnBeginExecute)); @@ -114,7 +115,7 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) var dataSource = (string)eventData.Payload[1]; var databaseName = (string)eventData.Payload[2]; - var startTags = SqlActivitySourceHelper.GetTagListFromConnectionInfo(dataSource, databaseName, this.options, out var activityName); + var startTags = SqlActivitySourceHelper.GetTagListFromConnectionInfo(dataSource, databaseName, options, out var activityName); var activity = SqlActivitySourceHelper.ActivitySource.StartActivity( activityName, ActivityKind.Client, @@ -130,14 +131,14 @@ private void OnBeginExecute(EventWrittenEventArgs eventData) if (activity.IsAllDataRequested) { var commandText = (string)eventData.Payload[3]; - if (!string.IsNullOrEmpty(commandText) && this.options.SetDbStatementForText) + if (!string.IsNullOrEmpty(commandText) && options.SetDbStatementForText) { - if (this.options.EmitOldAttributes) + if (options.EmitOldAttributes) { activity.SetTag(SemanticConventions.AttributeDbStatement, commandText); } - if (this.options.EmitNewAttributes) + if (options.EmitNewAttributes) { activity.SetTag(SemanticConventions.AttributeDbQueryText, commandText); } @@ -154,6 +155,11 @@ private void OnEndExecute(EventWrittenEventArgs eventData) [2] -> SqlExceptionNumber */ + if (SqlClientInstrumentation.TracingHandles == 0) + { + return; + } + if (eventData.Payload.Count < 3) { SqlClientInstrumentationEventSource.Log.InvalidPayload(nameof(SqlEventSourceListener), nameof(this.OnEndExecute)); diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs index c1b3ea6988..ccb4526a74 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs @@ -13,10 +13,13 @@ namespace OpenTelemetry.Instrumentation.SqlClient; /// internal sealed class SqlClientInstrumentation : IDisposable { + public static readonly SqlClientInstrumentation Instance = new SqlClientInstrumentation(); + internal const string SqlClientDiagnosticListenerName = "SqlClientDiagnosticListener"; #if NET internal const string SqlClientTrimmingUnsupportedMessage = "Trimming is not yet supported with SqlClient instrumentation."; #endif + internal static int TracingHandles; #if NETFRAMEWORK private readonly SqlEventSourceListener sqlEventSourceListener; #else @@ -39,18 +42,16 @@ internal sealed class SqlClientInstrumentation : IDisposable /// /// Initializes a new instance of the class. /// - /// Configuration options for sql instrumentation. #if NET [RequiresUnreferencedCode(SqlClientTrimmingUnsupportedMessage)] #endif - public SqlClientInstrumentation( - SqlClientTraceInstrumentationOptions? options = null) + private SqlClientInstrumentation() { #if NETFRAMEWORK - this.sqlEventSourceListener = new SqlEventSourceListener(options); + this.sqlEventSourceListener = new SqlEventSourceListener(); #else this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( - name => new SqlClientDiagnosticListener(name, options), + name => new SqlClientDiagnosticListener(name), listener => listener.Name == SqlClientDiagnosticListenerName, this.isEnabled, SqlClientInstrumentationEventSource.Log.UnknownErrorProcessingEvent); @@ -58,6 +59,8 @@ public SqlClientInstrumentation( #endif } + public static SqlClientTraceInstrumentationOptions TracingOptions { get; set; } = new SqlClientTraceInstrumentationOptions(); + /// public void Dispose() { @@ -67,4 +70,23 @@ public void Dispose() this.diagnosticSourceSubscriber?.Dispose(); #endif } + + internal class TracingHandle : IDisposable + { + private bool disposed; + + public TracingHandle() + { + Interlocked.Increment(ref TracingHandles); + } + + public void Dispose() + { + if (!this.disposed) + { + Interlocked.Decrement(ref TracingHandles); + this.disposed = true; + } + } + } } diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs index 2959484eb3..ce58d7799d 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs @@ -70,8 +70,8 @@ public static TracerProviderBuilder AddSqlClientInstrumentation( builder.AddInstrumentation(sp => { var sqlOptions = sp.GetRequiredService>().Get(name); - - return new SqlClientInstrumentation(sqlOptions); + SqlClientInstrumentation.TracingOptions = sqlOptions; + return new SqlClientInstrumentation.TracingHandle(); }); builder.AddSource(SqlActivitySourceHelper.ActivitySourceName); From 3e11aac335e3e1f149252a6457314fb1d00f10d4 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Thu, 7 Nov 2024 10:59:52 -0800 Subject: [PATCH 2/2] Tweaks and trim warning cleanup. --- .../SqlClientInstrumentation.cs | 13 +++++++++---- .../TracerProviderBuilderExtensions.cs | 3 +-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs index ccb4526a74..be77ab5796 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs @@ -11,6 +11,9 @@ namespace OpenTelemetry.Instrumentation.SqlClient; /// /// SqlClient instrumentation. /// +#if NET +[RequiresUnreferencedCode(SqlClientTrimmingUnsupportedMessage)] +#endif internal sealed class SqlClientInstrumentation : IDisposable { public static readonly SqlClientInstrumentation Instance = new SqlClientInstrumentation(); @@ -42,9 +45,6 @@ internal sealed class SqlClientInstrumentation : IDisposable /// /// Initializes a new instance of the class. /// -#if NET - [RequiresUnreferencedCode(SqlClientTrimmingUnsupportedMessage)] -#endif private SqlClientInstrumentation() { #if NETFRAMEWORK @@ -61,6 +61,8 @@ private SqlClientInstrumentation() public static SqlClientTraceInstrumentationOptions TracingOptions { get; set; } = new SqlClientTraceInstrumentationOptions(); + public static IDisposable AddTracingHandle() => new TracingHandle(); + /// public void Dispose() { @@ -71,7 +73,10 @@ public void Dispose() #endif } - internal class TracingHandle : IDisposable +#if NET + [RequiresUnreferencedCode(SqlClientTrimmingUnsupportedMessage)] +#endif + private sealed class TracingHandle : IDisposable { private bool disposed; diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs index ce58d7799d..e7145081d2 100644 --- a/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.SqlClient/TracerProviderBuilderExtensions.cs @@ -52,7 +52,6 @@ public static TracerProviderBuilder AddSqlClientInstrumentation( #if NET [RequiresUnreferencedCode(SqlClientInstrumentation.SqlClientTrimmingUnsupportedMessage)] #endif - public static TracerProviderBuilder AddSqlClientInstrumentation( this TracerProviderBuilder builder, string? name, @@ -71,7 +70,7 @@ public static TracerProviderBuilder AddSqlClientInstrumentation( { var sqlOptions = sp.GetRequiredService>().Get(name); SqlClientInstrumentation.TracingOptions = sqlOptions; - return new SqlClientInstrumentation.TracingHandle(); + return SqlClientInstrumentation.AddTracingHandle(); }); builder.AddSource(SqlActivitySourceHelper.ActivitySourceName);