Skip to content

Commit 26986bd

Browse files
suraj-qlogicsuraj-qlogic
committed
feat: fix review changes
1 parent da7fead commit 26986bd

File tree

3 files changed

+20
-44
lines changed

3 files changed

+20
-44
lines changed

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

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,13 @@ final class DatastoreImpl extends BaseService<DatastoreOptions> implements Datas
5454
private final RetrySettings retrySettings;
5555
private static final ExceptionHandler TRANSACTION_EXCEPTION_HANDLER =
5656
TransactionExceptionHandler.build();
57-
private final TraceUtil traceUtil;
57+
private final TraceUtil traceUtil = TraceUtil.getInstance();
5858

5959
DatastoreImpl(DatastoreOptions options) {
6060
super(options);
6161
this.datastoreRpc = options.getDatastoreRpcV1();
6262
retrySettings =
6363
MoreObjects.firstNonNull(options.getRetrySettings(), ServiceOptions.getNoRetrySettings());
64-
traceUtil = TraceUtil.getInstance();
6564
}
6665

6766
@Override
@@ -135,41 +134,35 @@ public T call() throws DatastoreException {
135134

136135
@Override
137136
public <T> T runInTransaction(final TransactionCallable<T> callable) {
138-
final DatastoreImpl self = this;
139137
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_TRANSACTION);
140-
Scope scope = traceUtil.getTracer().withSpan(span);
141-
try {
138+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
142139
return RetryHelper.runWithRetries(
143-
new ReadWriteTransactionCallable<T>(self, callable, null),
140+
new ReadWriteTransactionCallable<T>(this, callable, null),
144141
retrySettings,
145142
TRANSACTION_EXCEPTION_HANDLER,
146143
getOptions().getClock());
147144
} catch (RetryHelperException e) {
148145
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
149146
throw DatastoreException.translateAndThrow(e);
150147
} finally {
151-
scope.close();
152148
span.end(TraceUtil.END_SPAN_OPTIONS);
153149
}
154150
}
155151

