Skip to content

Commit 5c4f798

Browse files
jimit-j-shahjimit-j-shah
authored
test: Additional Transaction Testing and cleanup OpenCensus usage (#1505)
* test: newTransactionReadWriteTraceTest * fix: test literal * feat: Added tests for transaction cases * fix: Cleanup OpenCensus dead code * fix: updating version from 2.20.1 -> 2.21.0 * fix: reverting version from 2.21.0 -> 2.20.1 * fix: Adding an exception to the clirr-maven-plugin for an internal API parameter change from com.google.cloud.datastore.TraceUtil -> com.google.cloud.datastore.telemetry.TraceUtil * fix: Fixing the differenceType in clirr exception * fix: add an exception for removal of an internal class (com.google.cloud.datastore.TraceUtil) * fix: fixing incomplete difference details for type 7005 * fix: Fixing `to` of the difference to be the entire signature * fix: typo
1 parent 82a8d78 commit 5c4f798

File tree

6 files changed

+139
-97
lines changed

6 files changed

+139
-97
lines changed

‎google-cloud-datastore/clirr-ignored-differences.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,16 @@
5555
<method>java.lang.Object execute(com.google.cloud.datastore.Query, com.google.cloud.datastore.ReadOption[])</method>
5656
<differenceType>7004</differenceType>
5757
</difference>
58+
<difference>
59+
<className>com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator</className>
60+
<method>RetryAndTraceDatastoreRpcDecorator(com.google.cloud.datastore.spi.v1.DatastoreRpc, com.google.cloud.datastore.TraceUtil, com.google.api.gax.retrying.RetrySettings, com.google.cloud.datastore.DatastoreOptions)</method>
61+
<to>RetryAndTraceDatastoreRpcDecorator(com.google.cloud.datastore.spi.v1.DatastoreRpc, com.google.cloud.datastore.telemetry.TraceUtil, com.google.api.gax.retrying.RetrySettings, com.google.cloud.datastore.DatastoreOptions)</to>
62+
<differenceType>7005</differenceType>
63+
</difference>
64+
65+
<!-- Class removed -->
66+
<difference>
67+
<className>com/google/cloud/datastore/TraceUtil</className>
68+
<differenceType>8001</differenceType>
69+
</difference>
5870
</differences>

‎google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreImpl.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.google.cloud.ServiceOptions;
2626
import com.google.cloud.datastore.execution.AggregationQueryExecutor;
2727
import com.google.cloud.datastore.spi.v1.DatastoreRpc;
28+
import com.google.cloud.datastore.telemetry.TraceUtil.Scope;
2829
import com.google.common.base.MoreObjects;
2930
import com.google.common.base.Preconditions;
3031
import com.google.common.base.Throwables;
@@ -39,9 +40,6 @@
3940
import com.google.datastore.v1.RunQueryResponse;
4041
import com.google.datastore.v1.TransactionOptions;
4142
import com.google.protobuf.ByteString;
42-
import io.opencensus.common.Scope;
43-
import io.opencensus.trace.Span;
44-
import io.opencensus.trace.Status;
4543
import io.opentelemetry.api.common.Attributes;
4644
import io.opentelemetry.api.trace.SpanBuilder;
4745
import io.opentelemetry.api.trace.SpanKind;
@@ -68,7 +66,6 @@ final class DatastoreImpl extends BaseService<DatastoreOptions> implements Datas
6866
TransactionExceptionHandler.build();
6967
private static final ExceptionHandler TRANSACTION_OPERATION_EXCEPTION_HANDLER =
7068
TransactionOperationExceptionHandler.build();
71-
private final TraceUtil traceUtil = TraceUtil.getInstance();
7269

7370
private final com.google.cloud.datastore.telemetry.TraceUtil otelTraceUtil =
7471
getOptions().getTraceUtil();
@@ -85,7 +82,8 @@ final class DatastoreImpl extends BaseService<DatastoreOptions> implements Datas
8582
readOptionProtoPreparer = new ReadOptionProtoPreparer();
8683
aggregationQueryExecutor =
8784
new AggregationQueryExecutor(
88-
new RetryAndTraceDatastoreRpcDecorator(datastoreRpc, traceUtil, retrySettings, options),
85+
new RetryAndTraceDatastoreRpcDecorator(
86+
datastoreRpc, otelTraceUtil, retrySettings, options),
8987
options);
9088
}
9189

@@ -744,8 +742,9 @@ void rollbackTransaction(ByteString transaction) {
744742
}
745743

746744
void rollback(final com.google.datastore.v1.RollbackRequest requestPb) {
747-
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_ROLLBACK);
748-
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
745+
com.google.cloud.datastore.telemetry.TraceUtil.Span span =
746+
otelTraceUtil.startSpan(com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_ROLLBACK);
747+
try (Scope scope = span.makeCurrent()) {
749748
RetryHelper.runWithRetries(
750749
new Callable<Void>() {
751750
@Override
@@ -758,10 +757,10 @@ public Void call() throws DatastoreException {
758757
EXCEPTION_HANDLER,
759758
getOptions().getClock());
760759
} catch (RetryHelperException e) {
761-
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
760+
span.end(e);
762761
throw DatastoreException.translateAndThrow(e);
763762
} finally {
764-
span.end(TraceUtil.END_SPAN_OPTIONS);
763+
span.end();
765764
}
766765
}
767766
}

