Skip to content

Commit

Permalink
feat: (observability) propagate database name for every span generate…
Browse files Browse the repository at this point in the history
…d to aid in quick debugging

With this change customers shall always be able to identify which
database is being connected to.

Updates #2079
  • Loading branch information
odeke-em committed Oct 9, 2024
1 parent a464bdb commit 8f5ea2b
Show file tree
Hide file tree
Showing 9 changed files with 218 additions and 22 deletions.
33 changes: 33 additions & 0 deletions observability-test/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {Instance, Spanner} from '../src';
import * as pfy from '@google-cloud/promisify';
import {grpc} from 'google-gax';
import {MockError} from '../test/mockserver/mockspanner';
const {generateWithAllSpansHaveDBName} = require('./helper');

const fakePfy = extend({}, pfy, {
promisifyAll(klass, options) {
Expand Down Expand Up @@ -235,6 +236,10 @@ describe('Database', () => {
DatabaseCached = Object.assign({}, Database);
});

const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
INSTANCE.formattedName_ + '/databases/' + NAME
);

beforeEach(() => {
fakeCodec.encode = util.noop;
extend(Database, DatabaseCached);
Expand Down Expand Up @@ -285,6 +290,8 @@ describe('Database', () => {
const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');

withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -333,6 +340,7 @@ describe('Database', () => {
traceExporter.forceFlush();
const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -414,6 +422,7 @@ describe('Database', () => {
traceExporter.forceFlush();
const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -495,6 +504,7 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 2, 'Exactly 2 spans expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -597,6 +607,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -662,6 +674,7 @@ describe('Database', () => {
assert.strictEqual(resp, RESPONSE);
const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -727,6 +740,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -802,6 +817,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -849,6 +866,8 @@ describe('Database', () => {
assert.strictEqual(transaction, fakeTransaction);

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

assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -928,6 +947,7 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -982,6 +1002,7 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -1040,6 +1061,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -1114,6 +1137,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -1165,6 +1190,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -1268,6 +1295,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -1319,6 +1348,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
spans.forEach(span => {
Expand Down Expand Up @@ -1383,6 +1414,8 @@ describe('Database', () => {

const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 2, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spans.forEach(span => {
Expand Down
15 changes: 15 additions & 0 deletions observability-test/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/

import {ContextManager, context} from '@opentelemetry/api';
import * as assert from 'assert';
const {ReadableSpan} = require('@opentelemetry/sdk-trace-base');
import {SEMATTRS_DB_NAME} from '@opentelemetry/semantic-conventions';

/**
* This utility exists as a test helper because mocha has builtin "context"
Expand All @@ -32,3 +35,15 @@ export function disableContextAndManager(manager: ContextManager) {
manager.disable();
context.disable();
}

export function generateWithAllSpansHaveDBName(dbName: String): Function {
return function (spans: (typeof ReadableSpan)[]) {
spans.forEach(span => {
assert.deepStrictEqual(
span.attributes[SEMATTRS_DB_NAME],
dbName,
`Span ${span.name} has mismatched DB_NAME`
);
});
};
}
48 changes: 47 additions & 1 deletion observability-test/spanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ const {
} = require('@opentelemetry/sdk-trace-node');
// eslint-disable-next-line n/no-extraneous-require
const {SimpleSpanProcessor} = require('@opentelemetry/sdk-trace-base');
const {disableContextAndManager, setGlobalContextManager} = require('./helper');
const {
disableContextAndManager,
generateWithAllSpansHaveDBName,
setGlobalContextManager,
} = require('./helper');
const {
AsyncHooksContextManager,
} = require('@opentelemetry/context-async-hooks');
Expand Down Expand Up @@ -169,9 +173,13 @@ describe('EndToEnd', () => {
it('getSessions', async () => {
const [rows] = await database.getSessions();

const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);
traceExporter.forceFlush();
const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand All @@ -198,6 +206,10 @@ describe('EndToEnd', () => {
});

it('getSnapshot', done => {
const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);

database.getSnapshot((err, transaction) => {
assert.ifError(err);

Expand All @@ -206,6 +218,7 @@ describe('EndToEnd', () => {

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

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -241,13 +254,17 @@ describe('EndToEnd', () => {
});

it('getTransaction', done => {
const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);
database.getTransaction((err, transaction) => {
assert.ifError(err);
assert.ok(transaction);

traceExporter.forceFlush();
const spans = traceExporter.getFinishedSpans();
assert.strictEqual(spans.length, 1, 'Exactly 1 span expected');
withAllSpansHaveDBName(spans);

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -277,13 +294,17 @@ describe('EndToEnd', () => {
});

it('runStream', done => {
const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);
database
.runStream('SELECT 1')
.on('data', row => {})
.on('error', assert.ifError)
.on('end', () => {
traceExporter.forceFlush();
const spans = traceExporter.getFinishedSpans();
withAllSpansHaveDBName(spans);

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -316,10 +337,14 @@ describe('EndToEnd', () => {
});

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

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

// Sort the spans by duration.
spans.sort((spanA, spanB) => {
Expand Down Expand Up @@ -375,13 +400,17 @@ describe('EndToEnd', () => {
});

it('runTransaction', done => {
const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);
database.runTransaction((err, transaction) => {
assert.ifError(err);
transaction!.run('SELECT 1', (err, rows) => {
assert.ifError(err);

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

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -416,13 +445,17 @@ describe('EndToEnd', () => {
});

it('writeAtLeastOnce', done => {
const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);
const blankMutations = new MutationSet();
database.writeAtLeastOnce(blankMutations, (err, response) => {
assert.ifError(err);
assert.ok(response);

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

const actualEventNames: string[] = [];
const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -552,6 +585,10 @@ describe('ObservabilityOptions injection and propagation', async () => {
describe('Transaction', () => {
const traceExporter = injectedTraceExporter;

const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);

it('run', done => {
database.getTransaction((err, tx) => {
assert.ifError(err);
Expand All @@ -560,6 +597,7 @@ describe('ObservabilityOptions injection and propagation', async () => {
traceExporter.forceFlush();

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

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -616,6 +654,7 @@ describe('ObservabilityOptions injection and propagation', async () => {
traceExporter.forceFlush();

const spans = traceExporter.getFinishedSpans();
withAllSpansHaveDBName(spans);
assert.strictEqual(spans.length, 4);

const actualSpanNames: string[] = [];
Expand Down Expand Up @@ -669,6 +708,7 @@ describe('ObservabilityOptions injection and propagation', async () => {
traceExporter.forceFlush();

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

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -716,6 +756,7 @@ describe('ObservabilityOptions injection and propagation', async () => {
traceExporter.forceFlush();

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

const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
Expand Down Expand Up @@ -762,6 +803,10 @@ describe('ObservabilityOptions injection and propagation', async () => {
const instance = spanner.instance('instance');
const database = instance.database('database');

const withAllSpansHaveDBName = generateWithAllSpansHaveDBName(
database.formattedName_
);

database.run('SELECT 1', (err, rows) => {
assert.ifError(err);

Expand All @@ -784,6 +829,7 @@ describe('ObservabilityOptions injection and propagation', async () => {
spansFromInjected.sort((spanA, spanB) => {
spanA.startTime < spanB.startTime;
});
withAllSpansHaveDBName(spansFromInjected);
const actualSpanNames: string[] = [];
const actualEventNames: string[] = [];
spansFromInjected.forEach(span => {
Expand Down
Loading

0 comments on commit 8f5ea2b

Please sign in to comment.