Skip to content

Commit 9cdba74

Browse files
surbhigarg92surbhigarg92
authored
chore: Improve traces (googleapis#4150)
This PR does below - Add request-id as span attribute - Add common attributes for all the Spans
1 parent 54f4b5a commit 9cdba74

File tree

6 files changed

+47
-32
lines changed

6 files changed

+47
-32
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseClientImpl.java‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class DatabaseClientImpl implements DatabaseClient {
4242
private static final String READ_ONLY_TRANSACTION = "CloudSpanner.ReadOnlyTransaction";
4343
private static final String PARTITION_DML_TRANSACTION = "CloudSpanner.PartitionDMLTransaction";
4444
private final TraceWrapper tracer;
45-
private Attributes commonAttributes;
45+
private final Attributes databaseAttributes;
4646
@VisibleForTesting final String clientId;
4747
@VisibleForTesting final SessionPool pool;
4848
@VisibleForTesting final MultiplexedSessionDatabaseClient multiplexedSessionDatabaseClient;
@@ -88,15 +88,15 @@ class DatabaseClientImpl implements DatabaseClient {
8888
boolean useMultiplexedSessionPartitionedOps,
8989
TraceWrapper tracer,
9090
boolean useMultiplexedSessionForRW,
91-
Attributes commonAttributes) {
91+
Attributes databaseAttributes) {
9292
this.clientId = clientId;
9393
this.pool = pool;
9494
this.useMultiplexedSessionBlindWrite = useMultiplexedSessionBlindWrite;
9595
this.multiplexedSessionDatabaseClient = multiplexedSessionDatabaseClient;
9696
this.useMultiplexedSessionPartitionedOps = useMultiplexedSessionPartitionedOps;
9797
this.tracer = tracer;
9898
this.useMultiplexedSessionForRW = useMultiplexedSessionForRW;
99-
this.commonAttributes = commonAttributes;
99+
this.databaseAttributes = databaseAttributes;
100100

101101
this.clientIdToOrdinalMap = new HashMap<String, Integer>();
102102
this.dbId = this.dbIdFromClientId(this.clientId);
@@ -203,7 +203,7 @@ public Timestamp write(final Iterable<Mutation> mutations) throws SpannerExcepti
203203
public CommitResponse writeWithOptions(
204204
final Iterable<Mutation> mutations, final TransactionOption... options)
205205
throws SpannerException {
206-
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, commonAttributes, options);
206+
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, databaseAttributes, options);
207207
try (IScope s = tracer.withSpan(span)) {
208208
if (canUseMultiplexedSessionsForRW() && getMultiplexedSessionDatabaseClient() != null) {
209209
return getMultiplexedSessionDatabaseClient().writeWithOptions(mutations, options);
@@ -230,7 +230,7 @@ public Timestamp writeAtLeastOnce(final Iterable<Mutation> mutations) throws Spa
230230
public CommitResponse writeAtLeastOnceWithOptions(
231231
final Iterable<Mutation> mutations, final TransactionOption... options)
232232
throws SpannerException {
233-
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, commonAttributes, options);
233+
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, databaseAttributes, options);
234234
try (IScope s = tracer.withSpan(span)) {
235235
if (useMultiplexedSessionBlindWrite && getMultiplexedSessionDatabaseClient() != null) {
236236
return getMultiplexedSessionDatabaseClient()
@@ -260,7 +260,7 @@ int getNthRequest() {
260260
public ServerStream<BatchWriteResponse> batchWriteAtLeastOnce(
261261
final Iterable<MutationGroup> mutationGroups, final TransactionOption... options)
262262
throws SpannerException {
263-
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, commonAttributes, options);
263+
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, databaseAttributes, options);
264264
try (IScope s = tracer.withSpan(span)) {
265265
if (canUseMultiplexedSessionsForRW() && getMultiplexedSessionDatabaseClient() != null) {
266266
return getMultiplexedSessionDatabaseClient().batchWriteAtLeastOnce(mutationGroups, options);
@@ -278,7 +278,7 @@ public ServerStream<BatchWriteResponse> batchWriteAtLeastOnce(
278278

279279
@Override
280280
public ReadContext singleUse() {
281-
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, commonAttributes);
281+
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, databaseAttributes);
282282
try (IScope s = tracer.withSpan(span)) {
283283
return getMultiplexedSession().singleUse();
284284
} catch (RuntimeException e) {
@@ -290,7 +290,7 @@ public ReadContext singleUse() {
290290

291291
@Override
292292
public ReadContext singleUse(TimestampBound bound) {
293-
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, commonAttributes);
293+
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, databaseAttributes);
294294
try (IScope s = tracer.withSpan(span)) {
295295
return getMultiplexedSession().singleUse(bound);
296296
} catch (RuntimeException e) {
@@ -302,7 +302,7 @@ public ReadContext singleUse(TimestampBound bound) {
302302

303303
@Override
304304
public ReadOnlyTransaction singleUseReadOnlyTransaction() {
305-
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, commonAttributes);
305+
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, databaseAttributes);
306306
try (IScope s = tracer.withSpan(span)) {
307307
return getMultiplexedSession().singleUseReadOnlyTransaction();
308308
} catch (RuntimeException e) {
@@ -314,7 +314,7 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction() {
314314

315315
@Override
316316
public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) {
317-
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, commonAttributes);
317+
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, databaseAttributes);
318318
try (IScope s = tracer.withSpan(span)) {
319319
return getMultiplexedSession().singleUseReadOnlyTransaction(bound);
320320
} catch (RuntimeException e) {
@@ -326,7 +326,7 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) {
326326

327327
@Override
328328
public ReadOnlyTransaction readOnlyTransaction() {
329-
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, commonAttributes);
329+
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, databaseAttributes);
330330
try (IScope s = tracer.withSpan(span)) {
331331
return getMultiplexedSession().readOnlyTransaction();
332332
} catch (RuntimeException e) {
@@ -338,7 +338,7 @@ public ReadOnlyTransaction readOnlyTransaction() {
338338

339339
@Override
340340
public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) {
341-
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, commonAttributes);
341+
ISpan span = tracer.spanBuilder(READ_ONLY_TRANSACTION, databaseAttributes);
342342
try (IScope s = tracer.withSpan(span)) {
343343
return getMultiplexedSession().readOnlyTransaction(bound);
344344
} catch (RuntimeException e) {
@@ -350,7 +350,7 @@ public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) {
350350

351351
@Override
352352
public TransactionRunner readWriteTransaction(TransactionOption... options) {
353-
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, commonAttributes, options);
353+
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, databaseAttributes, options);
354354
try (IScope s = tracer.withSpan(span)) {
355355
return getMultiplexedSessionForRW().readWriteTransaction(options);
356356
} catch (RuntimeException e) {
@@ -362,7 +362,7 @@ public TransactionRunner readWriteTransaction(TransactionOption... options) {
362362

363363
@Override
364364
public TransactionManager transactionManager(TransactionOption... options) {
365-
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, commonAttributes, options);
365+
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, databaseAttributes, options);
366366
try (IScope s = tracer.withSpan(span)) {
367367
return getMultiplexedSessionForRW().transactionManager(options);
368368
} catch (RuntimeException e) {
@@ -374,7 +374,7 @@ public TransactionManager transactionManager(TransactionOption... options) {
374374

375375
@Override
376376
public AsyncRunner runAsync(TransactionOption... options) {
377-
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, commonAttributes, options);
377+
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, databaseAttributes, options);
378378
try (IScope s = tracer.withSpan(span)) {
379379
return getMultiplexedSessionForRW().runAsync(options);
380380
} catch (RuntimeException e) {
@@ -386,7 +386,7 @@ public AsyncRunner runAsync(TransactionOption... options) {
386386

387387
@Override
388388
public AsyncTransactionManager transactionManagerAsync(TransactionOption... options) {
389-
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, commonAttributes, options);
389+
ISpan span = tracer.spanBuilder(READ_WRITE_TRANSACTION, databaseAttributes, options);
390390
try (IScope s = tracer.withSpan(span)) {
391391
return getMultiplexedSessionForRW().transactionManagerAsync(options);
392392
} catch (RuntimeException e) {
@@ -449,7 +449,7 @@ private TransactionOption[] withReqId(
449449

450450
private long executePartitionedUpdateWithPooledSession(
451451
final Statement stmt, final UpdateOption... options) {
452-
ISpan span = tracer.spanBuilder(PARTITION_DML_TRANSACTION, commonAttributes);
452+
ISpan span = tracer.spanBuilder(PARTITION_DML_TRANSACTION, databaseAttributes);
453453
try (IScope s = tracer.withSpan(span)) {
454454
return runWithSessionRetry(
455455
(session, reqId) -> {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void run() {
140140
List<SessionImpl> sessions;
141141
int remainingSessionsToCreate = sessionCount;
142142
ISpan span =
143-
spanner.getTracer().spanBuilder(SpannerImpl.BATCH_CREATE_SESSIONS, commonAttributes);
143+
spanner.getTracer().spanBuilder(SpannerImpl.BATCH_CREATE_SESSIONS, databaseAttributes);
144144
try (IScope s = spanner.getTracer().withSpan(span)) {
145145
spanner
146146
.getTracer()
@@ -185,7 +185,7 @@ interface SessionConsumer {
185185
private final ExecutorFactory<ScheduledExecutorService> executorFactory;
186186
private final ScheduledExecutorService executor;
187187
private final DatabaseId db;
188-
private final Attributes commonAttributes;
188+
private final Attributes databaseAttributes;
189189

190190
// SessionClient is created long before a DatabaseClientImpl is created,
191191
// as batch sessions are firstly created then later attached to each Client.
@@ -204,7 +204,7 @@ interface SessionConsumer {
204204
this.db = db;
205205
this.executorFactory = executorFactory;
206206
this.executor = executorFactory.get();
207-
this.commonAttributes = spanner.getTracer().createCommonAttributes(db);
207+
this.databaseAttributes = spanner.getTracer().createDatabaseAttributes(db);
208208
}
209209

210210
@Override
@@ -236,7 +236,8 @@ SessionImpl createSession() {
236236
sessionChannelCounter++;
237237
}
238238
XGoogSpannerRequestId reqId = nextRequestId(channelId, 1);
239-
ISpan span = spanner.getTracer().spanBuilder(SpannerImpl.CREATE_SESSION, this.commonAttributes);
239+
ISpan span =
240+
spanner.getTracer().spanBuilder(SpannerImpl.CREATE_SESSION, this.databaseAttributes);
240241
try (IScope s = spanner.getTracer().withSpan(span)) {
241242
com.google.spanner.v1.Session session =
242243
spanner
@@ -289,7 +290,7 @@ SessionImpl createMultiplexedSession() {
289290
ISpan span =
290291
spanner
291292
.getTracer()
292-
.spanBuilder(SpannerImpl.CREATE_MULTIPLEXED_SESSION, this.commonAttributes);
293+
.spanBuilder(SpannerImpl.CREATE_MULTIPLEXED_SESSION, this.databaseAttributes);
293294
// MultiplexedSession doesn't use a channelId hence this hard-coded value.
294295
int channelId = 0;
295296
XGoogSpannerRequestId reqId = nextRequestId(channelId, 1);

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ public DatabaseClient getDatabaseClient(DatabaseId db) {
360360
multiplexedSessionDatabaseClient,
361361
getOptions().getSessionPoolOptions().getUseMultiplexedSessionPartitionedOps(),
362362
useMultiplexedSessionForRW,
363-
this.tracer.createCommonAttributes(db));
363+
this.tracer.createDatabaseAttributes(db));
364364
dbClients.put(db, dbClient);
365365
return dbClient;
366366
}
@@ -375,7 +375,7 @@ DatabaseClientImpl createDatabaseClient(
375375
@Nullable MultiplexedSessionDatabaseClient multiplexedSessionClient,
376376
boolean useMultiplexedSessionPartitionedOps,
377377
boolean useMultiplexedSessionForRW,
378-
Attributes commonAttributes) {
378+
Attributes databaseAttributes) {
379379
if (multiplexedSessionClient != null) {
380380
// Set the session pool in the multiplexed session client.
381381
// This is required to handle fallback to regular sessions for in-progress transactions that
@@ -390,7 +390,7 @@ DatabaseClientImpl createDatabaseClient(
390390
useMultiplexedSessionPartitionedOps,
391391
tracer,
392392
useMultiplexedSessionForRW,
393-
commonAttributes);
393+
databaseAttributes);
394394
}
395395

396396
@Override

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ class TraceWrapper {
6060
private final Tracer openCensusTracer;
6161
private final io.opentelemetry.api.trace.Tracer openTelemetryTracer;
6262
private final boolean enableExtendedTracing;
63+
private final Attributes commonAttributes;
6364

6465
TraceWrapper(
6566
Tracer openCensusTracer,
@@ -68,20 +69,25 @@ class TraceWrapper {
6869
this.openTelemetryTracer = openTelemetryTracer;
6970
this.openCensusTracer = openCensusTracer;
7071
this.enableExtendedTracing = enableExtendedTracing;
72+
this.commonAttributes = createCommonAttributes();
7173
}
7274

7375
ISpan spanBuilder(String spanName) {
7476
return spanBuilder(spanName, Attributes.empty());
7577
}
7678

77-
ISpan spanBuilder(String spanName, Attributes commonAttributes, TransactionOption... options) {
78-
return spanBuilder(spanName, createTransactionAttributes(commonAttributes, options));
79+
ISpan spanBuilder(String spanName, Attributes attributes, TransactionOption... options) {
80+
return spanBuilder(spanName, createTransactionAttributes(attributes, options));
7981
}
8082

8183
ISpan spanBuilder(String spanName, Attributes attributes) {
8284
if (SpannerOptions.getActiveTracingFramework().equals(TracingFramework.OPEN_TELEMETRY)) {
8385
return new OpenTelemetrySpan(
84-
openTelemetryTracer.spanBuilder(spanName).setAllAttributes(attributes).startSpan());
86+
openTelemetryTracer
87+
.spanBuilder(spanName)
88+
.setAllAttributes(attributes)
89+
.setAllAttributes(commonAttributes)
90+
.startSpan());
8591
} else {
8692
return new OpenCensusSpan(openCensusTracer.spanBuilder(spanName).startSpan());
8793
}
@@ -209,10 +215,15 @@ Attributes createTableAttributes(String tableName, Options options) {
209215
return builder.build();
210216
}
211217

212-
Attributes createCommonAttributes(DatabaseId db) {
218+
Attributes createDatabaseAttributes(DatabaseId db) {
213219
AttributesBuilder builder = Attributes.builder();
214220
builder.put(DB_NAME_KEY, db.getDatabase());
215221
builder.put(INSTANCE_NAME_KEY, db.getInstanceId().getInstance());
222+
return builder.build();
223+
}
224+
225+
private Attributes createCommonAttributes() {
226+
AttributesBuilder builder = Attributes.builder();
216227
builder.put(GCP_CLIENT_SERVICE_KEY, "spanner");
217228
builder.put(GCP_CLIENT_REPO_KEY, "googleapis/java-spanner");
218229
builder.put(GCP_CLIENT_VERSION_KEY, GaxProperties.getLibraryVersion(TraceWrapper.class));

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/XGoogSpannerRequestId.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public class XGoogSpannerRequestId {
3535
@VisibleForTesting
3636
static final String RAND_PROCESS_ID = XGoogSpannerRequestId.generateRandProcessId();
3737

38-
static String REQUEST_ID = "x-goog-spanner-request-id";
38+
public static String REQUEST_ID = "x-goog-spanner-request-id";
3939
public static final Metadata.Key<String> REQUEST_HEADER_KEY =
4040
Metadata.Key.of(REQUEST_ID, Metadata.ASCII_STRING_MARSHALLER);
4141

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,17 @@ public void start(Listener<RespT> responseListener, Metadata headers) {
112112
Span span = Span.current();
113113
DatabaseName databaseName = extractDatabaseName(headers);
114114
String key = extractKey(databaseName, method.getFullMethodName());
115+
String requestId = extractRequestId(headers);
115116
TagContext tagContext = getTagContext(key, method.getFullMethodName(), databaseName);
116117
Attributes attributes =
117118
getMetricAttributes(key, method.getFullMethodName(), databaseName);
118119
Map<String, String> builtInMetricsAttributes =
119120
getBuiltInMetricAttributes(key, databaseName);
120-
builtInMetricsAttributes.put(
121-
BuiltInMetricsConstant.REQUEST_ID_KEY.getKey(), extractRequestId(headers));
121+
builtInMetricsAttributes.put(BuiltInMetricsConstant.REQUEST_ID_KEY.getKey(), requestId);
122122
addBuiltInMetricAttributes(compositeTracer, builtInMetricsAttributes);
123+
if (span != null) {
124+
span.setAttribute(XGoogSpannerRequestId.REQUEST_ID, requestId);
125+
}
123126
super.start(
124127
new SimpleForwardingClientCallListener<RespT>(responseListener) {
125128
@Override

0 commit comments

Comments
 (0)