olim7t commented on code in PR #2037:
URL:
https://github.com/apache/cassandra-java-driver/pull/2037#discussion_r2064980661
##########
core/src/main/java/com/datastax/oss/driver/api/core/session/SessionBuilder.java:
##########
@@ -318,6 +319,19 @@ public SelfT addRequestTracker(@NonNull RequestTracker
requestTracker) {
return self;
}
+ /**
+ * Registers a distributed trace ID generator The driver will use the
distributed trace ID in the
+ * logs So that users can correlate logs about the same request from
different loggers.
Review Comment:
nitpick: it looks like a period is missing here.
```suggestion
* Registers a distributed trace ID generator. The driver will use the
distributed trace ID in the
* logs So that users can correlate logs about the same request from
different loggers.
```
Also if there's nothing useful to say in `@param` and `@return` (and I would
agree it is the case here), it's fine to omit them altogether.
##########
core/src/main/java/com/datastax/oss/driver/api/core/tracker/DistributedTraceIdGenerator.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.datastax.oss.driver.api.core.tracker;
+
+import com.datastax.oss.driver.api.core.session.Request;
+import edu.umd.cs.findbugs.annotations.NonNull;
+
+public interface DistributedTraceIdGenerator {
Review Comment:
nitpick: this is subjective, but the name is a bit long. Have you considered
just `TraceIdGenerator`?
##########
core/src/main/java/com/datastax/oss/driver/internal/core/tracker/UuidDistributedTraceIdGenerator.java:
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.datastax.oss.driver.internal.core.tracker;
+
+import com.datastax.oss.driver.api.core.context.DriverContext;
+import com.datastax.oss.driver.api.core.session.Request;
+import com.datastax.oss.driver.api.core.tracker.DistributedTraceIdGenerator;
+import com.datastax.oss.driver.api.core.uuid.Uuids;
+import edu.umd.cs.findbugs.annotations.NonNull;
+
+public class UuidDistributedTraceIdGenerator implements
DistributedTraceIdGenerator {
+ public UuidDistributedTraceIdGenerator(DriverContext context) {}
+
+ @Override
+ public String getSessionRequestId(
+ @NonNull Request statement, @NonNull String sessionName, int hashCode) {
+ return Uuids.random().toString();
+ }
+
+ @Override
+ public String getNodeRequestId(
+ @NonNull Request statement, @NonNull String sessionRequestId, int
executionCount) {
+ return sessionRequestId + "-" + Uuids.random().toString();
Review Comment:
nitpick: the `.toString()` at the end is unnecessary.
##########
core/src/main/java/com/datastax/oss/driver/api/core/tracker/DistributedTraceIdGenerator.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.datastax.oss.driver.api.core.tracker;
+
+import com.datastax.oss.driver.api.core.session.Request;
+import edu.umd.cs.findbugs.annotations.NonNull;
+
+public interface DistributedTraceIdGenerator {
+ /**
+ * Generates a unique identifier for the session request. This identifier
will be added to logs.
Review Comment:
I think we should be more descriptive here, it was not obvious to me on the
first read .
Something along the lines of:
- sessionRequestId: an identifier for an entire `session.execute()` call
- nodeRequestId: an identifier for the execution of a statement against a
particular node. There can be multiple nodeRequestId for a given
sessionRequestId (because of retries and speculative executions). Can
optionally be sent to the node in the custom payload.
Both are propagated to request trackers (btw, we should rename the
corresponding parameters in `RequestTracker` and adjust the descriptions).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