156152
@Override
157153
public <T> T runInTransaction(
158154
final TransactionCallable<T> callable, TransactionOptions transactionOptions) {
159-
final DatastoreImpl self = this;
160155
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_TRANSACTION);
161-
Scope scope = traceUtil.getTracer().withSpan(span);
162-
try {
156+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
163157
return RetryHelper.runWithRetries(
164-
new ReadWriteTransactionCallable<T>(self, callable, transactionOptions),
158+
new ReadWriteTransactionCallable<T>(this, callable, transactionOptions),
165159
retrySettings,
166160
TRANSACTION_EXCEPTION_HANDLER,
167161
getOptions().getClock());
168162
} catch (RetryHelperException e) {
169163
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
170164
throw DatastoreException.translateAndThrow(e);
171165
} finally {
172-
scope.close();
173166
span.end(TraceUtil.END_SPAN_OPTIONS);
174167
}
175168
}
@@ -191,8 +184,7 @@ <T> QueryResults<T> run(com.google.datastore.v1.ReadOptions readOptionsPb, Query
191184
com.google.datastore.v1.RunQueryResponse runQuery(
192185
final com.google.datastore.v1.RunQueryRequest requestPb) {
193186
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_RUNQUERY);
194-
Scope scope = traceUtil.getTracer().withSpan(span);
195-
try {
187+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
196188
return RetryHelper.runWithRetries(
197189
new Callable<com.google.datastore.v1.RunQueryResponse>() {
198190
@Override
@@ -207,7 +199,6 @@ public com.google.datastore.v1.RunQueryResponse call() throws DatastoreException
207199
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
208200
throw DatastoreException.translateAndThrow(e);
209201
} finally {
210-
scope.close();
211202
span.end(TraceUtil.END_SPAN_OPTIONS);
212203
}
213204
}
@@ -249,8 +240,7 @@ public List<Key> allocateId(IncompleteKey... keys) {
249240
private com.google.datastore.v1.AllocateIdsResponse allocateIds(
250241
final com.google.datastore.v1.AllocateIdsRequest requestPb) {
251242
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_ALLOCATEIDS);
252-
Scope scope = traceUtil.getTracer().withSpan(span);
253-
try {
243+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
254244
return RetryHelper.runWithRetries(
255245
new Callable<com.google.datastore.v1.AllocateIdsResponse>() {
256246
@Override
@@ -265,7 +255,6 @@ public com.google.datastore.v1.AllocateIdsResponse call() throws DatastoreExcept
265255
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
266256
throw DatastoreException.translateAndThrow(e);
267257
} finally {
268-
scope.close();
269258
span.end(TraceUtil.END_SPAN_OPTIONS);
270259
}
271260
}
@@ -415,8 +404,7 @@ protected Entity computeNext() {
415404
com.google.datastore.v1.LookupResponse lookup(
416405
final com.google.datastore.v1.LookupRequest requestPb) {
417406
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_LOOKUP);
418-
Scope scope = traceUtil.getTracer().withSpan(span);
419-
try {
407+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
420408
return RetryHelper.runWithRetries(
421409
new Callable<com.google.datastore.v1.LookupResponse>() {
422410
@Override
@@ -431,7 +419,6 @@ public com.google.datastore.v1.LookupResponse call() throws DatastoreException {
431419
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
432420
throw DatastoreException.translateAndThrow(e);
433421
} finally {
434-
scope.close();
435422
span.end(TraceUtil.END_SPAN_OPTIONS);
436423
}
437424
}
@@ -455,8 +442,7 @@ public List<Key> reserveIds(Key... keys) {
455442
com.google.datastore.v1.ReserveIdsResponse reserveIds(
456443
final com.google.datastore.v1.ReserveIdsRequest requestPb) {
457444
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_RESERVEIDS);
458-
Scope scope = traceUtil.getTracer().withSpan(span);
459-
try {
445+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
460446
return RetryHelper.runWithRetries(
461447
new Callable<com.google.datastore.v1.ReserveIdsResponse>() {
462448
@Override
@@ -471,7 +457,6 @@ public com.google.datastore.v1.ReserveIdsResponse call() throws DatastoreExcepti
471457
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
472458
throw DatastoreException.translateAndThrow(e);
473459
} finally {
474-
scope.close();
475460
span.end(TraceUtil.END_SPAN_OPTIONS);
476461
}
477462
}
@@ -565,8 +550,7 @@ private com.google.datastore.v1.CommitResponse commitMutation(
565550
com.google.datastore.v1.CommitResponse commit(
566551
final com.google.datastore.v1.CommitRequest requestPb) {
567552
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_COMMIT);
568-
Scope scope = traceUtil.getTracer().withSpan(span);
569-
try {
553+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
570554
return RetryHelper.runWithRetries(
571555
new Callable<com.google.datastore.v1.CommitResponse>() {
572556
@Override
@@ -581,7 +565,6 @@ public com.google.datastore.v1.CommitResponse call() throws DatastoreException {
581565
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
582566
throw DatastoreException.translateAndThrow(e);
583567
} finally {
584-
scope.close();
585568
span.end(TraceUtil.END_SPAN_OPTIONS);
586569
}
587570
}
@@ -594,8 +577,7 @@ ByteString requestTransactionId(
594577
com.google.datastore.v1.BeginTransactionResponse beginTransaction(
595578
final com.google.datastore.v1.BeginTransactionRequest requestPb) {
596579
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_BEGINTRANSACTION);
597-
Scope scope = traceUtil.getTracer().withSpan(span);
598-
try {
580+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
599581
return RetryHelper.runWithRetries(
600582
new Callable<com.google.datastore.v1.BeginTransactionResponse>() {
601583
@Override
@@ -611,7 +593,6 @@ public com.google.datastore.v1.BeginTransactionResponse call()
611593
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
612594
throw DatastoreException.translateAndThrow(e);
613595
} finally {
614-
scope.close();
615596
span.end(TraceUtil.END_SPAN_OPTIONS);
616597
}
617598
}
@@ -625,8 +606,7 @@ void rollbackTransaction(ByteString transaction) {
625606

626607
void rollback(final com.google.datastore.v1.RollbackRequest requestPb) {
627608
Span span = traceUtil.startSpan(TraceUtil.SPAN_NAME_ROLLBACK);
628-
Scope scope = traceUtil.getTracer().withSpan(span);
629-
try {
609+
try (Scope scope = traceUtil.getTracer().withSpan(span)) {
630610
RetryHelper.runWithRetries(
631611
new Callable<Void>() {
632612
@Override
@@ -642,7 +622,6 @@ public Void call() throws DatastoreException {
642622
span.setStatus(Status.UNKNOWN.withDescription(e.getMessage()));
643623
throw DatastoreException.translateAndThrow(e);
644624
} finally {
645-
scope.close();
646625
span.end(TraceUtil.END_SPAN_OPTIONS);
647626
}
648627
}

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@
2323
import io.opencensus.trace.Tracing;
2424

2525
/**
26-
* Helper class for tracing utility. It is used for instrumenting {@link HttpDatastoreRpc} with Open
27-
* Census APIs.
26+
* Helper class for tracing utility. It is used for instrumenting {@link HttpDatastoreRpc} with
27+
* OpenCensus APIs.
2828
*
29-
* <p>TraceUtil Instance are created by the {@link TraceUtil#getInstance()} method.
29+
* <p>TraceUtil instances are created by the {@link TraceUtil#getInstance()} method.
3030
*/
3131
public class TraceUtil {
3232
private final Tracer tracer = Tracing.getTracer();
33-
private static TraceUtil traceUtil;
33+
private static final TraceUtil traceUtil = new TraceUtil();
3434
static final String SPAN_NAME_ALLOCATEIDS = "CloudDatastoreOperation.allocateIds";
3535
static final String SPAN_NAME_TRANSACTION = "CloudDatastoreOperation.readWriteTransaction";
3636
static final String SPAN_NAME_BEGINTRANSACTION = "CloudDatastoreOperation.beginTransaction";
@@ -67,9 +67,6 @@ public Tracer getTracer() {
6767
* @return An instance of {@link TraceUtil}
6868
*/
6969
public static TraceUtil getInstance() {
70-
if (traceUtil == null) {
71-
traceUtil = new TraceUtil();
72-
}
7370
return traceUtil;
7471
}
7572

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,18 @@ public HttpDatastoreRpc(DatastoreOptions options) {
7777

7878
private HttpRequestInitializer getHttpRequestInitializer(
7979
final DatastoreOptions options, HttpTransportOptions httpTransportOptions) {
80-
HttpRequestInitializer delegate = httpTransportOptions.getHttpRequestInitializer(options);
8180
// Open Census initialization
8281
CensusHttpModule censusHttpModule =
8382
new CensusHttpModule(TraceUtil.getInstance().getTracer(), true);
84-
delegate = censusHttpModule.getHttpRequestInitializer(delegate);
83+
final HttpRequestInitializer censusHttpModuleHttpRequestInitializer =
84+
censusHttpModule.getHttpRequestInitializer(
85+
httpTransportOptions.getHttpRequestInitializer(options));
8586

8687
final String applicationName = options.getApplicationName();
87-
final HttpRequestInitializer localDelegate = delegate;
8888
return new HttpRequestInitializer() {
8989
@Override
9090
public void initialize(HttpRequest httpRequest) throws IOException {
91-
localDelegate.initialize(httpRequest);
91+
censusHttpModuleHttpRequestInitializer.initialize(httpRequest);
9292
httpRequest.getHeaders().setUserAgent(applicationName);
9393
}
9494
};

0 commit comments

Comments
 (0)