Skip to content

Commit

Permalink
Nullable Instrumentation.EntityFrameworkCore (#1366)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kielek authored Nov 2, 2023
1 parent 2f29ddb commit a372bc0
Show file tree
Hide file tree
Showing 19 changed files with 76 additions and 74 deletions.
8 changes: 7 additions & 1 deletion build/Common.props
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,20 @@
</ItemGroup>

<ItemGroup Condition="'$(IncludeSharedInstrumentationSource)'=='true'">
<Compile Include="$(RepoRoot)\src\Shared\ActivityInstrumentationHelper.cs" Link="Includes\ActivityInstrumentationHelper.cs" />
<Compile Include="$(RepoRoot)\src\Shared\MultiTypePropertyFetcher.cs" Link="Includes\MultiTypePropertyFetcher.cs" />
<Compile Include="$(RepoRoot)\src\Shared\PropertyFetcher.cs" Link="Includes\PropertyFetcher.cs" />
<Compile Include="$(RepoRoot)\src\Shared\SemanticConventions.cs" Link="Includes\SemanticConventions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\SpanAttributeConstants.cs" Link="Includes\SpanAttributeConstants.cs" />
</ItemGroup>

<ItemGroup Condition="'$(IncludeSharedSpanHelper)'=='true'">
<Compile Include="$(RepoRoot)\src\Shared\SpanHelper.cs" Link="Includes\SpanHelper.cs" />
</ItemGroup>

<ItemGroup Condition="'$(IncludeSharedActivityInstrumentationHelper)'=='true'">
<Compile Include="$(RepoRoot)\src\Shared\ActivityInstrumentationHelper.cs" Link="Includes\ActivityInstrumentationHelper.cs" />
</ItemGroup>

<ItemGroup Condition="'$(IncludeSharedDiagnosticSourceSubscriber)'=='true'">
<Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceListener.cs" Link="Includes\DiagnosticSourceListener.cs" />
<Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceSubscriber.cs" Link="Includes\DiagnosticSourceSubscriber.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
using System.Text;
using System.Text.Json;
using System.Text.RegularExpressions;
using OpenTelemetry.Internal;
using OpenTelemetry.Trace;

namespace OpenTelemetry.Instrumentation.ElasticsearchClient.Implementation;
Expand Down Expand Up @@ -67,6 +68,7 @@ public override void OnStartActivity(Activity activity, object payload)
return;
}

