From e33930c44eb1da7d3b48b7faa18bf05c5ebd1a3b Mon Sep 17 00:00:00 2001 From: David Hayden Date: Wed, 19 Jan 2022 21:27:01 +0000 Subject: [PATCH 1/6] Support for enriching activities --- .devcontainer/db/init-db.sh | 0 .github/workflows/build.yml | 4 +- Directory.Packages.props | 1 + Npgsql.sln | 11 +++ .../Npgsql.OpenTelemetry.csproj | 6 +- .../NpgsqlTracingInstrumentation.cs | 16 +++++ src/Npgsql.OpenTelemetry/README.md | 39 ++++++++++ .../TracerProviderBuilderExtensions.cs | 11 ++- src/Npgsql/NpgsqlActivitySource.cs | 14 +++- src/Npgsql/NpgsqlCommand.cs | 4 +- src/Npgsql/NpgsqlTracingOptions.cs | 13 ++++ src/Npgsql/Properties/AssemblyInfo.cs | 7 ++ src/Npgsql/PublicAPI.Unshipped.txt | 2 + .../Npgsql.OpenTelemetry.Tests.csproj | 16 +++++ .../NpgsqlTracingOptionsTests.cs | 71 +++++++++++++++++++ 15 files changed, 206 insertions(+), 9 deletions(-) mode change 100644 => 100755 .devcontainer/db/init-db.sh create mode 100644 src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs create mode 100644 test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj create mode 100644 test/Npgsql.OpenTelemetry.Tests/NpgsqlTracingOptionsTests.cs diff --git a/.devcontainer/db/init-db.sh b/.devcontainer/db/init-db.sh old mode 100644 new mode 100755 diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7232c7da34..8909b34153 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -278,7 +278,9 @@ jobs: # TODO: Once test/Npgsql.Specification.Tests work, switch to just testing on the solution - name: Test - run: dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.Tests --logger "GitHubActions;report-warnings=false" + run: | + dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.Tests --logger "GitHubActions;report-warnings=false" + dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.OpenTelemetry.Tests --logger "GitHubActions;report-warnings=false" shell: bash - name: Test Plugins diff --git a/Directory.Packages.props b/Directory.Packages.props index e0d3c6a801..8cae8985af 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -37,6 +37,7 @@ + diff --git a/Npgsql.sln b/Npgsql.sln index b6137866a8..4e1d42fde9 100644 --- a/Npgsql.sln +++ b/Npgsql.sln @@ -47,6 +47,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Github", "Github", "{BA7B6F .github\workflows\rich-code-nav.yml = .github\workflows\rich-code-nav.yml EndProjectSection EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Npgsql.OpenTelemetry.Tests", "test\Npgsql.OpenTelemetry.Tests\Npgsql.OpenTelemetry.Tests.csproj", "{44E4A08B-3D46-43CF-B030-8501F98EA1D7}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -151,6 +153,14 @@ Global {DA29F063-1828-47D8-B051-800AF7C9A0BE}.Release|Any CPU.Build.0 = Release|Any CPU {DA29F063-1828-47D8-B051-800AF7C9A0BE}.Release|x86.ActiveCfg = Release|Any CPU {DA29F063-1828-47D8-B051-800AF7C9A0BE}.Release|x86.Build.0 = Release|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|Any CPU.Build.0 = Debug|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|x86.ActiveCfg = Debug|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|x86.Build.0 = Debug|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|Any CPU.ActiveCfg = Release|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|Any CPU.Build.0 = Release|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|x86.ActiveCfg = Release|Any CPU + {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -169,6 +179,7 @@ Global {C00D2EB1-5719-4372-9E1C-5ED05DC23A00} = {ED612DB1-AB32-4603-95E7-891BACA71C39} {DA29F063-1828-47D8-B051-800AF7C9A0BE} = {8537E50E-CF7F-49CB-B4EF-3E2A1B11F050} {BA7B6F53-D24D-45AC-927A-266857EA8D1E} = {004A2E0F-D34A-44D4-8DF0-D2BC63B57073} + {44E4A08B-3D46-43CF-B030-8501F98EA1D7} = {ED612DB1-AB32-4603-95E7-891BACA71C39} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {C90AEECD-DB4C-4BE6-B506-16A449852FB8} diff --git a/src/Npgsql.OpenTelemetry/Npgsql.OpenTelemetry.csproj b/src/Npgsql.OpenTelemetry/Npgsql.OpenTelemetry.csproj index 56f95dcab1..7869b997f6 100644 --- a/src/Npgsql.OpenTelemetry/Npgsql.OpenTelemetry.csproj +++ b/src/Npgsql.OpenTelemetry/Npgsql.OpenTelemetry.csproj @@ -17,6 +17,10 @@ - + + + + + diff --git a/src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs b/src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs new file mode 100644 index 0000000000..ed9577e3b7 --- /dev/null +++ b/src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs @@ -0,0 +1,16 @@ +using System; + +namespace Npgsql.OpenTelemetry; + +internal sealed class NpgsqlTracingInstrumentation : IDisposable +{ + readonly NpgsqlTracingOptions? _originalOptions; + + public NpgsqlTracingInstrumentation(NpgsqlTracingOptions options) + { + _originalOptions = NpgsqlActivitySource.Options; + NpgsqlActivitySource.Options = options; + } + + public void Dispose() => NpgsqlActivitySource.Options = _originalOptions; +} diff --git a/src/Npgsql.OpenTelemetry/README.md b/src/Npgsql.OpenTelemetry/README.md index 97f1300d6a..d9cdb1c01e 100644 --- a/src/Npgsql.OpenTelemetry/README.md +++ b/src/Npgsql.OpenTelemetry/README.md @@ -20,3 +20,42 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder() Once this is done, you should start seeing Npgsql trace data appearing in your application's console. At this point, you can look into exporting your trace data to a more useful destination: systems such as [Zipkin](https://zipkin.io/) or [Jaeger](https://www.jaegertracing.io/) can efficiently collect and store your data, and provide user interfaces for querying and exploring it. For more information, [visit the diagnostics documentation page](https://www.npgsql.org/doc/diagnostics.html). + +### Enrich + +This option allows one to enrich the activity with additional information from the `NpgsqlCommand`, or on any exception. +The `Enrich` action is called only when `activity.IsAllDataRequested` is `true`. +It contains the activity itself (which can be enriched), the name of the event, and either the `NpgsqlCommand` or an exception, depending on the event name: + +For event name "OnStartActivity", the actual object will be `NpgsqlCommand`. + +For event name "OnStopActivity", the actual object will be `NpgsqlCommand`. + +For event name "OnException", the actual object will be `Exception`. + +Example: + +```csharp +using System; +using Npgsql; +using OpenTelemetry; + +var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddNpgsql(options => options.Enrich + = (activity, eventName, rawObject) => + { + switch (eventName, rawObject) + { + case ("OnStartActivity", NpgsqlCommand command): + activity.SetTag("command.type", command.CommandType); + break; + case ("OnStopActivity", NpgsqlCommand command): + activity.SetTag("succeeded", true); + break; + case ("OnException", Exception exception): + activity.SetTag("succeeded", false); + activity.SetTag("stackTrace", exception.StackTrace); + break; + } + }).Build(); +``` diff --git a/src/Npgsql.OpenTelemetry/TracerProviderBuilderExtensions.cs b/src/Npgsql.OpenTelemetry/TracerProviderBuilderExtensions.cs index 0c34138278..9f5af4f8f3 100644 --- a/src/Npgsql.OpenTelemetry/TracerProviderBuilderExtensions.cs +++ b/src/Npgsql.OpenTelemetry/TracerProviderBuilderExtensions.cs @@ -1,4 +1,5 @@ using System; +using Npgsql.OpenTelemetry; using OpenTelemetry.Trace; // ReSharper disable once CheckNamespace @@ -14,6 +15,12 @@ public static class TracerProviderBuilderExtensions /// public static TracerProviderBuilder AddNpgsql( this TracerProviderBuilder builder, - Action? options = null) - => builder.AddSource("Npgsql"); + Action? configure = null) + { + var options = new NpgsqlTracingOptions(); + configure?.Invoke(options); + return builder + .AddSource("Npgsql") + .AddInstrumentation(() => new NpgsqlTracingInstrumentation(options)); + } } \ No newline at end of file diff --git a/src/Npgsql/NpgsqlActivitySource.cs b/src/Npgsql/NpgsqlActivitySource.cs index 002cf4a638..b99c309fea 100644 --- a/src/Npgsql/NpgsqlActivitySource.cs +++ b/src/Npgsql/NpgsqlActivitySource.cs @@ -19,8 +19,10 @@ static NpgsqlActivitySource() } internal static bool IsEnabled => Source.HasListeners(); + + internal static NpgsqlTracingOptions? Options { get; set; } - internal static Activity? CommandStart(NpgsqlConnector connector, string sql) + internal static Activity? CommandStart(NpgsqlConnector connector, NpgsqlCommand command) { var settings = connector.Settings; var activity = Source.StartActivity(settings.Database!, ActivityKind.Client); @@ -31,7 +33,7 @@ static NpgsqlActivitySource() activity.SetTag("db.connection_string", connector.UserFacingConnectionString); activity.SetTag("db.user", settings.Username); activity.SetTag("db.name", settings.Database); - activity.SetTag("db.statement", sql); + activity.SetTag("db.statement", command.CommandText); activity.SetTag("db.connection_id", connector.Id); var endPoint = connector.ConnectedEndPoint; @@ -54,6 +56,8 @@ static NpgsqlActivitySource() default: throw new ArgumentOutOfRangeException("Invalid endpoint type: " + endPoint.GetType()); } + + Options?.Enrich?.Invoke(activity, "OnStartActivity", command); return activity; } @@ -64,9 +68,11 @@ internal static void ReceivedFirstResponse(Activity activity) activity.AddEvent(activityEvent); } - internal static void CommandStop(Activity activity) + internal static void CommandStop(Activity activity, NpgsqlCommand command) { activity.SetTag("otel.status_code", "OK"); + activity.SetEndTime(DateTime.UtcNow); + Options?.Enrich?.Invoke(activity, "OnStopActivity", command); activity.Dispose(); } @@ -83,6 +89,8 @@ internal static void SetException(Activity activity, Exception ex, bool escaped activity.AddEvent(activityEvent); activity.SetTag("otel.status_code", "ERROR"); activity.SetTag("otel.status_description", ex is PostgresException pgEx ? pgEx.SqlState : ex.Message); + activity.SetEndTime(DateTime.UtcNow); + Options?.Enrich?.Invoke(activity, "OnException", ex); activity.Dispose(); } } \ No newline at end of file diff --git a/src/Npgsql/NpgsqlCommand.cs b/src/Npgsql/NpgsqlCommand.cs index 97fea04593..32c3b7e51d 100644 --- a/src/Npgsql/NpgsqlCommand.cs +++ b/src/Npgsql/NpgsqlCommand.cs @@ -1577,7 +1577,7 @@ internal void TraceCommandStart(NpgsqlConnector connector) { Debug.Assert(CurrentActivity is null); if (NpgsqlActivitySource.IsEnabled) - CurrentActivity = NpgsqlActivitySource.CommandStart(connector, CommandText); + CurrentActivity = NpgsqlActivitySource.CommandStart(connector, this); } internal void TraceReceivedFirstResponse() @@ -1592,7 +1592,7 @@ internal void TraceCommandStop() { if (CurrentActivity is not null) { - NpgsqlActivitySource.CommandStop(CurrentActivity); + NpgsqlActivitySource.CommandStop(CurrentActivity, this); CurrentActivity = null; } } diff --git a/src/Npgsql/NpgsqlTracingOptions.cs b/src/Npgsql/NpgsqlTracingOptions.cs index 4aa61beec6..b79042c849 100644 --- a/src/Npgsql/NpgsqlTracingOptions.cs +++ b/src/Npgsql/NpgsqlTracingOptions.cs @@ -1,3 +1,6 @@ +using System; +using System.Diagnostics; + namespace Npgsql; /// @@ -6,4 +9,14 @@ namespace Npgsql; /// public class NpgsqlTracingOptions { + /// + /// Gets or sets an action to enrich an Activity. + /// + /// + /// : the activity being enriched. + /// string: the name of the event. + /// object: the raw object from which additional information can be extracted to enrich the activity. + /// The type of this object depends on the event, which is given by the above parameter. + /// + public Action? Enrich { get; set; } } \ No newline at end of file diff --git a/src/Npgsql/Properties/AssemblyInfo.cs b/src/Npgsql/Properties/AssemblyInfo.cs index a95cc5d548..b8d52c8d06 100644 --- a/src/Npgsql/Properties/AssemblyInfo.cs +++ b/src/Npgsql/Properties/AssemblyInfo.cs @@ -42,3 +42,10 @@ "8078a5df97a62d83c9a2db2d072523a8fc491398254c6b89329b8c1dcef43a1e" + "7aa16153bcea2ae9a471145624826f60d7c8e71cd025b554a0177bd935a78096" + "29f0a7afc778ebb4ad033e1bf512c1a9c6ceea26b077bc46cac93800435e77ee")] + +[assembly: InternalsVisibleTo("Npgsql.OpenTelemetry, PublicKey=" + +"0024000004800000940000000602000000240000525341310004000001000100" + +"2b3c590b2a4e3d347e6878dc0ff4d21eb056a50420250c6617044330701d35c9" + +"8078a5df97a62d83c9a2db2d072523a8fc491398254c6b89329b8c1dcef43a1e" + +"7aa16153bcea2ae9a471145624826f60d7c8e71cd025b554a0177bd935a78096" + +"29f0a7afc778ebb4ad033e1bf512c1a9c6ceea26b077bc46cac93800435e77ee")] diff --git a/src/Npgsql/PublicAPI.Unshipped.txt b/src/Npgsql/PublicAPI.Unshipped.txt index 4e67f19797..077def87d6 100644 --- a/src/Npgsql/PublicAPI.Unshipped.txt +++ b/src/Npgsql/PublicAPI.Unshipped.txt @@ -1,5 +1,7 @@ #nullable enable Npgsql.NpgsqlLoggingConfiguration +Npgsql.NpgsqlTracingOptions.Enrich.get -> System.Action? +Npgsql.NpgsqlTracingOptions.Enrich.set -> void static Npgsql.NpgsqlLoggingConfiguration.InitializeLogging(Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory, bool parameterLoggingEnabled = false) -> void *REMOVED*abstract Npgsql.Logging.NpgsqlLogger.IsEnabled(Npgsql.Logging.NpgsqlLogLevel level) -> bool *REMOVED*abstract Npgsql.Logging.NpgsqlLogger.Log(Npgsql.Logging.NpgsqlLogLevel level, int connectorId, string! msg, System.Exception? exception = null) -> void diff --git a/test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj b/test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj new file mode 100644 index 0000000000..794fba0088 --- /dev/null +++ b/test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj @@ -0,0 +1,16 @@ + + + false + + + + + + + + + + + + + diff --git a/test/Npgsql.OpenTelemetry.Tests/NpgsqlTracingOptionsTests.cs b/test/Npgsql.OpenTelemetry.Tests/NpgsqlTracingOptionsTests.cs new file mode 100644 index 0000000000..3f1116d870 --- /dev/null +++ b/test/Npgsql.OpenTelemetry.Tests/NpgsqlTracingOptionsTests.cs @@ -0,0 +1,71 @@ +using System.Collections.Generic; +using System.Diagnostics; +using Npgsql.Tests; +using NUnit.Framework; +using OpenTelemetry; +using OpenTelemetry.Trace; + +namespace Npgsql.OpenTelemetry.Tests; + +[NonParallelizable] +public class NpgsqlTracingOptionsTests : TestBase +{ + [Test] + public void Activity_start_stop() + { + using (var conn = OpenConnection()) + { + conn.ExecuteScalar("SELECT 1"); + } + + Assert.That(_enrichInvocations, Has.Count.EqualTo(2)); + + var (startActivity, startEventName, startObject) = _enrichInvocations[0]; + Assert.That(startEventName, Is.EqualTo("OnStartActivity")); + Assert.That(startObject, Is.TypeOf().With.Property("CommandText").EqualTo("SELECT 1")); + Assert.That(startActivity.Kind, Is.EqualTo(ActivityKind.Client)); + + var (stopActivity, stopEventName, stopObject) = _enrichInvocations[1]; + Assert.That(stopEventName, Is.EqualTo("OnStopActivity")); + Assert.That(stopObject, Is.SameAs(startObject)); + Assert.That(stopActivity, Is.SameAs(startActivity)); + } + + [Test] + public void Activity_start_exception() + { + var exception = Assert.Throws(() => + { + using var conn = OpenConnection(); + conn.ExecuteScalar("BO SELECTA"); + }); + + Assert.That(_enrichInvocations, Has.Count.EqualTo(2)); + + var (startActivity, startEventName, startObject) = _enrichInvocations[0]; + Assert.That(startEventName, Is.EqualTo("OnStartActivity")); + Assert.That(startObject, Is.TypeOf().With.Property("CommandText").EqualTo("BO SELECTA")); + Assert.That(startActivity.Kind, Is.EqualTo(ActivityKind.Client)); + + var (stopActivity, stopEventName, stopObject) = _enrichInvocations[1]; + Assert.That(stopEventName, Is.EqualTo("OnException")); + Assert.That(stopObject, Is.SameAs(exception)); + Assert.That(stopActivity, Is.SameAs(startActivity)); + } + + [SetUp] + public void SetUp() + { + _enrichInvocations.Clear(); + _tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddNpgsql(o => o.Enrich = (activity, eventName, rawObject) => _enrichInvocations.Add((activity, eventName, rawObject))) + .Build(); + } + + [TearDown] + public void TearDown() => _tracerProvider.Dispose(); + + TracerProvider _tracerProvider = null!; + + readonly List<(Activity activity, string eventName, object rawObject)> _enrichInvocations = new(); +} From 172f1db6b862ffa5300cf51564bdc7d2cca71eeb Mon Sep 17 00:00:00 2001 From: David Hayden Date: Thu, 20 Jan 2022 18:22:27 +0000 Subject: [PATCH 2/6] Address minor PR feedback - nullability of options - documentation --- .../NpgsqlTracingInstrumentation.cs | 4 +- src/Npgsql.OpenTelemetry/README.md | 41 +------------------ src/Npgsql/NpgsqlActivitySource.cs | 10 ++--- src/Npgsql/NpgsqlTracingOptions.cs | 6 +-- 4 files changed, 9 insertions(+), 52 deletions(-) diff --git a/src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs b/src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs index ed9577e3b7..9f7c047b7f 100644 --- a/src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs +++ b/src/Npgsql.OpenTelemetry/NpgsqlTracingInstrumentation.cs @@ -2,9 +2,9 @@ namespace Npgsql.OpenTelemetry; -internal sealed class NpgsqlTracingInstrumentation : IDisposable +sealed class NpgsqlTracingInstrumentation : IDisposable { - readonly NpgsqlTracingOptions? _originalOptions; + readonly NpgsqlTracingOptions _originalOptions; public NpgsqlTracingInstrumentation(NpgsqlTracingOptions options) { diff --git a/src/Npgsql.OpenTelemetry/README.md b/src/Npgsql.OpenTelemetry/README.md index d9cdb1c01e..4e891f9a6e 100644 --- a/src/Npgsql.OpenTelemetry/README.md +++ b/src/Npgsql.OpenTelemetry/README.md @@ -19,43 +19,4 @@ using var tracerProvider = Sdk.CreateTracerProviderBuilder() Once this is done, you should start seeing Npgsql trace data appearing in your application's console. At this point, you can look into exporting your trace data to a more useful destination: systems such as [Zipkin](https://zipkin.io/) or [Jaeger](https://www.jaegertracing.io/) can efficiently collect and store your data, and provide user interfaces for querying and exploring it. -For more information, [visit the diagnostics documentation page](https://www.npgsql.org/doc/diagnostics.html). - -### Enrich - -This option allows one to enrich the activity with additional information from the `NpgsqlCommand`, or on any exception. -The `Enrich` action is called only when `activity.IsAllDataRequested` is `true`. -It contains the activity itself (which can be enriched), the name of the event, and either the `NpgsqlCommand` or an exception, depending on the event name: - -For event name "OnStartActivity", the actual object will be `NpgsqlCommand`. - -For event name "OnStopActivity", the actual object will be `NpgsqlCommand`. - -For event name "OnException", the actual object will be `Exception`. - -Example: - -```csharp -using System; -using Npgsql; -using OpenTelemetry; - -var tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddNpgsql(options => options.Enrich - = (activity, eventName, rawObject) => - { - switch (eventName, rawObject) - { - case ("OnStartActivity", NpgsqlCommand command): - activity.SetTag("command.type", command.CommandType); - break; - case ("OnStopActivity", NpgsqlCommand command): - activity.SetTag("succeeded", true); - break; - case ("OnException", Exception exception): - activity.SetTag("succeeded", false); - activity.SetTag("stackTrace", exception.StackTrace); - break; - } - }).Build(); -``` +For more information, [visit the diagnostics documentation page](https://www.npgsql.org/doc/diagnostics.html). \ No newline at end of file diff --git a/src/Npgsql/NpgsqlActivitySource.cs b/src/Npgsql/NpgsqlActivitySource.cs index b99c309fea..32dfc69170 100644 --- a/src/Npgsql/NpgsqlActivitySource.cs +++ b/src/Npgsql/NpgsqlActivitySource.cs @@ -19,8 +19,8 @@ static NpgsqlActivitySource() } internal static bool IsEnabled => Source.HasListeners(); - - internal static NpgsqlTracingOptions? Options { get; set; } + + internal static NpgsqlTracingOptions Options { get; set; } = new(); internal static Activity? CommandStart(NpgsqlConnector connector, NpgsqlCommand command) { @@ -57,7 +57,7 @@ static NpgsqlActivitySource() throw new ArgumentOutOfRangeException("Invalid endpoint type: " + endPoint.GetType()); } - Options?.Enrich?.Invoke(activity, "OnStartActivity", command); + Options.Enrich?.Invoke(activity, "OnStartActivity", command); return activity; } @@ -72,7 +72,7 @@ internal static void CommandStop(Activity activity, NpgsqlCommand command) { activity.SetTag("otel.status_code", "OK"); activity.SetEndTime(DateTime.UtcNow); - Options?.Enrich?.Invoke(activity, "OnStopActivity", command); + Options.Enrich?.Invoke(activity, "OnStopActivity", command); activity.Dispose(); } @@ -90,7 +90,7 @@ internal static void SetException(Activity activity, Exception ex, bool escaped activity.SetTag("otel.status_code", "ERROR"); activity.SetTag("otel.status_description", ex is PostgresException pgEx ? pgEx.SqlState : ex.Message); activity.SetEndTime(DateTime.UtcNow); - Options?.Enrich?.Invoke(activity, "OnException", ex); + Options.Enrich?.Invoke(activity, "OnException", ex); activity.Dispose(); } } \ No newline at end of file diff --git a/src/Npgsql/NpgsqlTracingOptions.cs b/src/Npgsql/NpgsqlTracingOptions.cs index b79042c849..2aa46d22a4 100644 --- a/src/Npgsql/NpgsqlTracingOptions.cs +++ b/src/Npgsql/NpgsqlTracingOptions.cs @@ -5,7 +5,6 @@ namespace Npgsql; /// /// Options to configure Npgsql's support for OpenTelemetry tracing. -/// Currently no options are available. /// public class NpgsqlTracingOptions { @@ -13,10 +12,7 @@ public class NpgsqlTracingOptions /// Gets or sets an action to enrich an Activity. /// /// - /// : the activity being enriched. - /// string: the name of the event. - /// object: the raw object from which additional information can be extracted to enrich the activity. - /// The type of this object depends on the event, which is given by the above parameter. + /// /// public Action? Enrich { get; set; } } \ No newline at end of file From cb58126406efe79d9b32df90207cb5cb009d40d3 Mon Sep 17 00:00:00 2001 From: David Hayden Date: Thu, 20 Jan 2022 18:40:29 +0000 Subject: [PATCH 3/6] Remove dedicated test project --- .github/workflows/build.yml | 4 +--- Npgsql.sln | 11 ----------- .../Npgsql.OpenTelemetry.Tests.csproj | 16 ---------------- test/Npgsql.Tests/Npgsql.Tests.csproj | 2 ++ .../OpenTelemetry}/NpgsqlTracingOptionsTests.cs | 3 +-- 5 files changed, 4 insertions(+), 32 deletions(-) delete mode 100644 test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj rename test/{Npgsql.OpenTelemetry.Tests => Npgsql.Tests/OpenTelemetry}/NpgsqlTracingOptionsTests.cs (97%) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8909b34153..7232c7da34 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -278,9 +278,7 @@ jobs: # TODO: Once test/Npgsql.Specification.Tests work, switch to just testing on the solution - name: Test - run: | - dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.Tests --logger "GitHubActions;report-warnings=false" - dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.OpenTelemetry.Tests --logger "GitHubActions;report-warnings=false" + run: dotnet test -c ${{ matrix.config }} -f ${{ matrix.test_tfm }} test/Npgsql.Tests --logger "GitHubActions;report-warnings=false" shell: bash - name: Test Plugins diff --git a/Npgsql.sln b/Npgsql.sln index 4e1d42fde9..b6137866a8 100644 --- a/Npgsql.sln +++ b/Npgsql.sln @@ -47,8 +47,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Github", "Github", "{BA7B6F .github\workflows\rich-code-nav.yml = .github\workflows\rich-code-nav.yml EndProjectSection EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Npgsql.OpenTelemetry.Tests", "test\Npgsql.OpenTelemetry.Tests\Npgsql.OpenTelemetry.Tests.csproj", "{44E4A08B-3D46-43CF-B030-8501F98EA1D7}" -EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -153,14 +151,6 @@ Global {DA29F063-1828-47D8-B051-800AF7C9A0BE}.Release|Any CPU.Build.0 = Release|Any CPU {DA29F063-1828-47D8-B051-800AF7C9A0BE}.Release|x86.ActiveCfg = Release|Any CPU {DA29F063-1828-47D8-B051-800AF7C9A0BE}.Release|x86.Build.0 = Release|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|Any CPU.Build.0 = Debug|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|x86.ActiveCfg = Debug|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Debug|x86.Build.0 = Debug|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|Any CPU.ActiveCfg = Release|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|Any CPU.Build.0 = Release|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|x86.ActiveCfg = Release|Any CPU - {44E4A08B-3D46-43CF-B030-8501F98EA1D7}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -179,7 +169,6 @@ Global {C00D2EB1-5719-4372-9E1C-5ED05DC23A00} = {ED612DB1-AB32-4603-95E7-891BACA71C39} {DA29F063-1828-47D8-B051-800AF7C9A0BE} = {8537E50E-CF7F-49CB-B4EF-3E2A1B11F050} {BA7B6F53-D24D-45AC-927A-266857EA8D1E} = {004A2E0F-D34A-44D4-8DF0-D2BC63B57073} - {44E4A08B-3D46-43CF-B030-8501F98EA1D7} = {ED612DB1-AB32-4603-95E7-891BACA71C39} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {C90AEECD-DB4C-4BE6-B506-16A449852FB8} diff --git a/test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj b/test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj deleted file mode 100644 index 794fba0088..0000000000 --- a/test/Npgsql.OpenTelemetry.Tests/Npgsql.OpenTelemetry.Tests.csproj +++ /dev/null @@ -1,16 +0,0 @@ - - - false - - - - - - - - - - - - - diff --git a/test/Npgsql.Tests/Npgsql.Tests.csproj b/test/Npgsql.Tests/Npgsql.Tests.csproj index 7952ad6301..c8bb528be8 100644 --- a/test/Npgsql.Tests/Npgsql.Tests.csproj +++ b/test/Npgsql.Tests/Npgsql.Tests.csproj @@ -6,8 +6,10 @@ + + diff --git a/test/Npgsql.OpenTelemetry.Tests/NpgsqlTracingOptionsTests.cs b/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs similarity index 97% rename from test/Npgsql.OpenTelemetry.Tests/NpgsqlTracingOptionsTests.cs rename to test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs index 3f1116d870..6f0bca9b37 100644 --- a/test/Npgsql.OpenTelemetry.Tests/NpgsqlTracingOptionsTests.cs +++ b/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs @@ -1,11 +1,10 @@ using System.Collections.Generic; using System.Diagnostics; -using Npgsql.Tests; using NUnit.Framework; using OpenTelemetry; using OpenTelemetry.Trace; -namespace Npgsql.OpenTelemetry.Tests; +namespace Npgsql.Tests.OpenTelemetry; [NonParallelizable] public class NpgsqlTracingOptionsTests : TestBase From dd77ff0cf5bd52591bc55e388e5f80e70bad42cc Mon Sep 17 00:00:00 2001 From: David Hayden Date: Thu, 20 Jan 2022 18:56:36 +0000 Subject: [PATCH 4/6] Rename Enrich -> EnrichCommandExecution --- src/Npgsql/NpgsqlActivitySource.cs | 6 +++--- src/Npgsql/NpgsqlTracingOptions.cs | 4 ++-- src/Npgsql/PublicAPI.Unshipped.txt | 4 ++-- .../Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Npgsql/NpgsqlActivitySource.cs b/src/Npgsql/NpgsqlActivitySource.cs index 32dfc69170..5dd49f2947 100644 --- a/src/Npgsql/NpgsqlActivitySource.cs +++ b/src/Npgsql/NpgsqlActivitySource.cs @@ -57,7 +57,7 @@ static NpgsqlActivitySource() throw new ArgumentOutOfRangeException("Invalid endpoint type: " + endPoint.GetType()); } - Options.Enrich?.Invoke(activity, "OnStartActivity", command); + Options.EnrichCommandExecution?.Invoke(activity, "OnStartActivity", command); return activity; } @@ -72,7 +72,7 @@ internal static void CommandStop(Activity activity, NpgsqlCommand command) { activity.SetTag("otel.status_code", "OK"); activity.SetEndTime(DateTime.UtcNow); - Options.Enrich?.Invoke(activity, "OnStopActivity", command); + Options.EnrichCommandExecution?.Invoke(activity, "OnStopActivity", command); activity.Dispose(); } @@ -90,7 +90,7 @@ internal static void SetException(Activity activity, Exception ex, bool escaped activity.SetTag("otel.status_code", "ERROR"); activity.SetTag("otel.status_description", ex is PostgresException pgEx ? pgEx.SqlState : ex.Message); activity.SetEndTime(DateTime.UtcNow); - Options.Enrich?.Invoke(activity, "OnException", ex); + Options.EnrichCommandExecution?.Invoke(activity, "OnException", ex); activity.Dispose(); } } \ No newline at end of file diff --git a/src/Npgsql/NpgsqlTracingOptions.cs b/src/Npgsql/NpgsqlTracingOptions.cs index 2aa46d22a4..5e198fccac 100644 --- a/src/Npgsql/NpgsqlTracingOptions.cs +++ b/src/Npgsql/NpgsqlTracingOptions.cs @@ -9,10 +9,10 @@ namespace Npgsql; public class NpgsqlTracingOptions { /// - /// Gets or sets an action to enrich an Activity. + /// Gets or sets an action to enrich a Command Execution Activity. /// /// /// /// - public Action? Enrich { get; set; } + public Action? EnrichCommandExecution { get; set; } } \ No newline at end of file diff --git a/src/Npgsql/PublicAPI.Unshipped.txt b/src/Npgsql/PublicAPI.Unshipped.txt index 077def87d6..852cfcd186 100644 --- a/src/Npgsql/PublicAPI.Unshipped.txt +++ b/src/Npgsql/PublicAPI.Unshipped.txt @@ -1,7 +1,7 @@ #nullable enable Npgsql.NpgsqlLoggingConfiguration -Npgsql.NpgsqlTracingOptions.Enrich.get -> System.Action? -Npgsql.NpgsqlTracingOptions.Enrich.set -> void +Npgsql.NpgsqlTracingOptions.EnrichCommandExecution.get -> System.Action? +Npgsql.NpgsqlTracingOptions.EnrichCommandExecution.set -> void static Npgsql.NpgsqlLoggingConfiguration.InitializeLogging(Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory, bool parameterLoggingEnabled = false) -> void *REMOVED*abstract Npgsql.Logging.NpgsqlLogger.IsEnabled(Npgsql.Logging.NpgsqlLogLevel level) -> bool *REMOVED*abstract Npgsql.Logging.NpgsqlLogger.Log(Npgsql.Logging.NpgsqlLogLevel level, int connectorId, string! msg, System.Exception? exception = null) -> void diff --git a/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs b/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs index 6f0bca9b37..d44ee0a751 100644 --- a/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs +++ b/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs @@ -10,7 +10,7 @@ namespace Npgsql.Tests.OpenTelemetry; public class NpgsqlTracingOptionsTests : TestBase { [Test] - public void Activity_start_stop() + public void CommandExecution_start_stop() { using (var conn = OpenConnection()) { @@ -31,7 +31,7 @@ public void Activity_start_stop() } [Test] - public void Activity_start_exception() + public void CommandExecution_start_exception() { var exception = Assert.Throws(() => { @@ -57,7 +57,7 @@ public void SetUp() { _enrichInvocations.Clear(); _tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddNpgsql(o => o.Enrich = (activity, eventName, rawObject) => _enrichInvocations.Add((activity, eventName, rawObject))) + .AddNpgsql(o => o.EnrichCommandExecution = (activity, eventName, rawObject) => _enrichInvocations.Add((activity, eventName, rawObject))) .Build(); } From 21a616aa14d15e0cc5e62018ab650908943e079b Mon Sep 17 00:00:00 2001 From: David Hayden Date: Mon, 7 Mar 2022 19:21:22 +0000 Subject: [PATCH 5/6] Pass command + exception to enrich for OnException event --- src/Npgsql/NpgsqlActivitySource.cs | 4 +-- src/Npgsql/NpgsqlCommand.cs | 2 +- .../NpgsqlTracingOptionsTests.cs | 30 ++++++++++++++++++- 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/src/Npgsql/NpgsqlActivitySource.cs b/src/Npgsql/NpgsqlActivitySource.cs index 5dd49f2947..aa17b94d88 100644 --- a/src/Npgsql/NpgsqlActivitySource.cs +++ b/src/Npgsql/NpgsqlActivitySource.cs @@ -76,7 +76,7 @@ internal static void CommandStop(Activity activity, NpgsqlCommand command) activity.Dispose(); } - internal static void SetException(Activity activity, Exception ex, bool escaped = true) + internal static void SetException(Activity activity, NpgsqlCommand command, Exception ex, bool escaped = true) { var tags = new ActivityTagsCollection { @@ -90,7 +90,7 @@ internal static void SetException(Activity activity, Exception ex, bool escaped activity.SetTag("otel.status_code", "ERROR"); activity.SetTag("otel.status_description", ex is PostgresException pgEx ? pgEx.SqlState : ex.Message); activity.SetEndTime(DateTime.UtcNow); - Options.EnrichCommandExecution?.Invoke(activity, "OnException", ex); + Options.EnrichCommandExecution?.Invoke(activity, "OnException", (command, ex)); activity.Dispose(); } } \ No newline at end of file diff --git a/src/Npgsql/NpgsqlCommand.cs b/src/Npgsql/NpgsqlCommand.cs index 32c3b7e51d..a9500554f4 100644 --- a/src/Npgsql/NpgsqlCommand.cs +++ b/src/Npgsql/NpgsqlCommand.cs @@ -1601,7 +1601,7 @@ internal void TraceSetException(Exception e) { if (CurrentActivity is not null) { - NpgsqlActivitySource.SetException(CurrentActivity, e); + NpgsqlActivitySource.SetException(CurrentActivity, this, e); CurrentActivity = null; } } diff --git a/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs b/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs index d44ee0a751..0920950e11 100644 --- a/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs +++ b/test/Npgsql.Tests/OpenTelemetry/NpgsqlTracingOptionsTests.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Diagnostics; using NUnit.Framework; @@ -48,10 +49,37 @@ public void CommandExecution_start_exception() var (stopActivity, stopEventName, stopObject) = _enrichInvocations[1]; Assert.That(stopEventName, Is.EqualTo("OnException")); - Assert.That(stopObject, Is.SameAs(exception)); + Assert.That(stopObject, Is.TypeOf>()); + var (stopCommand, stopException) = (ValueTuple)stopObject; + Assert.That(stopCommand.CommandText, Is.EqualTo("BO SELECTA")); + Assert.That(stopException, Is.SameAs(exception)); Assert.That(stopActivity, Is.SameAs(startActivity)); } + [Test] + public void CommandExecution_start_exception_patternmatch() + { + var exception = Assert.Throws(() => + { + using var conn = OpenConnection(); + conn.ExecuteScalar("BO SELECTA"); + }); + + Assert.That(_enrichInvocations, Has.Count.EqualTo(2)); + var (_, stopEventName, stopObject) = _enrichInvocations[1]; + + switch (stopEventName, stopObject) + { + case ("OnException", (NpgsqlCommand stopCommand, Exception stopException)): + Assert.That(stopCommand.CommandText, Is.EqualTo("BO SELECTA")); + Assert.That(stopException, Is.SameAs(exception)); + break; + default: + Assert.Fail($"{nameof(stopEventName)}: '{stopEventName}', {nameof(stopObject)}.GetType(): '{stopObject.GetType()}'"); + break; + } + } + [SetUp] public void SetUp() { From 48badfe4a6cb3692b5650d1366c5692673eda796 Mon Sep 17 00:00:00 2001 From: David Hayden Date: Tue, 19 Apr 2022 19:13:22 +0100 Subject: [PATCH 6/6] Only enrich if IsAllDataRequested --- src/Npgsql/NpgsqlActivitySource.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Npgsql/NpgsqlActivitySource.cs b/src/Npgsql/NpgsqlActivitySource.cs index aa17b94d88..b5b9777aa9 100644 --- a/src/Npgsql/NpgsqlActivitySource.cs +++ b/src/Npgsql/NpgsqlActivitySource.cs @@ -56,7 +56,7 @@ static NpgsqlActivitySource() default: throw new ArgumentOutOfRangeException("Invalid endpoint type: " + endPoint.GetType()); } - + Options.EnrichCommandExecution?.Invoke(activity, "OnStartActivity", command); return activity; @@ -72,7 +72,11 @@ internal static void CommandStop(Activity activity, NpgsqlCommand command) { activity.SetTag("otel.status_code", "OK"); activity.SetEndTime(DateTime.UtcNow); - Options.EnrichCommandExecution?.Invoke(activity, "OnStopActivity", command); + if (activity.IsAllDataRequested) + { + Options.EnrichCommandExecution?.Invoke(activity, "OnStopActivity", command); + } + activity.Dispose(); } @@ -90,7 +94,11 @@ internal static void SetException(Activity activity, NpgsqlCommand command, Exce activity.SetTag("otel.status_code", "ERROR"); activity.SetTag("otel.status_description", ex is PostgresException pgEx ? pgEx.SqlState : ex.Message); activity.SetEndTime(DateTime.UtcNow); - Options.EnrichCommandExecution?.Invoke(activity, "OnException", (command, ex)); + if (activity.IsAllDataRequested) + { + Options.EnrichCommandExecution?.Invoke(activity, "OnException", (command, ex)); + } + activity.Dispose(); } } \ No newline at end of file