From bf187e367f3ca8e6fbe10e30e0f827114a9ddc43 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 30 Sep 2024 03:31:09 -0700 Subject: [PATCH] Document ObservabilityOptions plus prefix _observabilityOptions --- observability-test/batch-transaction.ts | 2 +- observability-test/database.ts | 2 +- observability-test/spanner.ts | 32 ++++++++++++------------- observability-test/table.ts | 2 +- src/batch-transaction.ts | 6 ++--- src/database.ts | 28 +++++++++++----------- src/index.ts | 6 ++--- src/instance.ts | 8 +++---- src/instrument.ts | 9 +++++++ src/table.ts | 6 ++--- src/transaction.ts | 2 +- test/spanner.ts | 2 +- 12 files changed, 56 insertions(+), 49 deletions(-) diff --git a/observability-test/batch-transaction.ts b/observability-test/batch-transaction.ts index 50e25abc5..763fb7e36 100644 --- a/observability-test/batch-transaction.ts +++ b/observability-test/batch-transaction.ts @@ -139,7 +139,7 @@ describe('BatchTransaction', () => { batchTransaction = new BatchTransaction(SESSION as {} as Session); batchTransaction.session = SESSION as {} as Session; batchTransaction.id = ID; - batchTransaction.observabilityOptions_ = {tracerProvider: provider}; + batchTransaction._observabilityOptions = {tracerProvider: provider}; REQUEST.callsFake((_, callback) => callback(null, RESPONSE)); }); diff --git a/observability-test/database.ts b/observability-test/database.ts index 945b7afdc..cbcc73572 100644 --- a/observability-test/database.ts +++ b/observability-test/database.ts @@ -241,7 +241,7 @@ describe('Database', () => { database = new Database(INSTANCE, NAME, POOL_OPTIONS); database.parent = INSTANCE; database.databaseRole = 'parent_role'; - database.observabilityOptions_ = { + database._observabilityOptions = { tracerProvider: provider, enableExtendedTracing: false, }; diff --git a/observability-test/spanner.ts b/observability-test/spanner.ts index 67b386c3f..bf3d93538 100644 --- a/observability-test/spanner.ts +++ b/observability-test/spanner.ts @@ -127,7 +127,16 @@ describe('EndToEnd', () => { }); beforeEach(async () => { - const setupResult = await setup(); + traceExporter = new InMemorySpanExporter(); + const sampler = new AlwaysOnSampler(); + const provider = new NodeTracerProvider({ + sampler: sampler, + exporter: traceExporter, + }); + const setupResult = await setup({ + tracerProvider: provider, + enableExtendedTracing: false, + }); spanner = setupResult.spanner; server = setupResult.server; spannerMock = setupResult.spannerMock; @@ -143,21 +152,10 @@ describe('EndToEnd', () => { mock.StatementResult.updateCount(1) ); - traceExporter = new InMemorySpanExporter(); - const sampler = new AlwaysOnSampler(); - - const provider = new NodeTracerProvider({ - sampler: sampler, - exporter: traceExporter, - }); provider.addSpanProcessor(new SimpleSpanProcessor(traceExporter)); const instance = spanner.instance('instance'); database = instance.database('database'); - database.observabilityOptions_ = { - tracerProvider: provider, - enableExtendedTracing: false, - }; }); afterEach(() => { @@ -488,26 +486,26 @@ describe('ObservabilityOptions injection and propagation', async () => { it('Passed into Spanner, Instance and Database', done => { // Ensure that the same observability configuration is set on the Spanner client. - assert.deepStrictEqual(spanner.observabilityOptions_, observabilityOptions); + assert.deepStrictEqual(spanner._observabilityOptions, observabilityOptions); // Acquire a handle to the Instance through spanner.instance. const instanceByHandle = spanner.instance('instance'); assert.deepStrictEqual( - instanceByHandle.observabilityOptions_, + instanceByHandle._observabilityOptions, observabilityOptions ); // Create the Instance by means of a constructor directly. const instanceByConstructor = new Instance(spanner, 'myInstance'); assert.deepStrictEqual( - instanceByConstructor.observabilityOptions_, + instanceByConstructor._observabilityOptions, observabilityOptions ); // Acquire a handle to the Database through instance.database. const databaseByHandle = instanceByHandle.database('database'); assert.deepStrictEqual( - databaseByHandle.observabilityOptions_, + databaseByHandle._observabilityOptions, observabilityOptions ); @@ -517,7 +515,7 @@ describe('ObservabilityOptions injection and propagation', async () => { 'myDatabase' ); assert.deepStrictEqual( - databaseByConstructor.observabilityOptions_, + databaseByConstructor._observabilityOptions, observabilityOptions ); diff --git a/observability-test/table.ts b/observability-test/table.ts index 8bc7dcaa1..86f6145f9 100644 --- a/observability-test/table.ts +++ b/observability-test/table.ts @@ -92,7 +92,7 @@ describe('Table', () => { extend(Table, TableCached); table = new Table(DATABASE, NAME); transaction = new FakeTransaction(); - table.observabilityOptions_ = {tracerProvider: provider}; + table._observabilityOptions = {tracerProvider: provider}; }); afterEach(() => { diff --git a/src/batch-transaction.ts b/src/batch-transaction.ts index 067539812..403f7dd6e 100644 --- a/src/batch-transaction.ts +++ b/src/batch-transaction.ts @@ -139,7 +139,7 @@ class BatchTransaction extends Snapshot { const traceConfig: traceConfig = { sql: query, - opts: this.observabilityOptions_, + opts: this._observabilityOptions, }; return startTrace( 'BatchTransaction.createQueryPartitions', @@ -182,7 +182,7 @@ class BatchTransaction extends Snapshot { */ createPartitions_(config, callback) { const traceConfig: traceConfig = { - opts: this.observabilityOptions_, + opts: this._observabilityOptions, }; return startTrace( @@ -259,7 +259,7 @@ class BatchTransaction extends Snapshot { */ createReadPartitions(options, callback) { const traceConfig: traceConfig = { - opts: this.observabilityOptions_, + opts: this._observabilityOptions, }; return startTrace( diff --git a/src/database.ts b/src/database.ts index 28eb7a451..917bcabcf 100644 --- a/src/database.ts +++ b/src/database.ts @@ -344,7 +344,7 @@ class Database extends common.GrpcServiceObject { databaseDialect?: EnumKey< typeof databaseAdmin.spanner.admin.database.v1.DatabaseDialect > | null; - observabilityOptions_?: ObservabilityOptions; + _observabilityOptions?: ObservabilityOptions; constructor( instance: Instance, name: string, @@ -467,7 +467,7 @@ class Database extends common.GrpcServiceObject { Object.assign({}, queryOptions), Database.getEnvironmentQueryOptions() ); - this.observabilityOptions_ = instance.observabilityOptions_; + this._observabilityOptions = instance._observabilityOptions; } /** * @typedef {array} SetDatabaseMetadataResponse @@ -693,7 +693,7 @@ class Database extends common.GrpcServiceObject { const sessions = (resp!.session || []).map(metadata => { const session = this.session(metadata.name!); - session.observabilityOptions_ = this.observabilityOptions_; + session._observabilityOptions = this._observabilityOptions; session.metadata = metadata; return session; }); @@ -738,7 +738,7 @@ class Database extends common.GrpcServiceObject { const id = identifier.transaction; const transaction = new BatchTransaction(session, options); transaction.id = id; - transaction.observabilityOptions_ = this.observabilityOptions_; + transaction._observabilityOptions = this._observabilityOptions; transaction.readTimestamp = identifier.timestamp as PreciseDate; return transaction; } @@ -828,7 +828,7 @@ class Database extends common.GrpcServiceObject { ? (optionsOrCallback as TimestampBounds) : {}; - const q = {opts: this.observabilityOptions_}; + const q = {opts: this._observabilityOptions}; return startTrace('Database.createBatchTransaction', q, span => { this.pool_.getSession((err, session) => { if (err) { @@ -1086,7 +1086,7 @@ class Database extends common.GrpcServiceObject { /CREATE TABLE `*([^\s`(]+)/ )![1]; const table = this.table(tableName!); - table.observabilityOptions_ = this.observabilityOptions_; + table._observabilityOptions = this._observabilityOptions; callback!(null, table, operation!, resp!); }); } @@ -1875,7 +1875,7 @@ class Database extends common.GrpcServiceObject { delete (gaxOpts as GetSessionsOptions).pageToken; } - const q = {opts: this.observabilityOptions_}; + const q = {opts: this._observabilityOptions}; return startTrace('Database.getSessions', q, span => { this.request< google.spanner.v1.ISession, @@ -1897,7 +1897,7 @@ class Database extends common.GrpcServiceObject { sessionInstances = sessions.map(metadata => { const session = self.session(metadata.name!); session.metadata = metadata; - session.observabilityOptions_ = this.observabilityOptions_; + session._observabilityOptions = this._observabilityOptions; return session; }); } @@ -2058,7 +2058,7 @@ class Database extends common.GrpcServiceObject { ? (optionsOrCallback as TimestampBounds) : {}; - const q = {opts: this.observabilityOptions_}; + const q = {opts: this._observabilityOptions}; return startTrace('Database.getSnapshot', q, span => { this.pool_.getSession((err, session) => { if (err) { @@ -2159,7 +2159,7 @@ class Database extends common.GrpcServiceObject { ? (optionsOrCallback as GetTransactionOptions) : {}; - const q = {opts: this.observabilityOptions_}; + const q = {opts: this._observabilityOptions}; return startTrace('Database.getTransaction', q, span => { this.pool_.getSession((err, session, transaction) => { if (options.requestOptions) { @@ -2786,7 +2786,7 @@ class Database extends common.GrpcServiceObject { ? (optionsOrCallback as TimestampBounds) : {}; - const q = {sql: query, opts: this.observabilityOptions_}; + const q = {sql: query, opts: this._observabilityOptions}; return startTrace('Database.run', q, span => { this.runStream(query, options) .on('error', err => { @@ -3007,7 +3007,7 @@ class Database extends common.GrpcServiceObject { options?: TimestampBounds ): PartialResultStream { const proxyStream: Transform = through.obj(); - const q = {sql: query, opts: this.observabilityOptions_}; + const q = {sql: query, opts: this._observabilityOptions}; return startTrace('Database.runStream', q, span => { this.pool_.getSession((err, session) => { if (err) { @@ -3185,7 +3185,7 @@ class Database extends common.GrpcServiceObject { ? (optionsOrRunFn as RunTransactionOptions) : {}; - const q = {opts: this.observabilityOptions_}; + const q = {opts: this._observabilityOptions}; startTrace('Database.runTransaction', q, span => { this.pool_.getSession((err, session?, transaction?) => { if (err) { @@ -3578,7 +3578,7 @@ class Database extends common.GrpcServiceObject { ? (optionsOrCallback as CallOptions) : {}; - const q = {opts: this.observabilityOptions_}; + const q = {opts: this._observabilityOptions}; return startTrace('Database.writeAtLeastOnce', q, span => { this.pool_.getSession((err, session?, transaction?) => { if (err && isSessionNotFoundError(err as grpc.ServiceError)) { diff --git a/src/index.ts b/src/index.ts index 39aa43e2d..2299dd27a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -241,7 +241,7 @@ class Spanner extends GrpcService { resourceHeader_: {[k: string]: string}; routeToLeaderEnabled = true; directedReadOptions: google.spanner.v1.IDirectedReadOptions | null; - observabilityOptions_: ObservabilityOptions | undefined; + _observabilityOptions: ObservabilityOptions | undefined; /** * Placeholder used to auto populate a column with the commit timestamp. @@ -368,7 +368,7 @@ class Spanner extends GrpcService { [CLOUD_RESOURCE_HEADER]: this.projectFormattedName_, }; this.directedReadOptions = directedReadOptions; - this.observabilityOptions_ = options.observabilityOptions; + this._observabilityOptions = options.observabilityOptions; } /** @@ -589,7 +589,7 @@ class Spanner extends GrpcService { return; } const instance = this.instance(formattedName); - instance.observabilityOptions_ = this.observabilityOptions_; + instance._observabilityOptions = this._observabilityOptions; callback!(null, instance, operation, resp); } ); diff --git a/src/instance.ts b/src/instance.ts index 443f13bd3..b72f24622 100644 --- a/src/instance.ts +++ b/src/instance.ts @@ -165,7 +165,7 @@ class Instance extends common.GrpcServiceObject { databases_: Map; metadata?: IInstance; resourceHeader_: {[k: string]: string}; - observabilityOptions_?: ObservabilityOptions; + _observabilityOptions?: ObservabilityOptions; constructor(spanner: Spanner, name: string) { const formattedName_ = Instance.formatName_(spanner.projectId, name); const methods = { @@ -241,7 +241,7 @@ class Instance extends common.GrpcServiceObject { this.resourceHeader_ = { [CLOUD_RESOURCE_HEADER]: this.formattedName_, }; - this.observabilityOptions_ = spanner.observabilityOptions_; + this._observabilityOptions = spanner._observabilityOptions; } /** @@ -928,7 +928,7 @@ class Instance extends common.GrpcServiceObject { return; } const database = this.database(name, poolOptions || poolCtor); - database.observabilityOptions_ = this.observabilityOptions_; + database._observabilityOptions = this._observabilityOptions; callback(null, database, operation, resp); } ); @@ -978,7 +978,7 @@ class Instance extends common.GrpcServiceObject { const key = name.split('/').pop() + optionsKey; if (!this.databases_.has(key!)) { const db = new Database(this, name, poolOptions, queryOptions); - db.observabilityOptions_ = this.observabilityOptions_; + db._observabilityOptions = this._observabilityOptions; this.databases_.set(key!, db); } return this.databases_.get(key!)!; diff --git a/src/instrument.ts b/src/instrument.ts index fb725b69d..1ffc8cf89 100644 --- a/src/instrument.ts +++ b/src/instrument.ts @@ -45,6 +45,15 @@ interface SQLStatement { sql: string; } +/* + * ObservabilityOptions defines the configuration with which + * startTrace may be invoked if set. + * @param [tracerProvider] is the injected TracerProvider to use, + * otherwise the global TracerProvider shall be used. + * @param [enableExtendedTracing] when set signifies that spans started + * with an accompanying an SQL statement shall be annotated with that SQL. + * Alternatively, you could set environment variable `SPANNER_ENABLE_EXTENDED_TRACING=true`. + */ interface ObservabilityOptions { tracerProvider: TracerProvider; enableExtendedTracing?: boolean; diff --git a/src/table.ts b/src/table.ts index 1e5078b73..a435b7a40 100644 --- a/src/table.ts +++ b/src/table.ts @@ -99,7 +99,7 @@ const POSTGRESQL = 'POSTGRESQL'; class Table { database: Database; name: string; - observabilityOptions_?: ObservabilityOptions; + _observabilityOptions?: ObservabilityOptions; constructor(database: Database, name: string) { /** * The {@link Database} instance of this {@link Table} instance. @@ -113,7 +113,7 @@ class Table { * @type {string} */ this.name = name; - this.observabilityOptions_ = database.observabilityOptions_; + this._observabilityOptions = database._observabilityOptions; } /** * Create a table. @@ -1081,7 +1081,7 @@ class Table { callback: CommitCallback ): void { const traceConfig: traceConfig = { - opts: this.observabilityOptions_, + opts: this._observabilityOptions, }; startTrace('Table.' + method, traceConfig, span => { diff --git a/src/transaction.ts b/src/transaction.ts index 60366a58b..ca96864e1 100644 --- a/src/transaction.ts +++ b/src/transaction.ts @@ -286,7 +286,7 @@ export class Snapshot extends EventEmitter { queryOptions?: IQueryOptions; resourceHeader_: {[k: string]: string}; requestOptions?: Pick; - observabilityOptions_?: ObservabilityOptions; + _observabilityOptions?: ObservabilityOptions; /** * The transaction ID. diff --git a/test/spanner.ts b/test/spanner.ts index 2b49a7904..fc4e11b91 100644 --- a/test/spanner.ts +++ b/test/spanner.ts @@ -5015,7 +5015,7 @@ describe('Spanner with mock server', () => { const opts: typeof ObservabilityOptions = {tracerProvider: provider}; startTrace('aSpan', {opts: opts}, span => { const database = newTestDatabase(); - database.observabilityOptions_ = opts; + database._observabilityOptions = opts; async function runIt() { const query = {