Guard.ThrowIfNull(activity);
if (activity.IsAllDataRequested)
{
var uri = this.uriFetcher.Fetch(payload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
<IncludeSharedInstrumentationSource>true</IncludeSharedInstrumentationSource>
<IncludeSharedExceptionExtensionsSource>true</IncludeSharedExceptionExtensionsSource>
<IncludeSharedDiagnosticSourceSubscriber>true</IncludeSharedDiagnosticSourceSubscriber>
<IncludeSharedActivityInstrumentationHelper>true</IncludeSharedActivityInstrumentationHelper>
<IncludeSharedSpanHelper>true</IncludeSharedSpanHelper>
<Nullable>disable</Nullable>
</PropertyGroup>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#nullable enable
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EntityFrameworkInstrumentationOptions() -> void
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.Filter.get -> System.Func<string, System.Data.IDbCommand, bool>
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.Filter.get -> System.Func<string?, System.Data.IDbCommand!, bool>?
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.Filter.set -> void
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForStoredProcedure.get -> bool
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForStoredProcedure.set -> void
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForText.get -> bool
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.SetDbStatementForText.set -> void
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.get -> System.Action<System.Diagnostics.Activity, System.Data.IDbCommand>
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.get -> System.Action<System.Diagnostics.Activity!, System.Data.IDbCommand!>?
OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions.EnrichWithIDbCommand.set -> void
OpenTelemetry.Trace.TracerProviderBuilderExtensions
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<OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddEntityFrameworkCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder builder, System.Action<OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions> configure) -> OpenTelemetry.Trace.TracerProviderBuilder
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<OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions!>? configure) -> OpenTelemetry.Trace.TracerProviderBuilder!
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddEntityFrameworkCoreInstrumentation(this OpenTelemetry.Trace.TracerProviderBuilder! builder, System.Action<OpenTelemetry.Instrumentation.EntityFrameworkCore.EntityFrameworkInstrumentationOptions!>? configure) -> OpenTelemetry.Trace.TracerProviderBuilder!
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal class EntityFrameworkInstrumentation : IDisposable
{
private readonly DiagnosticSourceSubscriber diagnosticSourceSubscriber;

public EntityFrameworkInstrumentation(EntityFrameworkInstrumentationOptions options)
public EntityFrameworkInstrumentation(EntityFrameworkInstrumentationOptions? options)
{
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
name => new EntityFrameworkDiagnosticListener(name, options),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class EntityFrameworkInstrumentationOptions
/// <para><see cref="Activity"/>: the activity being enriched.</para>
/// <para><see cref="IDbCommand"/>: db command to allow access to command.</para>
/// </remarks>
public Action<Activity, IDbCommand> EnrichWithIDbCommand { get; set; }
public Action<Activity, IDbCommand>? EnrichWithIDbCommand { get; set; }

/// <summary>
/// Gets or sets a filter function that determines whether or not to
Expand All @@ -64,5 +64,5 @@ public class EntityFrameworkInstrumentationOptions
/// </list></item>
/// </list>
/// </remarks>
public Func<string, IDbCommand, bool> Filter { get; set; }
public Func<string?, IDbCommand, bool>? Filter { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,31 +40,31 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler

private static readonly Version Version = typeof(EntityFrameworkDiagnosticListener).Assembly.GetName().Version;
#pragma warning disable SA1202 // Elements should be ordered by access <- In this case, Version MUST come before SqlClientActivitySource otherwise null ref exception is thrown.
internal static readonly ActivitySource SqlClientActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());
internal static readonly ActivitySource SqlClientActivitySource = new(ActivitySourceName, Version.ToString());
#pragma warning restore SA1202 // Elements should be ordered by access

private readonly PropertyFetcher<object> commandFetcher = new PropertyFetcher<object>("Command");
private readonly PropertyFetcher<object> connectionFetcher = new PropertyFetcher<object>("Connection");
private readonly PropertyFetcher<object> dbContextFetcher = new PropertyFetcher<object>("Context");
private readonly PropertyFetcher<object> dbContextDatabaseFetcher = new PropertyFetcher<object>("Database");
private readonly PropertyFetcher<string> providerNameFetcher = new PropertyFetcher<string>("ProviderName");
private readonly PropertyFetcher<object> dataSourceFetcher = new PropertyFetcher<object>("DataSource");
private readonly PropertyFetcher<object> databaseFetcher = new PropertyFetcher<object>("Database");
private readonly PropertyFetcher<CommandType> commandTypeFetcher = new PropertyFetcher<CommandType>("CommandType");
private readonly PropertyFetcher<string> commandTextFetcher = new PropertyFetcher<string>("CommandText");
private readonly PropertyFetcher<Exception> exceptionFetcher = new PropertyFetcher<Exception>("Exception");
private readonly PropertyFetcher<object> commandFetcher = new("Command");
private readonly PropertyFetcher<object> connectionFetcher = new("Connection");
private readonly PropertyFetcher<object> dbContextFetcher = new("Context");
private readonly PropertyFetcher<object> dbContextDatabaseFetcher = new("Database");
private readonly PropertyFetcher<string> providerNameFetcher = new("ProviderName");
private readonly PropertyFetcher<object> dataSourceFetcher = new("DataSource");
private readonly PropertyFetcher<object> databaseFetcher = new("Database");
private readonly PropertyFetcher<CommandType> commandTypeFetcher = new("CommandType");
private readonly PropertyFetcher<string> commandTextFetcher = new("CommandText");
private readonly PropertyFetcher<Exception> exceptionFetcher = new("Exception");

private readonly EntityFrameworkInstrumentationOptions options;

public EntityFrameworkDiagnosticListener(string sourceName, EntityFrameworkInstrumentationOptions options)
public EntityFrameworkDiagnosticListener(string sourceName, EntityFrameworkInstrumentationOptions? options)
: base(sourceName)
{
this.options = options ?? new EntityFrameworkInstrumentationOptions();
}

public override bool SupportsNullActivity => true;

public override void OnCustom(string name, Activity activity, object payload)
public override void OnCustom(string name, Activity? activity, object? payload)
{
switch (name)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,6 @@ public void NullPayload(string handlerName, string eventName)
this.WriteEvent(3, handlerName, eventName);
}

[Event(4, Message = "Payload is invalid in event '{1}' from handler '{0}', span will not be recorded.", Level = EventLevel.Warning)]
public void InvalidPayload(string handlerName, string eventName)
{
this.WriteEvent(4, handlerName, eventName);
}

[Event(5, Message = "Enrichment threw exception. Exception {0}.", Level = EventLevel.Error)]
public void EnrichmentException(string eventName, string exception)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
<IncludeSharedInstrumentationSource>true</IncludeSharedInstrumentationSource>
<IncludeSharedExceptionExtensionsSource>true</IncludeSharedExceptionExtensionsSource>
<IncludeSharedDiagnosticSourceSubscriber>true</IncludeSharedDiagnosticSourceSubscriber>
<Nullable>disable</Nullable>
</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public static TracerProviderBuilder AddEntityFrameworkCoreInstrumentation(
/// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
public static TracerProviderBuilder AddEntityFrameworkCoreInstrumentation(
this TracerProviderBuilder builder,
Action<EntityFrameworkInstrumentationOptions> configure) =>
Action<EntityFrameworkInstrumentationOptions>? configure) =>
AddEntityFrameworkCoreInstrumentation(builder, name: null, configure);

/// <summary>
Expand All @@ -57,8 +57,8 @@ public static TracerProviderBuilder AddEntityFrameworkCoreInstrumentation(
/// <returns>The instance of <see cref="TracerProviderBuilder"/> to chain the calls.</returns>
public static TracerProviderBuilder AddEntityFrameworkCoreInstrumentation(
this TracerProviderBuilder builder,
string name,
Action<EntityFrameworkInstrumentationOptions> configure)
string? name,
Action<EntityFrameworkInstrumentationOptions>? configure)
{
Guard.ThrowIfNull(builder);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
<Description>OpenTelemetry instrumentation for OWIN</Description>
<PackageTags>$(PackageTags);distributed-tracing;OWIN</PackageTags>
<MinVerTagPrefix>Instrumentation.Owin-</MinVerTagPrefix>
<IncludeSharedSpanHelper>true</IncludeSharedSpanHelper>
<Nullable>disable</Nullable>
</PropertyGroup>

<ItemGroup>
<Compile Include="$(RepoRoot)\src\Shared\SemanticConventions.cs" Link="Includes\SemanticConventions.cs"/>
<Compile Include="$(RepoRoot)\src\Shared\SpanHelper.cs" Link="Includes\SpanHelper.cs"/>
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
</ItemGroup>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ public QuartzDiagnosticListener(string sourceName, QuartzInstrumentationOptions
this.options = options;
}

public override void OnStartActivity(Activity activity, object payload)
public override void OnStartActivity(Activity? activity, object? payload)
{
Guard.ThrowIfNull(activity);
if (activity.IsAllDataRequested)
{
if (!this.options.TracedOperations.Contains(activity.OperationName))
Expand Down Expand Up @@ -73,8 +74,9 @@ public override void OnStartActivity(Activity activity, object payload)
}
}

public override void OnStopActivity(Activity activity, object payload)
public override void OnStopActivity(Activity? activity, object? payload)
{
Guard.ThrowIfNull(activity);
if (activity.IsAllDataRequested)
{
try
Expand All @@ -92,8 +94,9 @@ public override void OnStopActivity(Activity activity, object payload)
}
}

public override void OnException(Activity activity, object payload)
public override void OnException(Activity? activity, object? payload)
{
Guard.ThrowIfNull(activity);
if (activity.IsAllDataRequested)
{
var exc = payload as Exception;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
<TargetFrameworks>netstandard2.0</TargetFrameworks>
<IncludeSharedExceptionExtensionsSource>true</IncludeSharedExceptionExtensionsSource>
<IncludeSharedDiagnosticSourceSubscriber>true</IncludeSharedDiagnosticSourceSubscriber>
<IncludeSharedActivityInstrumentationHelper>true</IncludeSharedActivityInstrumentationHelper>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="OpenTelemetry.Api" Version="$(OpenTelemetryCoreLatestVersion)" />
Expand Down
8 changes: 4 additions & 4 deletions src/Shared/DiagnosticSourceListener.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// limitations under the License.
// </copyright>

#nullable disable
#nullable enable

#pragma warning disable IDE0005 // Using directive is unnecessary.
using System;
Expand All @@ -25,7 +25,7 @@

namespace OpenTelemetry.Instrumentation;

internal sealed class DiagnosticSourceListener : IObserver<KeyValuePair<string, object>>
internal sealed class DiagnosticSourceListener : IObserver<KeyValuePair<string, object?>>
{
private readonly ListenerHandler handler;

Expand All @@ -47,7 +47,7 @@ public void OnError(Exception error)
{
}

public void OnNext(KeyValuePair<string, object> value)
public void OnNext(KeyValuePair<string, object?> value)
{
if (!this.handler.SupportsNullActivity && Activity.Current == null)
{
Expand Down Expand Up @@ -75,7 +75,7 @@ public void OnNext(KeyValuePair<string, object> value)
}
catch (Exception ex)
{
this.logUnknownException?.Invoke(this.handler?.SourceName, value.Key, ex);
this.logUnknownException?.Invoke(this.handler.SourceName, value.Key, ex);
}
}
}
10 changes: 5 additions & 5 deletions src/Shared/DiagnosticSourceSubscriber.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// limitations under the License.
// </copyright>

#nullable disable
#nullable enable

#pragma warning disable IDE0005 // Using directive is unnecessary.
using System;
Expand All @@ -31,14 +31,14 @@ internal sealed class DiagnosticSourceSubscriber : IDisposable, IObserver<Diagno
private readonly List<IDisposable> listenerSubscriptions;
private readonly Func<string, ListenerHandler> handlerFactory;
private readonly Func<DiagnosticListener, bool> diagnosticSourceFilter;
private readonly Func<string, object, object, bool> isEnabledFilter;
private readonly Func<string, object?, object?, bool>? isEnabledFilter;
private readonly Action<string, string, Exception> logUnknownException;
private long disposed;
private IDisposable allSourcesSubscription;
private IDisposable? allSourcesSubscription;

public DiagnosticSourceSubscriber(
ListenerHandler handler,
Func<string, object, object, bool> isEnabledFilter,
Func<string, object?, object?, bool> isEnabledFilter,
Action<string, string, Exception> logUnknownException)
: this(_ => handler, value => handler.SourceName == value.Name, isEnabledFilter, logUnknownException)
{
Expand All @@ -47,7 +47,7 @@ public DiagnosticSourceSubscriber(
public DiagnosticSourceSubscriber(
Func<string, ListenerHandler> handlerFactory,
Func<DiagnosticListener, bool> diagnosticSourceFilter,
Func<string, object, object, bool> isEnabledFilter,
Func<string, object?, object?, bool>? isEnabledFilter,
Action<string, string, Exception> logUnknownException)
{
Guard.ThrowIfNull(handlerFactory);
Expand Down
13 changes: 8 additions & 5 deletions src/Shared/ListenerHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

#nullable enable

using System.Diagnostics;

namespace OpenTelemetry.Instrumentation;
Expand All @@ -26,7 +29,7 @@ internal abstract class ListenerHandler
/// Initializes a new instance of the <see cref="ListenerHandler"/> class.
/// </summary>
/// <param name="sourceName">The name of the <see cref="ListenerHandler"/>.</param>
public ListenerHandler(string sourceName)
protected ListenerHandler(string sourceName)
{
this.SourceName = sourceName;
}
Expand All @@ -46,7 +49,7 @@ public ListenerHandler(string sourceName)
/// </summary>
/// <param name="activity">The <see cref="Activity"/> to be started.</param>
/// <param name="payload">An object that represent the value being passed as a payload for the event.</param>
public virtual void OnStartActivity(Activity activity, object payload)
public virtual void OnStartActivity(Activity? activity, object? payload)
{
}

Expand All @@ -55,7 +58,7 @@ public virtual void OnStartActivity(Activity activity, object payload)
/// </summary>
/// <param name="activity">The <see cref="Activity"/> to be stopped.</param>
/// <param name="payload">An object that represent the value being passed as a payload for the event.</param>
public virtual void OnStopActivity(Activity activity, object payload)
public virtual void OnStopActivity(Activity? activity, object? payload)
{
}

Expand All @@ -64,7 +67,7 @@ public virtual void OnStopActivity(Activity activity, object payload)
/// </summary>
/// <param name="activity">The <see cref="Activity"/>.</param>
/// <param name="payload">An object that represent the value being passed as a payload for the event.</param>
public virtual void OnException(Activity activity, object payload)
public virtual void OnException(Activity? activity, object? payload)
{
}

Expand All @@ -74,7 +77,7 @@ public virtual void OnException(Activity activity, object payload)
/// <param name="name">Custom name.</param>
/// <param name="activity">The <see cref="Activity"/> to be processed.</param>
/// <param name="payload">An object that represent the value being passed as a payload for the event.</param>
public virtual void OnCustom(string name, Activity activity, object payload)
public virtual void OnCustom(string name, Activity? activity, object? payload)
{
}
}
Loading

0 comments on commit a372bc0

Please sign in to comment.