From 2f2425e6066edbc68e63248e24e715041d425a19 Mon Sep 17 00:00:00 2001 From: zivaninstana Date: Thu, 31 Oct 2024 12:51:40 +0100 Subject: [PATCH 01/11] EF Core and SqlClient instrumentation enabled at the same time produce wrong spans --- .../EntityFrameworkDiagnosticListener.cs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index d46ef6ed67..31d567e7cf 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. @@ -168,7 +168,7 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source != EntityFrameworkActivitySource) { return; } @@ -267,12 +267,18 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source == EntityFrameworkActivitySource) { - return; + activity.Stop(); } - activity.Stop(); + // from some reason this EF event comes before SQLClient SqlMicrosoftAfterExecuteCommand event + // EF span should not be parrent of any other span except SQLClient, because of that it can be closed safetly + // can result in a slightly strange timeline where the EF span finishes before its child SQLClient but based on EventSources it is true + if (activity.Parent?.Source == EntityFrameworkActivitySource) + { + activity.Parent.Stop(); + } } break; @@ -285,7 +291,7 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source != EntityFrameworkActivitySource) { return; } From b4e2966940bc24d2c4728bc9b4c83a56e6e421e3 Mon Sep 17 00:00:00 2001 From: zivaninstana Date: Thu, 31 Oct 2024 13:56:36 +0100 Subject: [PATCH 02/11] EF Core instrumentation fix - Trailing space fix --- .../Implementation/EntityFrameworkDiagnosticListener.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 31d567e7cf..c4b65c05f3 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -272,9 +272,9 @@ public override void OnEventWritten(string name, object? payload) activity.Stop(); } - // from some reason this EF event comes before SQLClient SqlMicrosoftAfterExecuteCommand event + // From some reason this EF event comes before SQLClient SqlMicrosoftAfterExecuteCommand event // EF span should not be parrent of any other span except SQLClient, because of that it can be closed safetly - // can result in a slightly strange timeline where the EF span finishes before its child SQLClient but based on EventSources it is true + // Can result in a slightly strange timeline where the EF span finishes before its child SQLClient but based on EventSources it is true if (activity.Parent?.Source == EntityFrameworkActivitySource) { activity.Parent.Stop(); From 58d18e254b2ebbf8dbd003541df8a6563e8454bc Mon Sep 17 00:00:00 2001 From: zivaninstana Date: Thu, 31 Oct 2024 14:13:38 +0100 Subject: [PATCH 03/11] EF Core instrumentation fix - Update changelog --- .../CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index bc04010a01..49377ed3c0 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +* EF Core instrumentation support together with SqlClient instrumentation. + ([#2280](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2280)) + ## 1.12.0-beta.1 Released 2025-May-05 From 0ee81c188139d3017643e0987c881875cc2182dd Mon Sep 17 00:00:00 2001 From: martincostello Date: Mon, 9 Jun 2025 16:12:09 +0100 Subject: [PATCH 04/11] Fix comment Fix spelling in grammar in comment. --- .../Implementation/EntityFrameworkDiagnosticListener.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index c4b65c05f3..568adad6b9 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -272,9 +272,9 @@ public override void OnEventWritten(string name, object? payload) activity.Stop(); } - // From some reason this EF event comes before SQLClient SqlMicrosoftAfterExecuteCommand event - // EF span should not be parrent of any other span except SQLClient, because of that it can be closed safetly - // Can result in a slightly strange timeline where the EF span finishes before its child SQLClient but based on EventSources it is true + // 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(); From 02afbecae6850fb35f800363de2c713fa1fcc8b9 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 10 Jun 2025 11:34:24 +0100 Subject: [PATCH 05/11] Add integration tests Add integration tests for EFCore using SQL Server and SQLite. --- .../EntityFrameworkDiagnosticListenerTests.cs | 34 +- .../EntityFrameworkIntegrationTests.cs | 314 ++++++++++++++++++ .../Item.cs | 11 + .../ItemsContext.cs | 11 + ...mentation.EntityFrameworkCore.Tests.csproj | 9 +- ...try.Instrumentation.SqlClient.Tests.csproj | 2 +- 6 files changed, 348 insertions(+), 33 deletions(-) create mode 100644 test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs create mode 100644 test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/Item.cs create mode 100644 test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/ItemsContext.cs 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..fbe1d500ec --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs @@ -0,0 +1,314 @@ +// 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, true, commandType, system, database); + testCases.Add(provider, "select 1/1", true, false, true, 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, false, commandType, system, database); + testCases.Add(provider, "select 1/0", false, 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, + 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? provider, IDbCommand command) + { + filtered = true; + + Assert.Equal(provider, provider); + Assert.IsType(expectedCommandType, command, false); + + return true; + } + + var activities = new List(); + using var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddInMemoryExporter(activities) + .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); + + try + { + await context.Database.ExecuteSqlRawAsync(commandText); + } + catch + { + // Ignore + } + + Assert.Single(activities); + var activity = activities[0]; + + VerifyActivityData( + commandText, + captureTextCommandContent, + isFailure, + shouldEnrich, + expectedSystemName, + expectedDatabaseName, + activity); + + if (isFailure) + { + Assert.Equal(ActivityStatusCode.Error, activity.Status); + Assert.Equal("Divide by zero error encountered.", activity.StatusDescription); + } + + Assert.Equal(shouldEnrich, enriched); + 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? provider, IDbCommand command) + { + filtered = true; + + Assert.Equal(provider, provider); + Assert.IsType(expectedCommandType, command, false); + + return true; + } + + var activities = new List(); + using var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddInMemoryExporter(activities) + .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(); + + var activity = Assert.Single(activities); + + 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( + string? commandText, + bool captureTextCommandContent, + bool isFailure, + bool shouldEnrich, + string expectedSystemName, + string expectedDatabaseName, + Activity activity) + { + Assert.Equal(ActivitySourceName, activity.Source.Name); + + 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); + } + + if (shouldEnrich) + { + Assert.Contains(activity.Tags, tag => tag.Key == "enriched"); + Assert.Equal("yes", activity.Tags.FirstOrDefault(tag => tag.Key == "enriched").Value); + } + else + { + Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enriched"); + } + + Assert.Equal(expectedSystemName, activity.GetTagValue(SemanticConventions.AttributeDbSystem)); + Assert.Equal(expectedDatabaseName, activity.GetTagValue(SemanticConventions.AttributeDbName)); + + if (captureTextCommandContent) + { + Assert.Equal(commandText, 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..f89c905ad0 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj @@ -7,9 +7,12 @@ - + + + + @@ -17,7 +20,11 @@ + + + + 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 @@ - + From 46ccb5bfe0c85d4464959fc8d63a71904b097173 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 10 Jun 2025 12:33:51 +0100 Subject: [PATCH 06/11] Fix tests - Fix missing SqlClient instrumentation. - Fix assertions. - Fix project reference. --- ...Instrumentation.EntityFrameworkCore.csproj | 2 +- .../EntityFrameworkIntegrationTests.cs | 78 +++++++++---------- ...mentation.EntityFrameworkCore.Tests.csproj | 5 +- 3 files changed, 41 insertions(+), 44 deletions(-) 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/EntityFrameworkIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs index fbe1d500ec..eac3928fa0 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs @@ -30,7 +30,7 @@ public EntityFrameworkIntegrationTests(SqlClientIntegrationTestsFixture fixture) this.fixture = fixture; } - public static TheoryData RawSqlTestCases() + public static TheoryData RawSqlTestCases() { (string, Type, string, string)[] providers = [ @@ -38,18 +38,18 @@ public static TheoryData (SqlServerProvider, typeof(SqlCommand), "mssql", "master"), ]; - var testCases = new TheoryData(); + var testCases = new TheoryData(); foreach ((var provider, var commandType, var system, var database) in providers) { - testCases.Add(provider, "select 1/1", false, false, true, commandType, system, database); - testCases.Add(provider, "select 1/1", true, false, true, commandType, system, database); + 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, false, commandType, system, database); - testCases.Add(provider, "select 1/0", false, true, true, commandType, system, database); + testCases.Add(provider, "select 1/0", false, true, commandType, system, database); + testCases.Add(provider, "select 1/0", true, true, commandType, system, database); } } @@ -89,20 +89,15 @@ public async Task TracesRawSql( string commandText, bool captureTextCommandContent, bool isFailure, - 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); - } + // 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; @@ -119,14 +114,14 @@ bool ActivityFilter(string? provider, IDbCommand command) 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(); @@ -145,14 +140,18 @@ bool ActivityFilter(string? provider, IDbCommand command) // Ignore } - Assert.Single(activities); - var activity = activities[0]; + 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( - commandText, captureTextCommandContent, isFailure, - shouldEnrich, expectedSystemName, expectedDatabaseName, activity); @@ -163,7 +162,6 @@ bool ActivityFilter(string? provider, IDbCommand command) Assert.Equal("Divide by zero error encountered.", activity.StatusDescription); } - Assert.Equal(shouldEnrich, enriched); Assert.True(filtered); } @@ -202,6 +200,10 @@ bool ActivityFilter(string? provider, IDbCommand command) var activities = new List(); using var tracerProvider = Sdk.CreateTracerProviderBuilder() .AddInMemoryExporter(activities) + .AddSqlClientInstrumentation(options => + { + options.SetDbStatementForText = captureTextCommandContent; + }) .AddEntityFrameworkCoreInstrumentation(options => { options.Filter = ActivityFilter; @@ -225,7 +227,15 @@ bool ActivityFilter(string? provider, IDbCommand command) var result = await context.Items.ToListAsync(); - var activity = Assert.Single(activities); + 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); @@ -238,16 +248,12 @@ bool ActivityFilter(string? provider, IDbCommand command) } private static void VerifyActivityData( - string? commandText, bool captureTextCommandContent, bool isFailure, - bool shouldEnrich, string expectedSystemName, string expectedDatabaseName, Activity activity) { - Assert.Equal(ActivitySourceName, activity.Source.Name); - Assert.Equal(expectedDatabaseName, activity.DisplayName); Assert.Equal(ActivityKind.Client, activity.Kind); @@ -263,22 +269,12 @@ private static void VerifyActivityData( Assert.Empty(activity.Events); } - if (shouldEnrich) - { - Assert.Contains(activity.Tags, tag => tag.Key == "enriched"); - Assert.Equal("yes", activity.Tags.FirstOrDefault(tag => tag.Key == "enriched").Value); - } - else - { - Assert.DoesNotContain(activity.Tags, tag => tag.Key == "enriched"); - } - Assert.Equal(expectedSystemName, activity.GetTagValue(SemanticConventions.AttributeDbSystem)); Assert.Equal(expectedDatabaseName, activity.GetTagValue(SemanticConventions.AttributeDbName)); if (captureTextCommandContent) { - Assert.Equal(commandText, activity.GetTagValue(SemanticConventions.AttributeDbStatement)); + Assert.NotNull(activity.GetTagValue(SemanticConventions.AttributeDbStatement)); } else { 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 f89c905ad0..c72b2d91df 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests.csproj @@ -17,14 +17,15 @@ + + + - - From 195b88e81774833d998590fd242d01d614e8fedf Mon Sep 17 00:00:00 2001 From: Martin Costello Date: Tue, 10 Jun 2025 12:43:43 +0100 Subject: [PATCH 07/11] Update CHANGELOG Rewrite the message. --- .../CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index 49377ed3c0..dcd0f3c60a 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -* EF Core instrumentation support together with SqlClient instrumentation. +* Support use with `SqlClient` instrumentation. ([#2280](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2280)) ## 1.12.0-beta.1 From 48f49d039eb68706d9adeff7e995aebf9b637b0a Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 10 Jun 2025 12:57:14 +0100 Subject: [PATCH 08/11] Fix assertions Fix incorrect assertion of a value with itself. --- .../EntityFrameworkIntegrationTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs index eac3928fa0..038dee129c 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs @@ -101,11 +101,11 @@ public async Task TracesRawSql( var filtered = false; - bool ActivityFilter(string? provider, IDbCommand command) + bool ActivityFilter(string? providerName, IDbCommand command) { filtered = true; - Assert.Equal(provider, provider); + Assert.Equal(provider, providerName); Assert.IsType(expectedCommandType, command, false); return true; @@ -187,11 +187,11 @@ void ActivityEnrichment(Activity activity, IDbCommand command) var filtered = false; - bool ActivityFilter(string? provider, IDbCommand command) + bool ActivityFilter(string? providerName, IDbCommand command) { filtered = true; - Assert.Equal(provider, provider); + Assert.Equal(provider, providerName); Assert.IsType(expectedCommandType, command, false); return true; From 68a4780cddfb4e502f71f91d31e10cea0a5fa510 Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 24 Jun 2025 13:36:20 +0100 Subject: [PATCH 09/11] Avoid ArgumentException when no DbContext Avoid `ArgumentException` if the `DbContext` is not available in the event payload. See https://github.com/dotnet/efcore/pull/36286. --- .../EntityFrameworkDiagnosticListener.cs | 31 ++++++++++++++----- .../EntityFrameworkIntegrationTests.cs | 4 +-- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 568adad6b9..446cfe6c77 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -80,11 +80,21 @@ 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": activity.AddTag(AttributeDbSystem, "mssql"); @@ -92,6 +102,7 @@ public override void OnEventWritten(string name, object? payload) 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"); @@ -136,7 +147,7 @@ public override void OnEventWritten(string name, object? payload) break; default: activity.AddTag(AttributeDbSystem, "other_sql"); - activity.AddTag("ef.provider", providerName); + activity.AddTag("ef.provider", providerOrCommandName); break; } @@ -179,9 +190,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) { diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs index 038dee129c..734373655f 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkIntegrationTests.cs @@ -105,7 +105,7 @@ bool ActivityFilter(string? providerName, IDbCommand command) { filtered = true; - Assert.Equal(provider, providerName); + Assert.True(providerName == provider || providerName == null, $"The provider name {providerName} is not null or the expected value."); Assert.IsType(expectedCommandType, command, false); return true; @@ -191,7 +191,7 @@ bool ActivityFilter(string? providerName, IDbCommand command) { filtered = true; - Assert.Equal(provider, providerName); + Assert.True(providerName == provider || providerName == null, $"The provider name {providerName} is not null or the expected value."); Assert.IsType(expectedCommandType, command, false); return true; From 1786f606bd384a2cb98f964c8c9a8740aa5e99dd Mon Sep 17 00:00:00 2001 From: martincostello Date: Tue, 24 Jun 2025 14:11:58 +0100 Subject: [PATCH 10/11] Add more command hints Add more `DbCommand` class names for if `DbContext` is not available. --- .../EntityFrameworkDiagnosticListener.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 446cfe6c77..a65bf5a575 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -97,6 +97,7 @@ public override void OnEventWritten(string name, object? payload) switch (providerOrCommandName) { case "Microsoft.EntityFrameworkCore.SqlServer": + case "Microsoft.Data.SqlClient.SqlCommand": activity.AddTag(AttributeDbSystem, "mssql"); break; case "Microsoft.EntityFrameworkCore.Cosmos": @@ -108,21 +109,28 @@ public override void OnEventWritten(string name, object? payload) 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; @@ -131,17 +139,21 @@ 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; From a99a3db9bd3ebcfa29af7115c698ca31904cc74b Mon Sep 17 00:00:00 2001 From: Alan West <3676547+alanwest@users.noreply.github.com> Date: Tue, 24 Jun 2025 14:49:52 -0700 Subject: [PATCH 11/11] Update src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md --- .../CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md index dcd0f3c60a..b15dade3da 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/CHANGELOG.md @@ -3,7 +3,8 @@ ## Unreleased * Support use with `SqlClient` instrumentation. - ([#2280](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2280)) + ([#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