Skip to content

fix: adding span attributes for request tag and transaction tag #2236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gcf-merge-on-green merged 12 commits into from
Apr 15, 2025
Merged

fix: adding span attributes for request tag and transaction tag #2236

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a0a6bff
fix: adding span attributes for request tag and transaction tag
surbhigarg92 Feb 18, 2025
8db5c17
request tag and refactor
surbhigarg92 Feb 27, 2025
189fee1
tests
surbhigarg92 Apr 10, 2025
75979f4
Merge branch 'main' into spans_tag_attributes
surbhigarg92 Apr 11, 2025
d99a5c3
request options
surbhigarg92 Apr 11, 2025
0145d91
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Apr 11, 2025
154ea00
Merge branch 'main' into spans_tag_attributes
surbhigarg92 Apr 11, 2025
3a483b2
extra tests
surbhigarg92 Apr 15, 2025
cecbc85
Merge branch 'main' into spans_tag_attributes
surbhigarg92 Apr 15, 2025
36bcba6
Merge branch 'main' into spans_tag_attributes
surbhigarg92 Apr 15, 2025
027a8be
Merge branch 'main' into spans_tag_attributes
surbhigarg92 Apr 15, 2025
52802ec
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions observability-test/observability.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
SEMATTRS_DB_NAME,
SEMATTRS_DB_SQL_TABLE,
SEMATTRS_DB_STATEMENT,
SEMATTRS_DB_SYSTEM,

Check warning on line 42 in observability-test/observability.ts

View workflow job for this annotation

GitHub Actions / lint 

'SEMATTRS_DB_SYSTEM' is assigned a value but never used
SEMATTRS_EXCEPTION_MESSAGE,
} = require('@opentelemetry/semantic-conventions');

Expand Down Expand Up @@ -160,12 +160,6 @@
'Missing gcp.client.repo attribute'
);

assert.equal(
span.attributes[SEMATTRS_DB_SYSTEM],
'spanner',
'Missing DB_SYSTEM attribute'
);

