Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix warning caused by db connection span closed prematurely #2068

Merged
merged 10 commits into from
Dec 2, 2022
Merged

Fix warning caused by db connection span closed prematurely #2068

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
db0a8dc
Capture db connection in integration tests
mattjohnsonpint Nov 29, 2022
bed4e57
Update verification snapshots
mattjohnsonpint Nov 29, 2022
176f523
Remove TransactionCommitAfter logic
mattjohnsonpint Nov 30, 2022
8974e67
Update tests
mattjohnsonpint Nov 30, 2022
0bb4eca
Update verification snapshots
mattjohnsonpint Nov 30, 2022
0b940a9
Update CHANGELOG.md
mattjohnsonpint Nov 30, 2022
91c3d80
.
mattjohnsonpint Nov 30, 2022
43025d3
.
mattjohnsonpint Nov 30, 2022
e01abcc
.
mattjohnsonpint Nov 30, 2022
8dbedec
Merge branch 'main' into diagnosticsource
mattjohnsonpint Dec 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Fix warning caused by db connection span closed prematurely ([#2068](https://github.com/getsentry/sentry-dotnet/pull/2068))

## 3.24.0

### Features
Expand Down
55 changes: 17 additions & 38 deletions src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ 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";

Expand Down Expand Up @@ -110,17 +107,15 @@ private void AddSpan(SentrySqlSpanType type, string operation, object? value)
return;
}

var operationId = kvp.Value?.GetGuidProperty("OperationId");

switch (type)
{
case SentrySqlSpanType.Execution:
{

var operationId = kvp.Value?.GetGuidProperty("OperationId");
if (TryGetQuerySpan(scope, operationId) is not { } querySpan)
{
_options.LogWarning(
"Trying to get a span of type {0} with operation id {1}, but it was not found.",
type,
"Trying to get an execution span with operation id {0}, but it was not found.",
operationId);
return;
}
Expand All @@ -137,35 +132,24 @@ span is SpanTracer executionTracer &&
}

break;
}

case SentrySqlSpanType.Connection:
switch (kvp.Key)

var connectionId = kvp.Value?.GetGuidProperty("ConnectionId");
if (TryGetConnectionSpan(scope, connectionId) is not { } connectionSpan)
{
case SqlMicrosoftWriteConnectionCloseAfterCommand or
SqlDataWriteConnectionCloseAfterCommand when
kvp.Value?.GetGuidProperty("ConnectionId") is { } id &&
TryGetConnectionSpan(scope, id) is { } connectionSpan:
span = connectionSpan;
break;

case SqlMicrosoftWriteTransactionCommitAfter or
SqlDataWriteTransactionCommitAfter when
kvp.Value?.GetProperty("Connection")?
.GetGuidProperty("ClientConnectionId") is { } commitId &&
TryGetConnectionSpan(scope, commitId) is { } commitSpan:
span = commitSpan;
break;

default:
_options.LogWarning(
"Trying to get a span of type {0} with operation id {1}, but it was not found.",
type, operationId);
break;
_options.LogWarning(
"Trying to get a connection span with connection id {0}, but it was not found.",
connectionId);
return;
}

span = connectionSpan;

break;
}
});

return span;
}

