Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Event-Exception: "Microsoft.Data.SqlClient.WriteCommandBefore" #89

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
+136 −47
Open

Fix Event-Exception: "Microsoft.Data.SqlClient.WriteCommandBefore" #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e4a2c20
Use Expression.New instead of Activator.CreateInstance
AndrewP-GH Feb 9, 2021
c4b038a
Use ConcurrentDictionary to save fetchers
AndrewP-GH Feb 9, 2021
0f3d20d
Add tests
AndrewP-GH Feb 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/OpenTracing.Contrib.NetCore/Internal/PropertyFetcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;

namespace OpenTracing.Contrib.NetCore.Internal
Expand Down Expand Up @@ -33,6 +34,7 @@ public object Fetch(object obj)
_fetchForExpectedType = PropertyFetch.FetcherForProperty(propertyInfo);
_expectedType = objType;
}

return _fetchForExpectedType.Fetch(obj);
}

Expand All @@ -51,13 +53,18 @@ private class PropertyFetch
public static PropertyFetch FetcherForProperty(PropertyInfo propertyInfo)
{
if (propertyInfo == null)
return new PropertyFetch(); // returns null on any fetch.
return new PropertyFetch(); // returns null on any fetch.

Type typedPropertyFetcher = typeof(TypedFetchProperty<,>);
Type instantiatedTypedPropertyFetcher = typedPropertyFetcher.GetTypeInfo().MakeGenericType(
propertyInfo.DeclaringType, propertyInfo.PropertyType);

return (PropertyFetch)Activator.CreateInstance(instantiatedTypedPropertyFetcher, propertyInfo);
Type propertyInfoType = typeof(PropertyInfo);
ConstructorInfo ctor = instantiatedTypedPropertyFetcher.GetConstructor(new[] {propertyInfoType});
ParameterExpression ctorParameter = Expression.Parameter(propertyInfoType);
Delegate expression = Expression.Lambda(Expression.New(ctor, ctorParameter), ctorParameter)
.Compile();
return (PropertyFetch)expression.DynamicInvoke(propertyInfo);
}

/// <summary>
Expand All @@ -74,10 +81,12 @@ public TypedFetchProperty(PropertyInfo property)
{
_propertyFetch = (Func<TObject, TProperty>)property.GetMethod.CreateDelegate(typeof(Func<TObject, TProperty>));
}

public override object Fetch(object obj)
{
return _propertyFetch((TObject)obj);
}

