Cannot associate log messages with specific instance of SftpClient

[shadytester]

Assume I have a few instances of SftpClient, a few open connections and want to get log messages from each of them separately. How can I do this?
I can register custom implementation of ILoggerFactory but it will receive log messages from all existing instances and these messages do not have any info that would allow to distinglish their senders.

It looks like a design bug.

[mus65]

You usually only have a single ILoggerFactory in an application. This type of context-specific information is usually added with a Logging Scope. So you could e.g. add an instance specific Guid to the logging scope while using the instance. Then this Guid is automatically added to all log messages.

If you e.g. use the client as part of an ASP .NET Core Http Request, you may not even have to do this because ASP .NET Core will automatically create a scope which already includes e.g. the Request Path and a Request Id.

I suppose SSH.NET could add some kind of instance id to the scope itself, but I have not seen other libraries do this, probably because you usually already have other Scope information from e.g. an ASP .NET Core Http Request or a scope specific to a hosted service.

[shadytester]

I need to use a few instances of SftpClient in my application and without ability to distinglish log messages the overal logging system becomes useless. Any workaround will work. Maybe it's possible to add a reference to related client object or any kind of its artificial "id" to the "state" parameter (it's a collection of key-value pairs)...

[desdesdes]

1+. I would really love to see this feature implemented. We use different settings to communicate with various providers, and if there are any issues, we would appreciate being able to trace a specific SftpClient session without interfering with the global logging.

You should be able to implement this in a backward-compatible manner by allowing the LoggerFactory to be set on individual objects (for example, on the ConnectionInfo), while falling back to SshNetLoggingConfiguration.LoggerFactory if not set.

This approach would also enhance our ability to conduct unit tests, as singletons are not very unit test-friendly.

If I would create a pull request for this you you consider merging it?

[mus65]

You should be able to implement this in a backward-compatible manner by allowing the LoggerFactory to be set on individual objects (for example, on the ConnectionInfo), while falling back to SshNetLoggingConfiguration.LoggerFactory if not set.

I agree passing an ILoggerFactory per instance would be better and I originally planned to implement it this way, but it simply turned out to be too much work, because the factory must be passed through a lot of constructors and layers in the code base, see #1509 "Injecting ILoggerFactory vs static instance".

Feel free to try though, maybe there is a simpler approach to this I didn't think of (I'm not a maintainer though).

And as mentioned, you can already add context specific information yourself with a logging scope, e.g.:

using var scope = logger.BeginScope(new Dictionary<string, object>()
{
    ["SshClientName"] = "foo", 
});

var client = new SftpClient("host", "user", "password");
await client.ConnectAsync(CancellationToken.None);

But I can see how that might not solve everyone's use case. If you actually need different logger implementations per client, this doesn't help.

[shadytester]

@mus65 the BeginScope method of custom ILogger implementation is never called. I'm not sure I understand how your suggestion can help.

[mus65]

@shadytester A custom ILogger implementation actually has to support logging scopes. How to do this is afaik not well documented. You may use the .NET Console Logger sources as a reference. Basically:

• your logger provider has to implement ISupportExternalScope
• .NET will then automatically call SetScopeProvider on the provider.
• Your logger provider has to pass the scopeProvider to the logging instances
• the logging instances have to call scopeProvider.Push() in the BeginScope method

Then your ILogger.Log should be able to access the scope variables by using scopeProvider.ForEachScope.

[desdesdes]

I've create a pull request, hopefully someone will review and eventually merge #1673.

[Rob-Hague]

done in #1673