Skip to content

Commit 17f2aa9

Browse files
surbhigarg92surbhigarg92
committed
remove snapshot.run span
1 parent 6a8a8a0 commit 17f2aa9

File tree

5 files changed

+23
-167
lines changed

5 files changed

+23
-167
lines changed

‎observability-test/spanner.ts‎

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,6 @@ describe('EndToEnd', async () => {
200200
'CloudSpanner.Snapshot.begin',
201201
'CloudSpanner.Database.getSnapshot',
202202
'CloudSpanner.Snapshot.runStream',
203-
'CloudSpanner.Snapshot.run',
204203
];
205204
const expectedEventNames = [
206205
'Begin Transaction',
@@ -288,16 +287,15 @@ describe('EndToEnd', async () => {
288287
await transaction!.end();
289288
const expectedSpanNames = [
290289
'CloudSpanner.Snapshot.runStream',
291-
'CloudSpanner.Snapshot.run',
292290
'CloudSpanner.Transaction.commit',
293291
'CloudSpanner.Database.runTransaction',
294292
];
295293
const expectedEventNames = [
296294
'Starting stream',
297-
'Transaction Creation Done',
298295
'Starting Commit',
299296
'Commit Done',
300297
...cacheSessionEvents,
298+
'Transaction Creation Done',
301299
];
302300

303301
await verifySpansAndEvents(
@@ -317,14 +315,13 @@ describe('EndToEnd', async () => {
317315

318316
const expectedSpanNames = [
319317
'CloudSpanner.Snapshot.runStream',
320-
'CloudSpanner.Snapshot.run',
321318
'CloudSpanner.Database.runTransactionAsync',
322319
];
323320
const expectedEventNames = [
324321
'Starting stream',
325-
'Transaction Creation Done',
326322
...cacheSessionEvents,
327323
'Using Session',
324+
'Transaction Creation Done',
328325
];
329326
await verifySpansAndEvents(
330327
traceExporter,
@@ -333,7 +330,7 @@ describe('EndToEnd', async () => {
333330
);
334331
});
335332

336-
it.only('runTransaction with abort', done => {
333+
it.skip('runTransaction with abort', done => {
337334
let attempts = 0;
338335
let rowCount = 0;
339336
const database = newTestDatabase();
@@ -354,9 +351,7 @@ describe('EndToEnd', async () => {
354351
'CloudSpanner.Database.batchCreateSessions',
355352
'CloudSpanner.SessionPool.createSessions',
356353
'CloudSpanner.Snapshot.runStream',
357-
'CloudSpanner.Snapshot.run',
358354
'CloudSpanner.Snapshot.runStream',
359-
'CloudSpanner.Snapshot.run',
360355
'CloudSpanner.Transaction.commit',
361356
'CloudSpanner.Snapshot.begin',
362357
'CloudSpanner.Database.runTransaction',
@@ -383,7 +378,7 @@ describe('EndToEnd', async () => {
383378
});
384379
});
385380

386-
it('runTransactionAsync with abort', async () => {
381+
it.skip('runTransactionAsync with abort', async () => {
387382
let attempts = 0;
388383
const database = newTestDatabase();
389384
await database.runTransactionAsync((transaction): Promise<number> => {
@@ -402,10 +397,8 @@ describe('EndToEnd', async () => {
402397
'CloudSpanner.Database.batchCreateSessions',
403398
'CloudSpanner.SessionPool.createSessions',
404399
'CloudSpanner.Snapshot.runStream',
405-
'CloudSpanner.Snapshot.run',
406400
'CloudSpanner.Snapshot.begin',
407401
'CloudSpanner.Snapshot.runStream',
408-
'CloudSpanner.Snapshot.run',
409402
'CloudSpanner.Transaction.commit',
410403
'CloudSpanner.Database.runTransactionAsync',
411404
];
@@ -476,7 +469,6 @@ describe('EndToEnd', async () => {
476469
const expectedSpanNames = [
477470
'CloudSpanner.Snapshot.begin',
478471
'CloudSpanner.Snapshot.runStream',
479-
'CloudSpanner.Snapshot.run',
480472
'CloudSpanner.Dml.runUpdate',
481473
'CloudSpanner.PartitionedDml.runUpdate',
482474
'CloudSpanner.Database.runPartitionedUpdate',
@@ -635,7 +627,6 @@ describe('ObservabilityOptions injection and propagation', async () => {
635627
const expectedSpanNames = [
636628
'CloudSpanner.Database.getTransaction',
637629
'CloudSpanner.Snapshot.runStream',
638-
'CloudSpanner.Snapshot.run',
639630
];
640631
assert.deepStrictEqual(
641632
actualSpanNames,
@@ -690,7 +681,6 @@ describe('ObservabilityOptions injection and propagation', async () => {
690681
const expectedSpanNames = [
691682
'CloudSpanner.Snapshot.begin',
692683
'CloudSpanner.Snapshot.runStream',
693-
'CloudSpanner.Snapshot.run',
694684
'CloudSpanner.Dml.runUpdate',
695685
];
696686
assert.deepStrictEqual(
@@ -799,7 +789,6 @@ describe('ObservabilityOptions injection and propagation', async () => {
799789
const expectedSpanNames = [
800790
'CloudSpanner.Snapshot.begin',
801791
'CloudSpanner.Snapshot.runStream',
802-
'CloudSpanner.Snapshot.run',
803792
'CloudSpanner.Dml.runUpdate',
804793
'CloudSpanner.Transaction.rollback',
805794
];
@@ -1352,7 +1341,6 @@ SELECT 1p
13521341
'CloudSpanner.Database.batchCreateSessions',
13531342
'CloudSpanner.SessionPool.createSessions',
13541343
'CloudSpanner.Snapshot.runStream',
1355-
'CloudSpanner.Snapshot.run',
13561344
'CloudSpanner.Snapshot.begin',
13571345
'CloudSpanner.Snapshot.begin',
13581346
'CloudSpanner.Transaction.commit',
@@ -1364,19 +1352,6 @@ SELECT 1p
13641352
expectedSpanNames,
13651353
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
13661354
);
1367-
const spanSnapshotRun = spans[3];
1368-
assert.strictEqual(spanSnapshotRun.name, 'CloudSpanner.Snapshot.run');
1369-
const wantSpanErr = '6 ALREADY_EXISTS: ' + messageBadInsertAlreadyExistent;
1370-
assert.deepStrictEqual(
1371-
spanSnapshotRun.status.code,
1372-
SpanStatusCode.ERROR,
1373-
'Unexpected status code'
1374-
);
1375-
assert.deepStrictEqual(
1376-
spanSnapshotRun.status.message,
1377-
wantSpanErr,
1378-
'Unexpexcted error message'
1379-
);
13801355

13811356
const databaseBatchCreateSessionsSpan = spans[0];
13821357
assert.strictEqual(
@@ -1405,8 +1380,7 @@ SELECT 1p
14051380

14061381
// We need to ensure a strict relationship between the spans.
14071382
// |-Database.runTransactionAsync |-------------------------------------|
1408-
// |-Snapshot.run |------------------------|
1409-
// |-Snapshot.runStream |---------------------|
1383+
// |-Snapshot.runStream |---------------------|
14101384
// |-Transaction.commit |--------|
14111385
// |-Snapshot.begin |------|
14121386
// |-Snapshot.commit |-----|
@@ -1427,12 +1401,6 @@ SELECT 1p
14271401
'Expected that Database.runTransaction is the parent to Transaction.commmit'
14281402
);
14291403

1430-
assert.deepStrictEqual(
1431-
spanSnapshotRun.parentSpanId,
1432-
spanDatabaseRunTransactionAsync.spanContext().spanId,
1433-
'Expected that Database.runTransaction is the parent to Snapshot.run'
1434-
);
1435-
14361404
// Assert that despite all being exported, SessionPool.createSessions
14371405
// is not in the same trace as runStream, createSessions is invoked at
14381406
// Spanner Client instantiation, thus before database.run is invoked.
@@ -1953,7 +1921,6 @@ describe('Traces for ExecuteStream broken stream retries', () => {
19531921
'CloudSpanner.Database.batchCreateSessions',
19541922
'CloudSpanner.SessionPool.createSessions',
19551923
'CloudSpanner.Snapshot.runStream',
1956-
'CloudSpanner.Snapshot.run',
19571924
'CloudSpanner.Dml.runUpdate',
19581925
'CloudSpanner.Snapshot.begin',
19591926
'CloudSpanner.Transaction.commit',

‎observability-test/transaction.ts‎

Lines changed: 1 addition & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -331,107 +331,7 @@ describe('Transaction', () => {
331331
fakeStream.emit('end');
332332
});
333333
});
334-
335-
describe('run', () => {
336-
const QUERY = 'SELET * FROM `MyTable`';
337-
338-
let fakeStream;
339-
340-
beforeEach(() => {
341-
fakeStream = new EventEmitter();
342-
sandbox.stub(snapshot, 'runStream').returns(fakeStream);
343-
});
344-
345-
it('without error', done => {
346-
const fakeRows = [{a: 'b'}, {c: 'd'}, {e: 'f'}];
347-
348-
snapshot.run(QUERY, (err, rows) => {
349-
assert.ifError(err);
350-
assert.deepStrictEqual(rows, fakeRows);
351-
352-
const exportResults = extractExportedSpans();
353-
const actualSpanNames = exportResults.spanNames;
354-
const actualEventNames = exportResults.spanEventNames;
355-
356-
const expectedSpanNames = ['CloudSpanner.Snapshot.run'];
357-
assert.deepStrictEqual(
358-
actualSpanNames,
359-
expectedSpanNames,
360-
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
361-
);
362-
363-
const expectedEventNames = [];
364-
assert.deepStrictEqual(
365-
actualEventNames,
366-
expectedEventNames,
367-
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
368-
);
369-
370-
// Ensure that the final span that got retries did not error.
371-
const spans = exportResults.spans;
372-
const firstSpan = spans[0];
373-
assert.strictEqual(
374-
SpanStatusCode.UNSET,
375-
firstSpan.status.code,
376-
'Unexpected an span status code'
377-
);
378-
assert.strictEqual(
379-
undefined,
380-
firstSpan.status.message,
381-
'Unexpected span status message'
382-
);
383-
done();
384-
});
385-
386-
fakeRows.forEach(row => fakeStream.emit('data', row));
387-
fakeStream.emit('end');
388-
});
389-
390-
it('with errors', done => {
391-
const fakeError = new Error('run.error');
392-
393-
snapshot.run(QUERY, err => {
394-
assert.strictEqual(err, fakeError);
395-
396-
const exportResults = extractExportedSpans();
397-
const actualSpanNames = exportResults.spanNames;
398-
const actualEventNames = exportResults.spanEventNames;
399-
400-
const expectedSpanNames = ['CloudSpanner.Snapshot.run'];
401-
assert.deepStrictEqual(
402-
actualSpanNames,
403-
expectedSpanNames,
404-
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
405-
);
406-
407-
const expectedEventNames = [];
408-
assert.deepStrictEqual(
409-
actualEventNames,
410-
expectedEventNames,
411-
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
412-
);
413-
414-
// Ensure that the final span that got retries did not error.
415-
const spans = exportResults.spans;
416-
const firstSpan = spans[0];
417-
assert.strictEqual(
418-
SpanStatusCode.ERROR,
419-
firstSpan.status.code,
420-
'Unexpected an span status code'
421-
);
422-
assert.strictEqual(
423-
'run.error',
424-
firstSpan.status.message,
425-
'Unexpected span status message'
426-
);
427-
428-
done();
429-
});
430-
431-
fakeStream.emit('error', fakeError);
432-
});
433-
});
434-
334+
435335
describe('runStream', () => {
436336
const QUERY = {
437337
sql: 'SELECT * FROM `MyTable`',

‎src/partial-result-stream.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ export function partialResultStream(
569569
// checkpoint stream has queued. After that, we will destroy the
570570
// user's stream with the same error.
571571
setImmediate(() => batchAndSplitOnTokenStream.destroy(err));
572-
setSpanErrorAndException(span, err as Error);
572+
// setSpanErrorAndException(span, err as Error);
573573
return;
574574
}
575575

‎src/transaction-runner.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,6 @@ export class TransactionRunner extends Runner<void> {
323323
}
324324

325325
stream.unpipe(proxyStream);
326-
// proxyStream.emit('error', err);
327326
reject(err);
328327
})
329328
.pipe(proxyStream);

‎src/transaction.ts‎

Lines changed: 16 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,33 +1093,23 @@ export class Snapshot extends EventEmitter {
10931093
let stats: google.spanner.v1.ResultSetStats;
10941094
let metadata: google.spanner.v1.ResultSetMetadata;
10951095

1096-
const traceConfig = {
1097-
sql: query,
1098-
opts: this._observabilityOptions,
1099-
dbName: this._dbName!,
1100-
};
1101-
startTrace('Snapshot.run', traceConfig, span => {
1102-
return this.runStream(query)
1103-
.on('error', (err, rows, stats, metadata) => {
1104-
setSpanError(span, err);
1105-
span.end();
1106-
callback!(err, rows, stats, metadata);
1107-
})
1108-
.on('response', response => {
1109-
if (response.metadata) {
1110-
metadata = response.metadata;
1111-
if (metadata.transaction && !this.id) {
1112-
this._update(metadata.transaction);
1113-
}
1096+
this.runStream(query)
1097+
.on('error', (err, rows, stats, metadata) => {
1098+
callback!(err, rows, stats, metadata);
1099+
})
1100+
.on('response', response => {
1101+
if (response.metadata) {
1102+
metadata = response.metadata;
1103+
if (metadata.transaction && !this.id) {
1104+
this._update(metadata.transaction);
11141105
}
1115-
})
1116-
.on('data', row => rows.push(row))
1117-
.on('stats', _stats => (stats = _stats))
1118-
.on('end', () => {
1119-
span.end();
1120-
callback!(null, rows, stats, metadata);
1121-
});
1122-
});
1106+
}
1107+
})
1108+
.on('data', row => rows.push(row))
1109+
.on('stats', _stats => (stats = _stats))
1110+
.on('end', () => {
1111+
callback!(null, rows, stats, metadata);
1112+
});
11231113
}
11241114

11251115
/**

0 commit comments

Comments
 (0)