[ https://issues.apache.org/jira/browse/TINKERPOP-2471?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17616544#comment-17616544 ]
ASF GitHub Bot commented on TINKERPOP-2471: ------------------------------------------- vkagamlyk commented on code in PR #1827: URL: https://github.com/apache/tinkerpop/pull/1827#discussion_r993653729 ########## gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs: ########## @@ -38,68 +40,69 @@ public class DriverRemoteConnection : IRemoteConnection, IDisposable { private readonly IGremlinClient _client; private readonly string _traversalSource; - + private readonly ILogger<DriverRemoteConnection> _logger; + /// <summary> /// Filter on these keys provided to OptionsStrategy and apply them to the request. Note that /// "scriptEvaluationTimeout" was deprecated in 3.3.9 but still supported in server implementations and will /// be removed in later versions. /// </summary> - private readonly List<String> _allowedKeys = new List<string> - {Tokens.ArgsEvalTimeout, "scriptEvaluationTimeout", Tokens.ArgsBatchSize, - Tokens.RequestId, Tokens.ArgsUserAgent}; + private readonly List<string> _allowedKeys = new() + { + Tokens.ArgsEvalTimeout, "scriptEvaluationTimeout", Tokens.ArgsBatchSize, + Tokens.RequestId, Tokens.ArgsUserAgent + }; private readonly string _sessionId; private string Processor => IsSessionBound ? Tokens.ProcessorSession : Tokens.ProcessorTraversal; /// <inheritdoc /> public bool IsSessionBound => _sessionId != null; - - /// <summary> - /// Initializes a new <see cref="IRemoteConnection" /> using "g" as the default remote TraversalSource name. - /// </summary> - /// <param name="host">The host to connect to.</param> - /// <param name="port">The port to connect to.</param> - /// <exception cref="ArgumentNullException">Thrown when client is null.</exception> - public DriverRemoteConnection(string host, int port):this(host, port, "g") - { - } - + /// <summary> /// Initializes a new <see cref="IRemoteConnection" />. /// </summary> /// <param name="host">The host to connect to.</param> /// <param name="port">The port to connect to.</param> /// <param name="traversalSource">The name of the traversal source on the server to bind to.</param> + /// <param name="loggerFactory">A factory to create loggers. If not provided, then nothing will be logged.</param> /// <exception cref="ArgumentNullException">Thrown when client is null.</exception> - public DriverRemoteConnection(string host, int port, string traversalSource):this(new GremlinClient(new GremlinServer(host, port)), traversalSource) + public DriverRemoteConnection(string host, int port, string traversalSource = "g", + ILoggerFactory loggerFactory = null) : this( + new GremlinClient(new GremlinServer(host, port), loggerFactory: loggerFactory), traversalSource, + logger: loggerFactory != null + ? loggerFactory.CreateLogger<DriverRemoteConnection>() + : NullLogger<DriverRemoteConnection>.Instance) { } /// <summary> - /// Initializes a new <see cref="IRemoteConnection" /> using "g" as the default remote TraversalSource name. + /// Initializes a new <see cref="IRemoteConnection" />. /// </summary> /// <param name="client">The <see cref="IGremlinClient" /> that will be used for the connection.</param> - /// <exception cref="ArgumentNullException">Thrown when client is null.</exception> - public DriverRemoteConnection(IGremlinClient client):this(client, "g") + /// <param name="traversalSource">The name of the traversal source on the server to bind to.</param> + /// <exception cref="ArgumentNullException">Thrown when client or the traversalSource is null.</exception> + public DriverRemoteConnection(IGremlinClient client, string traversalSource = "g") + : this(client, traversalSource, null) { } - /// <summary> - /// Initializes a new <see cref="IRemoteConnection" />. - /// </summary> - /// <param name="client">The <see cref="IGremlinClient" /> that will be used for the connection.</param> - /// <param name="traversalSource">The name of the traversal source on the server to bind to.</param> - /// <exception cref="ArgumentNullException">Thrown when client is null.</exception> - public DriverRemoteConnection(IGremlinClient client, string traversalSource) + private DriverRemoteConnection(IGremlinClient client, string traversalSource, string sessionId = null, + ILogger<DriverRemoteConnection> logger = null) { _client = client ?? throw new ArgumentNullException(nameof(client)); _traversalSource = traversalSource ?? throw new ArgumentNullException(nameof(traversalSource)); - } - private DriverRemoteConnection(IGremlinClient client, string traversalSource, Guid sessionId) - : this(client, traversalSource) - { - _sessionId = sessionId.ToString(); + if (logger == null) Review Comment: Logging is not covered by unit tests > Add logging to Gremlin.Net driver > --------------------------------- > > Key: TINKERPOP-2471 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2471 > Project: TinkerPop > Issue Type: Improvement > Components: dotnet > Reporter: Florian Hockmann > Priority: Minor > > It would be helpful to have logging in the driver to get insights into its > internal state. Examples for events that we could log are: > * A connection is dead & will be replaced. > * The pool is empty, so we cannot get a connection for the current request > (but will probably try again) > This should make it more transparent for users how the driver operates and > what its current state is. -- This message was sent by Atlassian Jira (v8.20.10#820010)