private readonly Func<TObject, TProperty> _propertyFetch;
}
}
Expand Down
106 changes: 62 additions & 44 deletions src/OpenTracing.Contrib.NetCore/MicrosoftSqlClient/MicrosoftSqlClientDiagnostics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,27 @@ internal sealed class MicrosoftSqlClientDiagnostics : DiagnosticEventObserver
{
public const string DiagnosticListenerName = "SqlClientDiagnosticListener";

private static readonly PropertyFetcher _activityCommand_RequestFetcher = new PropertyFetcher("Command");
private static readonly PropertyFetcher _exception_ExceptionFetcher = new PropertyFetcher("Exception");
private static Func<Type, PropertyFetcher> CommandFetcherFactoryMethod =>
_ => new PropertyFetcher("Command");

private static Func<Type, PropertyFetcher> ExceptionFetcherFactoryMethod =>
_ => new PropertyFetcher("Exception");

private readonly MicrosoftSqlClientDiagnosticOptions _options;
private readonly ConcurrentDictionary<object, ISpan> _spanStorage;

public MicrosoftSqlClientDiagnostics(ILoggerFactory loggerFactory, ITracer tracer, IOptions<MicrosoftSqlClientDiagnosticOptions> options)
: base(loggerFactory, tracer, options?.Value)
private readonly ConcurrentDictionary<Type, PropertyFetcher> _activityCommandFetchers;
private readonly ConcurrentDictionary<Type, PropertyFetcher> _exceptionFetchers;

public MicrosoftSqlClientDiagnostics(
ILoggerFactory loggerFactory,
ITracer tracer,
IOptions<MicrosoftSqlClientDiagnosticOptions> options)
: base(loggerFactory, tracer, options?.Value)
{
_options = options?.Value ?? throw new ArgumentNullException(nameof(options));
_spanStorage = new ConcurrentDictionary<object, ISpan>();
_activityCommandFetchers = new ConcurrentDictionary<Type, PropertyFetcher>();
_exceptionFetchers = new ConcurrentDictionary<Type, PropertyFetcher>();
}

protected override string GetListenerName() => DiagnosticListenerName;
Expand All @@ -44,68 +54,76 @@ protected override IEnumerable<string> HandledEventNames()

protected override void HandleEvent(string eventName, object untypedArg)
{
var untypedArgType = untypedArg.GetType();
switch (eventName)
{
case MicrosoftSqlClientDiagnosticOptions.EventNames.WriteCommandBefore:
{
var cmd = (SqlCommand)_activityCommand_RequestFetcher.Fetch(untypedArg);
{
var commandFetcher = _activityCommandFetchers.GetOrAdd(untypedArgType, CommandFetcherFactoryMethod);
var cmd = (SqlCommand)commandFetcher.Fetch(untypedArg);

var activeSpan = Tracer.ActiveSpan;
var activeSpan = Tracer.ActiveSpan;

if (activeSpan == null && !_options.StartRootSpans)
if (activeSpan == null && !_options.StartRootSpans)
{
if (IsLogLevelTraceEnabled)
{
if (IsLogLevelTraceEnabled)
{
Logger.LogTrace("Ignoring SQL command due to missing parent span");
}
return;
Logger.LogTrace("Ignoring SQL command due to missing parent span");
}

if (IgnoreEvent(cmd))
return;
}

if (IgnoreEvent(cmd))
{
if (IsLogLevelTraceEnabled)
{
if (IsLogLevelTraceEnabled)
{
Logger.LogTrace("Ignoring SQL command due to IgnorePatterns");
}
return;
Logger.LogTrace("Ignoring SQL command due to IgnorePatterns");
}

string operationName = _options.OperationNameResolver(cmd);
return;
}

var span = Tracer.BuildSpan(operationName)
.AsChildOf(activeSpan)
.WithTag(Tags.SpanKind, Tags.SpanKindClient)
.WithTag(Tags.Component, _options.ComponentName)
.WithTag(Tags.DbInstance, cmd.Connection.Database)
.WithTag(Tags.DbStatement, cmd.CommandText)
.Start();
string operationName = _options.OperationNameResolver(cmd);

_spanStorage.TryAdd(cmd, span);
}
var span = Tracer.BuildSpan(operationName)
.AsChildOf(activeSpan)
.WithTag(Tags.SpanKind, Tags.SpanKindClient)
.WithTag(Tags.Component, _options.ComponentName)
.WithTag(Tags.DbInstance, cmd.Connection.Database)
.WithTag(Tags.DbStatement, cmd.CommandText)
.Start();

_spanStorage.TryAdd(cmd, span);
}
break;

case MicrosoftSqlClientDiagnosticOptions.EventNames.WriteCommandError:
{
var cmd = (SqlCommand)_activityCommand_RequestFetcher.Fetch(untypedArg);
var ex = (Exception)_exception_ExceptionFetcher.Fetch(untypedArg);
{
var commandFetcher = _activityCommandFetchers[untypedArgType];
var cmd = (SqlCommand)commandFetcher.Fetch(untypedArg);

if (_spanStorage.TryRemove(cmd, out var span))
{
span.SetException(ex);
span.Finish();
}
var exceptionFetcher = _exceptionFetchers.GetOrAdd(untypedArgType, ExceptionFetcherFactoryMethod);
var ex = (Exception)exceptionFetcher.Fetch(untypedArg);

if (_spanStorage.TryRemove(cmd, out var span))
{
span.SetException(ex);
span.Finish();
}
}
break;

case MicrosoftSqlClientDiagnosticOptions.EventNames.WriteCommandAfter:
{
var cmd = (SqlCommand)_activityCommand_RequestFetcher.Fetch(untypedArg);
{
var commandFetcher = _activityCommandFetchers[untypedArgType];
var cmd = (SqlCommand)commandFetcher.Fetch(untypedArg);

if (_spanStorage.TryRemove(cmd, out var span))
{
span.Finish();
}
if (_spanStorage.TryRemove(cmd, out var span))
{
span.Finish();
}
}
break;

default:
Expand Down
57 changes: 57 additions & 0 deletions test/OpenTracing.Contrib.NetCore.Tests/MicrosoftSqlClient/HandleEventTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.Data.SqlClient;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using OpenTracing.Contrib.NetCore.Configuration;
using OpenTracing.Contrib.NetCore.MicrosoftSqlClient;
using OpenTracing.Mock;
using OpenTracing.Noop;
using Xunit;

namespace OpenTracing.Contrib.NetCore.Tests.MicrosoftSqlClient
{
public class HandleEventTest
{
private readonly IObserver<KeyValuePair<string, object>> _microsoftSqlClientDiagnostics;

public HandleEventTest()
{
_microsoftSqlClientDiagnostics = new MicrosoftSqlClientDiagnostics(
NullLoggerFactory.Instance,
NoopTracerFactory.Create(),
Options.Create(new MicrosoftSqlClientDiagnosticOptions {StartRootSpans = false}));
}

[Fact]
async Task CanHandleTwoDifferentTypesOfWriteCommandBeforeInParallel()
{
const string eventName = MicrosoftSqlClientDiagnosticOptions.EventNames.WriteCommandBefore;
var command1 = new {Command = new SqlCommand("Insert into"), Id = Guid.NewGuid()};
var command2 = new {Command = new SqlCommand("Update where"), Id = Guid.NewGuid(), ExtraProp = "any value"};
var kv1 = new KeyValuePair<string, object>(eventName, command1);
var kv2 = new KeyValuePair<string, object>(eventName, command2);

var tasks1 = Enumerable.Range(0, 100)
.Select(i => Task.Run(() => _microsoftSqlClientDiagnostics.OnNext(i % 2 == 0 ? kv1 : kv2)));

await Task.WhenAll(tasks1);
}

[Fact]
void CanHandleWriteCommandAfter()
{
const string eventNameBefore = MicrosoftSqlClientDiagnosticOptions.EventNames.WriteCommandBefore;
var commandBefore = new {Command = new SqlCommand("Insert into"), Id = Guid.NewGuid()};
var kvBefore = new KeyValuePair<string, object>(eventNameBefore, commandBefore);
_microsoftSqlClientDiagnostics.OnNext(kvBefore);

const string eventNameAfter = MicrosoftSqlClientDiagnosticOptions.EventNames.WriteCommandAfter;
var commandAfter = new {Command = new SqlCommand("Insert into"), Id = Guid.NewGuid()};
var kvAfter = new KeyValuePair<string, object>(eventNameAfter, commandAfter);
_microsoftSqlClientDiagnostics.OnNext(kvAfter);
}
}
}
7 changes: 6 additions & 1 deletion test/OpenTracing.Contrib.NetCore.Tests/OpenTracing.Contrib.NetCore.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netcoreapp2.1</TargetFrameworks>
<TargetFramework>netcoreapp2.1</TargetFramework>
</PropertyGroup>

<ItemGroup>
Expand All @@ -17,13 +17,18 @@
<PackageReference Include="Microsoft.AspNetCore.Mvc.Core" Version="[2.1.16,3)" />
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="[2.1.14,3)" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="[2.1.14,3)" />
<PackageReference Include="Microsoft.Data.SqlClient" Version="1.1.3" />
<PackageReference Include="System.Data.SqlClient" Version="4.6.1" />
</ItemGroup>

<ItemGroup Condition="$(TargetFramework)=='netcoreapp3.1'">
<FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>

<ItemGroup Condition="$(TargetFramework)=='net5.0'">
<PackageReference Include="Microsoft.Data.SqlClient" Version="2.0.1" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\OpenTracing.Contrib.NetCore\OpenTracing.Contrib.NetCore.csproj" />
</ItemGroup>
Expand Down