From 43cb6c39963907a5d533aa346ad0528099462e1b Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Wed, 12 Mar 2025 20:27:44 -0400 Subject: [PATCH 1/9] borrow from sqlclient metrics impl --- ...eworkCoreMeterProviderBuilderExtensions.cs | 39 ++++++++++++++++ .../EntityFrameworkInstrumentation.cs | 46 +++++++++++++++++++ .../EntityFrameworkDiagnosticListener.cs | 14 +++++- .../TracerProviderBuilderExtensions.cs | 3 +- 4 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkCoreMeterProviderBuilderExtensions.cs diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkCoreMeterProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkCoreMeterProviderBuilderExtensions.cs new file mode 100644 index 0000000000..5a7465bfd6 --- /dev/null +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkCoreMeterProviderBuilderExtensions.cs @@ -0,0 +1,39 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +#if NET +using System.Diagnostics.CodeAnalysis; +#endif +using OpenTelemetry.Instrumentation.EntityFrameworkCore; +using OpenTelemetry.Instrumentation.EntityFrameworkCore.Implementation; +using OpenTelemetry.Internal; + +namespace OpenTelemetry.Metrics; + +/// +/// Extension methods to simplify registering of dependency instrumentation. +/// +public static class EntityFrameworkCoreMeterProviderBuilderExtensions +{ + /// + /// Enables EntityFrameworkCore instrumentation. + /// + /// being configured. + /// The instance of to chain the calls. +#if NET + // TODO [RequiresUnreferencedCode(SqlClientInstrumentation.SqlClientTrimmingUnsupportedMessage)] +#endif + public static MeterProviderBuilder AddEntityFrameworkCoreInstrumentation(this MeterProviderBuilder builder) + { + Guard.ThrowIfNull(builder); + + builder.AddInstrumentation(sp => + { + return EntityFrameworkInstrumentation.AddMetricHandle(); + }); + + builder.AddMeter(EntityFrameworkDiagnosticListener.MeterName); + + return builder; + } +} diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs index 7902787113..bb0439eaa4 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs @@ -8,6 +8,8 @@ namespace OpenTelemetry.Instrumentation.EntityFrameworkCore; internal class EntityFrameworkInstrumentation : IDisposable { private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; + internal static int MetricHandles; + internal static int TracingHandles; public EntityFrameworkInstrumentation(EntityFrameworkInstrumentationOptions? options) { @@ -19,8 +21,52 @@ public EntityFrameworkInstrumentation(EntityFrameworkInstrumentationOptions? opt this.diagnosticSourceSubscriber.Subscribe(); } + public static EntityFrameworkInstrumentationOptions TracingOptions { get; set; } = new(); + + public static IDisposable AddMetricHandle() => new MetricHandle(); + + public static IDisposable AddTracingHandle() => new TracingHandle(); + public void Dispose() { this.diagnosticSourceSubscriber?.Dispose(); } + + private sealed class MetricHandle : IDisposable + { + private bool disposed; + + public MetricHandle() + { + Interlocked.Increment(ref MetricHandles); + } + + public void Dispose() + { + if (!this.disposed) + { + Interlocked.Decrement(ref MetricHandles); + this.disposed = true; + } + } + } + + private sealed class TracingHandle : IDisposable + { + private bool disposed; + + public TracingHandle() + { + Interlocked.Increment(ref TracingHandles); + } + + public void Dispose() + { + if (!this.disposed) + { + Interlocked.Decrement(ref TracingHandles); + this.disposed = true; + } + } + } } diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 636a02c589..92c34b500b 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -3,6 +3,7 @@ using System.Data; using System.Diagnostics; +using System.Diagnostics.Metrics; using System.Reflection; using OpenTelemetry.Internal; @@ -26,10 +27,20 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler internal const string AttributeDbQueryText = "db.query.text"; internal static readonly Assembly Assembly = typeof(EntityFrameworkDiagnosticListener).Assembly; - internal static readonly string ActivitySourceName = Assembly.GetName().Name; + public static readonly AssemblyName AssemblyName = Assembly.GetName(); + + internal static readonly string ActivitySourceName = AssemblyName.Name!; internal static readonly string ActivityName = ActivitySourceName + ".Execute"; internal static readonly ActivitySource SqlClientActivitySource = new(ActivitySourceName, Assembly.GetPackageVersion()); + internal static readonly string MeterName = AssemblyName.Name!; + internal static readonly Meter Meter = new(MeterName, Assembly.GetPackageVersion()); + internal static readonly Histogram DbClientOperationDuration = Meter.CreateHistogram( + "db.client.operation.duration", + unit: "s", + description: "Duration of database client operations.", + advice: new InstrumentAdvice { HistogramBucketBoundaries = [0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10] }); + private readonly PropertyFetcher commandFetcher = new("Command"); private readonly PropertyFetcher connectionFetcher = new("Connection"); private readonly PropertyFetcher dbContextFetcher = new("Context"); @@ -40,6 +51,7 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler private readonly PropertyFetcher commandTypeFetcher = new("CommandType"); private readonly PropertyFetcher commandTextFetcher = new("CommandText"); private readonly PropertyFetcher exceptionFetcher = new("Exception"); + private readonly AsyncLocal beginTimestamp = new(); private readonly EntityFrameworkInstrumentationOptions options; diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/TracerProviderBuilderExtensions.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/TracerProviderBuilderExtensions.cs index e0bb7bd938..88aba67ea1 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/TracerProviderBuilderExtensions.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/TracerProviderBuilderExtensions.cs @@ -58,7 +58,8 @@ public static TracerProviderBuilder AddEntityFrameworkCoreInstrumentation( builder.AddInstrumentation(sp => { var options = sp.GetRequiredService>().Get(name); - return new EntityFrameworkInstrumentation(options); + EntityFrameworkInstrumentation.TracingOptions = options; + return EntityFrameworkInstrumentation.AddTracingHandle(); }); builder.AddSource(EntityFrameworkDiagnosticListener.ActivitySourceName); From 46f06cebbaa00099782fb85eba7bbb508cf1aac1 Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Fri, 21 Mar 2025 22:32:26 -0400 Subject: [PATCH 2/9] instance + lift out options --- .../EntityFrameworkInstrumentation.cs | 9 ++++++--- .../Implementation/EntityFrameworkDiagnosticListener.cs | 6 ++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs index bb0439eaa4..8c53867166 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs @@ -7,14 +7,17 @@ namespace OpenTelemetry.Instrumentation.EntityFrameworkCore; internal class EntityFrameworkInstrumentation : IDisposable { - private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; + public static readonly EntityFrameworkInstrumentation Instance = new EntityFrameworkInstrumentation(); internal static int MetricHandles; internal static int TracingHandles; - public EntityFrameworkInstrumentation(EntityFrameworkInstrumentationOptions? options) + private readonly EntityFrameworkInstrumentationOptions options; + private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; + + public EntityFrameworkInstrumentation() { this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber( - name => new EntityFrameworkDiagnosticListener(name, options), + name => new EntityFrameworkDiagnosticListener(name), listener => listener.Name == EntityFrameworkDiagnosticListener.DiagnosticSourceName, null, EntityFrameworkInstrumentationEventSource.Log.UnknownErrorProcessingEvent); diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 8845b61412..f0c6841726 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -53,18 +53,16 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler private readonly PropertyFetcher exceptionFetcher = new("Exception"); private readonly AsyncLocal beginTimestamp = new(); - private readonly EntityFrameworkInstrumentationOptions options; - - public EntityFrameworkDiagnosticListener(string sourceName, EntityFrameworkInstrumentationOptions? options) + public EntityFrameworkDiagnosticListener(string sourceName) : base(sourceName) { - this.options = options ?? new EntityFrameworkInstrumentationOptions(); } public override bool SupportsNullActivity => true; public override void OnEventWritten(string name, object? payload) { + var options = EntityFrameworkInstrumentation.TracingOptions; var activity = Activity.Current; switch (name) From 870dbdc1230025489d49fa9c2b510dac3e96e13e Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Fri, 21 Mar 2025 22:33:05 -0400 Subject: [PATCH 3/9] setup start attr extract more extraction cleanup use new methods activityName --- .../EntityFrameworkDiagnosticListener.cs | 265 +++++++++++------- ...Instrumentation.EntityFrameworkCore.csproj | 1 + 2 files changed, 163 insertions(+), 103 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index f0c6841726..d617147c8e 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -6,6 +6,7 @@ using System.Diagnostics.Metrics; using System.Reflection; using OpenTelemetry.Internal; +using OpenTelemetry.Trace; namespace OpenTelemetry.Instrumentation.EntityFrameworkCore.Implementation; @@ -31,7 +32,7 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler internal static readonly string ActivitySourceName = AssemblyName.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()); internal static readonly string MeterName = AssemblyName.Name!; internal static readonly Meter Meter = new(MeterName, Assembly.GetPackageVersion()); @@ -41,6 +42,14 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler description: "Duration of database client operations.", advice: new InstrumentAdvice { HistogramBucketBoundaries = [0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10] }); + private static readonly string[] SharedTagNames = + [ + SemanticConventions.AttributeDbSystem, + SemanticConventions.AttributeDbCollectionName, + SemanticConventions.AttributeDbNamespace, + SemanticConventions.AttributeDbResponseStatusCode, + ]; + private readonly PropertyFetcher commandFetcher = new("Command"); private readonly PropertyFetcher connectionFetcher = new("Connection"); private readonly PropertyFetcher dbContextFetcher = new("Context"); @@ -60,8 +69,89 @@ public EntityFrameworkDiagnosticListener(string sourceName) public override bool SupportsNullActivity => true; + private static double CalculateDurationFromTimestamp(long begin, long? end = null) + { + end ??= Stopwatch.GetTimestamp(); + var timestampToTicks = TimeSpan.TicksPerSecond / (double)Stopwatch.Frequency; + var delta = end - begin; + var ticks = (long)(timestampToTicks * delta); + var duration = new TimeSpan(ticks); + return duration.TotalSeconds; + } + + private static string GetDbSystemFromProviderName(string? providerName) + { + return providerName switch + { + "Microsoft.EntityFrameworkCore.SqlServer" => "mssql", + "Microsoft.EntityFrameworkCore.Cosmos" => "cosmosdb", + "Microsoft.EntityFrameworkCore.Sqlite" or "Devart.Data.SQLite.Entity.EFCore" => "sqlite", + "MySql.Data.EntityFrameworkCore" or "Pomelo.EntityFrameworkCore.MySql" or "Devart.Data.MySql.Entity.EFCore" => "mysql", + "Npgsql.EntityFrameworkCore.PostgreSQL" or "Devart.Data.PostgreSql.Entity.EFCore" => "postgresql", + "Oracle.EntityFrameworkCore" or "Devart.Data.Oracle.Entity.EFCore" => "oracle", + "Microsoft.EntityFrameworkCore.InMemory" => "efcoreinmemory", + "FirebirdSql.EntityFrameworkCore.Firebird" => "firebird", + "FileContextCore" => "filecontextcore", + "EntityFrameworkCore.SqlServerCompact35" or "EntityFrameworkCore.SqlServerCompact40" => "mssqlcompact", + "EntityFrameworkCore.OpenEdge" => "openedge", + "EntityFrameworkCore.Jet" => "jet", + "Google.Cloud.EntityFrameworkCore.Spanner" => "spanner", + "Teradata.EntityFrameworkCore" => "teradata", + _ => "other_sql", + }; + } + + private TagList CreateTagsFromConnectionInfo(object? payload, object? command, EntityFrameworkInstrumentationOptions options, out string activityName) + { + activityName = ActivityName; + var tags = default(TagList); + + if (payload == null || command == null) + { + return tags; + } + + var connection = this.connectionFetcher.Fetch(command); + var dataSource = (string)this.dataSourceFetcher.Fetch(connection); + var database = (string)this.databaseFetcher.Fetch(connection); + activityName = database; + + var dbContext = this.dbContextFetcher.Fetch(payload); + var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); + var providerName = this.providerNameFetcher.Fetch(dbContextDatabase); + + var dbSystem = GetDbSystemFromProviderName(providerName); + tags.Add(SemanticConventions.AttributeDbSystem, dbSystem); + if (dbSystem == "other_sql" && providerName != null) + { + tags.Add("ef.provider", providerName); + } + + if (!string.IsNullOrEmpty(dataSource)) + { + tags.Add(SemanticConventions.AttributeServerAddress, dataSource); + } + + if (options.EmitOldAttributes) + { + tags.Add(SemanticConventions.AttributeDbName, database); + } + + if (options.EmitNewAttributes) + { + tags.Add(SemanticConventions.AttributeDbNamespace, database); + } + + return tags; + } + public override void OnEventWritten(string name, object? payload) { + if (EntityFrameworkInstrumentation.TracingHandles == 0 && EntityFrameworkInstrumentation.MetricHandles == 0) + { + return; + } + var options = EntityFrameworkInstrumentation.TracingOptions; var activity = Activity.Current; @@ -69,102 +159,24 @@ public override void OnEventWritten(string name, object? payload) { case EntityFrameworkCoreCommandCreated: { - activity = SqlClientActivitySource.StartActivity(ActivityName, ActivityKind.Client); - if (activity == null) - { - // There is no listener or it decided not to sample the current request. - return; - } - var command = this.commandFetcher.Fetch(payload); if (command == null) { EntityFrameworkInstrumentationEventSource.Log.NullPayload(nameof(EntityFrameworkDiagnosticListener), name); - activity.Stop(); return; } - var connection = this.connectionFetcher.Fetch(command); - var database = (string)this.databaseFetcher.Fetch(connection); - activity.DisplayName = database; + var startTags = this.CreateTagsFromConnectionInfo(payload, command, options, out var activityName); + activity = EntityFrameworkActivitySource.StartActivity( + activityName, + ActivityKind.Client, + default(ActivityContext), + startTags); - if (activity.IsAllDataRequested) + if (activity == null) { - var dbContext = this.dbContextFetcher.Fetch(payload); - var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); - var providerName = this.providerNameFetcher.Fetch(dbContextDatabase); - - switch (providerName) - { - case "Microsoft.EntityFrameworkCore.SqlServer": - activity.AddTag(AttributeDbSystem, "mssql"); - break; - case "Microsoft.EntityFrameworkCore.Cosmos": - activity.AddTag(AttributeDbSystem, "cosmosdb"); - break; - case "Microsoft.EntityFrameworkCore.Sqlite": - case "Devart.Data.SQLite.Entity.EFCore": - activity.AddTag(AttributeDbSystem, "sqlite"); - break; - case "MySql.Data.EntityFrameworkCore": - case "Pomelo.EntityFrameworkCore.MySql": - case "Devart.Data.MySql.Entity.EFCore": - activity.AddTag(AttributeDbSystem, "mysql"); - break; - case "Npgsql.EntityFrameworkCore.PostgreSQL": - case "Devart.Data.PostgreSql.Entity.EFCore": - activity.AddTag(AttributeDbSystem, "postgresql"); - break; - case "Oracle.EntityFrameworkCore": - case "Devart.Data.Oracle.Entity.EFCore": - activity.AddTag(AttributeDbSystem, "oracle"); - break; - case "Microsoft.EntityFrameworkCore.InMemory": - activity.AddTag(AttributeDbSystem, "efcoreinmemory"); - break; - case "FirebirdSql.EntityFrameworkCore.Firebird": - activity.AddTag(AttributeDbSystem, "firebird"); - break; - case "FileContextCore": - activity.AddTag(AttributeDbSystem, "filecontextcore"); - break; - case "EntityFrameworkCore.SqlServerCompact35": - case "EntityFrameworkCore.SqlServerCompact40": - activity.AddTag(AttributeDbSystem, "mssqlcompact"); - break; - case "EntityFrameworkCore.OpenEdge": - activity.AddTag(AttributeDbSystem, "openedge"); - break; - case "EntityFrameworkCore.Jet": - activity.AddTag(AttributeDbSystem, "jet"); - break; - case "Google.Cloud.EntityFrameworkCore.Spanner": - activity.AddTag(AttributeDbSystem, "spanner"); - break; - case "Teradata.EntityFrameworkCore": - activity.AddTag(AttributeDbSystem, "teradata"); - break; - default: - activity.AddTag(AttributeDbSystem, "other_sql"); - activity.AddTag("ef.provider", providerName); - break; - } - - var dataSource = (string)this.dataSourceFetcher.Fetch(connection); - if (!string.IsNullOrEmpty(dataSource)) - { - activity.AddTag(AttributeServerAddress, dataSource); - } - - if (this.options.EmitOldAttributes) - { - activity.AddTag(AttributeDbName, database); - } - - if (this.options.EmitNewAttributes) - { - activity.AddTag(AttributeDbNamespace, database); - } + this.beginTimestamp.Value = Stopwatch.GetTimestamp(); + return; } } @@ -178,7 +190,7 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source != EntityFrameworkActivitySource) { return; } @@ -193,7 +205,7 @@ public override void OnEventWritten(string name, object? payload) var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); var providerName = this.providerNameFetcher.Fetch(dbContextDatabase); - if (command is IDbCommand typedCommand && this.options.Filter?.Invoke(providerName, typedCommand) == false) + if (command is IDbCommand typedCommand && options.Filter?.Invoke(providerName, typedCommand) == false) { EntityFrameworkInstrumentationEventSource.Log.CommandIsFilteredOut(activity.OperationName); activity.IsAllDataRequested = false; @@ -215,32 +227,32 @@ public override void OnEventWritten(string name, object? payload) switch (commandType) { case CommandType.StoredProcedure: - if (this.options.SetDbStatementForStoredProcedure) + if (options.SetDbStatementForStoredProcedure) { - if (this.options.EmitOldAttributes) + if (options.EmitOldAttributes) { - activity.AddTag(AttributeDbStatement, commandText); + activity.AddTag(SemanticConventions.AttributeDbStatement, commandText); } - if (this.options.EmitNewAttributes) + if (options.EmitNewAttributes) { - activity.AddTag(AttributeDbQueryText, commandText); + activity.AddTag(SemanticConventions.AttributeDbQueryText, commandText); } } break; case CommandType.Text: - if (this.options.SetDbStatementForText) + if (options.SetDbStatementForText) { - if (this.options.EmitOldAttributes) + if (options.EmitOldAttributes) { - activity.AddTag(AttributeDbStatement, commandText); + activity.AddTag(SemanticConventions.AttributeDbStatement, commandText); } - if (this.options.EmitNewAttributes) + if (options.EmitNewAttributes) { - activity.AddTag(AttributeDbQueryText, commandText); + activity.AddTag(SemanticConventions.AttributeDbQueryText, commandText); } } @@ -257,7 +269,7 @@ public override void OnEventWritten(string name, object? payload) { if (command is IDbCommand typedCommand) { - this.options.EnrichWithIDbCommand?.Invoke(activity, typedCommand); + options.EnrichWithIDbCommand?.Invoke(activity, typedCommand); } } catch (Exception ex) @@ -277,7 +289,7 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source != EntityFrameworkActivitySource) { return; } @@ -295,8 +307,9 @@ public override void OnEventWritten(string name, object? payload) return; } - if (activity.Source != SqlClientActivitySource) + if (activity.Source != EntityFrameworkActivitySource) { + this.RecordDuration(null, payload, true); return; } @@ -306,6 +319,7 @@ public override void OnEventWritten(string name, object? payload) { if (this.exceptionFetcher.Fetch(payload) is Exception exception) { + activity.AddTag(SemanticConventions.AttributeErrorType, exception.GetType().FullName); activity.SetStatus(ActivityStatusCode.Error, exception.Message); } else @@ -317,6 +331,7 @@ public override void OnEventWritten(string name, object? payload) finally { activity.Stop(); + this.RecordDuration(activity, payload, hasError: true); } } @@ -325,4 +340,48 @@ public override void OnEventWritten(string name, object? payload) break; } } + + private void RecordDuration(Activity? activity, object? payload, bool hasError = false) + { + if (EntityFrameworkInstrumentation.MetricHandles == 0) + { + return; + } + + var tags = default(TagList); + + if (activity != null && activity.IsAllDataRequested) + { + foreach (var tag in SharedTagNames) + { + var value = activity.GetTagItem(tag); + if (value != null) + { + tags.Add(tag, value); + } + } + } + else if (payload != null) + { + var command = this.commandFetcher.Fetch(payload); + var startTags = this.CreateTagsFromConnectionInfo(payload, command, EntityFrameworkInstrumentation.TracingOptions); + + foreach (var tag in startTags) + { + tags.Add(tag.Key, tag.Value); + } + + if (hasError) + { + if (this.exceptionFetcher.Fetch(payload) is Exception exception) + { + tags.Add(SemanticConventions.AttributeErrorType, exception.GetType().FullName); + } + } + } + + var duration = activity?.Duration.TotalSeconds + ?? CalculateDurationFromTimestamp(this.beginTimestamp.Value); + DbClientOperationDuration.Record(duration, tags); + } } diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj index 484fcdaeef..300e060638 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/OpenTelemetry.Instrumentation.EntityFrameworkCore.csproj @@ -30,6 +30,7 @@ + From b443e16cd46827644787ce8d4081274281035ce5 Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Mon, 24 Mar 2025 21:18:02 -0400 Subject: [PATCH 4/9] record duration record duration readability verify helper ShouldNotCollectMetrics test --- .../EntityFrameworkInstrumentation.cs | 3 +- .../EntityFrameworkDiagnosticListener.cs | 14 +- .../EntityFrameworkDiagnosticListenerTests.cs | 176 +++++++++++++++++- 3 files changed, 184 insertions(+), 9 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs index 8c53867166..596fc82b28 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/EntityFrameworkInstrumentation.cs @@ -7,11 +7,10 @@ namespace OpenTelemetry.Instrumentation.EntityFrameworkCore; internal class EntityFrameworkInstrumentation : IDisposable { - public static readonly EntityFrameworkInstrumentation Instance = new EntityFrameworkInstrumentation(); + public static readonly EntityFrameworkInstrumentation Instance = new(); internal static int MetricHandles; internal static int TracingHandles; - private readonly EntityFrameworkInstrumentationOptions options; private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber; public EntityFrameworkInstrumentation() diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index d617147c8e..50f8f2355d 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -46,8 +46,10 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler [ SemanticConventions.AttributeDbSystem, SemanticConventions.AttributeDbCollectionName, + SemanticConventions.AttributeDbName, SemanticConventions.AttributeDbNamespace, SemanticConventions.AttributeDbResponseStatusCode, + SemanticConventions.AttributeServerAddress, ]; private readonly PropertyFetcher commandFetcher = new("Command"); @@ -286,15 +288,18 @@ public override void OnEventWritten(string name, object? payload) if (activity == null) { EntityFrameworkInstrumentationEventSource.Log.NullActivity(name); + this.RecordDuration(null, payload); return; } if (activity.Source != EntityFrameworkActivitySource) { + this.RecordDuration(null, payload); return; } activity.Stop(); + this.RecordDuration(activity, payload); } break; @@ -304,12 +309,13 @@ public override void OnEventWritten(string name, object? payload) if (activity == null) { EntityFrameworkInstrumentationEventSource.Log.NullActivity(name); + this.RecordDuration(null, payload, hasError: true); return; } if (activity.Source != EntityFrameworkActivitySource) { - this.RecordDuration(null, payload, true); + this.RecordDuration(null, payload, hasError: true); return; } @@ -364,7 +370,11 @@ private void RecordDuration(Activity? activity, object? payload, bool hasError = else if (payload != null) { var command = this.commandFetcher.Fetch(payload); - var startTags = this.CreateTagsFromConnectionInfo(payload, command, EntityFrameworkInstrumentation.TracingOptions); + var startTags = this.CreateTagsFromConnectionInfo( + payload, + command, + EntityFrameworkInstrumentation.TracingOptions, + out _); foreach (var tag in startTags) { diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs index bfdee50fe0..ca871b4ce8 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs @@ -8,6 +8,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Infrastructure; using OpenTelemetry.Instrumentation.EntityFrameworkCore.Implementation; +using OpenTelemetry.Metrics; using OpenTelemetry.Trace; using Xunit; @@ -15,6 +16,7 @@ namespace OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests; public class EntityFrameworkDiagnosticListenerTests : IDisposable { + private static readonly string DbClientOperationDurationName = EntityFrameworkDiagnosticListener.DbClientOperationDuration.Name; private readonly DbContextOptions contextOptions; private readonly DbConnection connection; @@ -123,9 +125,14 @@ public void EntityFrameworkEnrichDisplayNameWithEnrichWithIDbCommand_New() [Fact] public void EntityFrameworkContextExceptionEventsInstrumentedTest() { - var exportedItems = new List(); - using var shutdownSignal = Sdk.CreateTracerProviderBuilder() - .AddInMemoryExporter(exportedItems) + var activities = new List(); + var metrics = new List(); + var tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddInMemoryExporter(activities) + .AddEntityFrameworkCoreInstrumentation() + .Build(); + var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddInMemoryExporter(metrics) .AddEntityFrameworkCoreInstrumentation() .Build(); @@ -141,10 +148,16 @@ public void EntityFrameworkContextExceptionEventsInstrumentedTest() } } - Assert.Single(exportedItems); - var activity = exportedItems[0]; + tracerProvider?.Dispose(); + meterProvider?.Dispose(); + var activity = Assert.Single(activities); + var dbClientOperationDurationMetrics = metrics + .Where(metric => metric.Name == DbClientOperationDurationName) + .ToArray(); + var metric = Assert.Single(dbClientOperationDurationMetrics); VerifyActivityData(activity, isError: true); + VerifyDurationMetricData(metric, activity, hasError: true); } [Fact] @@ -250,6 +263,110 @@ public void ShouldCollectTelemetryWhenFilterEvaluatesToTrueByProviderName() Assert.True(activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded)); } + [Fact] + public void EntityFrameworkMetricsAreCollectedSuccessfully() + { + var activities = new List(); + var metrics = new List(); + + var traceProvider = Sdk.CreateTracerProviderBuilder() + .AddEntityFrameworkCoreInstrumentation() + .AddInMemoryExporter(activities) + .Build(); + var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddEntityFrameworkCoreInstrumentation() + .AddInMemoryExporter(metrics) + .Build(); + + try + { + using (var context = new ItemsContext(this.contextOptions)) + { + var items = context.Set().OrderBy(e => e.Name).ToList(); + Assert.Equal(3, items.Count); + + //items = context.Set().Where(i => i.Name.StartsWith("Item")).ToList(); + //Assert.Equal(3, items.Count); + + //// Execute a query that will fail + //try + //{ + // context.Database.ExecuteSqlRaw("select * from no_table"); + //} + //catch + //{ + // // Expected exception + //} + } + } + finally + { + traceProvider.Dispose(); + meterProvider.Dispose(); + } + + var activity = Assert.Single(activities); + var dbClientOperationDurationMetrics = metrics + .Where(metric => metric.Name == EntityFrameworkDiagnosticListener.DbClientOperationDuration.Name) + .ToArray(); + var metric = Assert.Single(dbClientOperationDurationMetrics); + VerifyDurationMetricData(metric, activity, hasError: false); + } + + [Fact] + public void ShouldCollectMetrics() + { + var metrics = new List(); + var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddEntityFrameworkCoreInstrumentation() + .AddInMemoryExporter(metrics) + .Build(); + + try + { + using (var context = new ItemsContext(this.contextOptions)) + { + var items = context.Set().OrderBy(e => e.Name).ToList(); + Assert.Equal(3, items.Count); + } + } + finally + { + meterProvider.Dispose(); + } + + Assert.Contains( + metrics, + m => m.Name.StartsWith("db.", StringComparison.InvariantCulture)); + var dbClientOperationDurationMetrics = metrics + .Where(metric => metric.Name == DbClientOperationDurationName) + .ToArray(); + var metric = Assert.Single(dbClientOperationDurationMetrics); + VerifyDurationMetricData(metric, null); + } + + [Fact] + public void ShouldNotCollectMetrics() + { + var metrics = new List(); + using var meterProvider = Sdk.CreateMeterProviderBuilder() + .AddInMemoryExporter(metrics) + .Build(); + + using (var context = new ItemsContext(this.contextOptions)) + { + var items = context.Set().OrderBy(e => e.Name).ToList(); + Assert.Equal(3, items.Count); + } + + meterProvider.ForceFlush(); + + // Verify no db.* metrics were collected + Assert.DoesNotContain( + metrics, + m => m.Name.StartsWith("db.", StringComparison.InvariantCulture)); + } + public void Dispose() => this.connection.Dispose(); private static SqliteConnection CreateInMemoryDatabase() @@ -292,6 +409,55 @@ private static void VerifyActivityData(Activity activity, bool isError = false, } } + private static void VerifyDurationMetricData( + Metric metric, + Activity? activity = null, + bool hasError = false, + bool emitNewAttributes = false) + { + Assert.Equal(DbClientOperationDurationName, metric.Name); + Assert.Equal(MetricType.Histogram, metric.MetricType); + Assert.Equal("s", metric.Unit); + + var metricPoints = new List(); + foreach (var point in metric.GetMetricPoints()) + { + metricPoints.Add(point); + } + + var metricPoint = Assert.Single(metricPoints); + + Assert.True(metricPoint.GetHistogramSum() > 0); + if (activity != null) + { + Assert.Equal(activity.Duration.TotalSeconds, metricPoint.GetHistogramSum()); + } + + var tags = new Dictionary(); + foreach (var tag in metricPoint.Tags) + { + tags[tag.Key] = tag.Value; + } + + Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeDbSystem); + Assert.Equal("sqlite", tags[SemanticConventions.AttributeDbSystem]); + + if (!emitNewAttributes) + { + Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeDbName); + } + else if (emitNewAttributes) + { + Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeDbNamespace); + } + + if (hasError) + { + Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeErrorType); + Assert.Equal(typeof(SqliteException).FullName, tags[SemanticConventions.AttributeErrorType]); + } + } + private void Seed() { using var context = new ItemsContext(this.contextOptions); From d33d4a95eea72d7fe2155af0541108d46e0d0bc2 Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Tue, 22 Apr 2025 11:23:24 -0400 Subject: [PATCH 5/9] add meter extension to public API --- .../.publicApi/PublicAPI.Unshipped.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt index fdae0869f7..16fbdfd340 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/.publicApi/PublicAPI.Unshipped.txt @@ -8,7 +8,9 @@ OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentation OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForText.set -> void OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.get -> System.Action? OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.set -> void +OpenTelemetry.Metrics.EntityFrameworkCoreMeterProviderBuilderExtensions OpenTelemetry.Trace.TracerProviderBuilderExtensions +static OpenTelemetry.Metrics.EntityFrameworkCoreMeterProviderBuilderExtensions.AddEntityFrameworkCoreInstrumentation(this OpenTelemetry.Metrics.MeterProviderBuilder! builder) -> OpenTelemetry.Metrics.MeterProviderBuilder! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddEntityFrameworkCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder! builder) -> OpenTelemetry.Trace.TracerProviderBuilder! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddEntityFrameworkCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder! builder, string? name, System.Action? configure) -> OpenTelemetry.Trace.TracerProviderBuilder! static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddEntityFrameworkCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Action? configure) -> OpenTelemetry.Trace.TracerProviderBuilder! From 2949416c33a80c6c601fb45c90ff8d5fb3ffc58b Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Tue, 22 Apr 2025 11:23:35 -0400 Subject: [PATCH 6/9] linter --- .../EntityFrameworkDiagnosticListener.cs | 154 +++++++++--------- 1 file changed, 77 insertions(+), 77 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 50f8f2355d..2e4930a9a5 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 const string AttributeDbQueryText = "db.query.text"; internal static readonly Assembly Assembly = typeof(EntityFrameworkDiagnosticListener).Assembly; - public static readonly AssemblyName AssemblyName = Assembly.GetName(); + internal static readonly AssemblyName AssemblyName = Assembly.GetName(); internal static readonly string ActivitySourceName = AssemblyName.Name!; internal static readonly string ActivityName = ActivitySourceName + ".Execute"; @@ -71,82 +71,6 @@ public EntityFrameworkDiagnosticListener(string sourceName) public override bool SupportsNullActivity => true; - private static double CalculateDurationFromTimestamp(long begin, long? end = null) - { - end ??= Stopwatch.GetTimestamp(); - var timestampToTicks = TimeSpan.TicksPerSecond / (double)Stopwatch.Frequency; - var delta = end - begin; - var ticks = (long)(timestampToTicks * delta); - var duration = new TimeSpan(ticks); - return duration.TotalSeconds; - } - - private static string GetDbSystemFromProviderName(string? providerName) - { - return providerName switch - { - "Microsoft.EntityFrameworkCore.SqlServer" => "mssql", - "Microsoft.EntityFrameworkCore.Cosmos" => "cosmosdb", - "Microsoft.EntityFrameworkCore.Sqlite" or "Devart.Data.SQLite.Entity.EFCore" => "sqlite", - "MySql.Data.EntityFrameworkCore" or "Pomelo.EntityFrameworkCore.MySql" or "Devart.Data.MySql.Entity.EFCore" => "mysql", - "Npgsql.EntityFrameworkCore.PostgreSQL" or "Devart.Data.PostgreSql.Entity.EFCore" => "postgresql", - "Oracle.EntityFrameworkCore" or "Devart.Data.Oracle.Entity.EFCore" => "oracle", - "Microsoft.EntityFrameworkCore.InMemory" => "efcoreinmemory", - "FirebirdSql.EntityFrameworkCore.Firebird" => "firebird", - "FileContextCore" => "filecontextcore", - "EntityFrameworkCore.SqlServerCompact35" or "EntityFrameworkCore.SqlServerCompact40" => "mssqlcompact", - "EntityFrameworkCore.OpenEdge" => "openedge", - "EntityFrameworkCore.Jet" => "jet", - "Google.Cloud.EntityFrameworkCore.Spanner" => "spanner", - "Teradata.EntityFrameworkCore" => "teradata", - _ => "other_sql", - }; - } - - private TagList CreateTagsFromConnectionInfo(object? payload, object? command, EntityFrameworkInstrumentationOptions options, out string activityName) - { - activityName = ActivityName; - var tags = default(TagList); - - if (payload == null || command == null) - { - return tags; - } - - var connection = this.connectionFetcher.Fetch(command); - var dataSource = (string)this.dataSourceFetcher.Fetch(connection); - var database = (string)this.databaseFetcher.Fetch(connection); - activityName = database; - - var dbContext = this.dbContextFetcher.Fetch(payload); - var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); - var providerName = this.providerNameFetcher.Fetch(dbContextDatabase); - - var dbSystem = GetDbSystemFromProviderName(providerName); - tags.Add(SemanticConventions.AttributeDbSystem, dbSystem); - if (dbSystem == "other_sql" && providerName != null) - { - tags.Add("ef.provider", providerName); - } - - if (!string.IsNullOrEmpty(dataSource)) - { - tags.Add(SemanticConventions.AttributeServerAddress, dataSource); - } - - if (options.EmitOldAttributes) - { - tags.Add(SemanticConventions.AttributeDbName, database); - } - - if (options.EmitNewAttributes) - { - tags.Add(SemanticConventions.AttributeDbNamespace, database); - } - - return tags; - } - public override void OnEventWritten(string name, object? payload) { if (EntityFrameworkInstrumentation.TracingHandles == 0 && EntityFrameworkInstrumentation.MetricHandles == 0) @@ -347,6 +271,38 @@ public override void OnEventWritten(string name, object? payload) } } + private static double CalculateDurationFromTimestamp(long begin, long? end = null) + { + end ??= Stopwatch.GetTimestamp(); + var timestampToTicks = TimeSpan.TicksPerSecond / (double)Stopwatch.Frequency; + var delta = end - begin; + var ticks = (long)(timestampToTicks * delta); + var duration = new TimeSpan(ticks); + return duration.TotalSeconds; + } + + private static string GetDbSystemFromProviderName(string? providerName) + { + return providerName switch + { + "Microsoft.EntityFrameworkCore.SqlServer" => "mssql", + "Microsoft.EntityFrameworkCore.Cosmos" => "cosmosdb", + "Microsoft.EntityFrameworkCore.Sqlite" or "Devart.Data.SQLite.Entity.EFCore" => "sqlite", + "MySql.Data.EntityFrameworkCore" or "Pomelo.EntityFrameworkCore.MySql" or "Devart.Data.MySql.Entity.EFCore" => "mysql", + "Npgsql.EntityFrameworkCore.PostgreSQL" or "Devart.Data.PostgreSql.Entity.EFCore" => "postgresql", + "Oracle.EntityFrameworkCore" or "Devart.Data.Oracle.Entity.EFCore" => "oracle", + "Microsoft.EntityFrameworkCore.InMemory" => "efcoreinmemory", + "FirebirdSql.EntityFrameworkCore.Firebird" => "firebird", + "FileContextCore" => "filecontextcore", + "EntityFrameworkCore.SqlServerCompact35" or "EntityFrameworkCore.SqlServerCompact40" => "mssqlcompact", + "EntityFrameworkCore.OpenEdge" => "openedge", + "EntityFrameworkCore.Jet" => "jet", + "Google.Cloud.EntityFrameworkCore.Spanner" => "spanner", + "Teradata.EntityFrameworkCore" => "teradata", + _ => "other_sql", + }; + } + private void RecordDuration(Activity? activity, object? payload, bool hasError = false) { if (EntityFrameworkInstrumentation.MetricHandles == 0) @@ -394,4 +350,48 @@ private void RecordDuration(Activity? activity, object? payload, bool hasError = ?? CalculateDurationFromTimestamp(this.beginTimestamp.Value); DbClientOperationDuration.Record(duration, tags); } + + private TagList CreateTagsFromConnectionInfo(object? payload, object? command, EntityFrameworkInstrumentationOptions options, out string activityName) + { + activityName = ActivityName; + var tags = default(TagList); + + if (payload == null || command == null) + { + return tags; + } + + var connection = this.connectionFetcher.Fetch(command); + var dataSource = (string)this.dataSourceFetcher.Fetch(connection); + var database = (string)this.databaseFetcher.Fetch(connection); + activityName = database; + + var dbContext = this.dbContextFetcher.Fetch(payload); + var dbContextDatabase = this.dbContextDatabaseFetcher.Fetch(dbContext); + var providerName = this.providerNameFetcher.Fetch(dbContextDatabase); + + var dbSystem = GetDbSystemFromProviderName(providerName); + tags.Add(SemanticConventions.AttributeDbSystem, dbSystem); + if (dbSystem == "other_sql" && providerName != null) + { + tags.Add("ef.provider", providerName); + } + + if (!string.IsNullOrEmpty(dataSource)) + { + tags.Add(SemanticConventions.AttributeServerAddress, dataSource); + } + + if (options.EmitOldAttributes) + { + tags.Add(SemanticConventions.AttributeDbName, database); + } + + if (options.EmitNewAttributes) + { + tags.Add(SemanticConventions.AttributeDbNamespace, database); + } + + return tags; + } } From 6572e0b5eda00b714868414b5dcc8aa908dff302 Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Tue, 22 Apr 2025 15:17:12 -0400 Subject: [PATCH 7/9] check combinations of tracing/metrics --- .../EntityFrameworkDiagnosticListenerTests.cs | 86 ++++++++++++------- 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs index ca871b4ce8..ed1e838662 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs @@ -263,54 +263,74 @@ public void ShouldCollectTelemetryWhenFilterEvaluatesToTrueByProviderName() Assert.True(activity.ActivityTraceFlags.HasFlag(ActivityTraceFlags.Recorded)); } - [Fact] - public void EntityFrameworkMetricsAreCollectedSuccessfully() + [Theory] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void ShouldConditionallyCollectTelemetry(bool enableTracing, bool enableMetrics) { var activities = new List(); var metrics = new List(); - var traceProvider = Sdk.CreateTracerProviderBuilder() - .AddEntityFrameworkCoreInstrumentation() - .AddInMemoryExporter(activities) - .Build(); - var meterProvider = Sdk.CreateMeterProviderBuilder() - .AddEntityFrameworkCoreInstrumentation() - .AddInMemoryExporter(metrics) - .Build(); + var traceProviderBuilder = Sdk.CreateTracerProviderBuilder(); + var meterProviderBuilder = Sdk.CreateMeterProviderBuilder(); - try + if (enableTracing) { - using (var context = new ItemsContext(this.contextOptions)) - { - var items = context.Set().OrderBy(e => e.Name).ToList(); - Assert.Equal(3, items.Count); + traceProviderBuilder.AddEntityFrameworkCoreInstrumentation(); + traceProviderBuilder.AddInMemoryExporter(activities); + } - //items = context.Set().Where(i => i.Name.StartsWith("Item")).ToList(); - //Assert.Equal(3, items.Count); - - //// Execute a query that will fail - //try - //{ - // context.Database.ExecuteSqlRaw("select * from no_table"); - //} - //catch - //{ - // // Expected exception - //} - } + if (enableMetrics) + { + meterProviderBuilder.AddEntityFrameworkCoreInstrumentation(); + meterProviderBuilder.AddInMemoryExporter(metrics); + } + + var traceProvider = traceProviderBuilder.Build(); + var meterProvider = meterProviderBuilder.Build(); + + try + { + using var context = new ItemsContext(this.contextOptions); + var items = context.Set().OrderBy(e => e.Name).ToList(); + Assert.Equal(3, items.Count); } finally { - traceProvider.Dispose(); - meterProvider.Dispose(); + traceProvider?.Dispose(); + meterProvider?.Dispose(); + } + + Activity? activity = null; + + if (enableTracing) + { + activity = Assert.Single(activities); + VerifyActivityData(activity, isError: false); + } + else + { + Assert.Empty(activities); } - var activity = Assert.Single(activities); var dbClientOperationDurationMetrics = metrics .Where(metric => metric.Name == EntityFrameworkDiagnosticListener.DbClientOperationDuration.Name) .ToArray(); - var metric = Assert.Single(dbClientOperationDurationMetrics); - VerifyDurationMetricData(metric, activity, hasError: false); + + if (enableMetrics) + { + Assert.Contains( + metrics, + m => m.Name.StartsWith("db.", StringComparison.InvariantCulture)); + var metric = Assert.Single(dbClientOperationDurationMetrics); + VerifyDurationMetricData(metric, activity, hasError: false); + } + else + { + Assert.Empty(dbClientOperationDurationMetrics); + } } [Fact] From 2d512cdc2edc9bf11fd41fa3f5eb11f4e1a25a8d Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Tue, 22 Apr 2025 17:16:38 -0400 Subject: [PATCH 8/9] copy error.type --- .../Implementation/EntityFrameworkDiagnosticListener.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs index 2e4930a9a5..b0537a5b36 100644 --- a/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs +++ b/src/OpenTelemetry.Instrumentation.EntityFrameworkCore/Implementation/EntityFrameworkDiagnosticListener.cs @@ -49,6 +49,7 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler SemanticConventions.AttributeDbName, SemanticConventions.AttributeDbNamespace, SemanticConventions.AttributeDbResponseStatusCode, + SemanticConventions.AttributeErrorType, SemanticConventions.AttributeServerAddress, ]; From 78fc694ec2cf796c7f113187cf01cb1daa5bcf8d Mon Sep 17 00:00:00 2001 From: Matthew Hensley Date: Tue, 22 Apr 2025 17:16:59 -0400 Subject: [PATCH 9/9] test cleanup for improved failure msgs --- .../EntityFrameworkDiagnosticListenerTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs index ed1e838662..aec2aeafde 100644 --- a/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs +++ b/test/OpenTelemetry.Instrumentation.EntityFrameworkCore.Tests/EntityFrameworkDiagnosticListenerTests.cs @@ -362,7 +362,7 @@ public void ShouldCollectMetrics() .Where(metric => metric.Name == DbClientOperationDurationName) .ToArray(); var metric = Assert.Single(dbClientOperationDurationMetrics); - VerifyDurationMetricData(metric, null); + VerifyDurationMetricData(metric); } [Fact] @@ -459,21 +459,21 @@ private static void VerifyDurationMetricData( tags[tag.Key] = tag.Value; } - Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeDbSystem); + Assert.Contains(SemanticConventions.AttributeDbSystem, tags.Keys); Assert.Equal("sqlite", tags[SemanticConventions.AttributeDbSystem]); if (!emitNewAttributes) { - Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeDbName); + Assert.Contains(SemanticConventions.AttributeDbName, tags.Keys); } else if (emitNewAttributes) { - Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeDbNamespace); + Assert.Contains(SemanticConventions.AttributeDbNamespace, tags.Keys); } if (hasError) { - Assert.Contains(tags, t => t.Key == SemanticConventions.AttributeErrorType); + Assert.Contains(SemanticConventions.AttributeErrorType, tags.Keys); Assert.Equal(typeof(SqliteException).FullName, tags[SemanticConventions.AttributeErrorType]); } }