Skip to content

Commit 3f69dad

Browse files
surbhigarg92surbhigarg92
authored
fix: adding span attributes for request tag and transaction tag (#2236)
1 parent 3d5f080 commit 3f69dad

File tree

11 files changed

+690
-542
lines changed

11 files changed

+690
-542
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ Samples are in the [`samples/`](https://github.com/googleapis/nodejs-spanner/tre
111111
| Creates an incremental backup schedule | [source code](https://github.com/googleapis/nodejs-spanner/blob/main/samples/create-incremental-backup-schedule.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/create-incremental-backup-schedule.js,samples/README.md) |
112112
| Create-instance-without-default-backup-schedules | [source code](https://github.com/googleapis/nodejs-spanner/blob/main/samples/create-instance-without-default-backup-schedules.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/create-instance-without-default-backup-schedules.js,samples/README.md) |
113113
| CRUD | [source code](https://github.com/googleapis/nodejs-spanner/blob/main/samples/crud.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/crud.js,samples/README.md) |
114+
| Adds split points to a database. | [source code](https://github.com/googleapis/nodejs-spanner/blob/main/samples/database-add-split-points.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/database-add-split-points.js,samples/README.md) |
114115
| Creates a new database with a specific default leader | [source code](https://github.com/googleapis/nodejs-spanner/blob/main/samples/database-create-with-default-leader.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/database-create-with-default-leader.js,samples/README.md) |
115116
| Database-create-with-encryption-key | [source code](https://github.com/googleapis/nodejs-spanner/blob/main/samples/database-create-with-encryption-key.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/database-create-with-encryption-key.js,samples/README.md) |
116117
| Database-create-with-multiple-kms-keys | [source code](https://github.com/googleapis/nodejs-spanner/blob/main/samples/database-create-with-multiple-kms-keys.js) | [![Open in Cloud Shell][shell_img]](https://console.cloud.google.com/cloudshell/open?git_repo=https://github.com/googleapis/nodejs-spanner&page=editor&open_in_editor=samples/database-create-with-multiple-kms-keys.js,samples/README.md) |

‎observability-test/database.ts‎

Lines changed: 114 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -938,54 +938,60 @@ describe('Database', () => {
938938

939939
getSessionStub.callsFake(callback => callback(fakeError));
940940

941-
database.getTransaction(async err => {
942-
assert.strictEqual(err, fakeError);
941+
database.getTransaction(
942+
{requestOptions: {transactionTag: 'transaction-tag'}},
943+
async err => {
944+
assert.strictEqual(err, fakeError);
943945

944-
await provider.forceFlush();
945-
traceExporter.forceFlush();
946-
const spans = traceExporter.getFinishedSpans();
947-
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
948-
withAllSpansHaveDBName(spans);
946+
await provider.forceFlush();
947+
traceExporter.forceFlush();
948+
const spans = traceExporter.getFinishedSpans();
949+
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
950+
withAllSpansHaveDBName(spans);
949951

950-
const actualEventNames: string[] = [];
951-
const actualSpanNames: string[] = [];
952-
spans.forEach(span => {
953-
actualSpanNames.push(span.name);
954-
span.events.forEach(event => {
955-
actualEventNames.push(event.name);
952+
const actualEventNames: string[] = [];
953+
const actualSpanNames: string[] = [];
954+
spans.forEach(span => {
955+
actualSpanNames.push(span.name);
956+
span.events.forEach(event => {
957+
actualEventNames.push(event.name);
958+
});
956959
});
957-
});
958-
959-
const expectedSpanNames = ['CloudSpanner.Database.getTransaction'];
960-
assert.deepStrictEqual(
961-
actualSpanNames,
962-
expectedSpanNames,
963-
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
964-
);
965960

966-
// In the event of a sessionPool error, we should not have events.
967-
const expectedEventNames = [];
968-
assert.deepStrictEqual(
969-
actualEventNames,
970-
expectedEventNames,
971-
`event names mismatch:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
972-
);
961+
const expectedSpanNames = ['CloudSpanner.Database.getTransaction'];
962+
assert.deepStrictEqual(
963+
actualSpanNames,
964+
expectedSpanNames,
965+
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
966+
);
973967

974-
// Ensure that the span actually produced an error that was recorded.
975-
const firstSpan = spans[0];
976-
assert.strictEqual(
977-
SpanStatusCode.ERROR,
978-
firstSpan.status.code,
979-
'Expected an ERROR span status'
980-
);
981-
assert.strictEqual(
982-
'pool error',
983-
firstSpan.status.message,
984-
'Mismatched span status message'
985-
);
968+
// In the event of a sessionPool error, we should not have events.
969+
const expectedEventNames = [];
970+
assert.deepStrictEqual(
971+
actualEventNames,
972+
expectedEventNames,
973+
`event names mismatch:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
974+
);
986975

987-
done();
988-
});
976+
// Ensure that the span actually produced an error that was recorded.
977+
const firstSpan = spans[0];
978+
assert.strictEqual(
979+
SpanStatusCode.ERROR,
980+
firstSpan.status.code,
981+
'Expected an ERROR span status'
982+
);
983+
assert.strictEqual(
984+
'pool error',
985+
firstSpan.status.message,
986+
'Mismatched span status message'
987+
);
988+
assert.strictEqual(
989+
spans[0].attributes['transaction.tag'],
990+
'transaction-tag'
991+
);
992+
done();
993+
}
994+
);
989995
});
990996

991997
it('with no errors', done => {
@@ -1342,7 +1348,10 @@ describe('Database', () => {
13421348
'Using Session',
13431349
];
13441350
assert.deepStrictEqual(actualEventNames, expectedEventNames);
1345-
1351+
assert.strictEqual(
1352+
spans[0].attributes['transaction.tag'],
1353+
'batch-write-tag'
1354+
);
13461355
done();
13471356
});
13481357

@@ -1475,52 +1484,59 @@ describe('Database', () => {
14751484
callback(fakeErr)
14761485
);
14771486

1478-
database.runTransaction(err => {
1479-
assert.strictEqual(err, fakeErr);
1487+
database.runTransaction(
1488+
{requestOptions: {transactionTag: 'transaction-tag'}},
1489+
err => {
1490+
assert.strictEqual(err, fakeErr);
14801491

1481-
const spans = traceExporter.getFinishedSpans();
1482-
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
1483-
withAllSpansHaveDBName(spans);
1492+
const spans = traceExporter.getFinishedSpans();
1493+
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
1494+
withAllSpansHaveDBName(spans);
14841495

1485-
const actualSpanNames: string[] = [];
1486-
const actualEventNames: string[] = [];
1487-
spans.forEach(span => {
1488-
actualSpanNames.push(span.name);
1489-
span.events.forEach(event => {
1490-
actualEventNames.push(event.name);
1496+
const actualSpanNames: string[] = [];
1497+
const actualEventNames: string[] = [];
1498+
spans.forEach(span => {
1499+
actualSpanNames.push(span.name);
1500+
span.events.forEach(event => {
1501+
actualEventNames.push(event.name);
1502+
});
14911503
});
1492-
});
14931504

1494-
const expectedSpanNames = ['CloudSpanner.Database.runTransaction'];
1495-
assert.deepStrictEqual(
1496-
actualSpanNames,
1497-
expectedSpanNames,
1498-
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
1499-
);
1505+
const expectedSpanNames = ['CloudSpanner.Database.runTransaction'];
1506+
assert.deepStrictEqual(
1507+
actualSpanNames,
1508+
expectedSpanNames,
1509+
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
1510+
);
15001511

1501-
// Ensure that the span actually produced an error that was recorded.
1502-
const firstSpan = spans[0];
1503-
assert.strictEqual(
1504-
SpanStatusCode.ERROR,
1505-
firstSpan.status.code,
1506-
'Expected an ERROR span status'
1507-
);
1508-
assert.strictEqual(
1509-
'getting a session',
1510-
firstSpan.status.message,
1511-
'Mismatched span status message'
1512-
);
1512+
// Ensure that the span actually produced an error that was recorded.
1513+
const firstSpan = spans[0];
1514+
assert.strictEqual(
1515+
SpanStatusCode.ERROR,
1516+
firstSpan.status.code,
1517+
'Expected an ERROR span status'
1518+
);
1519+
assert.strictEqual(
1520+
'getting a session',
1521+
firstSpan.status.message,
1522+
'Mismatched span status message'
1523+
);
15131524

1514-
// We don't expect events.
1515-
const expectedEventNames = [];
1516-
assert.deepStrictEqual(
1517-
actualEventNames,
1518-
expectedEventNames,
1519-
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
1520-
);
1525+
// We don't expect events.
1526+
const expectedEventNames = [];
1527+
assert.deepStrictEqual(
1528+
actualEventNames,
1529+
expectedEventNames,
1530+
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
1531+
);
15211532

1522-
done();
1523-
});
1533+
assert.strictEqual(
1534+
spans[0].attributes['transaction.tag'],
1535+
'transaction-tag'
1536+
);
1537+
done();
1538+
}
1539+
);
15241540
});
15251541

15261542
it('with other errors when running the transaction', done => {
@@ -1601,11 +1617,14 @@ describe('Database', () => {
16011617
.stub(FakeAsyncTransactionRunner.prototype, 'run')
16021618
.resolves(fakeValue);
16031619

1604-
const value = await database.runTransactionAsync(async txn => {
1605-
const result = await txn.run('SELECT 1');
1606-
await txn.commit();
1607-
return result;
1608-
});
1620+
const value = await database.runTransactionAsync(
1621+
{requestOptions: {transactionTag: 'transaction-tag'}},
1622+
async txn => {
1623+
const result = await txn.run('SELECT 1');
1624+
await txn.commit();
1625+
return result;
1626+
}
1627+
);
16091628

16101629
assert.strictEqual(value, fakeValue);
16111630

@@ -1649,6 +1668,10 @@ describe('Database', () => {
16491668
expectedEventNames,
16501669
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
16511670
);
1671+
assert.strictEqual(
1672+
spans[0].attributes['transaction.tag'],
1673+
'transaction-tag'
1674+
);
16521675
});
16531676

16541677
it('with error', async () => {
@@ -1713,6 +1736,7 @@ describe('Database', () => {
17131736
sql: 'SELECT * FROM table',
17141737
a: 'b',
17151738
c: 'd',
1739+
requestOptions: {requestTag: 'request-tag'},
17161740
};
17171741
let fakeSessionFactory: FakeSessionFactory;
17181742
let fakeSession: FakeSession;
@@ -1805,6 +1829,7 @@ describe('Database', () => {
18051829
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
18061830
);
18071831

1832+
assert.strictEqual(spans[0].attributes['request.tag'], 'request-tag');
18081833
done();
18091834
});
18101835
});
@@ -1962,6 +1987,7 @@ describe('Database', () => {
19621987
key: 'k999',
19631988
thing: 'abc',
19641989
},
1990+
requestOptions: {requestTag: 'request-tag'},
19651991
};
19661992

19671993
let fakeSessionFactory: FakeSessionFactory;
@@ -2070,6 +2096,7 @@ describe('Database', () => {
20702096
`Unexpected events:\n\tGot: ${actualEventNames}\n\tWant: ${expectedEventNames}`
20712097
);
20722098

2099+
assert.strictEqual(spans[0].attributes['request.tag'], 'request-tag');
20732100
done();
20742101
});
20752102
});

‎observability-test/observability.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,12 +160,6 @@ describe('startTrace', () => {
160160
'Missing gcp.client.repo attribute'
161161
);
162162

163-
assert.equal(
164-
span.attributes[SEMATTRS_DB_SYSTEM],
165-
'spanner',
166-
'Missing DB_SYSTEM attribute'
167-
);
168-
169163
assert.equal(
170164
span.attributes[SEMATTRS_DB_SQL_TABLE],
171165
'table',

0 commit comments

Comments
 (0)