Skip to content

Commit 0dd9a53

Browse files
fix(spanner): disable metrics tracer when built-in metrics are disabled (#8170)
* fix(spanner): disable metrics tracer when built-in metrics are disabled Ensure MetricsTracerFactory starts disabled and is synchronized with the built-in metrics configuration. This prevents MetricsTracer instances from being created when metrics are disabled via client options, environment, or insecure credentials. * fix(spanner): honor disabled built-in metrics per client Track built-in metrics enablement on each Spanner instance and use it to guard metric interceptors and MetricsTracer creation. This prevents clients that explicitly disable built-in metrics from creating or retaining metrics tracers without mutating global MetricsTracerFactory state for other clients. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix(spanner): include insecure credentials in metrics guard Update the per-instance metrics flag to include insecure credentials so metric interceptors and tracers are not created when built-in metrics are disabled for that client. Keep the mock metrics tests using an explicit in-memory metrics override because they intentionally run against insecure test credentials. * fix(spanner): simplify disabled metrics guard Default MetricsTracerFactory.enabled to false so tracer creation stays disabled until metrics configuration explicitly enables the singleton factory. * Revert "fix(spanner): simplify disabled metrics guard" This reverts commit 8bc203e. * fix(spanner): sync metrics factory enablement before setup Move MetricsTracerFactory.enabled assignment outside the metrics setup branch so explicitly disabled or insecure clients update the singleton guard before any request path can create metrics tracers. --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent 9e91cc8 commit 0dd9a53

3 files changed

Lines changed: 21 additions & 11 deletions

File tree

handwritten/spanner/src/index.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ class Spanner extends GrpcService {
325325
sessionLabels: {[key: string]: string} | null;
326326
private _universeDomain: string;
327327
private _isInSecureCredentials: boolean;
328+
private _metricsEnabled = false;
328329
private static _isAFEServerTimingEnabled: boolean | undefined;
329330
readonly _nthClientId: number;
330331

@@ -1626,11 +1627,13 @@ class Spanner extends GrpcService {
16261627
configureMetrics_(disableBuiltInMetrics?: boolean) {
16271628
// Only enable metrics if not explicitly disabled and we are not using
16281629
// insecure credentials.
1629-
const metricsEnabled =
1630-
process.env.SPANNER_DISABLE_BUILTIN_METRICS !== 'true' &&
1631-
!disableBuiltInMetrics &&
1632-
!this._isInSecureCredentials;
1633-
if (metricsEnabled) {
1630+
const metricsExplicitlyDisabled =
1631+
process.env.SPANNER_DISABLE_BUILTIN_METRICS === 'true' ||
1632+
!!disableBuiltInMetrics;
1633+
this._metricsEnabled =
1634+
!metricsExplicitlyDisabled && !this._isInSecureCredentials;
1635+
MetricsTracerFactory.enabled = this._metricsEnabled;
1636+
if (this._metricsEnabled) {
16341637
try {
16351638
this.auth.getProjectId((err, projectId) => {
16361639
if (err || !projectId) {
@@ -1642,7 +1645,6 @@ class Spanner extends GrpcService {
16421645
return;
16431646
}
16441647

1645-
MetricsTracerFactory.enabled = metricsEnabled;
16461648
this.projectId_ = projectId;
16471649
const factory = MetricsTracerFactory.getInstance(projectId);
16481650
const periodicReader = new PeriodicExportingMetricReader({
@@ -1728,7 +1730,7 @@ class Spanner extends GrpcService {
17281730
// Attach the x-goog-spanner-request-id to the currently active span.
17291731
attributeXGoogSpannerRequestIdToActiveSpan(config);
17301732
const interceptors: any[] = [];
1731-
if (MetricsTracerFactory.enabled) {
1733+
if (this._metricsEnabled) {
17321734
interceptors.push(MetricInterceptor);
17331735
}
17341736
const requestFn = gaxClient[config.method].bind(
@@ -1811,7 +1813,11 @@ class Spanner extends GrpcService {
18111813
// eslint-disable-next-line @typescript-eslint/no-explicit-any
18121814
request(config: any, callback?: any): any {
18131815
let metricsTracer: MetricsTracer | null = null;
1814-
if (config.client === 'SpannerClient' && this.projectId_) {
1816+
if (
1817+
this._metricsEnabled &&
1818+
config.client === 'SpannerClient' &&
1819+
this.projectId_
1820+
) {
18151821
metricsTracer =
18161822
MetricsTracerFactory?.getInstance(this.projectId_)?.createMetricsTracer(
18171823
config.method,
@@ -1875,7 +1881,11 @@ class Spanner extends GrpcService {
18751881
// eslint-disable-next-line @typescript-eslint/no-explicit-any
18761882
requestStream(config): any {
18771883
let metricsTracer: MetricsTracer | null = null;
1878-
if (config.client === 'SpannerClient' && this.projectId_) {
1884+
if (
1885+
this._metricsEnabled &&
1886+
config.client === 'SpannerClient' &&
1887+
this.projectId_
1888+
) {
18791889
metricsTracer =
18801890
MetricsTracerFactory?.getInstance(this.projectId_)?.createMetricsTracer(
18811891
config.method,

handwritten/spanner/test/index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,8 +339,7 @@ describe('Spanner', () => {
339339

340340
it('should optionally accept disableBuiltInMetrics', () => {
341341
const spanner = new Spanner({disableBuiltInMetrics: true});
342-
assert.strictEqual(MetricsTracerFactory.enabled, false);
343-
MetricsTracerFactory.enabled = true; // Reset for other tests.
342+
assert.strictEqual(asAny(spanner)._metricsEnabled, false);
344343
});
345344

346345
it('should optionally accept directedReadOptions', () => {

handwritten/spanner/test/metrics/metrics.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ describe('Test metrics with mock server', () => {
164164
port,
165165
sslCreds: grpc.credentials.createInsecure(),
166166
});
167+
(spanner as any)._metricsEnabled = true;
167168
instance = spanner.instance('instance');
168169
}
169170

0 commit comments

Comments
 (0)