diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index bc04010a01..b15dade3da 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Support use with `SqlClient` instrumentation. + ([#2280](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2280), + [#2829](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2829)) + ## 1.12.0-beta.1 Released 2025-May-05 diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index d46ef6ed67..a65bf5a575 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -28,7 +28,7 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler internal static readonly Assembly Assembly = typeof(EntityFrameworkDiagnosticListener).Assembly; internal static readonly string ActivitySourceName = Assembly.GetName().Name; internal static readonly string ActivityName = ActivitySourceName + ".Execute"; - internal static readonly ActivitySource SqlClientActivitySource = new(ActivitySourceName, Assembly.GetPackageVersion()); + internal static readonly ActivitySource EntityFrameworkActivitySource = new(ActivitySourceName, Assembly.GetPackageVersion()); private readonly PropertyFetcher commandFetcher = new("Command"); private readonly PropertyFetcher connectionFetcher = new("Connection"); @@ -59,7 +59,7 @@ public override void OnEventWritten(string name, object? payload) { case EntityFrameworkCoreCommandCreated: { - activity = SqlClientActivitySource.StartActivity(ActivityName, ActivityKind.Client); + activity = EntityFrameworkActivitySource.StartActivity(ActivityName, ActivityKind.Client); if (activity == null) { // There is no listener or it decided not to sample the current request. @@ -80,38 +80,57 @@ public override void OnEventWritten(string name, object? payload) if (activity.IsAllDataRequested) { - var dbContext = this.dbContextFetcher.Fetch(payload); - var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); - var providerName = this.providerNameFetcher.Fetch(dbContextDatabase); + string? providerOrCommandName = null; - switch (providerName) + if (this.dbContextFetcher.Fetch(payload) is { } dbContext) + { + var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); + providerOrCommandName = this.providerNameFetcher.Fetch(dbContextDatabase); + } + else + { + // Try to infer the database name from the command + // type if the DbContext is not available. + providerOrCommandName = command.GetType().FullName; + } + + switch (providerOrCommandName) { case "Microsoft.EntityFrameworkCore.SqlServer": + case "Microsoft.Data.SqlClient.SqlCommand": activity.AddTag(AttributeDbSystem, "mssql"); break; case "Microsoft.EntityFrameworkCore.Cosmos": activity.AddTag(AttributeDbSystem, "cosmosdb"); break; + case "Microsoft.Data.Sqlite.SqliteCommand": case "Microsoft.EntityFrameworkCore.Sqlite": case "Devart.Data.SQLite.Entity.EFCore": activity.AddTag(AttributeDbSystem, "sqlite"); break; case "MySql.Data.EntityFrameworkCore": + case "MySql.Data.MySqlClient.MySqlCommand": case "Pomelo.EntityFrameworkCore.MySql": case "Devart.Data.MySql.Entity.EFCore": + case "Devart.Data.MySql.MySqlCommand": activity.AddTag(AttributeDbSystem, "mysql"); break; case "Npgsql.EntityFrameworkCore.PostgreSQL": + case "Npgsql.NpgsqlCommand": case "Devart.Data.PostgreSql.Entity.EFCore": + case "Devart.Data.PostgreSql.PgSqlCommand": activity.AddTag(AttributeDbSystem, "postgresql"); break; case "Oracle.EntityFrameworkCore": + case "Oracle.ManagedDataAccess.Client.OracleCommand": case "Devart.Data.Oracle.Entity.EFCore": + case "Devart.Data.Oracle.OracleCommand": activity.AddTag(AttributeDbSystem, "oracle"); break; case "Microsoft.EntityFrameworkCore.InMemory": activity.AddTag(AttributeDbSystem, "efcoreinmemory"); break; + case "FirebirdSql.Data.FirebirdClient.FbCommand": case "FirebirdSql.EntityFrameworkCore.Firebird": activity.AddTag(AttributeDbSystem, "firebird"); break; @@ -120,23 +139,27 @@ public override void OnEventWritten(string name, object? payload) break; case "EntityFrameworkCore.SqlServerCompact35": case "EntityFrameworkCore.SqlServerCompact40": + case "System.Data.SqlServerCe.SqlCeCommand": activity.AddTag(AttributeDbSystem, "mssqlcompact"); break; case "EntityFrameworkCore.OpenEdge": activity.AddTag(AttributeDbSystem, "openedge"); break; case "EntityFrameworkCore.Jet": + case "EntityFrameworkCore.Jet.Data.JetCommand": activity.AddTag(AttributeDbSystem, "jet"); break; case "Google.Cloud.EntityFrameworkCore.Spanner": + case "Google.Cloud.Spanner.Data.SpannerCommand": activity.AddTag(AttributeDbSystem, "spanner"); break; + case "Teradata.Client.Provider.TdCommand": case "Teradata.EntityFrameworkCore": activity.AddTag(AttributeDbSystem, "teradata"); break; default: activity.AddTag(AttributeDbSystem, "other_sql"); - activity.AddTag("ef.provider", providerName); + activity.AddTag("ef.provider", providerOrCommandName); break; } @@ -168,7 +191,7 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source != EntityFrameworkActivitySource) { return; } @@ -179,9 +202,13 @@ public override void OnEventWritten(string name, object? payload) try { - var dbContext = this.dbContextFetcher.Fetch(payload); - var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); - var providerName = this.providerNameFetcher.Fetch(dbContextDatabase); + string? providerName = null; + + if (this.dbContextFetcher.Fetch(payload) is { } dbContext) + { + var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); + providerName = this.providerNameFetcher.Fetch(dbContextDatabase); + } if (command is IDbCommand typedCommand && this.options.Filter?.Invoke(providerName, typedCommand) == false) { @@ -267,12 +294,18 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source == EntityFrameworkActivitySource) { - return; + activity.Stop(); } - activity.Stop(); + // For some reason this EF event comes before the SQLClient SqlMicrosoftAfterExecuteCommand event. + // EF span should not be parent of any other span except SQLClient, because of that it can be closed safely. + // Can result in a slightly strange timeline where the EF span finishes before its child SQLClient but based on EventSource's it is true. + if (activity.Parent?.Source == EntityFrameworkActivitySource) + { + activity.Parent.Stop(); + } } break; @@ -285,7 +318,7 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source != EntityFrameworkActivitySource) { return; } diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj index 484fcdaeef..3f4c485894 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj @@ -14,7 +14,7 @@ - + diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs index bfdee50fe0..23eb00380b 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs @@ -299,42 +299,14 @@ private void Seed() context.Database.EnsureDeleted(); context.Database.EnsureCreated(); - var one = new Item("ItemOne"); + var one = new Item() { Name = "ItemOne" }; - var two = new Item("ItemTwo"); + var two = new Item() { Name = "ItemTwo" }; - var three = new Item("ItemThree"); + var three = new Item() { Name = "ItemThree" }; context.AddRange(one, two, three); context.SaveChanges(); } - - private class Item - { - public Item(string name) - { - this.Name = name; - } - - public string Name { get; } - } - - private class ItemsContext : DbContext - { - public ItemsContext(DbContextOptions options) - : base(options) - { - } - - protected override void OnModelCreating(ModelBuilder modelBuilder) - { - modelBuilder.Entity( - b => - { - b.Property(e => e.Name); - b.HasKey("Name"); - }); - } - } } diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs new file mode 100644 index 0000000000..734373655f --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs @@ -0,0 +1,310 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using System.Data; +using System.Diagnostics; +using Microsoft.Data.SqlClient; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using OpenTelemetry.Instrumentation.SqlClient.Tests; +using OpenTelemetry.Tests; +using OpenTelemetry.Trace; +using Testcontainers.MsSql; +using Testcontainers.SqlEdge; +using Xunit; + +namespace OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests; + +[Trait("CategoryName", "SqlIntegrationTests")] +public sealed class EntityFrameworkIntegrationTests : IClassFixture +{ + private const string SqliteProvider = "Microsoft.EntityFrameworkCore.Sqlite"; + private const string SqlServerProvider = "Microsoft.EntityFrameworkCore.SqlServer"; + + private const string ActivitySourceName = "OpenTelemetry.Instrumentation.EntityFrameworkCore"; + + private readonly SqlClientIntegrationTestsFixture fixture; + + public EntityFrameworkIntegrationTests(SqlClientIntegrationTestsFixture fixture) + { + this.fixture = fixture; + } + + public static TheoryData RawSqlTestCases() + { + (string, Type, string, string)[] providers = + [ + (SqliteProvider, typeof(SqliteCommand), "sqlite", "main"), + (SqlServerProvider, typeof(SqlCommand), "mssql", "master"), + ]; + + var testCases = new TheoryData(); + + foreach ((var provider, var commandType, var system, var database) in providers) + { + testCases.Add(provider, "select 1/1", false, false, commandType, system, database); + testCases.Add(provider, "select 1/1", true, false, commandType, system, database); + + // For some reason, SQLite does not throw an exception for division by zero + if (provider != SqliteProvider) + { + testCases.Add(provider, "select 1/0", false, true, commandType, system, database); + testCases.Add(provider, "select 1/0", true, true, commandType, system, database); + } + } + + return testCases; + } + + public static TheoryData DataContextTestCases() + { + (string, Type, string, string)[] providers = + [ + (SqliteProvider, typeof(SqliteCommand), "sqlite", "main"), + (SqlServerProvider, typeof(SqlCommand), "mssql", "master"), + ]; + + bool[] trueFalse = [true, false]; + + var testCases = new TheoryData(); + + foreach ((var provider, var commandType, var system, var database) in providers) + { + foreach (var captureTextCommandContent in trueFalse) + { + foreach (var shouldEnrich in trueFalse) + { + testCases.Add(provider, captureTextCommandContent, shouldEnrich, commandType, system, database); + } + } + } + + return testCases; + } + + [EnabledOnDockerPlatformTheory(EnabledOnDockerPlatformTheoryAttribute.DockerPlatform.Linux)] + [MemberData(nameof(RawSqlTestCases))] + public async Task TracesRawSql( + string provider, + string commandText, + bool captureTextCommandContent, + bool isFailure, + Type expectedCommandType, + string expectedSystemName, + string expectedDatabaseName) + { + // In the case of SQL Server, the activity we're interested in is the one + // created by the SqlClient instrumentation which is a child of EFCore. + var expectedSourceName = isFailure && provider is SqlServerProvider + ? "OpenTelemetry.Instrumentation.SqlClient" + : ActivitySourceName; + + var filtered = false; + + bool ActivityFilter(string? providerName, IDbCommand command) + { + filtered = true; + + Assert.True(providerName == provider || providerName == null, $"The provider name {providerName} is not null or the expected value."); + Assert.IsType(expectedCommandType, command, false); + + return true; + } + + var activities = new List(); + using var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddInMemoryExporter(activities) + .AddSqlClientInstrumentation(options => + { + options.SetDbStatementForText = captureTextCommandContent; + }) + .AddEntityFrameworkCoreInstrumentation(options => + { + options.Filter = ActivityFilter; + options.SetDbStatementForText = captureTextCommandContent; + }) + .Build(); + + var optionsBuilder = new DbContextOptionsBuilder(); + + this.ConfigureProvider(provider, optionsBuilder); + + await using var context = new ItemsContext(optionsBuilder.Options); + + try + { + await context.Database.ExecuteSqlRawAsync(commandText); + } + catch + { + // Ignore + } + + Assert.NotEmpty(activities); + + // All activities should either be the root EFCore activity or a child of it. + Assert.All(activities, activity => Assert.Equal(ActivitySourceName, (activity.Parent?.Source ?? activity.Source).Name)); + + var activity = activities.Last(); + + Assert.Equal(expectedSourceName, activity.Source.Name); + + VerifyActivityData( + captureTextCommandContent, + isFailure, + expectedSystemName, + expectedDatabaseName, + activity); + + if (isFailure) + { + Assert.Equal(ActivityStatusCode.Error, activity.Status); + Assert.Equal("Divide by zero error encountered.", activity.StatusDescription); + } + + Assert.True(filtered); + } + + [EnabledOnDockerPlatformTheory(EnabledOnDockerPlatformTheoryAttribute.DockerPlatform.Linux)] + [MemberData(nameof(DataContextTestCases))] + public async Task TracesDataContext( + string provider, + bool captureTextCommandContent, + bool shouldEnrich, + Type expectedCommandType, + string expectedSystemName, + string expectedDatabaseName) + { + var enriched = false; + + void ActivityEnrichment(Activity activity, IDbCommand command) + { + enriched = true; + + activity.SetTag("enriched", "yes"); + Assert.IsType(expectedCommandType, command, false); + } + + var filtered = false; + + bool ActivityFilter(string? providerName, IDbCommand command) + { + filtered = true; + + Assert.True(providerName == provider || providerName == null, $"The provider name {providerName} is not null or the expected value."); + Assert.IsType(expectedCommandType, command, false); + + return true; + } + + var activities = new List(); + using var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddInMemoryExporter(activities) + .AddSqlClientInstrumentation(options => + { + options.SetDbStatementForText = captureTextCommandContent; + }) + .AddEntityFrameworkCoreInstrumentation(options => + { + options.Filter = ActivityFilter; + options.SetDbStatementForText = captureTextCommandContent; + if (shouldEnrich) + { + options.EnrichWithIDbCommand = ActivityEnrichment; + } + }) + .Build(); + + var optionsBuilder = new DbContextOptionsBuilder(); + + this.ConfigureProvider(provider, optionsBuilder); + + await using var context = new ItemsContext(optionsBuilder.Options); + await context.Database.EnsureCreatedAsync(); + + // Clear activities from creating the database + activities.Clear(); + + var result = await context.Items.ToListAsync(); + + Assert.NotNull(result); + Assert.Empty(result); + + // All activities should either be the root EFCore activity or a child of it. + Assert.All(activities, activity => Assert.Equal(ActivitySourceName, (activity.Parent?.Source ?? activity.Source).Name)); + + // When using SQL Server there may be multiple activities, but we care + // about the EFCore activity which should be the parent activity. + var activity = Assert.Single(activities, activity => activity.Parent is null); + + Assert.Equal(ActivitySourceName, activity.Source.Name); + Assert.Null(activity.Parent); + + Assert.Equal(expectedSystemName, activity.GetTagValue(SemanticConventions.AttributeDbSystem)); + Assert.Equal(expectedDatabaseName, activity.GetTagValue(SemanticConventions.AttributeDbName)); + + Assert.Equal(shouldEnrich, enriched); + Assert.True(filtered); + } + + private static void VerifyActivityData( + bool captureTextCommandContent, + bool isFailure, + string expectedSystemName, + string expectedDatabaseName, + Activity activity) + { + Assert.Equal(expectedDatabaseName, activity.DisplayName); + + Assert.Equal(ActivityKind.Client, activity.Kind); + + if (!isFailure) + { + Assert.Equal(ActivityStatusCode.Unset, activity.Status); + } + else + { + Assert.Equal(ActivityStatusCode.Error, activity.Status); + Assert.NotNull(activity.StatusDescription); + Assert.Empty(activity.Events); + } + + Assert.Equal(expectedSystemName, activity.GetTagValue(SemanticConventions.AttributeDbSystem)); + Assert.Equal(expectedDatabaseName, activity.GetTagValue(SemanticConventions.AttributeDbName)); + + if (captureTextCommandContent) + { + Assert.NotNull(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + } + else + { + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + Assert.Null(activity.GetTagValue(SemanticConventions.AttributeDbQueryText)); + } + } + + private string GetConnectionString() => this.fixture.DatabaseContainer switch + { + SqlEdgeContainer container => container.GetConnectionString(), + MsSqlContainer container => container.GetConnectionString(), + _ => throw new InvalidOperationException($"Container type '${this.fixture.DatabaseContainer.GetType().Name}' is not supported."), + }; + + private void ConfigureProvider(string provider, DbContextOptionsBuilder builder) + { + switch (provider) + { + case "Microsoft.EntityFrameworkCore.Sqlite": + var file = Path.GetTempFileName(); + builder.UseSqlite($"Filename={file}"); + break; + + case "Microsoft.EntityFrameworkCore.SqlServer": + builder.UseSqlServer(this.GetConnectionString()); + break; + + default: + throw new NotSupportedException($"Unsupported provider: {provider}"); + } + } +} diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/Item.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/Item.cs new file mode 100644 index 0000000000..09320b3fe8 --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/Item.cs @@ -0,0 +1,11 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +namespace OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests; + +internal class Item +{ + public Guid Id { get; set; } + + public string Name { get; set; } = default!; +} diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/ItemsContext.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/ItemsContext.cs new file mode 100644 index 0000000000..09f54e6971 --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/ItemsContext.cs @@ -0,0 +1,11 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +using Microsoft.EntityFrameworkCore; + +namespace OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests; + +internal class ItemsContext(DbContextOptions options) : DbContext(options) +{ + public DbSet Items { get; set; } = null!; +} diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj index 74df3dd0dc..c72b2d91df 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj @@ -7,17 +7,25 @@ - + + + + + + + + + diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj index 663a4b77b9..c36949c255 100644 --- a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj @@ -16,7 +16,7 @@ - +