assert.equal(
span.attributes[SEMATTRS_DB_SQL_TABLE],
'table',
Expand Down
56 changes: 31 additions & 25 deletions observability-test/table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {SpanStatusCode} from '@opentelemetry/api';

// eslint-disable-next-line n/no-extraneous-require
const {SimpleSpanProcessor} = require('@opentelemetry/sdk-trace-base');
const {generateWithAllSpansHaveDBName} = require('./helper');

const fakePfy = extend({}, pfy, {
promisifyAll(klass, options) {
Expand Down Expand Up @@ -83,6 +82,12 @@ describe('Table', () => {

const NAME = 'table-name';

const ROW = {};

const mutateRowsOptions = {
requestOptions: {transactionTag: 'transaction-tag'},
};

before(() => {
Table = proxyquire('../src/table.js', {
'@google-cloud/promisify': fakePfy,
Expand All @@ -102,10 +107,6 @@ describe('Table', () => {
traceExporter.reset();
});

const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
DATABASE.formattedName_
);

function getExportedSpans(minCount: number) {
traceExporter.forceFlush();
const spans = traceExporter.getFinishedSpans();
Expand All @@ -131,6 +132,13 @@ describe('Table', () => {
return actualSpanNames;
}

function verifySpanAttributes(span) {
const attributes = span.attributes;
assert.strictEqual(attributes['transaction.tag'], 'transaction-tag');
assert.strictEqual(attributes['db.sql.table'], 'table-name');
assert.strictEqual(attributes['db.name'], 'formatted-db-name');
}

it('deleteRows', done => {
const KEYS = ['key'];
const stub = (
Expand All @@ -141,39 +149,39 @@ describe('Table', () => {
callback();
});

table.deleteRows(KEYS, err => {
table.deleteRows(KEYS, mutateRowsOptions, err => {
assert.ifError(err);
assert.strictEqual(stub.callCount, 1);
const actualSpanNames = spanNames(getExportedSpans(1));
const spans = getExportedSpans(1);
const actualSpanNames = spanNames(spans);
const expectedSpanNames = ['CloudSpanner.Table.deleteRows'];
assert.deepStrictEqual(
actualSpanNames,
expectedSpanNames,
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

verifySpanAttributes(spans[0]);
done();
});
});

const ROW = {};

it('insert', done => {
const stub = (
sandbox.stub(transaction, 'insert') as sinon.SinonStub
).withArgs(table.name, ROW);

table.insert(ROW, err => {
table.insert(ROW, mutateRowsOptions, err => {
assert.ifError(err);
assert.strictEqual(stub.callCount, 1);
const actualSpanNames = spanNames(getExportedSpans(1));
const spans = getExportedSpans(1);
const actualSpanNames = spanNames(spans);
const expectedSpanNames = ['CloudSpanner.Table.insert'];
assert.deepStrictEqual(
actualSpanNames,
expectedSpanNames,
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

verifySpanAttributes(spans[0]);
done();
});
});
Expand All @@ -184,7 +192,7 @@ describe('Table', () => {
.stub(DATABASE, 'runTransaction')
.callsFake((opts, callback) => callback(fakeError));

table.insert(ROW, err => {
table.insert(ROW, mutateRowsOptions, err => {
assert.strictEqual(err, fakeError);

const gotSpans = getExportedSpans(1);
Expand All @@ -207,7 +215,7 @@ describe('Table', () => {
expectedSpanNames,
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

verifySpanAttributes(gotSpans[0]);
done();
});
});
Expand All @@ -217,12 +225,11 @@ describe('Table', () => {
sandbox.stub(transaction, 'upsert') as sinon.SinonStub
).withArgs(table.name, ROW);

table.upsert(ROW, err => {
table.upsert(ROW, mutateRowsOptions, err => {
assert.ifError(err);
assert.strictEqual(stub.callCount, 1);

const gotSpans = getExportedSpans(1);
withAllSpansHaveDBName(gotSpans);

const actualSpanNames = spanNames(gotSpans);
const expectedSpanNames = ['CloudSpanner.Table.upsert'];
Expand All @@ -232,7 +239,7 @@ describe('Table', () => {
expectedSpanNames,
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

verifySpanAttributes(gotSpans[0]);
done();
});
});
Expand All @@ -243,11 +250,10 @@ describe('Table', () => {
.stub(DATABASE, 'runTransaction')
.callsFake((opts, callback) => callback(fakeError));

table.upsert(ROW, err => {
table.upsert(ROW, mutateRowsOptions, err => {
assert.strictEqual(err, fakeError);

const gotSpans = getExportedSpans(1);
withAllSpansHaveDBName(gotSpans);

const gotSpanStatus = gotSpans[0].status;
const wantSpanStatus = {
Expand All @@ -268,6 +274,7 @@ describe('Table', () => {
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

verifySpanAttributes[gotSpans[0]];
done();
});
});
Expand All @@ -277,12 +284,11 @@ describe('Table', () => {
sandbox.stub(transaction, 'replace') as sinon.SinonStub
).withArgs(table.name, ROW);

table.replace(ROW, err => {
table.replace(ROW, mutateRowsOptions, err => {
assert.ifError(err);
assert.strictEqual(stub.callCount, 1);

const gotSpans = getExportedSpans(1);
withAllSpansHaveDBName(gotSpans);

const actualSpanNames = spanNames(gotSpans);
const expectedSpanNames = ['CloudSpanner.Table.replace'];
Expand All @@ -292,6 +298,7 @@ describe('Table', () => {
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);

verifySpanAttributes(gotSpans[0]);
done();
});
});
Expand All @@ -302,7 +309,7 @@ describe('Table', () => {
.stub(DATABASE, 'runTransaction')
.callsFake((opts, callback) => callback(fakeError));

table.replace(ROW, err => {
table.replace(ROW, mutateRowsOptions, err => {
assert.strictEqual(err, fakeError);
const gotSpans = getExportedSpans(1);
const gotSpanStatus = gotSpans[0].status;
Expand All @@ -316,15 +323,14 @@ describe('Table', () => {
`mismatch in span status:\n\tGot: ${JSON.stringify(gotSpanStatus)}\n\tWant: ${JSON.stringify(wantSpanStatus)}`
);

withAllSpansHaveDBName(gotSpans);

const actualSpanNames = spanNames(gotSpans);
const expectedSpanNames = ['CloudSpanner.Table.replace'];
assert.deepStrictEqual(
actualSpanNames,
expectedSpanNames,
`span names mismatch:\n\tGot: ${actualSpanNames}\n\tWant: ${expectedSpanNames}`
);
verifySpanAttributes(gotSpans[0]);

done();
});
Expand Down
49 changes: 47 additions & 2 deletions observability-test/transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
SimpleSpanProcessor,
} = require('@opentelemetry/sdk-trace-base');
const {generateWithAllSpansHaveDBName} = require('./helper');
import {ExecuteSqlRequest, ReadRequest} from '../src/transaction';

describe('Transaction', () => {
const sandbox = sinon.createSandbox();
Expand Down Expand Up @@ -64,6 +65,7 @@
formattedName_: SESSION_NAME,
request: REQUEST,
requestStream: REQUEST_STREAM,
_observabilityOptions: {},
};

const PARTIAL_RESULT_STREAM = sandbox.stub();
Expand Down Expand Up @@ -100,11 +102,11 @@

const SNAPSHOT_OPTIONS = {a: 'b', c: 'd'};
sandbox.stub(Snapshot, 'encodeTimestampBounds').returns(SNAPSHOT_OPTIONS);
SESSION._observabilityOptions = {tracerProvider: tracerProvider};
snapshot = new Snapshot(SESSION);
snapshot._observabilityOptions = {tracerProvider: tracerProvider};

transaction = new Transaction(SESSION);
transaction._observabilityOptions = {tracerProvider: tracerProvider};
});

afterEach(async () => {
Expand Down Expand Up @@ -444,10 +446,12 @@
it('with error', done => {
REQUEST_STREAM.resetHistory();

const fakeQuery = Object.assign({}, QUERY, {
const fakeQuery: ExecuteSqlRequest = Object.assign({}, QUERY, {
params: {a: undefined},
requestOptions: {requestTag: 'request-tag'},
});

snapshot.requestOptions = {transactionTag: 'transaction-tag'};
const stream = snapshot.runStream(fakeQuery);
stream.on('error', error => {
assert.strictEqual(
Expand Down Expand Up @@ -487,11 +491,52 @@
'Unexpected span status message'
);

const attributes = exportResults.spans[0].attributes;
assert.strictEqual(attributes['transaction.tag'], 'transaction-tag');
assert.strictEqual(attributes['db.name'], 'formatted-database-name');
assert.strictEqual(attributes['request.tag'], 'request-tag');
done();
});
assert.ok(!REQUEST_STREAM.called, 'No request should be made');
});
});

describe('createReadStream', () => {
const TABLE = 'my-table-123';

beforeEach(() => {
PARTIAL_RESULT_STREAM.callsFake(makeRequest => makeRequest());
});

it('without error', done => {
const fakeStream = new EventEmitter();
REQUEST_STREAM.returns(fakeStream);
const request: ReadRequest = {
requestOptions: {requestTag: 'request-tag'},
};
snapshot.requestOptions = {transactionTag: 'transaction-tag'};
const stream = snapshot.createReadStream(TABLE, request);
stream.on('end', () => {
const exportResults = extractExportedSpans();
const actualSpanNames = exportResults.spanNames;

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

const attributes = exportResults.spans[0].attributes;
assert.strictEqual(attributes['transaction.tag'], 'transaction-tag');
assert.strictEqual(attributes['db.sql.table'], TABLE);
assert.strictEqual(attributes['db.name'], 'formatted-database-name');
assert.strictEqual(attributes['request.tag'], 'request-tag');
done();
});
fakeStream.emit('end');
});
});
});

describe('rollback', () => {
Expand All @@ -502,7 +547,7 @@
});

it('no error with unset `id`', done => {
const expectedError = new Error(

Check warning on line 550 in observability-test/transaction.ts

View workflow job for this annotation

GitHub Actions / lint 

'expectedError' is assigned a value but never used
'Transaction ID is unknown, nothing to rollback.'
);
delete transaction.id;
Expand Down
Loading
Loading