feat: Added DB attributes to spans (#2583)

getsentry · Sep 6, 2023 · db270dc · db270dc
1 parent c2a31b4
commit db270dc
Show file tree

Hide file tree

Showing 16 changed files with 184 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Features
 
+- Added additional `DB` attributes to automatically generated spans like `name` and `provider` ([#2583](https://github.com/getsentry/sentry-dotnet/pull/2583))
 - `Hints` now accept attachments provided as a file path via `AddAttachment` method ([#2585](https://github.com/getsentry/sentry-dotnet/pull/2585))
 
 ### Fixes

diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/DiagnosticSourceConstants.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/DiagnosticSourceConstants.cs
@@ -7,6 +7,8 @@ namespace Sentry.Internal.DiagnosticSource;
 internal static class OTelKeys
 {
     internal const string DbName = "db.name";
+    internal const string DbSystem = "db.system";
+    internal const string DbServer = "db.server";
 }
 
 /// <summary>
@@ -27,3 +29,40 @@ internal static class EFKeys
     internal const string DbConnectionId = "db.connection_id";
     internal const string DbCommandId = "db.command_id";
 }
+
+/// <summary>
+/// Mapping of Database Providers to known Open Telemetry db.system
+/// https://learn.microsoft.com/en-us/ef/core/providers/?tabs=dotnet-core-cli
+/// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md#notes-and-well-known-identifiers-for-dbsystem
+/// </summary>
+internal static class DatabaseProviderSystems
+{
+    public static readonly Dictionary<string, string> ProviderSystems = new()
+    {
+        { "Microsoft.EntityFrameworkCore.SqlServer", "mssql" },
+        { "Microsoft.EntityFrameworkCore.Sqlite", "sqlite" },
+        { "Microsoft.EntityFrameworkCore.InMemory", "inmemory" },
+        { "Microsoft.EntityFrameworkCore.Cosmos", "cosmosdb" },
+        { "Npgsql.EntityFrameworkCore.PostgreSQL", "postgresql" },
+        { "Pomelo.EntityFrameworkCore.MySql", "mysql" },
+        { "MySql.EntityFrameworkCore", "mysql" },
+        { "Oracle.EntityFrameworkCore", "oracle" },
+        { "Devart.Data.MySql.EFCore", "mysql" },
+        { "Devart.Data.Oracle.EFCore", "oracle" },
+        { "Devart.Data.PostgreSql.EFCore", "postgres" },
+        { "Devart.Data.SQLite.EFCore", "sqlite" },
+        { "InterBase", "interbase" },
+        { "FirebirdSql.EntityFrameworkCore.Firebird", "firebird" },
+        { "IBM.EntityFrameworkCore", "db2" },
+        { "IBM.EntityFrameworkCore-lnx", "db2" },
+        { "IBM.EntityFrameworkCore-osx", "db2" },
+        { "EntityFrameworkCore.Jet", "accessfiles" },
+        { "Google.Cloud.EntityFrameworkCore.Spanner", "spanner" },
+        { "Teradata.EntityFrameworkCore", "teradata" },
+        { "FileContextCore", "datainfiles" },
+        { "FileBaseContext", "tablesinfiles" },
+        { "EntityFrameworkCore.SqlServerCompact35", "mssqlcompact" },
+        { "EntityFrameworkCore.SqlServerCompact40", "mssqlcompact" },
+        { "EntityFrameworkCore.OpenEdge", "progress" },
+    };
+}
diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFCommandDiagnosticSourceHelper.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFCommandDiagnosticSourceHelper.cs
@@ -40,11 +40,6 @@ private static void SetCommandId(ISpan span, Guid? commandId)
 
     protected override void SetSpanReference(ISpan span, object? diagnosticSourceValue)
     {
-        if (GetDatabaseName(diagnosticSourceValue) is { } databaseName)
-        {
-            span.SetExtra(OTelKeys.DbName, databaseName);
-        }
-
         if (GetCommandId(diagnosticSourceValue) is { } commandId)
         {
             SetCommandId(span, commandId);

diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFConnectionDiagnosticSourceHelper.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFConnectionDiagnosticSourceHelper.cs
@@ -29,11 +29,6 @@ internal EFConnectionDiagnosticSourceHelper(IHub hub, SentryOptions options) : b
 
     protected override void SetSpanReference(ISpan span, object? diagnosticSourceValue)
     {
-        if (GetDatabaseName(diagnosticSourceValue) is { } databaseName)
-        {
-            span.SetExtra(OTelKeys.DbName, databaseName);
-        }
-
         if (GetConnectionId(diagnosticSourceValue) is { } connectionId)
         {
             SetConnectionId(span, connectionId);

diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFDiagnosticSourceHelper.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/EFDiagnosticSourceHelper.cs
@@ -14,6 +14,25 @@ internal abstract class EFDiagnosticSourceHelper
     protected static string? GetDatabaseName(object? diagnosticSourceValue) =>
         diagnosticSourceValue?.GetStringProperty("Connection.Database");
 
+    protected static string? GetDatabaseSystem(object? diagnosticSourceValue)
+    {
+        var providerName = diagnosticSourceValue?.GetStringProperty("Context.Database.ProviderName");
+        if (providerName is null)
+        {
+            return null;
+        }
+
+        if (DatabaseProviderSystems.ProviderSystems.TryGetValue(providerName, out var dbSystem))
+        {
+            return dbSystem;
+        }
+
+        return null;
+    }
+
+    protected static string? GetDatabaseServerAddress(object? diagnosticSourceValue) =>
+        diagnosticSourceValue?.GetStringProperty("Connection.DataSource");
+
     internal EFDiagnosticSourceHelper(IHub hub, SentryOptions options)
     {
         Options = options;
@@ -46,6 +65,7 @@ internal void AddSpan(object? diagnosticSourceValue)
         var parent = Transaction.GetDbParentSpan();
         var child = parent.StartChild(Operation, GetDescription(diagnosticSourceValue));
 
+        SetDbData(child, diagnosticSourceValue);
         SetSpanReference(child, diagnosticSourceValue);
     }
 
@@ -69,6 +89,24 @@ internal void FinishSpan(object? diagnosticSourceValue, SpanStatus status)
         sourceSpan.Finish(status);
     }
 
+    protected void SetDbData(ISpan span, object? diagnosticSourceValue)
+    {
+        if (GetDatabaseName(diagnosticSourceValue) is { } dataBaseName)
+        {
+            span.SetExtra(OTelKeys.DbName, dataBaseName);
+        }
+
+        if (GetDatabaseSystem(diagnosticSourceValue) is { } databaseProviderName)
+        {
+            span.SetExtra(OTelKeys.DbSystem, databaseProviderName);
+        }
+
+        if (GetDatabaseServerAddress(diagnosticSourceValue) is { } databaseServerAddress)
+        {
+            span.SetExtra(OTelKeys.DbServer, databaseServerAddress);
+        }
+    }
+
     protected void LogTransactionSpans()
     {
         if (Transaction == null)
@@ -102,7 +140,7 @@ protected void LogTransactionSpans()
         return str?[(str.IndexOf('\n') + 1)..];
     }
 
-    protected abstract void SetSpanReference(ISpan span, object? diagnosticSourceValue);
-
     protected abstract ISpan? GetSpanReference(ITransaction transaction, object? diagnosticSourceValue);
+
+    protected abstract void SetSpanReference(ISpan span, object? diagnosticSourceValue);
 }
diff --git a/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs b/src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs
@@ -45,6 +45,13 @@ private static void SetDatabaseName(ISpan span, string databaseName)
         span.SetExtra(OTelKeys.DbName, databaseName);
     }
 
+    private static void SetDatabaseAddress(ISpan span, string databaseAddress)
+    {
+        Debug.Assert(databaseAddress != string.Empty);
+
+        span.SetExtra(OTelKeys.DbServer, databaseAddress);
+    }
+
     private static void SetConnectionId(ISpan span, Guid? connectionId)
     {
         Debug.Assert(connectionId != Guid.Empty);
@@ -73,6 +80,7 @@ private void AddSpan(string operation, object? value)
 
         var parent = transaction.GetDbParentSpan();
         var span = parent.StartChild(operation);
+        span.SetExtra(OTelKeys.DbSystem, "sql");
         SetOperationId(span, value?.GetGuidProperty("OperationId"));
         SetConnectionId(span, value?.GetGuidProperty("ConnectionId"));
     }
@@ -161,6 +169,11 @@ private void UpdateConnectionSpan(object? value)
                 connectionSpan.Description = dbName;
                 SetDatabaseName(connectionSpan, dbName);
             }
+
+            if (value.GetStringProperty("Connection.DataSource") is { } dbSource)
+            {
+                SetDatabaseAddress(connectionSpan, dbSource);
+            }
         }
     }
 

diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/LocalDbFixture.cs b/test/Sentry.DiagnosticSource.IntegrationTests/LocalDbFixture.cs
@@ -12,7 +12,7 @@ public LocalDbFixture()
         }
 
         SqlInstance = new(
-            name: "SqlListenerTests" + Namer.RuntimeAndVersion,
+            name: "SqlListenerTests",
             buildTemplate: TestDbBuilder.CreateTableAsync);
     }
 