Expand Down Expand Up @@ -214,7 +198,7 @@ public void OnNext(KeyValuePair<string, object?> kvp)
{
switch (kvp.Key)
{
// Query.
// Query
case SqlMicrosoftBeforeExecuteCommand or SqlDataBeforeExecuteCommand:
AddSpan(SentrySqlSpanType.Execution, "db.query", kvp.Value);
return;
Expand All @@ -228,7 +212,8 @@ when GetSpan(SentrySqlSpanType.Execution, kvp) is { } errorSpan:
errorSpan.Description = kvp.Value?.GetProperty("Command")?.GetStringProperty("CommandText");
errorSpan.Finish(SpanStatus.InternalError);
return;
// Connection.

// Connection
case SqlMicrosoftWriteConnectionOpenBeforeCommand or SqlDataWriteConnectionOpenBeforeCommand:
AddSpan(SentrySqlSpanType.Connection, "db.connection", kvp.Value);
return;
Expand All @@ -240,12 +225,6 @@ when GetSpan(SentrySqlSpanType.Connection, kvp) is { } closeSpan:
TrySetConnectionStatistics(closeSpan, kvp.Value);
closeSpan.Finish(SpanStatus.Ok);
return;
case SqlMicrosoftWriteTransactionCommitAfter or SqlDataWriteTransactionCommitAfter
when GetSpan(SentrySqlSpanType.Connection, kvp) is { } commitSpan:
// If some query makes changes to the Database data, CloseAfterCommand event will not be invoked,
// instead, TransactionCommitAfter is invoked.
commitSpan.Finish(SpanStatus.Ok);
break;
mattjohnsonpint marked this conversation as resolved.
Show resolved Hide resolved
}
}
catch (Exception ex)
Expand Down
28 changes: 13 additions & 15 deletions .../Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.Logging.DotNet6_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,6 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
eventId: Microsoft.EntityFrameworkCore.Infrastructure.ContextInitialized
},
Category: Microsoft.EntityFrameworkCore.Infrastructure
},
{
Message:
Executed DbCommand
SET NOCOUNT ON;
INSERT INTO [TestEntities] ([Property])
VALUES (@p0);
SELECT [Id]
FROM [TestEntities]
WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();,
Data: {
eventId: Microsoft.EntityFrameworkCore.Database.Command.CommandExecuted
},
Category: Microsoft.EntityFrameworkCore.Database.Command
mattjohnsonpint marked this conversation as resolved.
Show resolved Hide resolved
}
],
Tags: {
Expand Down Expand Up @@ -78,6 +64,18 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();,
},
User: {},
Spans: [
{
IsFinished: true,
Operation: db.connection,
Status: Ok,
IsSampled: true,
Extra: {
bytes_sent : 510,
db.connection_id: Guid_1,
db.operation_id: Guid_2,
rows_sent: 0
}
},
mattjohnsonpint marked this conversation as resolved.
Show resolved Hide resolved
{
IsFinished: true,
Operation: db.query,
Expand All @@ -94,7 +92,7 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_2
db.operation_id: Guid_3
}
}
],
Expand Down
28 changes: 13 additions & 15 deletions .../Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.Logging.DotNet7_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,6 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
eventId: Microsoft.EntityFrameworkCore.Infrastructure.ContextInitialized
},
Category: Microsoft.EntityFrameworkCore.Infrastructure
},
{
Message:
Executed DbCommand
SET NOCOUNT ON;
INSERT INTO [TestEntities] ([Property])
VALUES (@p0);
SELECT [Id]
FROM [TestEntities]
WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();,
Data: {
eventId: Microsoft.EntityFrameworkCore.Database.Command.CommandExecuted
},
Category: Microsoft.EntityFrameworkCore.Database.Command
}
],
Tags: {
Expand Down Expand Up @@ -78,6 +64,18 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();,
},
User: {},
Spans: [
{
IsFinished: true,
Operation: db.connection,
Status: Ok,
IsSampled: true,
Extra: {
bytes_sent : 510,
db.connection_id: Guid_1,
db.operation_id: Guid_2,
rows_sent: 0
}
},
{
IsFinished: true,
Operation: db.query,
Expand All @@ -94,7 +92,7 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_2
db.operation_id: Guid_3
}
}
],
Expand Down
17 changes: 15 additions & 2 deletions .../Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.Core3_1.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@
},
User: {},
Spans: [
{
IsFinished: true,
Operation: db.connection,
Status: Ok,
IsSampled: true,
Extra: {
bytes_received: 304,
bytes_sent : 704,
db.connection_id: Guid_1,
db.operation_id: Guid_2,
rows_sent: 1
}
},
{
IsFinished: true,
Operation: db.query,
Expand All @@ -61,7 +74,7 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_2
db.operation_id: Guid_3
}
},
{
Expand All @@ -81,7 +94,7 @@ FROM [TestEntities] AS [t],
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_3
db.operation_id: Guid_4
}
}
],
Expand Down
17 changes: 15 additions & 2 deletions ...entry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.DotNet6_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@
},
User: {},
Spans: [
{
IsFinished: true,
Operation: db.connection,
Status: Ok,
IsSampled: true,
Extra: {
bytes_received: 304,
bytes_sent : 704,
db.connection_id: Guid_1,
db.operation_id: Guid_2,
rows_sent: 1
}
},
{
IsFinished: true,
Operation: db.query,
Expand All @@ -61,7 +74,7 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_2
db.operation_id: Guid_3
}
},
{
Expand All @@ -81,7 +94,7 @@ FROM [TestEntities] AS [t],
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_3
db.operation_id: Guid_4
}
}
],
Expand Down
17 changes: 15 additions & 2 deletions ...entry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEf.DotNet7_0.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@
},
User: {},
Spans: [
{
IsFinished: true,
Operation: db.connection,
Status: Ok,
IsSampled: true,
Extra: {
bytes_received: 304,
bytes_sent : 704,
db.connection_id: Guid_1,
db.operation_id: Guid_2,
rows_sent: 1
}
},
{
IsFinished: true,
Operation: db.query,
Expand All @@ -61,7 +74,7 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_2
db.operation_id: Guid_3
}
},
{
Expand All @@ -81,7 +94,7 @@ FROM [TestEntities] AS [t],
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_3
db.operation_id: Guid_4
}
}
],
Expand Down
17 changes: 15 additions & 2 deletions test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSql.verified.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,19 @@
},
User: {},
Spans: [
{
IsFinished: true,
Operation: db.connection,
Status: Ok,
IsSampled: true,
Extra: {
bytes_received: 167,
bytes_sent : 542,
db.connection_id: Guid_1,
db.operation_id: Guid_2,
rows_sent: 1
}
},
{
IsFinished: true,
Operation: db.query,
Expand All @@ -55,7 +68,7 @@ values (@value);,
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_2
db.operation_id: Guid_3
}
},
{
Expand All @@ -66,7 +79,7 @@ values (@value);,
IsSampled: true,
Extra: {
db.connection_id: Guid_1,
db.operation_id: Guid_3
db.operation_id: Guid_4
}
}
],
Expand Down
Loading