Skip to content

Commit 55a59b4

Browse files
shivanee-pshivanee-p
committed
fix(gax): align GOOGLE_SDK_NODE_LOGGING parsing with namespace filters and OR with enableTelemetryLogging
1 parent a4b2a43 commit 55a59b4

6 files changed

Lines changed: 195 additions & 93 deletions

File tree

‎core/packages/gax/src/clientInterface.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,9 @@ export interface ClientOptions
5454
* Whether to enable telemetry logging for the client.
5555
*
5656
* Logging is only enabled when `GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING` is enabled
57-
* AND either the `GOOGLE_SDK_NODE_LOGGING` environment variable is enabled
58-
* or a user passes `enableTelemetryLogging: true` in their client options. The
59-
* `GOOGLE_SDK_NODE_LOGGING` environment variable overrides this option
60-
* whenever it is set.
57+
* AND either a user passes `enableTelemetryLogging: true` in their client options
58+
* or the `GOOGLE_SDK_NODE_LOGGING` environment variable matches the client service
59+
* namespace.
6160
*/
6261
enableTelemetryLogging?: boolean;
6362
}

‎core/packages/gax/src/gax.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,7 @@ export interface ClientConfig {
870870
* when passed in client options (or overridden by `GOOGLE_SDK_NODE_ENABLE_TRACING`).
871871
* @param {unknown} [_legacyTelemetryInfo] - Unused legacy parameter kept for backward compatibility.
872872
* @param {boolean} [enableTelemetryLogging] - Flag to enable telemetry logging
873-
* when passed in client options (or overridden by `GOOGLE_SDK_NODE_LOGGING`).
873+
* when passed in client options (or when `GOOGLE_SDK_NODE_LOGGING` matches the service namespace).
874874
* @return {Object} A mapping from method name to CallSettings, or null if the
875875
* service is not found in the config.
876876
*/
@@ -905,13 +905,13 @@ export function constructSettings(
905905
env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING?.trim().toLowerCase();
906906
const isExperimentalLoggingEnabled =
907907
experimentalLogging === 'true' || experimentalLogging === '1';
908-
const envLogging = env.GOOGLE_SDK_NODE_LOGGING?.trim();
909-
const isLoggingEnvSet = envLogging !== undefined && envLogging !== '';
910-
const isLoggingEnvEnabled = checkLoggingEnabled();
908+
const isLoggingEnvEnabled = checkLoggingEnabled(
909+
new CallSettings({apiName: serviceName}),
910+
);
911911

912912
const loggingRequested =
913913
isExperimentalLoggingEnabled &&
914-
(isLoggingEnvSet ? isLoggingEnvEnabled : Boolean(enableTelemetryLogging));
914+
(isLoggingEnvEnabled || Boolean(enableTelemetryLogging));
915915

916916
otherArgs = otherArgs || {};
917917
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -982,10 +982,10 @@ export function constructSettings(
982982
apiName,
983983
enableTelemetryTracing: isEnvSet ? isEnvEnabled : enableTelemetryTracing,
984984
enableTelemetryLogging: isExperimentalLoggingEnabled
985-
? isLoggingEnvSet
986-
? isLoggingEnvEnabled
985+
? isLoggingEnvEnabled
986+
? true
987987
: enableTelemetryLogging
988-
: enableTelemetryLogging !== undefined || isLoggingEnvSet
988+
: enableTelemetryLogging !== undefined
989989
? false
990990
: undefined,
991991
});

‎core/packages/gax/src/grpc.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ export interface GrpcClientOptions extends GoogleAuthOptions {
6767
* Whether to enable telemetry logging for the client.
6868
*
6969
* Logging is only enabled when `GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING` is enabled
70-
* AND either the `GOOGLE_SDK_NODE_LOGGING` environment variable is enabled
71-
* or a user passes `enableTelemetryLogging: true` in their client options.
70+
* AND either a user passes `enableTelemetryLogging: true` in their client options
71+
* or the `GOOGLE_SDK_NODE_LOGGING` environment variable matches the client service
72+
* namespace.
7273
*/
7374
enableTelemetryLogging?: boolean;
7475
}

‎core/packages/gax/src/observability/util.ts‎

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
import {CallSettings} from '../gax';
18+
import {extractServiceFromApiName} from './contextUtil';
1819

1920
/**
2021
* Checks if telemetry tracing is enabled.
@@ -48,14 +49,56 @@ export function checkTracingEnabled(settings?: CallSettings): boolean {
4849
);
4950
}
5051

52+
/**
53+
* Matches a `GOOGLE_SDK_NODE_LOGGING` filter string against the logger namespace
54+
* following `DebugLogBackendBase` / `NodeBackend` semantics in `google-logging-utils`
55+
* (comma-separated case-insensitive filters with `*` wildcards, plus `'all'`, `'true'`,
56+
* and `'1'` enabling all namespaces and `'false'` / `'0'` matching none).
57+
*/
58+
function matchesLoggingEnvFilter(envOptIn: string, namespace: string): boolean {
59+
const lower = envOptIn.toLowerCase();
60+
if (lower === 'false' || lower === '0') {
61+
return false;
62+
}
63+
if (lower === 'true' || lower === '1' || lower === 'all' || lower === '*') {
64+
return true;
65+
}
66+
67+
for (const rawToken of envOptIn.split(',')) {
68+
const token = rawToken.trim();
69+
if (!token) {
70+
continue;
71+
}
72+
const lowerToken = token.toLowerCase();
73+
if (lowerToken === 'false' || lowerToken === '0') {
74+
continue;
75+
}
76+
if (
77+
lowerToken === 'true' ||
78+
lowerToken === '1' ||
79+
lowerToken === 'all' ||
80+
lowerToken === '*'
81+
) {
82+
return true;
83+
}
84+
const pattern = token
85+
.replace(/[|\\{}()[\]^$+?.]/g, '\\$&')
86+
.replace(/\*+/g, '.*');
87+
if (new RegExp(`^${pattern}$`, 'i').test(namespace)) {
88+
return true;
89+
}
90+
}
91+
return false;
92+
}
93+
5194
/**
5295
* Checks if telemetry logging is enabled.
5396
*
5497
* Logging is only enabled when `GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING` is enabled
55-
* AND either the `GOOGLE_SDK_NODE_LOGGING` environment variable is enabled
56-
* or a user passes `enableTelemetryLogging: true` in their client options.
57-
* If `GOOGLE_SDK_NODE_LOGGING` is explicitly set, it takes precedence over
58-
* the client option.
98+
* AND either a user passes `enableTelemetryLogging: true` in their client options
99+
* or the `GOOGLE_SDK_NODE_LOGGING` environment variable matches the client service
100+
* namespace (supporting `'true'`, `'1'`, `'all'`, `'*'`, or comma-separated wildcard
101+
* filters matching `google-logging-utils`).
59102
*
60103
* @param settings
61104
* @returns true if telemetry logging is enabled, false otherwise
@@ -71,15 +114,26 @@ export function checkLoggingEnabled(settings?: CallSettings): boolean {
71114
return false;
72115
}
73116

74-
const envOptIn = env.GOOGLE_SDK_NODE_LOGGING?.trim();
117+
if (
118+
settings?.enableTelemetryLogging ||
119+
settings?.otherArgs?.enableTelemetryLogging
120+
) {
121+
return true;
122+
}
75123

76-
if (envOptIn !== undefined && envOptIn !== '') {
77-
const lower = envOptIn.toLowerCase();
78-
return lower === 'true' || lower === '1';
124+
const envOptIn = env.GOOGLE_SDK_NODE_LOGGING?.trim();
125+
if (envOptIn === undefined || envOptIn === '') {
126+
return false;
79127
}
80128

81-
return Boolean(
82-
settings?.enableTelemetryLogging ||
83-
settings?.otherArgs?.enableTelemetryLogging,
84-
);
129+
const envService = (
130+
env.GOOGLE_SDK_NODE_CLIENT_SERVICE || env.GCP_CLIENT_SERVICE
131+
)?.trim();
132+
const apiService =
133+
typeof settings?.apiName === 'string' && settings.apiName
134+
? extractServiceFromApiName(settings.apiName)
135+
: undefined;
136+
const namespace = envService || apiService || 'google-gax';
137+
138+
return matchesLoggingEnvFilter(envOptIn, namespace);
85139
}

‎core/packages/gax/test/unit/gax.ts‎

Lines changed: 59 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -317,42 +317,69 @@ describe('gax construct settings', () => {
317317
assert.strictEqual(settings.otherArgs.internalMethodName, undefined);
318318
});
319319

320-
it('ignores client option when GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING=true and GOOGLE_SDK_NODE_LOGGING=true', () => {
320+
it('enables logging when GOOGLE_SDK_NODE_LOGGING matches the service namespace or wildcard', () => {
321321
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'true';
322-
process.env.GOOGLE_SDK_NODE_LOGGING = 'true';
323-
const defaults = gax.constructSettings(
324-
SERVICE_NAME,
325-
A_CONFIG,
326-
{},
327-
RETRY_DICT,
328-
undefined,
329-
undefined,
330-
undefined,
331-
false,
332-
);
333-
const settings = defaults.bundlingMethod;
334-
assert.strictEqual(settings.enableTelemetryLogging, true);
335-
assert.strictEqual(settings.otherArgs.internalMethodName, 'BundlingMethod');
336-
assert.strictEqual(settings.otherArgs.enableTelemetryTracing, false);
337-
assert.strictEqual(settings.otherArgs.enableTelemetryLogging, true);
322+
for (const envValue of ['true', '*', 'all', 'auth,interface']) {
323+
process.env.GOOGLE_SDK_NODE_LOGGING = envValue;
324+
const defaults = gax.constructSettings(
325+
SERVICE_NAME,
326+
A_CONFIG,
327+
{},
328+
RETRY_DICT,
329+
undefined,
330+
undefined,
331+
undefined,
332+
false,
333+
);
334+
const settings = defaults.bundlingMethod;
335+
assert.strictEqual(settings.enableTelemetryLogging, true);
336+
assert.strictEqual(
337+
settings.otherArgs.internalMethodName,
338+
'BundlingMethod',
339+
);
340+
assert.strictEqual(settings.otherArgs.enableTelemetryTracing, false);
341+
assert.strictEqual(settings.otherArgs.enableTelemetryLogging, true);
342+
}
338343
});
339344

340-
it('disables logging when GOOGLE_SDK_NODE_LOGGING=false even with client option', () => {
345+
it('preserves enableTelemetryLogging=true when GOOGLE_SDK_NODE_LOGGING targets another namespace or is false', () => {
341346
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'true';
342-
process.env.GOOGLE_SDK_NODE_LOGGING = 'false';
343-
const defaults = gax.constructSettings(
344-
SERVICE_NAME,
345-
A_CONFIG,
346-
{},
347-
RETRY_DICT,
348-
undefined,
349-
undefined,
350-
undefined,
351-
true,
352-
);
353-
const settings = defaults.bundlingMethod;
354-
assert.strictEqual(settings.enableTelemetryLogging, false);
355-
assert.strictEqual(settings.otherArgs.internalMethodName, undefined);
347+
for (const envValue of ['auth,storage', 'false']) {
348+
process.env.GOOGLE_SDK_NODE_LOGGING = envValue;
349+
const enabledDefaults = gax.constructSettings(
350+
SERVICE_NAME,
351+
A_CONFIG,
352+
{},
353+
RETRY_DICT,
354+
undefined,
355+
undefined,
356+
undefined,
357+
true,
358+
);
359+
const enabledSettings = enabledDefaults.bundlingMethod;
360+
assert.strictEqual(enabledSettings.enableTelemetryLogging, true);
361+
assert.strictEqual(
362+
enabledSettings.otherArgs.internalMethodName,
363+
'BundlingMethod',
364+
);
365+
assert.strictEqual(
366+
enabledSettings.otherArgs.enableTelemetryLogging,
367+
true,
368+
);
369+
370+
const unsetDefaults = gax.constructSettings(
371+
SERVICE_NAME,
372+
A_CONFIG,
373+
{},
374+
RETRY_DICT,
375+
undefined,
376+
undefined,
377+
undefined,
378+
);
379+
const unsetSettings = unsetDefaults.bundlingMethod;
380+
assert.strictEqual(unsetSettings.enableTelemetryLogging, undefined);
381+
assert.strictEqual(unsetSettings.otherArgs.internalMethodName, undefined);
382+
}
356383
});
357384

358385
describe('CallSettings telemetry fields', () => {

‎core/packages/gax/test/unit/util.ts‎

Lines changed: 56 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -313,12 +313,18 @@ describe('util.ts', () => {
313313
afterEach(() => {
314314
delete process.env.GOOGLE_SDK_NODE_LOGGING;
315315
delete process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING;
316+
delete process.env.GOOGLE_SDK_NODE_CLIENT_SERVICE;
317+
delete process.env.GCP_CLIENT_SERVICE;
316318
});
317319

318320
const mockSettings = new CallSettings({
319321
enableTelemetryLogging: true,
320322
});
321323

324+
const storageSettings = new CallSettings({
325+
apiName: 'google.cloud.storage.v1.Storage',
326+
});
327+
322328
it('returns false when GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING is not set even if logging is enabled', () => {
323329
process.env.GOOGLE_SDK_NODE_LOGGING = 'true';
324330
assert.strictEqual(checkLoggingEnabled(mockSettings), false);
@@ -353,54 +359,69 @@ describe('util.ts', () => {
353359
assert.strictEqual(checkLoggingEnabled(undefined), false);
354360
});
355361

356-
it('returns true when GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING=true and GOOGLE_SDK_NODE_LOGGING=true', () => {
357-
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'true';
358-
process.env.GOOGLE_SDK_NODE_LOGGING = 'true';
359-
assert.strictEqual(checkLoggingEnabled(new CallSettings({})), true);
360-
assert.strictEqual(checkLoggingEnabled(), true);
361-
});
362-
363-
it('accepts GOOGLE_SDK_NODE_LOGGING and GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING case-insensitively', () => {
362+
it('returns true when GOOGLE_SDK_NODE_LOGGING is true, 1, all, or * (case-insensitively)', () => {
364363
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'TRUE';
365-
process.env.GOOGLE_SDK_NODE_LOGGING = 'TRUE';
366-
assert.strictEqual(checkLoggingEnabled(new CallSettings({})), true);
364+
for (const value of ['true', 'TRUE', '1', 'all', 'ALL', '*']) {
365+
process.env.GOOGLE_SDK_NODE_LOGGING = value;
366+
assert.strictEqual(checkLoggingEnabled(storageSettings), true);
367+
assert.strictEqual(checkLoggingEnabled(new CallSettings({})), true);
368+
assert.strictEqual(checkLoggingEnabled(), true);
369+
}
367370
});
368371

369-
it('returns false when GOOGLE_SDK_NODE_LOGGING=false or 0 overrides the client option', () => {
372+
it('matches comma-separated and wildcard namespace filters against client service or google-gax fallback', () => {
370373
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'true';
371-
process.env.GOOGLE_SDK_NODE_LOGGING = 'false';
372-
assert.strictEqual(checkLoggingEnabled(mockSettings), false);
373374

374-
process.env.GOOGLE_SDK_NODE_LOGGING = '0';
375-
assert.strictEqual(checkLoggingEnabled(mockSettings), false);
376-
});
375+
process.env.GOOGLE_SDK_NODE_LOGGING = 'storage';
376+
assert.strictEqual(checkLoggingEnabled(storageSettings), true);
377+
assert.strictEqual(checkLoggingEnabled(new CallSettings({})), false);
377378

378-
it('returns true when GOOGLE_SDK_NODE_LOGGING=true or 1 overrides enableTelemetryLogging=false on settings', () => {
379-
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'true';
380-
const disabledSettings = new CallSettings({
381-
enableTelemetryLogging: false,
382-
});
379+
process.env.GOOGLE_SDK_NODE_LOGGING = 'auth,STORAGE,bigquery';
380+
assert.strictEqual(checkLoggingEnabled(storageSettings), true);
383381

384-
process.env.GOOGLE_SDK_NODE_LOGGING = 'true';
385-
assert.strictEqual(checkLoggingEnabled(disabledSettings), true);
382+
process.env.GOOGLE_SDK_NODE_LOGGING = 'auth, stor*';
383+
assert.strictEqual(checkLoggingEnabled(storageSettings), true);
386384

387-
process.env.GOOGLE_SDK_NODE_LOGGING = '1';
388-
assert.strictEqual(checkLoggingEnabled(disabledSettings), true);
389-
});
385+
process.env.GOOGLE_SDK_NODE_LOGGING = 'auth,bigquery';
386+
assert.strictEqual(checkLoggingEnabled(storageSettings), false);
390387

391-
it('falls back to the client option when GOOGLE_SDK_NODE_LOGGING is empty or whitespace', () => {
392-
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'true';
393-
process.env.GOOGLE_SDK_NODE_LOGGING = '';
394-
assert.strictEqual(checkLoggingEnabled(mockSettings), true);
388+
process.env.GOOGLE_SDK_NODE_LOGGING = 'google-gax';
389+
assert.strictEqual(checkLoggingEnabled(new CallSettings({})), true);
390+
assert.strictEqual(checkLoggingEnabled(), true);
391+
assert.strictEqual(checkLoggingEnabled(storageSettings), false);
395392

396-
process.env.GOOGLE_SDK_NODE_LOGGING = ' ';
397-
assert.strictEqual(checkLoggingEnabled(mockSettings), true);
393+
process.env.GOOGLE_SDK_NODE_CLIENT_SERVICE = 'custom.svc';
394+
process.env.GOOGLE_SDK_NODE_LOGGING = 'custom.svc';
395+
assert.strictEqual(checkLoggingEnabled(storageSettings), true);
396+
process.env.GOOGLE_SDK_NODE_LOGGING = 'customXsvc';
397+
assert.strictEqual(checkLoggingEnabled(storageSettings), false);
398398
});
399399

400-
it('treats an unrecognized GOOGLE_SDK_NODE_LOGGING value as disabled', () => {
400+
it('combines GOOGLE_SDK_NODE_LOGGING and enableTelemetryLogging with logical OR', () => {
401401
process.env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING = 'true';
402-
process.env.GOOGLE_SDK_NODE_LOGGING = 'yes';
403-
assert.strictEqual(checkLoggingEnabled(mockSettings), false);
402+
const disabledStorageSettings = new CallSettings({
403+
apiName: 'google.cloud.storage.v1.Storage',
404+
enableTelemetryLogging: false,
405+
});
406+
407+
// Matching GOOGLE_SDK_NODE_LOGGING enables logging even when client option is false.
408+
for (const value of ['true', '1', 'all', '*', 'auth,storage']) {
409+
process.env.GOOGLE_SDK_NODE_LOGGING = value;
410+
assert.strictEqual(checkLoggingEnabled(disabledStorageSettings), true);
411+
}
412+
413+
// Non-matching or false/0 GOOGLE_SDK_NODE_LOGGING does not disable explicit enableTelemetryLogging: true.
414+
for (const value of ['auth', 'false', '0', '', ' ']) {
415+
process.env.GOOGLE_SDK_NODE_LOGGING = value;
416+
assert.strictEqual(checkLoggingEnabled(mockSettings), true);
417+
}
418+
419+
// false, 0, and non-matching filters stay disabled when client option is not enabled.
420+
for (const value of ['false', '0', 'yes']) {
421+
process.env.GOOGLE_SDK_NODE_LOGGING = value;
422+
assert.strictEqual(checkLoggingEnabled(disabledStorageSettings), false);
423+
assert.strictEqual(checkLoggingEnabled(storageSettings), false);
424+
}
404425
});
405426

406427
it('does not throw when process is undefined, as in a browser', () => {

0 commit comments

Comments
 (0)