Query Interception - Disposing of diagnostic listeners

Question

We are using DiagnosticListeners in order to modify the SQL command text that EF Core produces. The problem is that our listeners need to be modifying the SQL command based on some user specific data that comes into our Api via HttpRequests. Our current solution is extremely hacky and likely to cause issues in the future. We register a new listener each time DbContext is created:

public class MyContext : DbContext
{
    private readonly HReCommandAdapter _adapter;


    public MyContext(DbContextOptions options) : base(options)
    {
        _adapter = new DbCommandAdapter();

        var listener = this.GetService<DiagnosticSource>();
        (listener as DiagnosticListener).SubscribeWithAdapter(_adapter);
    }   

    public override void Dispose()
    {
        _adapter.Dispose();
        base.Dispose();
    }

    //DbSets and stuff
}

The simplified listener code looks like below:

public class DbCommandAdapter : IDisposable
{
    private bool _hasExecuted = false;
    private Guid? _lastExecId = null;

    [DiagnosticName("Microsoft.EntityFrameworkCore.Database.Command.CommandExecuting")]
    public void OnCommandExecuting(DbCommand command, DbCommandMethod executeMethod, Guid commandId, Guid connectionId, bool async, DateTimeOffset startTime)
    {
        if (!_lastExecId.HasValue)
            _lastExecId = connectionId;

        if (_lastExecId != connectionId)
            return;

        //We are modifying command text here
    }

    [DiagnosticName("Microsoft.EntityFrameworkCore.Database.Command.CommandExecuted")]
    public void OnCommandExecuted(object result, bool async)
    {
    }

    [DiagnosticName("Microsoft.EntityFrameworkCore.Database.Command.CommandError")]
    public void OnCommandError(Exception exception, bool async)
    {
    }

    public void Dispose() { //No code in here }
} 

As you can see, our current approach is to use the connectionId which will be different each time DbContext is created. The reason for this hacky approach is because the listener instances are not disposed even though the DbContext.Dispose() is called every time the HttpRequest is processed. So connectionId allows to basically have an illusion of 1:1 mapping between a listener and a given DbContext instance.

What happens though is that the amount of listener instances keep piling up throughout the lifetime of the api and the only time the instances go away is when the application pools stop or recycle.

Is it possible to somehow dispose of these listener instances and how? I'm also open to a different approach in order to modify SQL commands (the diagnostic listeners was the only viable one we found for EF Core).

EDIT: I am modifying only SELECT commands. I omitted the details, but the DbCommandAdapter gets created with a user specific prefix that is different based on the user trying to access the API.

So for example if the query is:

SELECT FIELD1, FIELD2 FROM EMPLOYEES

and the user specific prefix is USER_SOMENUMBER, then the modified query ends up:

SELECT FIELD1, FIELD2 FROM USER_SOMENUMBER_EMPLOYEES

I understand that this is fragile but we guarantee that the schema of the tablename that we change is identical and it is not a concern.

Answer 1

If you can't dispose of the listeners why not pool them and reuse them? Pooling is a good software pattern when disposing or constructing is expensive. Preventing infinite growth is a reasonable usage too.

The following is only pseudo-code. It requires knowing when an adapter transaction has completed, so that it can be marked available and reused. It also assumes the updated myDbContext will have what you need to perform your work.

public static class DbCommandAdapterPool
{
    private static ConcurrentBag<DbCommandAdapter> _pool = new ConcurrentBag<DbCommandAdapter>();

    public static DbCommandAdapter SubscribeAdapter(MyContext context)
    {
        var adapter = _pool.FirstOrDefault(a => a.IsAvailable);
        if (adapter == null)
        {
            adapter = new DbCommandAdapter(context);
            _pool.Add(adapter);
        }
        else adapter.Reuse(context);

        return adapter;
    }
}

public class MyContext : DbContext
{
    private readonly HReCommandAdapter _adapter;


    public MyContext(DbContextOptions options) : base(options)
    {
        //_adapter = new DbCommandAdapter();

        //var listener = this.GetService<DiagnosticSource>();
        //(listener as DiagnosticListener).SubscribeWithAdapter(_adapter);

        DbCommandAdapterPool.SubscribeAdapter(this);
    }

    public override void Dispose()
    {
        _adapter.Dispose();
        base.Dispose();
    }

    //DbSets and stuff
}

public class DbCommandAdapter : IDisposable
{
    private bool _hasExecuted = false;
    private Guid? _lastExecId = null;
    private MyContext _context;
    private DiagnosticListener _listener;//added for correlation

    public bool IsAvailable { get; } = false;//Not sure what constitutes a complete transaction.

    public DbCommandAdapter(MyContext context)
    {
        this._context = context;
        this._listener = context.GetService<DiagnosticSource>();
    }


    ...

    public void Reuse(MyContext context)
    {
        this.IsAvailable = false;
        this._context = context;
    }
}

NOTE: I haven't tried this myself. Ivan Stoev recommends injecting dependency to ICurrentDbContext to CustomSqlServerQuerySqlGeneratorFactory which is then available inside CustomSqlServerQuerySqlGenerator. see: Ef-Core - What regex can I use to replace table names with nolock ones in Db Interceptor

Answer 2

How about creating the subscription once in the Startup and letting it live until the end of the application.

The thing is, that the subscription is based on the DiagnosticSource Object (EntityFramework in my case) which is generated by the ServiceProvider.

So everytime you create a MyContext in your code, you add another adapter and another subscription to the DiagnosticSource. The adapter lives with the subscription and the subscription is not disposed (it would be disposed with the DiagnosticSource or at least get useless when the Source is disposed).

Therefore the listener instances keep piling up throughout the lifetime of the api.

I suggest to initialize your subscription once after building your host and before running it. You need to get the DiagnosticSource that is used later on in your application, that's why you need the host. Otherwise the subscription would be on another DiagnosticSource object and would never be called.

var host = new WebHostBuilder()
     .UseConfiguration(config)
      // ...
     .Build();

using (var subscription = InitializeSqlSubscription(host))
{
  host.Run();
}

private static IDisposable InitializeSqlSubscription(IWebHost host)
{
   // TODO: remove Workaround with .Net Core 3.1
   // we need a scope for the ServiceProvider
   using (var serviceScope = host.Services.CreateScope())
   {
      var services = serviceScope.ServiceProvider;

      try
      {
         var adapter = new DbCommandAdapter();

         // context needed to get DiagnosticSource (EntityFramework)
         var myContext = services.GetRequiredService<MyContext>();

         // DiagnosticSource is Singleton in ServiceProvider (i guess), and spanning across scopes
         // -> Disposal of scope has no effect on DiagnosticSource or its subscriptions
         var diagnosticSource = myContext.GetService<DiagnosticSource>();

         // adapter Object is referenced and kept alive by the subscription
         // DiagnosticListener is Disposable, but was created (and should be disposed) by ServiceProvider
         // subscription itself is returned and can be disposed at the end of the application lifetime
         return (diagnosticSource as DiagnosticListener).SubscribeWithAdapter(adapter);
      }
      catch (Exception ex)
      {
         var logger = services.GetRequiredService<ILogger<Startup>>();
         logger.LogError(ex, "An error occurred while initializing subscription");
         throw;
      }
   }
}