Skip to content
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

Jump to bottom

feat: (observability) propagate database name for every span generated to aid in quick debugging #2155

Merged
merged 1 commit into from
Oct 10, 2024
+253 −62
Merged

feat: (observability) propagate database name for every span generated to aid in quick debugging #2155

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 37 additions & 4 deletions observability-test/database.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
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 @@ -84,7 +85,7 @@

class FakeSession {
calledWith_: IArguments;
formattedName_: any;

Check warning on line 88 in observability-test/database.ts

View workflow job for this annotation

GitHub Actions / lint 

Unexpected any. Specify a different type
constructor() {
this.calledWith_ = arguments;
}
Expand Down Expand Up @@ -235,16 +236,20 @@
DatabaseCached = Object.assign({}, Database);
});

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

beforeEach(() => {
fakeCodec.encode = util.noop;
extend(Database, DatabaseCached);
database = new Database(INSTANCE, NAME, POOL_OPTIONS);
database.parent = INSTANCE;
database.databaseRole = 'parent_role';
database._observabilityOptions = {
INSTANCE._observabilityOptions = {
tracerProvider: provider,
enableExtendedTracing: false,
};
database = new Database(INSTANCE, NAME, POOL_OPTIONS);
database.parent = INSTANCE;
database.databaseRole = 'parent_role';
const gaxOpts = {};
const options: {
a: string;
Expand Down Expand Up @@ -285,6 +290,8 @@
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 @@
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 @@ -491,7 +499,7 @@

let beginSnapshotStub: sinon.SinonStub;
let getSessionStub: sinon.SinonStub;
let snapshotStub: sinon.SinonStub;

Check warning on line 502 in observability-test/database.ts

View workflow job for this annotation

GitHub Actions / lint 

'snapshotStub' is assigned a value but never used

beforeEach(() => {
fakePool = database.pool_;
Expand Down Expand Up @@ -523,6 +531,7 @@
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 @@ -604,6 +613,7 @@

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 @@ -706,6 +716,8 @@

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 @@ -771,6 +783,7 @@
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 @@ -836,6 +849,8 @@

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 @@ -911,6 +926,8 @@

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 @@ -958,6 +975,8 @@
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 @@ -1037,6 +1056,7 @@

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 @@ -1091,6 +1111,7 @@

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 @@ -1137,7 +1158,7 @@

it('with error on null mutation should catch thrown error', done => {
try {
database.writeAtLeastOnce(null, (err, res) => {});

Check warning on line 1161 in observability-test/database.ts

View workflow job for this annotation

GitHub Actions / lint 

'err' is defined but never used
} catch (err) {
// Performing a substring search on the error because
// depending on the version of Node.js, the error might be either of:
Expand All @@ -1148,6 +1169,8 @@

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 @@ -1222,6 +1245,8 @@

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 @@ -1273,6 +1298,8 @@

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 @@ -1376,6 +1403,8 @@

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 @@ -1427,6 +1456,8 @@

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 @@ -1491,6 +1522,8 @@

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
View file
Open in desktop
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`
);
});
};
}
Loading
Loading