diff --git a/...ry.DiagnosticSource.IntegrationTests/SqlListenerTests.LoggingAsync.DotNet6_0.verified.txt b/...ry.DiagnosticSource.IntegrationTests/SqlListenerTests.LoggingAsync.DotNet6_0.verified.txt
@@ -79,6 +79,8 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();,
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_LoggingAsync,
             db.operation_id: Guid_2,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: sql,
             rows_sent: 0
           }
         },
@@ -99,7 +101,8 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_LoggingAsync,
-            db.operation_id: Guid_3
+            db.operation_id: Guid_3,
+            db.system: sql
           }
         }
       ],

diff --git a/...ry.DiagnosticSource.IntegrationTests/SqlListenerTests.LoggingAsync.DotNet7_0.verified.txt b/...ry.DiagnosticSource.IntegrationTests/SqlListenerTests.LoggingAsync.DotNet7_0.verified.txt
@@ -79,6 +79,8 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();,
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_LoggingAsync,
             db.operation_id: Guid_2,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: sql,
             rows_sent: 0
           }
         },
@@ -99,7 +101,8 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_LoggingAsync,
-            db.operation_id: Guid_3
+            db.operation_id: Guid_3,
+            db.system: sql
           }
         }
       ],

diff --git a/...ry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.Core3_1.verified.txt b/...ry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.Core3_1.verified.txt
@@ -56,6 +56,8 @@
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
             db.operation_id: Guid_2,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: sql,
             rows_sent: 1
           }
         },
