2
\$\begingroup\$

I have created a database logger for ASP:NET Core's logging API and I'd like to publish it on NuGet when ready. The GitHub repository is here.

The library is partly modeled after Microsoft.Extensions.Logging.Console and I tried to follow good practices of object-oriented design.

I am slightly concerned about some decisions I made while implementing ILogWriter interface in SqlServerLogWriter class AND while designing the interface for the WriteBulk method (the method is called from the LogRecordCache class).

  1. Have I managed threading well enough? It works fine, but it might not look OK.
  2. Should I hide exceptions or let them propagate? Logging should be redundant in production environment, so I think hiding them would be fine.
  3. Is there a better way for column mapping (in SqlServerLogWriter)?
  4. Any advice or contribution would be appreciated.

LogRecordCache.cs

internal static class LogRecordCache
{
    private static readonly List<LogRecord> _logs;
    private static bool _flushingInProgress = false;
    private static object _lockObject = new object();

    public static bool IsEmpty { get { return _flushingInProgress || _logs.Count == 0; } }


    static LogRecordCache()
    {
        _logs = new List<LogRecord>();
    }


    public static void Add(LogRecord log)
    {
        if (_flushingInProgress)
        {
            lock (_lockObject)
            {
                _logs.Add(log);
            }
        }
        else
        {
            _logs.Add(log);
        }
    }

    public static bool IsFull(int maxCount)
    {
        return !_flushingInProgress && _logs.Count >= maxCount;
    }

    public static void Flush(ILogWriter writer)
    {
        if (!_flushingInProgress)
        {
            _flushingInProgress = true;
            Task.Run(() => writer.WriteBulk(_logs, _lockObject, ref _flushingInProgress));
        }
    }
}

WriteBulk() method

/// <summary>
/// Writes a collection of log records to database at once
/// </summary>
/// <param name="logs">List of <see cref="LogRecord"/> objects</param>
/// <param name="lockObject">Locking object</param>
/// <param name="flushingInProgress">Indicates if writing process is still in progress; set to false at the end of this method</param>
/// <exception cref="SqlException">If opening a connection through the provided connection string was not possible</exception>
public void WriteBulk(List<LogRecord> logs, object lockObject, ref bool flushingInProgress)
{
    var lockTaken = false;
    var connection = new SqlConnection(_connectionString);
    connection.Open();
    var transaction = connection.BeginTransaction();

    try
    {
        using (var bulkCopy = new SqlBulkCopy(connection, SqlBulkCopyOptions.TableLock, transaction))
        {
            bulkCopy.DestinationTableName = "Logging";
            foreach (var mapping in _columnMappings)
                bulkCopy.ColumnMappings.Add(mapping.Key, mapping.Value);

            Monitor.TryEnter(lockObject, ref lockTaken);
            if (lockTaken)
            {
                using (var reader = ObjectReader.Create(logs, _columnMappings.Keys.ToArray()))
                {
                    bulkCopy.BatchSize = logs.Count;
                    bulkCopy.WriteToServer(reader);
                }

                transaction.Commit();
            }
        }
    }
    catch
    {
        if (connection.State == ConnectionState.Open)
            transaction.Rollback();
        throw;
    }
    finally
    {
        if (lockTaken)
        {
            logs.Clear();
            Monitor.Exit(lockObject);
        }

        transaction.Dispose();
        connection.Close();
        flushingInProgress = false;
    }
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Should I hide exceptions or let them propagate? Logging should be redundant in production environment, so I think hiding them would be fine.

Both. I think this should be configurable. During setup / testing exceptions should be thrown for easier debugging (database permission issues etc.) but in production exceptions should not be thrown.

I have been using NLog and at least in my use this way has been working fine.

\$\endgroup\$
1
  • \$\begingroup\$ Hmmm... I also need to take care of what happens to the cache if the ILogWriter instance is not able to flush it. I like your idea, I'll have to think about an elegant solution for all of that. \$\endgroup\$ Commented Feb 6, 2017 at 17:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.