Skip to content

Commit f08cf4b

Browse files
surbhigarg92odeke-emalkatrivedi
authored
chore: refactor observability tests (#2177)
* chore: integration test fix * traces tests refactoring * feat: (observability): trace Database.runPartitionedUpdate (#2176) This change traces Database.runPartitionedUpdate along with the appropriate tests for it with and without errors. Updates #2079 * moving additional attributes to separate PR --------- Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com> Co-authored-by: alkatrivedi <58396306+alkatrivedi@users.noreply.github.com>
1 parent be063b0 commit f08cf4b

File tree

12 files changed

+263
-671
lines changed

12 files changed

+263
-671
lines changed

‎observability-test/batch-transaction.ts‎

Lines changed: 83 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -153,103 +153,95 @@ describe('BatchTransaction', () => {
153153
};
154154

155155
it('createQueryPartitions', done => {
156-
const REQUEST = sandbox.stub();
157-
158-
const res = batchTransaction.createQueryPartitions(
159-
QUERY,
160-
(err, part, resp) => {
161-
assert.ifError(err);
162-
traceExporter.forceFlush();
163-
const spans = traceExporter.getFinishedSpans();
164-
assert.strictEqual(spans.length, 2, 'Exactly 2 spans expected');
165-
166-
// Sort the spans by duration.
167-
spans.sort((spanA, spanB) => {
168-
spanA.duration < spanB.duration;
169-
});
170-
171-
const actualSpanNames: string[] = [];
172-
spans.forEach(span => {
173-
actualSpanNames.push(span.name);
174-
});
175-
176-
const expectedSpanNames = [
177-
'CloudSpanner.BatchTransaction.createPartitions_',
178-
'CloudSpanner.BatchTransaction.createQueryPartitions',
179-
];
180-
assert.deepStrictEqual(
181-
actualSpanNames,
182-
expectedSpanNames,
183-
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
184-
);
185-
186-
// Ensure that createPartitions_ is a child span of createQueryPartitions.
187-
const spanCreatePartitions_ = spans[0];
188-
const spanCreateQueryPartitions = spans[1];
189-
assert.ok(
190-
spanCreateQueryPartitions.spanContext().traceId,
191-
'Expected that createQueryPartitions has a defined traceId'
192-
);
193-
assert.ok(
194-
spanCreatePartitions_.spanContext().traceId,
195-
'Expected that createPartitions_ has a defined traceId'
196-
);
197-
assert.deepStrictEqual(
198-
spanCreatePartitions_.spanContext().traceId,
199-
spanCreateQueryPartitions.spanContext().traceId,
200-
'Expected that both spans share a traceId'
201-
);
202-
assert.ok(
203-
spanCreateQueryPartitions.spanContext().spanId,
204-
'Expected that createQueryPartitions has a defined spanId'
205-
);
206-
assert.ok(
207-
spanCreatePartitions_.spanContext().spanId,
208-
'Expected that createPartitions_ has a defined spanId'
209-
);
210-
assert.deepStrictEqual(
211-
spanCreatePartitions_.parentSpanId,
212-
spanCreateQueryPartitions.spanContext().spanId,
213-
'Expected that createQueryPartitions is the parent to createPartitions_'
214-
);
215-
done();
216-
}
217-
);
156+
batchTransaction.createQueryPartitions(QUERY, err => {
157+
assert.ifError(err);
158+
traceExporter.forceFlush();
159+
const spans = traceExporter.getFinishedSpans();
160+
assert.strictEqual(spans.length, 2, 'Exactly 2 spans expected');
161+
162+
// Sort the spans by duration.
163+
spans.sort((spanA, spanB) => {
164+
spanA.duration < spanB.duration;
165+
});
166+
167+
const actualSpanNames: string[] = [];
168+
spans.forEach(span => {
169+
actualSpanNames.push(span.name);
170+
});
171+
172+
const expectedSpanNames = [
173+
'CloudSpanner.BatchTransaction.createPartitions_',
174+
'CloudSpanner.BatchTransaction.createQueryPartitions',
175+
];
176+
assert.deepStrictEqual(
177+
actualSpanNames,
178+
expectedSpanNames,
179+
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
180+
);
181+
182+
// Ensure that createPartitions_ is a child span of createQueryPartitions.
183+
const spanCreatePartitions_ = spans[0];
184+
const spanCreateQueryPartitions = spans[1];
185+
assert.ok(
186+
spanCreateQueryPartitions.spanContext().traceId,
187+
'Expected that createQueryPartitions has a defined traceId'
188+
);
189+
assert.ok(
190+
spanCreatePartitions_.spanContext().traceId,
191+
'Expected that createPartitions_ has a defined traceId'
192+
);
193+
assert.deepStrictEqual(
194+
spanCreatePartitions_.spanContext().traceId,
195+
spanCreateQueryPartitions.spanContext().traceId,
196+
'Expected that both spans share a traceId'
197+
);
198+
assert.ok(
199+
spanCreateQueryPartitions.spanContext().spanId,
200+
'Expected that createQueryPartitions has a defined spanId'
201+
);
202+
assert.ok(
203+
spanCreatePartitions_.spanContext().spanId,
204+
'Expected that createPartitions_ has a defined spanId'
205+
);
206+
assert.deepStrictEqual(
207+
spanCreatePartitions_.parentSpanId,
208+
spanCreateQueryPartitions.spanContext().spanId,
209+
'Expected that createQueryPartitions is the parent to createPartitions_'
210+
);
211+
done();
212+
});
218213
});
219214

220215
it('createReadPartitions', done => {
221216
const REQUEST = sandbox.stub();
222217
const response = {};
223218
REQUEST.callsFake((_, callback) => callback(null, response));
224219

225-
const res = batchTransaction.createReadPartitions(
226-
QUERY,
227-
(err, part, resp) => {
228-
assert.ifError(err);
229-
traceExporter.forceFlush();
230-
const spans = traceExporter.getFinishedSpans();
231-
assert.strictEqual(spans.length, 2, 'Exactly 2 spans expected');
232-
233-
// Sort the spans by duration.
234-
spans.sort((spanA, spanB) => {
235-
spanA.duration < spanB.duration;
236-
});
237-
238-
const actualSpanNames: string[] = [];
239-
spans.forEach(span => {
240-
actualSpanNames.push(span.name);
241-
});
242-
const expectedSpanNames = [
243-
'CloudSpanner.BatchTransaction.createPartitions_',
244-
'CloudSpanner.BatchTransaction.createReadPartitions',
245-
];
246-
assert.deepStrictEqual(
247-
actualSpanNames,
248-
expectedSpanNames,
249-
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
250-
);
251-
done();
252-
}
253-
);
220+
batchTransaction.createReadPartitions(QUERY, err => {
221+
assert.ifError(err);
222+
traceExporter.forceFlush();
223+
const spans = traceExporter.getFinishedSpans();
224+
assert.strictEqual(spans.length, 2, 'Exactly 2 spans expected');
225+
226+
// Sort the spans by duration.
227+
spans.sort((spanA, spanB) => {
228+
spanA.duration < spanB.duration;
229+
});
230+
231+
const actualSpanNames: string[] = [];
232+
spans.forEach(span => {
233+
actualSpanNames.push(span.name);
234+
});
235+
const expectedSpanNames = [
236+
'CloudSpanner.BatchTransaction.createPartitions_',
237+
'CloudSpanner.BatchTransaction.createReadPartitions',
238+
];
239+
assert.deepStrictEqual(
240+
actualSpanNames,
241+
expectedSpanNames,
242+
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
243+
);
244+
done();
245+
});
254246
});
255247
});

‎observability-test/database.ts‎

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,6 @@ describe('Database', () => {
507507

508508
let beginSnapshotStub: sinon.SinonStub;
509509
let getSessionStub: sinon.SinonStub;
510-
let snapshotStub: sinon.SinonStub;
511510

512511
beforeEach(() => {
513512
fakePool = database.pool_;
@@ -524,9 +523,7 @@ describe('Database', () => {
524523
sandbox.stub(fakePool, 'getSession') as sinon.SinonStub
525524
).callsFake(callback => callback(null, fakeSession));
526525

527-
snapshotStub = sandbox
528-
.stub(fakeSession, 'snapshot')
529-
.returns(fakeSnapshot);
526+
sandbox.stub(fakeSession, 'snapshot').returns(fakeSnapshot);
530527
});
531528

532529
it('with error', done => {
@@ -1175,7 +1172,7 @@ describe('Database', () => {
11751172

11761173
it('with error on null mutation should catch thrown error', done => {
11771174
try {
1178-
database.writeAtLeastOnce(null, (err, res) => {});
1175+
database.writeAtLeastOnce(null, () => {});
11791176
} catch (err) {
11801177
// Performing a substring search on the error because
11811178
// depending on the version of Node.js, the error might be either of:
@@ -1250,7 +1247,6 @@ describe('Database', () => {
12501247
let fakeSession: FakeSession;
12511248
let fakeDataStream: Transform;
12521249
let getSessionStub: sinon.SinonStub;
1253-
let requestStreamStub: sinon.SinonStub;
12541250

12551251
const options = {
12561252
requestOptions: {
@@ -1269,9 +1265,7 @@ describe('Database', () => {
12691265
sandbox.stub(fakePool, 'getSession') as sinon.SinonStub
12701266
).callsFake(callback => callback(null, fakeSession));
12711267

1272-
requestStreamStub = sandbox
1273-
.stub(database, 'requestStream')
1274-
.returns(fakeDataStream);
1268+
sandbox.stub(database, 'requestStream').returns(fakeDataStream);
12751269
});
12761270

12771271
it('on retry with "Session not found" error', done => {
@@ -1320,7 +1314,6 @@ describe('Database', () => {
13201314
'Expected an ERROR span status'
13211315
);
13221316

1323-
const errorMessage = firstSpan.status.message;
13241317
assert.deepStrictEqual(
13251318
firstSpan.status.message,
13261319
sessionNotFoundError.message
@@ -1658,7 +1651,7 @@ describe('Database', () => {
16581651
.throws(ourException);
16591652

16601653
assert.rejects(async () => {
1661-
const value = await database.runTransactionAsync(async txn => {
1654+
await database.runTransactionAsync(async txn => {
16621655
const result = await txn.run('SELECT 1');
16631656
await txn.commit();
16641657
return result;
@@ -1724,8 +1717,6 @@ describe('Database', () => {
17241717
let fakeStream2: Transform;
17251718

17261719
let getSessionStub: sinon.SinonStub;
1727-
let snapshotStub: sinon.SinonStub;
1728-
let runStreamStub: sinon.SinonStub;
17291720

17301721
beforeEach(() => {
17311722
fakePool = database.pool_;
@@ -1746,15 +1737,11 @@ describe('Database', () => {
17461737
.onSecondCall()
17471738
.callsFake(callback => callback(null, fakeSession2));
17481739

1749-
snapshotStub = sandbox
1750-
.stub(fakeSession, 'snapshot')
1751-
.returns(fakeSnapshot);
1740+
sandbox.stub(fakeSession, 'snapshot').returns(fakeSnapshot);
17521741

17531742
sandbox.stub(fakeSession2, 'snapshot').returns(fakeSnapshot2);
17541743

1755-
runStreamStub = sandbox
1756-
.stub(fakeSnapshot, 'runStream')
1757-
.returns(fakeStream);
1744+
sandbox.stub(fakeSnapshot, 'runStream').returns(fakeStream);
17581745

17591746
sandbox.stub(fakeSnapshot2, 'runStream').returns(fakeStream2);
17601747
});
@@ -1975,7 +1962,6 @@ describe('Database', () => {
19751962

19761963
let getSessionStub;
19771964
let beginStub;
1978-
let runUpdateStub;
19791965

19801966
beforeEach(() => {
19811967
fakePool = database.pool_;
@@ -1996,7 +1982,7 @@ describe('Database', () => {
19961982
sandbox.stub(fakePartitionedDml, 'begin') as sinon.SinonStub
19971983
).callsFake(callback => callback(null));
19981984

1999-
runUpdateStub = (
1985+
(
20001986
sandbox.stub(fakePartitionedDml, 'runUpdate') as sinon.SinonStub
20011987
).callsFake((_, callback) => callback(null));
20021988
});
@@ -2031,7 +2017,6 @@ describe('Database', () => {
20312017

20322018
it('with pool errors', done => {
20332019
const fakeError = new Error('err');
2034-
const fakeCallback = sandbox.spy();
20352020

20362021
getSessionStub.callsFake(callback => callback(fakeError));
20372022
database.runPartitionedUpdate(QUERY, async (err, rowCount) => {
@@ -2132,7 +2117,7 @@ describe('Database', () => {
21322117
sandbox.stub(fakePool, 'release') as sinon.SinonStub
21332118
).withArgs(fakeSession);
21342119

2135-
database.runPartitionedUpdate(QUERY, async (err, rowCount) => {
2120+
database.runPartitionedUpdate(QUERY, async () => {
21362121
const exportResults = await getTraceExportResults();
21372122
const actualSpanNames = exportResults.spanNames;
21382123
const spans = exportResults.spans;

‎observability-test/helper.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,25 @@ import * as assert from 'assert';
1919
const {ReadableSpan} = require('@opentelemetry/sdk-trace-base');
2020
import {SEMATTRS_DB_NAME} from '@opentelemetry/semantic-conventions';
2121

22+
export const batchCreateSessionsEvents = [
23+
'Requesting 25 sessions',
24+
'Creating 25 sessions',
25+
'Requested for 25 sessions returned 25',
26+
];
27+
28+
export const waitingSessionsEvents = [
29+
'Acquiring session',
30+
'Waiting for a session to become available',
31+
'Acquired session',
32+
'Using Session',
33+
];
34+
35+
export const cacheSessionEvents = [
36+
'Acquiring session',
37+
'Cache hit: has usable session',
38+
'Acquired session',
39+
];
40+
2241
/**
2342
* This utility exists as a test helper because mocha has builtin "context"
2443
* and referring to context causes type/value collision errors.
@@ -47,3 +66,30 @@ export function generateWithAllSpansHaveDBName(dbName: String): Function {
4766
});
4867
};
4968
}
69+
70+
export async function verifySpansAndEvents(
71+
traceExporter,
72+
expectedSpans,
73+
expectedEvents
74+
) {
75+
await traceExporter.forceFlush();
76+
const spans = traceExporter.getFinishedSpans();
77+
const actualEventNames: string[] = [];
78+
const actualSpanNames: string[] = [];
79+
spans.forEach(span => {
80+
actualSpanNames.push(span.name);
81+
span.events.forEach(event => {
82+
actualEventNames.push(event.name);
83+
});
84+
});
85+
assert.deepStrictEqual(
86+
actualSpanNames,
87+
expectedSpans,
88+
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpans}`
89+
);
90+
assert.deepStrictEqual(
91+
actualEventNames,
92+
expectedEvents,
93+
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEvents}`
94+
);
95+
}

0 commit comments

Comments
 (0)