@@ -76,7 +78,8 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
-            db.operation_id: Guid_3
+            db.operation_id: Guid_3,
+            db.system: sql
           }
         },
         {
@@ -97,11 +100,12 @@ FROM [TestEntities] AS [t],
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
-            db.operation_id: Guid_4
+            db.operation_id: Guid_4,
+            db.system: sql
           }
         }
       ],
       IsFinished: true
     }
   }
-]
+]
diff --git a/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet6_0.verified.txt b/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet6_0.verified.txt
@@ -56,6 +56,8 @@
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
             db.operation_id: Guid_2,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: sql,
             rows_sent: 1
           }
         },
@@ -76,7 +78,8 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
-            db.operation_id: Guid_3
+            db.operation_id: Guid_3,
+            db.system: sql
           }
         },
         {
@@ -97,11 +100,12 @@ FROM [TestEntities] AS [t],
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
-            db.operation_id: Guid_4
+            db.operation_id: Guid_4,
+            db.system: sql
           }
         }
       ],
       IsFinished: true
     }
   }
-]
+]
diff --git a/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet7_0.verified.txt b/....DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.DotNet7_0.verified.txt
@@ -56,6 +56,8 @@
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
             db.operation_id: Guid_2,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: sql,
             rows_sent: 1
           }
         },
@@ -76,7 +78,8 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
-            db.operation_id: Guid_3
+            db.operation_id: Guid_3,
+            db.system: sql
           }
         },
         {
@@ -97,11 +100,12 @@ FROM [TestEntities] AS [t],
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsEfAsync,
-            db.operation_id: Guid_4
+            db.operation_id: Guid_4,
+            db.system: sql
           }
         }
       ],
       IsFinished: true
     }
   }
-]
+]
diff --git a/...try.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.Net4_8.verified.txt b/...try.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsEfAsync.Net4_8.verified.txt
@@ -59,7 +59,9 @@ WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();,
           Extra: {
             db.command_id: Guid_1,
             db.connection_id: Guid_2,
-            db.name: SqlListenerTests.verify_RecordsEfAsync
+            db.name: SqlListenerTests.verify_RecordsEfAsync,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: mssql
           }
         },
         {
@@ -73,11 +75,13 @@ FROM [TestEntities] AS [t],
           Extra: {
             db.command_id: Guid_3,
             db.connection_id: Guid_4,
-            db.name: SqlListenerTests.verify_RecordsEfAsync
+            db.name: SqlListenerTests.verify_RecordsEfAsync,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: mssql
           }
         }
       ],
       IsFinished: true
     }
   }
-]
+]
diff --git a/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSqlAsync.verified.txt b/test/Sentry.DiagnosticSource.IntegrationTests/SqlListenerTests.RecordsSqlAsync.verified.txt
@@ -56,6 +56,8 @@
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsSqlAsync,
             db.operation_id: Guid_2,
+            db.server: (LocalDb)\SqlListenerTests,
+            db.system: sql,
             rows_sent: 1
           }
         },
@@ -68,7 +70,8 @@
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsSqlAsync,
-            db.operation_id: Guid_3
+            db.operation_id: Guid_3,
+            db.system: sql
           }
         },
         {
@@ -80,11 +83,12 @@
           Extra: {
             db.connection_id: Guid_1,
             db.name: SqlListenerTests.verify_RecordsSqlAsync,
-            db.operation_id: Guid_4
+            db.operation_id: Guid_4,
+            db.system: sql
           }
         }
       ],
       IsFinished: true
     }
   }
-]
+]