[
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)
