Skip to content

Commit

Permalink
feat: (observability): trace Database.runTransactionAsync
Browse files Browse the repository at this point in the history
Extracted out of PR #2158, this change traces
Database.runTransactionAsync. However, testing isn't effective
because of bugs such as #2166.
  • Loading branch information
odeke-em committed Oct 21, 2024
1 parent 51bc8a7 commit 563b571
Show file tree
Hide file tree
Showing 3 changed files with 161 additions and 38 deletions.
69 changes: 69 additions & 0 deletions observability-test/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1565,6 +1565,75 @@ describe('Database', () => {
});
});

describe('runTransactionAsync', () => {
const SESSION = new FakeSession();
const TRANSACTION = new FakeTransaction(
{} as google.spanner.v1.TransactionOptions.ReadWrite
);

let pool: FakeSessionPool;

beforeEach(() => {
pool = database.pool_;

(sandbox.stub(pool, 'getSession') as sinon.SinonStub).callsFake(
callback => {
callback(null, SESSION, TRANSACTION);
}
);
});

it('with no error', async() => {
await database.runTransactionAsync(async (txn) => {
const [rows] = txn.run('SELECT 1');
await txn.commit();
});

const spans = traceExporter.getFinishedSpans();
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spans.forEach(span => {
actualSpanNames.push(span.name);
span.events.forEach(event => {
actualEventNames.push(event.name);
});
});

const expectedSpanNames = ['CloudSpanner.Database.runTransactionAsync'];
assert.deepStrictEqual(
actualSpanNames,
expectedSpanNames,
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

// Ensure that the span actually produced an error that was recorded.
const firstSpan = spans[0];
assert.strictEqual(
SpanStatusCode.ERROR,
firstSpan.status.code,
'Expected an ERROR span status'
);
assert.strictEqual(
'getting a session',
firstSpan.status.message,
'Mismatched span status message'
);

// We don't expect events.
const expectedEventNames = [];
assert.deepStrictEqual(
actualEventNames,
expectedEventNames,
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
);

done();

});
});

describe('runStream', () => {
const QUERY = {
sql: 'SELECT * FROM table',
Expand Down
46 changes: 46 additions & 0 deletions observability-test/spanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,52 @@ describe('EndToEnd', () => {
});
});

it('runTransactionAsync', async () => {
const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);
await database.runTransactionAsync(async transaction => {
const [rows] = await transaction!.run('SELECT 1');
});

traceExporter.forceFlush();
const spans = traceExporter.getFinishedSpans();
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
spans.forEach(span => {
actualSpanNames.push(span.name);
span.events.forEach(event => {
actualEventNames.push(event.name);
});
});

const expectedSpanNames = [
'CloudSpanner.Snapshot.runStream',
'CloudSpanner.Snapshot.run',
'CloudSpanner.Database.runTransactionAsync',
];
assert.deepStrictEqual(
actualSpanNames,
expectedSpanNames,
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

const expectedEventNames = [
'Transaction Creation Done',
'Acquiring session',
'Cache hit: has usable session',
'Acquired session',
'Using Session',
];
assert.deepStrictEqual(
actualEventNames,
expectedEventNames,
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
);
});

it('writeAtLeastOnce', done => {
const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
Expand Down
84 changes: 46 additions & 38 deletions src/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3363,46 +3363,54 @@ class Database extends common.GrpcServiceObject {

let sessionId = '';
const getSession = this.pool_.getSession.bind(this.pool_);
const span = getActiveOrNoopSpan();
// Loop to retry 'Session not found' errors.
// (and yes, we like while (true) more than for (;;) here)
// eslint-disable-next-line no-constant-condition
while (true) {
try {
const [session, transaction] = await promisify(getSession)();
transaction.requestOptions = Object.assign(
transaction.requestOptions || {},
options.requestOptions
);
if (options.optimisticLock) {
transaction.useOptimisticLock();
}
if (options.excludeTxnFromChangeStreams) {
transaction.excludeTxnFromChangeStreams();
}
sessionId = session?.id;
span.addEvent('Using Session', {'session.id': sessionId});
const runner = new AsyncTransactionRunner<T>(
session,
transaction,
runFn,
options
);

try {
return await runner.run();
} finally {
this.pool_.release(session);
}
} catch (e) {
if (!isSessionNotFoundError(e as ServiceError)) {
span.addEvent('No session available', {
'session.id': sessionId,
});
throw e;
return startTrace(
'Database.runTransactionAsync',
this._traceConfig,
async span => {
// Loop to retry 'Session not found' errors.
// (and yes, we like while (true) more than for (;;) here)
// eslint-disable-next-line no-constant-condition
while (true) {
try {
const [session, transaction] = await promisify(getSession)();
transaction.requestOptions = Object.assign(
transaction.requestOptions || {},
options.requestOptions
);
if (options.optimisticLock) {
transaction.useOptimisticLock();
}
if (options.excludeTxnFromChangeStreams) {
transaction.excludeTxnFromChangeStreams();
}
sessionId = session?.id;
span.addEvent('Using Session', {'session.id': sessionId});
const runner = new AsyncTransactionRunner<T>(
session,
transaction,
runFn,
options
);

try {
const result = await runner.run();
span.end();
return result;
} finally {
this.pool_.release(session);
}
} catch (e) {
if (!isSessionNotFoundError(e as ServiceError)) {
span.addEvent('No session available', {
'session.id': sessionId,
});
span.end();
throw e;
}
}
}
}
}
);
}

/**
Expand Down

0 comments on commit 563b571

Please sign in to comment.