From 177cb35aa11e1c58166a028d1e86e6b1514d141b Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Tue, 6 Aug 2024 05:02:36 +0300 Subject: [PATCH] test: reduce cross-pollution of spans between tests with better isolation --- observability-test/grpc-instrumentation.ts | 58 ++++--- observability-test/observability.ts | 175 +++++++++++++-------- 2 files changed, 144 insertions(+), 89 deletions(-) diff --git a/observability-test/grpc-instrumentation.ts b/observability-test/grpc-instrumentation.ts index e61c6237c..dee9fae73 100644 --- a/observability-test/grpc-instrumentation.ts +++ b/observability-test/grpc-instrumentation.ts @@ -11,7 +11,6 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -// describe('Enabled gRPC instrumentation with sampling on', () => { const assert = require('assert'); @@ -30,47 +29,58 @@ describe('Enabled gRPC instrumentation with sampling on', () => { instrumentations: [new GrpcInstrumentation()], }); const { - startTrace, disableContextAndManager, setGlobalContextManager, + setTracerProvider, + startTrace, } = require('../src/instrument'); - const contextManager = new AsyncHooksContextManager(); - setGlobalContextManager(contextManager); - + const projectId = process.env.SPANNER_TEST_PROJECTID || 'test-project'; const exporter = new InMemorySpanExporter(); const sampler = new AlwaysOnSampler(); - const provider = new NodeTracerProvider({ - sampler: sampler, - exporter: exporter, - }); - provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); - provider.register(); - const projectId = process.env.SPANNER_TEST_PROJECTID || 'test-project'; - const {Spanner} = require('../src'); - const spanner = new Spanner({ - projectId: projectId, - }); - const instance = spanner.instance('test-instance'); - const database = instance.database('test-db'); + const {Database, Spanner} = require('../src'); + + let provider: typeof NodeTracerProvider; + let contextManager: typeof ContextManager; + let spanner: typeof Spanner; + let database: typeof Database; beforeEach(async () => { + provider = new NodeTracerProvider({ + sampler: sampler, + exporter: exporter, + }); + provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); + provider.register(); + setTracerProvider(provider); + + contextManager = new AsyncHooksContextManager(); + setGlobalContextManager(contextManager); + + spanner = new Spanner({ + projectId: projectId, + }); + + const instance = spanner.instance('test-instance'); + database = instance.database('test-db'); + + // Warm up session creation. + await database.run('SELECT 2'); // Mimick usual customer usage in which at setup time, the // Spanner and Database handles are created once then sit // and wait until they service HTTP or gRPC calls that // come in say 5+ seconds after the service is fully started. // This gives time for the batch session creation to be be completed. await new Promise((resolve, reject) => setTimeout(resolve, 100)); - - exporter.reset(); }); - after(async () => { - database.close(); + afterEach(async () => { spanner.close(); + exporter.forceFlush(); + exporter.reset(); await provider.shutdown(); - fini(); + disableContextAndManager(contextManager); }); it('Invoking database methods creates spans: gRPC enabled', () => { @@ -88,7 +98,7 @@ describe('Enabled gRPC instrumentation with sampling on', () => { span.end(); - await new Promise((resolve, reject) => setTimeout(resolve, 1900)); + await new Promise((resolve, reject) => setTimeout(resolve, 600)); await exporter.forceFlush(); // We need to ensure that spans were generated and exported. diff --git a/observability-test/observability.ts b/observability-test/observability.ts index 0a4681df8..663713407 100644 --- a/observability-test/observability.ts +++ b/observability-test/observability.ts @@ -64,9 +64,6 @@ describe('Testing spans produced with a sampler on', () => { spanner = new Spanner({ projectId: projectId, - observabilityConfig: { - tracerProvider: provider, - }, }); const instance = spanner.instance('test-instance'); @@ -74,11 +71,10 @@ describe('Testing spans produced with a sampler on', () => { // Warm up session creation. await database.run('SELECT 2'); - await new Promise((resolve, reject) => setTimeout(resolve, 100)); + // await new Promise((resolve, reject) => setTimeout(resolve, 100)); }); afterEach(async () => { - database.close(); spanner.close(); exporter.forceFlush(); exporter.reset(); @@ -86,68 +82,59 @@ describe('Testing spans produced with a sampler on', () => { disableContextAndManager(contextManager); }); - it('Invoking database methods creates spans: no gRPC instrumentation', async () => { + it('Invoking database methods creates spans: no gRPC instrumentation', () => { const query = {sql: 'SELECT 1'}; - const [rows] = await database.run(query); - assert.strictEqual(rows.length, 1); - - const spans = exporter.getFinishedSpans(); - // We need to ensure that spans were generated and exported - // correctly. - assert.ok(spans.length > 0, 'at least 1 span must have been created'); - - // Now sort the spans by duration, in reverse magnitude order. - spans.sort((spanA, spanB) => { - return spanA.duration > spanB.duration; - }); - - const got: string[] = []; - spans.forEach(span => { - got.push(span.name); - }); - - const want = [ - 'cloud.google.com/nodejs/spanner/Database.runStream', - 'cloud.google.com/nodejs/spanner/Database.run', - 'cloud.google.com/nodejs/spanner/Transaction.runStream', - ]; - - assert.deepEqual( - want, - got, - 'The spans order by duration has been violated:\n\tGot: ' + - got.toString() + - '\n\tWant: ' + - want.toString() - ); - - // Ensure that each span has the attribute - // SEMATTRS_DB_SYSTEM, set to 'spanner' - spans.forEach(span => { - assert.equal( - span.attributes[SEMATTRS_DB_SYSTEM], - 'spanner', - 'Missing DB_SYSTEM attribute' - ); - assert.equal( - span.attributes[SEMATTRS_DB_STATEMENT], - undefined, - 'unexpected DB_STATEMENT attribute without it being toggled' + database.run(query, async (err, rows, stats, metadata) => { + assert.strictEqual(rows.length, 1); + + // Give sometime for spans to be exported. + await new Promise((resolve, reject) => setTimeout(resolve, 500)); + + const spans = exporter.getFinishedSpans(); + // We need to ensure that spans were generated and exported + // correctly. + assert.ok(spans.length > 0, 'at least 1 span must have been created'); + + // Now sort the spans by duration, in reverse magnitude order. + spans.sort((spanA, spanB) => { + return spanA.duration > spanB.duration; + }); + + const got: string[] = []; + spans.forEach(span => { + got.push(span.name); + }); + + const want = [ + 'cloud.google.com/nodejs/spanner/Database.runStream', + 'cloud.google.com/nodejs/spanner/Database.run', + 'cloud.google.com/nodejs/spanner/Transaction.runStream', + ]; + + assert.deepEqual( + want, + got, + 'The spans order by duration has been violated:\n\tGot: ' + + got.toString() + + '\n\tWant: ' + + want.toString() ); - }); - }); - - it('Closing the client creates the closing span', () => { - spanner.close(); - const spans = exporter.getFinishedSpans(); - // We need to ensure that spans were generated and exported - // correctly. - assert.ok(spans.length == 1, 'exactly 1 span must have been created'); - assert.strictEqual( - spans[0].name, - 'cloud.google.com/nodejs/spanner/Spanner.close' - ); + // Ensure that each span has the attribute + // SEMATTRS_DB_SYSTEM, set to 'spanner' + spans.forEach(span => { + assert.equal( + span.attributes[SEMATTRS_DB_SYSTEM], + 'spanner', + 'Missing DB_SYSTEM attribute' + ); + assert.equal( + span.attributes[SEMATTRS_DB_STATEMENT], + undefined, + 'unexpected DB_STATEMENT attribute without it being toggled' + ); + }); + }); }); }); @@ -176,7 +163,6 @@ describe('Extended tracing', () => { }); afterEach(async () => { - database.close(); spanner.close(); exporter.forceFlush(); exporter.reset(); @@ -184,6 +170,8 @@ describe('Extended tracing', () => { disableContextAndManager(contextManager); }); + after(async () => {}); + const methodsTakingSQL = { 'cloud.google.com/nodejs/spanner/Database.run': true, 'cloud.google.com/nodejs/spanner/Database.runStream': true, @@ -368,6 +356,63 @@ describe('Capturing sessionPool annotations', () => { }); }); +describe('Close span', () => { + const exporter = new InMemorySpanExporter(); + const sampler = new AlwaysOnSampler(); + + const {Spanner} = require('../src'); + + const provider = new NodeTracerProvider({ + sampler: sampler, + exporter: exporter, + }); + provider.addSpanProcessor(new SimpleSpanProcessor(exporter)); + provider.register(); + setTracerProvider(provider); + + const contextManager = new AsyncHooksContextManager(); + setGlobalContextManager(contextManager); + + afterEach(async () => { + exporter.forceFlush(); + exporter.reset(); + await provider.shutdown(); + disableContextAndManager(contextManager); + }); + + it('Close span must be emitted', async () => { + const spanner = new Spanner({ + projectId: projectId, + }); + spanner.close(); + + const spans = exporter.getFinishedSpans(); + + spans.sort((spanA, spanB) => { + return spanA.startTime < spanB.startTime; + }); + const gotSpans: string[] = []; + spans.forEach(span => { + gotSpans.push(span.name); + }); + // We need to ensure that spans were generated and exported + // correctly. + assert.strictEqual( + spans.length, + 1, + 'exactly 1 span must have been emitted ' + gotSpans.toString() + ); + + const got = spans[0].name; + const want = 'cloud.google.com/nodejs/spanner/Spanner.close'; + assert.deepEqual( + want, + got, + 'Mismatched span:\n\tGot: ' + got + '\n\tWant: ' + want + ); + }); +}); + describe('Always off sampler used', () => { const exporter = new InMemorySpanExporter(); const sampler = new AlwaysOffSampler();