‎google-cloud-datastore/src/main/java/com/google/cloud/datastore/RetryAndTraceDatastoreRpcDecorator.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.cloud.RetryHelper;
2323
import com.google.cloud.RetryHelper.RetryHelperException;
2424
import com.google.cloud.datastore.spi.v1.DatastoreRpc;
25+
import com.google.cloud.datastore.telemetry.TraceUtil;
2526
import com.google.datastore.v1.AllocateIdsRequest;
2627
import com.google.datastore.v1.AllocateIdsResponse;
2728
import com.google.datastore.v1.BeginTransactionRequest;
@@ -55,13 +56,13 @@ public class RetryAndTraceDatastoreRpcDecorator implements DatastoreRpc {
5556

5657
public RetryAndTraceDatastoreRpcDecorator(
5758
DatastoreRpc datastoreRpc,
58-
TraceUtil traceUtil,
59+
TraceUtil otelTraceUtil,
5960
RetrySettings retrySettings,
6061
DatastoreOptions datastoreOptions) {
6162
this.datastoreRpc = datastoreRpc;
6263
this.retrySettings = retrySettings;
6364
this.datastoreOptions = datastoreOptions;
64-
this.otelTraceUtil = datastoreOptions.getTraceUtil();
65+
this.otelTraceUtil = otelTraceUtil;
6566
}
6667

6768
@Override

‎google-cloud-datastore/src/main/java/com/google/cloud/datastore/TraceUtil.java

Lines changed: 0 additions & 75 deletions
This file was deleted.

‎google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/HttpDatastoreRpc.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import com.google.api.client.http.HttpTransport;
2222
import com.google.cloud.datastore.DatastoreException;
2323
import com.google.cloud.datastore.DatastoreOptions;
24-
import com.google.cloud.datastore.TraceUtil;
2524
import com.google.cloud.http.CensusHttpModule;
2625
import com.google.cloud.http.HttpTransportOptions;
2726
import com.google.datastore.v1.AllocateIdsRequest;
@@ -40,6 +39,7 @@
4039
import com.google.datastore.v1.RunAggregationQueryResponse;
4140
import com.google.datastore.v1.RunQueryRequest;
4241
import com.google.datastore.v1.RunQueryResponse;
42+
import io.opencensus.trace.Tracing;
4343
import java.io.IOException;
4444
import java.net.InetAddress;
4545
import java.net.URL;
@@ -80,8 +80,7 @@ public HttpDatastoreRpc(DatastoreOptions options) {
8080
private HttpRequestInitializer getHttpRequestInitializer(
8181
final DatastoreOptions options, HttpTransportOptions httpTransportOptions) {
8282
// Open Census initialization
83-
CensusHttpModule censusHttpModule =
84-
new CensusHttpModule(TraceUtil.getInstance().getTracer(), true);
83+
CensusHttpModule censusHttpModule = new CensusHttpModule(Tracing.getTracer(), true);
8584
final HttpRequestInitializer censusHttpModuleHttpRequestInitializer =
8685
censusHttpModule.getHttpRequestInitializer(
8786
httpTransportOptions.getHttpRequestInitializer(options));

‎google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITE2ETracingTest.java

Lines changed: 114 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_COMMIT;
2323
import static com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_LOOKUP;
2424
import static com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_RESERVE_IDS;
25+
import static com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_ROLLBACK;
2526
import static com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_RUN_AGGREGATION_QUERY;
2627
import static com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_RUN_QUERY;
2728
import static com.google.cloud.datastore.telemetry.TraceUtil.SPAN_NAME_TRANSACTION_COMMIT;
@@ -374,6 +375,7 @@ public void after() throws Exception {
374375
tracer = null;
375376
retrievedTrace = null;
376377
customSpanContext = null;
378+
openTelemetrySdk = null;
377379
}
378380

379381
@AfterClass
@@ -793,7 +795,7 @@ public void runAggregationQueryTraceTest() throws Exception {
793795
}
794796

795797
@Test
796-
public void transactionalLookupTest() throws Exception {
798+
public void newTransactionReadTest() throws Exception {
797799
assertNotNull(customSpanContext);
798800

799801
Span rootSpan = getNewRootSpanWithContext();
@@ -817,7 +819,7 @@ public void transactionalLookupTest() throws Exception {
817819
}
818820

819821
@Test
820-
public void transactionQueryTest() throws Exception {
822+
public void newTransactionQueryTest() throws Exception {
821823
// Set up
822824
Entity entity1 = Entity.newBuilder(KEY1).set("test_field", "test_value1").build();
823825
Entity entity2 = Entity.newBuilder(KEY2).set("test_field", "test_value2").build();
@@ -856,6 +858,116 @@ public void transactionQueryTest() throws Exception {
856858
Collections.singletonList(SPAN_NAME_TRANSACTION_COMMIT)));
857859
}
858860

861+
@Test
862+
public void newTransactionReadWriteTraceTest() throws Exception {
863+
// Set up
864+
Entity entity1 = Entity.newBuilder(KEY1).set("pepper_type", "jalapeno").build();
865+
Entity entity2 = Entity.newBuilder(KEY2).set("pepper_type", "habanero").build();
866+
List<Entity> entityList = new ArrayList<>();
867+
entityList.add(entity1);
868+
entityList.add(entity2);
869+
870+
List<Entity> response = datastore.add(entity1, entity2);
871+
assertEquals(entityList, response);
872+
873+
String simplified_spice_level = "not_spicy";
874+
Entity entity1update =
875+
Entity.newBuilder(entity1).set("spice_level", simplified_spice_level).build();
876+
877+
assertNotNull(customSpanContext);
878+
879+
// Test
880+
Span rootSpan = getNewRootSpanWithContext();
881+
try (Scope ignored = rootSpan.makeCurrent()) {
882+
Transaction transaction = datastore.newTransaction();
883+
entity1 = transaction.get(KEY1);
884+
switch (entity1.getString("pepper_type")) {
885+
case "jalapeno":
886+
simplified_spice_level = "mild";
887+
break;
888+
889+
case "habanero":
890+
simplified_spice_level = "hot";
891+
break;
892+
}
893+
transaction.update(entity1update);
894+
transaction.delete(KEY2);
895+
transaction.commit();
896+
assertFalse(transaction.isActive());
897+
} finally {
898+
rootSpan.end();
899+
}
900+
901+
waitForTracesToComplete();
902+
903+
List<Entity> list = datastore.fetch(KEY1, KEY2);
904+
assertEquals(list.get(0), entity1update);
905+
assertNull(list.get(1));
906+
907+
fetchAndValidateTrace(
908+
customSpanContext.getTraceId(),
909+
/*numExpectedSpans=*/ 3,
910+
Arrays.asList(
911+
Collections.singletonList(SPAN_NAME_BEGIN_TRANSACTION),
912+
Collections.singletonList(SPAN_NAME_TRANSACTION_LOOKUP),
913+
Collections.singletonList(SPAN_NAME_TRANSACTION_COMMIT)));
914+
}
915+
916+
@Test
917+
public void newTransactionRollbackTest() throws Exception {
918+
// Set up
919+
Entity entity1 = Entity.newBuilder(KEY1).set("pepper_type", "jalapeno").build();
920+
Entity entity2 = Entity.newBuilder(KEY2).set("pepper_type", "habanero").build();
921+
List<Entity> entityList = new ArrayList<>();
922+
entityList.add(entity1);
923+
entityList.add(entity2);
924+
925+
List<Entity> response = datastore.add(entity1, entity2);
926+
assertEquals(entityList, response);
927+
928+
String simplified_spice_level = "not_spicy";
929+
Entity entity1update =
930+
Entity.newBuilder(entity1).set("spice_level", simplified_spice_level).build();
931+
932+
assertNotNull(customSpanContext);
933+
934+
// Test
935+
Span rootSpan = getNewRootSpanWithContext();
936+
try (Scope ignored = rootSpan.makeCurrent()) {
937+
Transaction transaction = datastore.newTransaction();
938+
entity1 = transaction.get(KEY1);
939+
switch (entity1.getString("pepper_type")) {
940+
case "jalapeno":
941+
simplified_spice_level = "mild";
942+
break;
943+
944+
case "habanero":
945+
simplified_spice_level = "hot";
946+
break;
947+
}
948+
transaction.update(entity1update);
949+
transaction.delete(KEY2);
950+
transaction.rollback();
951+
assertFalse(transaction.isActive());
952+
} finally {
953+
rootSpan.end();
954+
}
955+
956+
waitForTracesToComplete();
957+
958+
List<Entity> list = datastore.fetch(KEY1, KEY2);
959+
assertEquals(list.get(0), entity1);
960+
assertEquals(list.get(1), entity2);
961+
962+
fetchAndValidateTrace(
963+
customSpanContext.getTraceId(),
964+
/*numExpectedSpans=*/ 3,
965+
Arrays.asList(
966+
Collections.singletonList(SPAN_NAME_BEGIN_TRANSACTION),
967+
Collections.singletonList(SPAN_NAME_TRANSACTION_LOOKUP),
968+
Collections.singletonList(SPAN_NAME_ROLLBACK)));
969+
}
970+
859971
@Test
860972
public void runInTransactionQueryTest() throws Exception {
861973
// Set up
@@ -897,10 +1009,4 @@ public void runInTransactionQueryTest() throws Exception {
8971009
Arrays.asList(SPAN_NAME_TRANSACTION_RUN, SPAN_NAME_RUN_QUERY),
8981010
Arrays.asList(SPAN_NAME_TRANSACTION_RUN, SPAN_NAME_TRANSACTION_COMMIT)));
8991011
}
900-
901-
@Test
902-
public void transactionRunQueryTest() throws Exception {}
903-
904-
@Test
905-
public void readWriteTransactionTraceTest() throws Exception {}
9061012
}

0 commit comments

Comments
 (0)