diff --git a/CHANGELOG.md b/CHANGELOG.md index 82b986cae4..ba87f09a7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Fix SQLClient unplanned behaviors ([#1179](https://github.com/getsentry/sentry-dotnet/pull/1179)) - Add fallback to Scope Stack from AspNet ([#1180](https://github.com/getsentry/sentry-dotnet/pull/1180)) ## 3.9.0 diff --git a/src/Sentry.DiagnosticSource/Internals/DiagnosticSource/SentrySqlListener.cs b/src/Sentry.DiagnosticSource/Internals/DiagnosticSource/SentrySqlListener.cs index b440298b19..01202439b8 100644 --- a/src/Sentry.DiagnosticSource/Internals/DiagnosticSource/SentrySqlListener.cs +++ b/src/Sentry.DiagnosticSource/Internals/DiagnosticSource/SentrySqlListener.cs @@ -28,6 +28,9 @@ private enum SentrySqlSpanType internal const string SqlMicrosoftWriteConnectionCloseAfterCommand = "Microsoft.Data.SqlClient.WriteConnectionCloseAfter"; internal const string SqlDataWriteConnectionCloseAfterCommand = "System.Data.SqlClient.WriteConnectionCloseAfter"; + internal const string SqlMicrosoftWriteTransactionCommitAfter = "Microsoft.Data.SqlClient.WriteTransactionCommitAfter"; + internal const string SqlDataWriteTransactionCommitAfter = "System.Data.SqlClient.WriteTransactionCommitAfter"; + internal const string SqlDataBeforeExecuteCommand = "System.Data.SqlClient.WriteCommandBefore"; internal const string SqlMicrosoftBeforeExecuteCommand = "Microsoft.Data.SqlClient.WriteCommandBefore"; @@ -46,22 +49,57 @@ public SentrySqlListener(IHub hub, SentryOptions options) _options = options; } + private void SetConnectionId(ISpan span, Guid? connectionId) + { + span.SetExtra(ConnectionExtraKey, connectionId); + } + + private void SetOperationId(ISpan span, Guid? operationId) + { + span.SetExtra(OperationExtraKey, operationId); + } + + private Guid? TryGetOperationId(ISpan span) + { + if (span.Extra.TryGetValue(OperationExtraKey, out var key) && key is Guid guid) + { + return guid; + } + return null; + } + + private Guid? TryGetConnectionId(ISpan span) + { + if (span.Extra.TryGetValue(ConnectionExtraKey, out var key) && key is Guid guid) + { + return guid; + } + return null; + } + private void AddSpan(SentrySqlSpanType type, string operation, string? description, Guid operationId, Guid? connectionId = null) { _hub.ConfigureScope(scope => { - if (type == SentrySqlSpanType.Connection && - scope.Transaction?.StartChild(operation, description) is { } connectionSpan) + if (scope.Transaction is { } transaction) { - connectionSpan.SetExtra(OperationExtraKey, operationId); - // ConnectionId is set afterwards. - } - else if (type == SentrySqlSpanType.Execution && - connectionId != null && - TryGetConnectionSpan(scope, connectionId.Value) is { } parentSpan) - { - var span = parentSpan.StartChild(operation, description); - span.SetExtra(OperationExtraKey, operationId); + if (type == SentrySqlSpanType.Connection && + transaction?.StartChild(operation, description) is { } connectionSpan) + { + SetOperationId(connectionSpan, operationId); + } + else if (type == SentrySqlSpanType.Execution && connectionId != null) + { + var span = TryStartChild( + TryGetConnectionSpan(scope, connectionId.Value) ?? transaction, + operation, + description); + if (span is not null) + { + SetOperationId(span, operationId); + SetConnectionId(span, connectionId); + } + } } }); } @@ -76,6 +114,16 @@ operationId is { } queryId && TryGetQuerySpan(scope, queryId) is { } querySpan) { span = querySpan; + + if (span.ParentSpanId == scope.Transaction?.SpanId && + TryGetConnectionId(span) is { } spanConnectionId && + spanConnectionId is Guid spanConnectionGuid && + span is SpanTracer executionTracer && + TryGetConnectionSpan(scope, spanConnectionGuid) is { } spanConnectionRef) + { + // Connection Span exist but wasn't set as the parent of the current Span. + executionTracer.ParentSpanId = spanConnectionRef.SpanId; + } } else if (type == SentrySqlSpanType.Connection && connectionId is { } id && @@ -91,18 +139,27 @@ connectionId is { } id && return span; } + private ISpan? TryStartChild(ISpan? parent, string operation, string? description) + => parent?.StartChild(operation, description); + private ISpan? TryGetConnectionSpan(Scope scope, Guid connectionId) - => scope.Transaction?.Spans.FirstOrDefault(span => TryGetKey(span.Extra, ConnectionExtraKey) is Guid id && id == connectionId); + => scope.Transaction?.Spans.FirstOrDefault(span => span.Operation is "db.connection" && TryGetConnectionId(span) == connectionId); private ISpan? TryGetQuerySpan(Scope scope, Guid operationId) - => scope.Transaction?.Spans.FirstOrDefault(span => TryGetKey(span.Extra, OperationExtraKey) is Guid id && id == operationId); + => scope.Transaction?.Spans.FirstOrDefault(span => TryGetOperationId(span) == operationId); private void UpdateConnectionSpan(Guid operationId, Guid connectionId) { _hub.ConfigureScope(scope => { - var span = scope.Transaction?.Spans.FirstOrDefault(span => TryGetKey(span.Extra, OperationExtraKey) is Guid id && id == operationId); - span?.SetExtra(ConnectionExtraKey, connectionId); + // We may have multiple Spans with different Operations for the same connection. + // So lets set the connection Id only if there are no connection spans with the same connectionId. + var connectionSpans = scope.Transaction?.Spans?.Where(span => span.Operation is "db.connection").ToList(); + if (connectionSpans?.Any(span => TryGetConnectionId(span) == connectionId) is false && + connectionSpans.FirstOrDefault(span => TryGetOperationId(span) == operationId) is { } span) + { + SetConnectionId(span, connectionId); + } }); } @@ -148,6 +205,13 @@ public void OnNext(KeyValuePair value) TrySetConnectionStatistics(connectionSpan, value); connectionSpan.Finish(SpanStatus.Ok); } + else if ((value.Key is SqlMicrosoftWriteTransactionCommitAfter || value.Key is SqlDataWriteTransactionCommitAfter) && + GetSpan(SentrySqlSpanType.Connection, null, value.GetSubProperty("Connection", "ClientConnectionId")) is { } connectionSpan2) + { + // If some query makes changes to the Database data, CloseAfterCommand event will not be invoked, + // instead, TransactionCommitAfter is invoked. + connectionSpan2.Finish(SpanStatus.Ok); + } } catch (Exception ex) { @@ -173,11 +237,5 @@ private void TrySetConnectionStatistics(ISpan span, KeyValuePair dictionary, string key) - { - var found = dictionary.TryGetValue(key, out var result); - return found ? result : null; - } } } diff --git a/src/Sentry/SpanTracer.cs b/src/Sentry/SpanTracer.cs index 4300c600e8..6bcb5898b8 100644 --- a/src/Sentry/SpanTracer.cs +++ b/src/Sentry/SpanTracer.cs @@ -16,7 +16,7 @@ public class SpanTracer : ISpan public SpanId SpanId { get; } /// - public SpanId? ParentSpanId { get; } + public SpanId? ParentSpanId { get; internal set; } /// public SentryId TraceId { get; } @@ -55,14 +55,14 @@ public void SetTag(string key, string value) => public void UnsetTag(string key) => (_tags ??= new ConcurrentDictionary()).TryRemove(key, out _); - private ConcurrentDictionary? _data; + private ConcurrentDictionary _data = new(); /// - public IReadOnlyDictionary Extra => _data ??= new ConcurrentDictionary(); + public IReadOnlyDictionary Extra => _data; /// public void SetExtra(string key, object? value) => - (_data ??= new ConcurrentDictionary())[key] = value; + _data[key] = value; /// /// Initializes an instance of . diff --git a/test/Sentry.DiagnosticSource.Tests/DiagnosticsSentryOptionsExtensionsTests.cs b/test/Sentry.DiagnosticSource.Tests/DiagnosticsSentryOptionsExtensionsTests.cs index cdd3925a31..505b48b63d 100644 --- a/test/Sentry.DiagnosticSource.Tests/DiagnosticsSentryOptionsExtensionsTests.cs +++ b/test/Sentry.DiagnosticSource.Tests/DiagnosticsSentryOptionsExtensionsTests.cs @@ -1,7 +1,7 @@ using Sentry.Internals.DiagnosticSource; using Xunit; -namespace Sentry.Diagnostics.DiagnosticSource.Internals +namespace Sentry.DiagnosticSource.Internals { public class DiagnosticsSentryOptionsExtensionsTests { diff --git a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Database.cs b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Database.cs index cc468bcca6..c8a3f5ce6d 100644 --- a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Database.cs +++ b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Database.cs @@ -3,7 +3,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; -namespace Sentry.Diagnostics.DiagnosticSource.Tests.Integration.SQLite +namespace Sentry.DiagnosticSource.Tests.Integration.SQLite { public class Database { diff --git a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Item.cs b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Item.cs index b48f1ba010..8a02e26824 100644 --- a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Item.cs +++ b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/Item.cs @@ -1,4 +1,4 @@ -namespace Sentry.Diagnostics.DiagnosticSource.Tests.Integration.SQLite +namespace Sentry.DiagnosticSource.Tests.Integration.SQLite { public class Item { diff --git a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/ItemsContext.cs b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/ItemsContext.cs index 047c90dc78..a8b43dfc70 100644 --- a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/ItemsContext.cs +++ b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/ItemsContext.cs @@ -1,6 +1,6 @@ using Microsoft.EntityFrameworkCore; -namespace Sentry.Diagnostics.DiagnosticSource.Tests.Integration.SQLite +namespace Sentry.DiagnosticSource.Tests.Integration.SQLite { public class ItemsContext : DbContext { diff --git a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/SentryDiagnosticListenerTests.cs b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/SentryDiagnosticListenerTests.cs index 8c260fd98c..f0bc5cb916 100644 --- a/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/SentryDiagnosticListenerTests.cs +++ b/test/Sentry.DiagnosticSource.Tests/Integration/SQLite/SentryDiagnosticListenerTests.cs @@ -11,7 +11,7 @@ using Sentry.Internals.DiagnosticSource; using Xunit; -namespace Sentry.Diagnostics.DiagnosticSource.Tests.Integration.SQLite +namespace Sentry.DiagnosticSource.Tests.Integration.SQLite { public class SentryDiagnosticListenerTests { diff --git a/test/Sentry.DiagnosticSource.Tests/Sentry.DiagnosticSource.Tests.csproj b/test/Sentry.DiagnosticSource.Tests/Sentry.DiagnosticSource.Tests.csproj index 500196781e..5c2fd2ac7c 100644 --- a/test/Sentry.DiagnosticSource.Tests/Sentry.DiagnosticSource.Tests.csproj +++ b/test/Sentry.DiagnosticSource.Tests/Sentry.DiagnosticSource.Tests.csproj @@ -22,6 +22,7 @@ + diff --git a/test/Sentry.DiagnosticSource.Tests/SentryEFCoreListenerTests.cs b/test/Sentry.DiagnosticSource.Tests/SentryEFCoreListenerTests.cs index 8d341cb8a4..923e221335 100644 --- a/test/Sentry.DiagnosticSource.Tests/SentryEFCoreListenerTests.cs +++ b/test/Sentry.DiagnosticSource.Tests/SentryEFCoreListenerTests.cs @@ -5,7 +5,7 @@ using Sentry.Internals.DiagnosticSource; using Xunit; -namespace Sentry.Diagnostics.DiagnosticSource.Tests +namespace Sentry.DiagnosticSource.Tests { public class SentryEFCoreListenerTests { diff --git a/test/Sentry.DiagnosticSource.Tests/SentrySqlListenerTests.cs b/test/Sentry.DiagnosticSource.Tests/SentrySqlListenerTests.cs index cd10fbc32e..a9a1cf67cb 100644 --- a/test/Sentry.DiagnosticSource.Tests/SentrySqlListenerTests.cs +++ b/test/Sentry.DiagnosticSource.Tests/SentrySqlListenerTests.cs @@ -1,13 +1,49 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using FluentAssertions; using NSubstitute; using Sentry.Internals.DiagnosticSource; +using Sentry.Testing; using Xunit; -namespace Sentry.Diagnostics.DiagnosticSource.Tests +namespace Sentry.DiagnosticSource.Tests { + internal static class SentrySqlListenerExtensions + { + public static void OpenConnectionStart(this SentrySqlListener listener, Guid operationId) + => listener.OnNext(new KeyValuePair( + SentrySqlListener.SqlMicrosoftWriteConnectionOpenBeforeCommand, + new { OperationId = operationId })); + + public static void OpenConnectionStarted(this SentrySqlListener listener, Guid operationId, Guid connectionId) + => listener.OnNext(new KeyValuePair( + SentrySqlListener.SqlMicrosoftWriteConnectionOpenAfterCommand, + new { OperationId = operationId, ConnectionId = connectionId })); + + public static void OpenConnectionClose(this SentrySqlListener listener, Guid operationId, Guid connectionId) + => listener.OnNext(new KeyValuePair( + SentrySqlListener.SqlMicrosoftWriteConnectionCloseAfterCommand, + new { OperationId = operationId, ConnectionId = connectionId })); + + public static void ExecuteQueryStart(this SentrySqlListener listener, Guid operationId, Guid connectionId) + => listener.OnNext(new KeyValuePair( + SentrySqlListener.SqlDataBeforeExecuteCommand, + new { OperationId = operationId, ConnectionId = connectionId })); + + public static void ExecuteQueryFinish(this SentrySqlListener listener, Guid operationId, Guid connectionId, string query) + => listener.OnNext(new KeyValuePair( + SentrySqlListener.SqlDataAfterExecuteCommand, + new { OperationId = operationId, ConnectionId = connectionId, Command = new { CommandText = query } })); + + public static void ExecuteQueryFinishWithError(this SentrySqlListener listener, Guid operationId, Guid connectionId, string query) + => listener.OnNext(new KeyValuePair( + SentrySqlListener.SqlDataWriteCommandError, + new { OperationId = operationId, ConnectionId = connectionId, Command = new { CommandText = query } })); + } + public class SentrySqlListenerTests { internal const string SqlDataWriteConnectionOpenBeforeCommand = SentrySqlListener.SqlDataWriteConnectionOpenBeforeCommand; @@ -28,6 +64,9 @@ public class SentrySqlListenerTests internal const string SqlDataWriteCommandError = SentrySqlListener.SqlDataWriteCommandError; internal const string SqlMicrosoftWriteCommandError = SentrySqlListener.SqlMicrosoftWriteCommandError; + internal const string SqlDataWriteTransactionCommitAfter = SentrySqlListener.SqlDataWriteTransactionCommitAfter; + internal const string SqlMicrosoftWriteTransactionCommitAfter = SentrySqlListener.SqlMicrosoftWriteTransactionCommitAfter; + private Func GetValidator(string type) => type switch { @@ -37,7 +76,9 @@ _ when type == SqlMicrosoftWriteConnectionOpenAfterCommand || type == SqlDataWriteConnectionOpenAfterCommand || type == SqlMicrosoftWriteConnectionCloseAfterCommand || - type == SqlDataWriteConnectionCloseAfterCommand + type == SqlDataWriteConnectionCloseAfterCommand || + type == SqlDataWriteTransactionCommitAfter || + type == SqlMicrosoftWriteTransactionCommitAfter => span => span.Description is null && span.Operation == "db.connection", _ when type == SqlDataBeforeExecuteCommand || @@ -66,10 +107,21 @@ private class Fixture private Scope _scope { get; } internal TransactionTracer Tracer { get; } + public InMemoryDiagnosticLogger Logger { get; } + + public SentryOptions Options { get; } + public IReadOnlyCollection Spans => Tracer?.Spans; public IHub Hub { get; set; } public Fixture() { + Logger = new InMemoryDiagnosticLogger(); + Options = new SentryOptions() + { + Debug = true, + DiagnosticLogger = Logger, + DiagnosticLevel = SentryLevel.Debug + }; Tracer = new TransactionTracer(Hub, "foo", "bar"); _scope = new Scope(); _scope.Transaction = Tracer; @@ -121,8 +173,10 @@ public void OnNext_KnownKey_GetSpanInvoked(string key, bool addConnectionSpan) Assert.True(GetValidator(key)(_fixture.Spans.First())); } - [Fact] - public void OnNext_HappyPath_IsValid() + [Theory] + [InlineData(SqlMicrosoftWriteConnectionOpenBeforeCommand, SqlMicrosoftWriteConnectionOpenAfterCommand, SqlMicrosoftWriteConnectionCloseAfterCommand, SqlMicrosoftBeforeExecuteCommand, SqlMicrosoftAfterExecuteCommand)] + [InlineData(SqlDataWriteConnectionOpenBeforeCommand, SqlDataWriteConnectionOpenAfterCommand, SqlDataWriteConnectionCloseAfterCommand, SqlDataBeforeExecuteCommand, SqlDataAfterExecuteCommand)] + public void OnNext_HappyPathsWithoutTransaction_IsValid(string connectionOpenKey, string connectionUpdateKey, string connectionCloseKey, string queryStartKey, string queryEndKey) { // Arrange var hub = _fixture.Hub; @@ -135,25 +189,126 @@ public void OnNext_HappyPath_IsValid() // Act interceptor.OnNext( - new(SqlMicrosoftWriteConnectionOpenBeforeCommand, + new(connectionOpenKey, new { OperationId = connectionOperationId })); interceptor.OnNext( - new(SqlMicrosoftWriteConnectionOpenAfterCommand, + new(connectionUpdateKey, new { OperationId = connectionOperationId, ConnectionId = connectionId })); interceptor.OnNext( - new(SqlMicrosoftBeforeExecuteCommand, + new(queryStartKey, new { OperationId = queryOperationId, ConnectionId = connectionId })); interceptor.OnNext( - new(SqlMicrosoftAfterExecuteCommand, + new(queryEndKey, new { OperationId = queryOperationId, ConnectionId = connectionId, Command = new { CommandText = query } })); interceptor.OnNext( - new(SqlMicrosoftWriteConnectionCloseAfterCommand, + new(connectionCloseKey, new { OperationId = connectionOperationIdClosed, ConnectionId = connectionId })); + //Connection", "ClientConnectionId + // Assert + _fixture.Spans.Should().HaveCount(2); + var connectionSpan = _fixture.Spans.First(s => GetValidator(connectionOpenKey)(s)); + var commandSpan = _fixture.Spans.First(s => GetValidator(queryStartKey)(s)); + + // Validate if all spans were finished. + Assert.All(_fixture.Spans, (span) => + { + Assert.True(span.IsFinished); + Assert.Equal(SpanStatus.Ok, span.Status); + }); + // Check connections between spans. + Assert.Equal(_fixture.Tracer.SpanId, connectionSpan.ParentSpanId); + Assert.Equal(connectionSpan.SpanId, commandSpan.ParentSpanId); + + Assert.Equal(query, commandSpan.Description); + } + + [Theory] + [InlineData(SqlMicrosoftWriteConnectionOpenBeforeCommand, SqlMicrosoftWriteConnectionOpenAfterCommand, SqlMicrosoftWriteTransactionCommitAfter, SqlMicrosoftBeforeExecuteCommand, SqlMicrosoftAfterExecuteCommand)] + [InlineData(SqlDataWriteConnectionOpenBeforeCommand, SqlDataWriteConnectionOpenAfterCommand, SqlDataWriteTransactionCommitAfter, SqlDataBeforeExecuteCommand, SqlDataAfterExecuteCommand)] + public void OnNext_HappyPathsWithTransaction_IsValid(string connectionOpenKey, string connectionUpdateKey, string connectionCloseKey, string queryStartKey, string queryEndKey) + { + // Arrange + var hub = _fixture.Hub; + var interceptor = new SentrySqlListener(hub, new SentryOptions()); + var query = "SELECT * FROM ..."; + var connectionId = Guid.NewGuid(); + var connectionOperationId = Guid.NewGuid(); + var connectionOperationIdClosed = Guid.NewGuid(); + var queryOperationId = Guid.NewGuid(); + + // Act + interceptor.OnNext( + new(connectionOpenKey, + new { OperationId = connectionOperationId })); + interceptor.OnNext( + new(connectionUpdateKey, + new { OperationId = connectionOperationId, ConnectionId = connectionId })); + interceptor.OnNext( + new(queryStartKey, + new { OperationId = queryOperationId, ConnectionId = connectionId })); + interceptor.OnNext( + new(queryEndKey, + new { OperationId = queryOperationId, ConnectionId = connectionId, Command = new { CommandText = query } })); + interceptor.OnNext( + new(connectionCloseKey, + new { OperationId = connectionOperationIdClosed, Connection = new { ClientConnectionId = connectionId } })); // Assert _fixture.Spans.Should().HaveCount(2); - var connectionSpan = _fixture.Spans.First(s => GetValidator(SqlMicrosoftWriteConnectionOpenBeforeCommand)(s)); - var commandSpan = _fixture.Spans.First(s => GetValidator(SqlMicrosoftBeforeExecuteCommand)(s)); + var connectionSpan = _fixture.Spans.First(s => GetValidator(connectionOpenKey)(s)); + var commandSpan = _fixture.Spans.First(s => GetValidator(queryStartKey)(s)); + + // Validate if all spans were finished. + Assert.All(_fixture.Spans, (span) => + { + Assert.True(span.IsFinished); + Assert.Equal(SpanStatus.Ok, span.Status); + }); + // Check connections between spans. + Assert.Equal(_fixture.Tracer.SpanId, connectionSpan.ParentSpanId); + Assert.Equal(connectionSpan.SpanId, commandSpan.ParentSpanId); + + Assert.Equal(query, commandSpan.Description); + } + + + [Theory] + [InlineData(SqlMicrosoftWriteConnectionOpenBeforeCommand, SqlMicrosoftWriteConnectionOpenAfterCommand, SqlMicrosoftWriteConnectionCloseAfterCommand, SqlMicrosoftBeforeExecuteCommand, SqlMicrosoftAfterExecuteCommand)] + [InlineData(SqlDataWriteConnectionOpenBeforeCommand, SqlDataWriteConnectionOpenAfterCommand, SqlDataWriteConnectionCloseAfterCommand, SqlDataBeforeExecuteCommand, SqlDataAfterExecuteCommand)] + public void OnNext_ExecuteQueryCalledBeforeConnectionId_ExecuteParentIsConnectionSpan(string connectionBeforeKey, string connectionUpdate, string connctionClose, string executeBeforeKey, string executeAfterKey) + { + // Arrange + var hub = _fixture.Hub; + var interceptor = new SentrySqlListener(hub, new SentryOptions()); + var query = "SELECT * FROM ..."; + var connectionId = Guid.NewGuid(); + var connectionOperationId = Guid.NewGuid(); + var queryOperationId = Guid.NewGuid(); + + // Act + interceptor.OnNext( + new(connectionBeforeKey, + new { OperationId = connectionOperationId })); + // Connection span has no connection ID and query will temporarily have Transaction as parent. + interceptor.OnNext( + new(executeBeforeKey, + new { OperationId = queryOperationId, ConnectionId = connectionId })); + // Connection Id is set. + interceptor.OnNext( + new(connectionUpdate, + new { OperationId = connectionOperationId, ConnectionId = connectionId })); + // Query sets ParentId to ConnectionId. + interceptor.OnNext( + new(executeAfterKey, + new { OperationId = queryOperationId, ConnectionId = connectionId, Command = new { CommandText = query } })); + interceptor.OnNext( + new(connctionClose, + new { OperationId = connectionOperationId, ConnectionId = connectionId })); + + // Assert + _fixture.Spans.Should().HaveCount(2); + var connectionSpan = _fixture.Spans.First(s => GetValidator(connectionBeforeKey)(s)); + var commandSpan = _fixture.Spans.First(s => GetValidator(executeBeforeKey)(s)); // Validate if all spans were finished. Assert.All(_fixture.Spans, span => @@ -168,6 +323,107 @@ public void OnNext_HappyPath_IsValid() Assert.Equal(query, commandSpan.Description); } + [Theory] + [InlineData(1)] + [InlineData(2)] + [InlineData(3)] + [InlineData(4)] + [InlineData(5)] + [InlineData(6)] + [InlineData(7)] + [InlineData(8)] + [InlineData(9)] + [InlineData(10)] + [InlineData(11)] + [InlineData(12)] + [InlineData(13)] + [InlineData(14)] + [InlineData(15)] + [InlineData(16)] + [InlineData(17)] + [InlineData(18)] + [InlineData(19)] + public async Task OnNext_ParallelExecution_IsValid(int testNumber) + { + _ = testNumber; + // Arrange + var hub = _fixture.Hub; + var interceptor = new SentrySqlListener(hub, _fixture.Options); + int maxItems = 8; + var query = "SELECT * FROM ..."; + var connectionsIds = Enumerable.Range(0, maxItems).Select((_) => Guid.NewGuid()).ToList(); + var connectionOperationsIds = Enumerable.Range(0, maxItems).Select((_) => Guid.NewGuid()).ToList(); + var connectionOperations2Ids = Enumerable.Range(0, maxItems).Select((_) => Guid.NewGuid()).ToList(); + var queryOperationsIds = Enumerable.Range(0, maxItems).Select((_) => Guid.NewGuid()).ToList(); + var queryOperations2Ids = Enumerable.Range(0, maxItems).Select((_) => Guid.NewGuid()).ToList(); + var evt = new ManualResetEvent(false); + var ready = new ManualResetEvent(false); + int counter = 0; + + // Act + var taskList = Enumerable.Range(1, maxItems).Select((_) => Task.Run(() => + { + var threadId = Interlocked.Increment(ref counter) - 1; + + if (threadId == maxItems - 1) + { + ready.Set(); + } + evt.WaitOne(); + + // 1 repeated connection with 1 query where the first query will start before connection span gets the connectionId. + void SimulateDbRequest(List connnectionOperationIds, List queryOperationIds) + { + interceptor.OpenConnectionStart(connnectionOperationIds[threadId]); + interceptor.ExecuteQueryStart(queryOperationIds[threadId], connectionsIds[threadId]); + interceptor.OpenConnectionStarted(connnectionOperationIds[threadId], connectionsIds[threadId]); + interceptor.ExecuteQueryFinish(queryOperationIds[threadId], connectionsIds[threadId], query); + interceptor.OpenConnectionClose(connnectionOperationIds[threadId], connectionsIds[threadId]); + } + SimulateDbRequest(connectionOperationsIds, queryOperationsIds); + SimulateDbRequest(connectionOperations2Ids, queryOperations2Ids); + })).ToList(); + ready.WaitOne(); + evt.Set(); + await Task.WhenAll(taskList); + + // Assert + // 1 connection span and 1 query span, executed twice for 11 threads. + _fixture.Spans.Should().HaveCount(2 * 2 * maxItems); + + var openSpans = _fixture.Spans.Where(span => span.IsFinished is false); + var closedSpans = _fixture.Spans.Where(span => span.IsFinished is true); + var connectionSpans = _fixture.Spans.Where(span => span.Operation is "db.connection"); + var closedConnectionSpans = connectionSpans.Where(span => span.IsFinished); + var querySpans = _fixture.Spans.Where(span => span.Operation is "db.query"); + + // We have two connections per thread, but since both share the same ConnectionId, only one will be closed. + closedConnectionSpans.Should().HaveCount(maxItems); + querySpans.Should().HaveCount(2 * maxItems); + + // Open Spans should not have any Connection key. + Assert.All(openSpans, (span) => Assert.False(span.Extra.ContainsKey(SentrySqlListener.ConnectionExtraKey))); + Assert.All(closedSpans, (span) => Assert.Equal(SpanStatus.Ok, span.Status)); + // Assert that all connectionIds will belong to a single connection Span and ParentId set to Trace. + Assert.All(connectionsIds, (connectionId) => + { + var connectionSpan = Assert.Single(closedConnectionSpans.Where(span => (Guid)span.Extra[SentrySqlListener.ConnectionExtraKey] == connectionId)); + Assert.Equal(_fixture.Tracer.SpanId, connectionSpan.ParentSpanId); + }); + // Assert all Query spans have the correct ParentId Set and not the Transaction Id. + Assert.All(querySpans, (querySpan) => + { + Assert.True(querySpan.IsFinished); + var connectionId = (Guid)querySpan.Extra[SentrySqlListener.ConnectionExtraKey]; + var connectionSpan = connectionSpans.Where(span => span.Extra.ContainsKey(SentrySqlListener.ConnectionExtraKey) && (Guid)span.Extra[SentrySqlListener.ConnectionExtraKey] == connectionId); + Assert.Single(connectionSpan); + Assert.NotEqual(_fixture.Tracer.SpanId, querySpan.ParentSpanId); + Assert.Equal(connectionSpan.First().SpanId, querySpan.ParentSpanId); + }); + + _fixture.Logger.Entries.Should().BeEmpty(); + } + [Fact] public void OnNext_HappyPathWithError_TransactionWithErroredCommand() { diff --git a/test/Sentry.Tests/Sentry.Tests.csproj b/test/Sentry.Tests/Sentry.Tests.csproj index 48e62cc2ae..aba1ba200a 100644 --- a/test/Sentry.Tests/Sentry.Tests.csproj +++ b/test/Sentry.Tests/Sentry.Tests.csproj @@ -42,7 +42,7 @@ - + Internals/DiagnosticSource/%(Filename)%(Extension) diff --git a/test/Sentry.Tests/SpanTracerTests.cs b/test/Sentry.Tests/SpanTracerTests.cs new file mode 100644 index 0000000000..5ab7bb8603 --- /dev/null +++ b/test/Sentry.Tests/SpanTracerTests.cs @@ -0,0 +1,50 @@ +using System; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using NSubstitute; +using Xunit; + +namespace Sentry.Tests +{ + public class SpanTracerTests + { + [Fact] + public async Task SetExtra_DataInserted_NoDataLoss() + { + // Run 20 times to avoid flacky tests scapping. + for (int amount = 0; amount < 20; amount++) + { + // Arrange + var hub = Substitute.For(); + var transaction = new SpanTracer(hub, null, null, SentryId.Empty, ""); + var evt = new ManualResetEvent(false); + var ready = new ManualResetEvent(false); + int counter = 0; + // Act + var tasks = Enumerable.Range(1, 4).Select((_) => Task.Run(() => + { + var threadId = Interlocked.Increment(ref counter); + + if (threadId == 4) + { + ready.Set(); + } + evt.WaitOne(); + + for (int i = 0; i < amount; i++) + { + transaction.SetExtra(Guid.NewGuid().ToString(), Guid.NewGuid()); + } + })).ToList(); + ready.WaitOne(); + evt.Set(); + await Task.WhenAll(tasks); + + // Arrange + // 4 tasks testing X amount should be the same amount as Extras. + Assert.Equal(4 * amount, transaction.Extra.Count); + } + } + } +}