Skip to content

Commit 0f7bb00

Browse files
committed
Ensure database.runTransaction async awaits on caller code + ensure context.active only in Spanner constructor
1 parent adfb777 commit 0f7bb00

File tree

4 files changed

+37
-11
lines changed

4 files changed

+37
-11
lines changed

src/database.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3223,8 +3223,8 @@ class Database extends common.GrpcServiceObject {
32233223
span.addEvent('No session available', {
32243224
'session.id': session?.id,
32253225
});
3226-
span.end();
32273226
this.runTransaction(options, runFn!);
3227+
span.end();
32283228
return;
32293229
}
32303230

@@ -3244,18 +3244,22 @@ class Database extends common.GrpcServiceObject {
32443244

32453245
const release = () => {
32463246
this.pool_.release(session!);
3247-
span.end();
3247+
if (span.isRecording()) {
3248+
// span.end() might have already been invoked inside
3249+
// Transactionrunner.
3250+
span.end();
3251+
}
32483252
};
32493253

32503254
const runner = new TransactionRunner(
32513255
session!,
32523256
transaction!,
3253-
(err, resp) => {
3257+
async (err, resp) => {
32543258
if (err) {
32553259
setSpanError(span, err!);
32563260
}
3261+
await runFn!(err, resp);
32573262
span.end();
3258-
runFn!(err, resp);
32593263
},
32603264
options
32613265
);

src/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@ import {
8080
import grpcGcpModule = require('grpc-gcp');
8181
const grpcGcp = grpcGcpModule(grpc);
8282
import * as v1 from './v1';
83-
import {ObservabilityOptions} from './instrument';
83+
import {
84+
ObservabilityOptions,
85+
ensureInitialContextManagerSet,
86+
} from './instrument';
8487

8588
// eslint-disable-next-line @typescript-eslint/no-var-requires
8689
const gcpApiConfig = require('./spanner_grpc_config.json');
@@ -370,6 +373,7 @@ class Spanner extends GrpcService {
370373
};
371374
this.directedReadOptions = directedReadOptions;
372375
this._observabilityOptions = options.observabilityOptions;
376+
ensureInitialContextManagerSet();
373377
}
374378

375379
/**

src/instrument.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ function ensureInitialContextManagerSet() {
117117
context.setGlobalContextManager(contextManager);
118118
}
119119
}
120+
export {ensureInitialContextManagerSet};
120121

121122
const debugTraces = process.env.SPANNER_DEBUG_TRACES === 'true';
122123

@@ -138,14 +139,19 @@ export function startTrace<T>(
138139
config = {} as traceConfig;
139140
}
140141

141-
ensureInitialContextManagerSet();
142+
const tracer = getTracer(config.opts?.tracerProvider);
143+
let parentSpanId: string | undefined;
144+
if (debugTraces) {
145+
const parentSpan = trace.getActiveSpan();
146+
parentSpanId = parentSpan?.spanContext()!.spanId;
147+
}
142148

143-
return getTracer(config.opts?.tracerProvider).startActiveSpan(
149+
return tracer.startActiveSpan(
144150
SPAN_NAMESPACE_PREFIX + '.' + spanNameSuffix,
145151
{kind: SpanKind.CLIENT},
146152
span => {
147153
if (debugTraces) {
148-
patchSpanEndForDebugging(span, spanNameSuffix);
154+
patchSpanEndForDebugging(span, spanNameSuffix, parentSpanId);
149155
}
150156

151157
span.setAttribute(SEMATTRS_DB_SYSTEM, 'spanner');
@@ -305,10 +311,19 @@ class noopSpan implements Span {
305311
}
306312
}
307313

308-
function patchSpanEndForDebugging(span: Span, spanNameSuffix: string) {
314+
function patchSpanEndForDebugging(
315+
span: Span,
316+
spanNameSuffix: string,
317+
parentSpanId: string | undefined
318+
) {
319+
console.trace(
320+
`\x1b[33m${spanNameSuffix}.start id=${span.spanContext().spanId} with parentSpanId: ${parentSpanId}\x1b[00m`
321+
);
309322
const origSpanEnd = span.end;
310323
const wrapSpanEnd = function (this: Span) {
311-
console.trace(`\x1b[35m${spanNameSuffix}.end()\x1b[00m`);
324+
console.trace(
325+
`\x1b[35m${spanNameSuffix}.end() id=${span.spanContext().spanId} with parentSpanId: ${parentSpanId}\x1b[00m`
326+
);
312327
return origSpanEnd.apply(this);
313328
};
314329
Object.defineProperty(span, 'end', {

src/transaction.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,7 @@ export class Snapshot extends EventEmitter {
744744
}
745745
})
746746
.on('error', err => {
747+
setSpanError(span, err);
747748
const isServiceError =
748749
err && typeof err === 'object' && 'code' in err;
749750
if (
@@ -754,9 +755,11 @@ export class Snapshot extends EventEmitter {
754755
(err as grpc.ServiceError).code === grpc.status.ABORTED
755756
)
756757
) {
758+
span.addEvent('Transaction Aborted, retrying begin', {
759+
'transaction.id': this.id?.toString(),
760+
});
757761
this.begin();
758762
}
759-
setSpanError(span, err);
760763
})
761764
.on('end', err => {
762765
if (err) {

0 commit comments

Comments
 (0)