Skip to content

Commit 57bed21

Browse files
committed
fix(gax): fix logging backend fallback, filter matching, url.domain resolution, and fail-open logCall
1 parent b3c1e77 commit 57bed21

15 files changed

Lines changed: 871 additions & 163 deletions

File tree

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ 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 a user passes `enableTelemetryLogging: true` in their client options
58-
* or the `GOOGLE_SDK_NODE_LOGGING` environment variable matches the client service
59-
* namespace.
57+
* AND either `enableTelemetryLogging: true` is passed here or, when unset, the
58+
* `GOOGLE_SDK_NODE_LOGGING` environment variable matches the client service
59+
* namespace. Explicitly setting `enableTelemetryLogging: false` disables logging
60+
* for this client even when `GOOGLE_SDK_NODE_LOGGING` matches.
6061
*/
6162
enableTelemetryLogging?: boolean;
6263
}

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {OutgoingHttpHeaders} from 'http';
1818
import objectHash from 'object-hash';
1919
import * as protobuf from 'protobufjs';
2020
import * as gax from './gax';
21+
import {setSettingsDefaultDomain} from './observability/contextUtil';
2122
import * as routingHeader from './routingHeader';
2223
import {Status} from './status';
2324
import {GoogleAuth, AuthClient} from 'google-auth-library';
@@ -305,7 +306,7 @@ export class GrpcClient {
305306
if (this._port !== undefined) {
306307
otherArgs.port = this._port;
307308
}
308-
return gax.constructSettings(
309+
const defaults = gax.constructSettings(
309310
serviceName,
310311
clientConfig,
311312
configOverrides,
@@ -315,6 +316,17 @@ export class GrpcClient {
315316
_legacyTelemetryInfo,
316317
enableTelemetryLogging ?? this._enableTelemetryLogging,
317318
);
319+
const defaultServicePath = (this.auth as {defaultServicePath?: unknown})
320+
?.defaultServicePath;
321+
if (
322+
typeof defaultServicePath === 'string' &&
323+
defaultServicePath.includes('.')
324+
) {
325+
for (const methodName of Object.keys(defaults)) {
326+
setSettingsDefaultDomain(defaults[methodName], defaultServicePath);
327+
}
328+
}
329+
return defaults;
318330
}
319331

320332
/**

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

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import {GoogleError} from './googleError';
2525
import {BundleOptions} from './bundlingCalls/bundleExecutor';
2626
import {toLowerCamelCase} from './util';
2727
import {checkTracingEnabled, checkLoggingEnabled} from './observability/util';
28+
import {
29+
getSettingsDefaultDomain,
30+
setSettingsDefaultDomain,
31+
} from './observability/contextUtil';
2832
import {Status} from './status';
2933
import {RequestType} from './apitypes';
3034

@@ -253,7 +257,12 @@ export class CallSettings {
253257
*/
254258
merge(options?: CallOptions | null) {
255259
if (!options) {
256-
return new CallSettings(this);
260+
const merged = new CallSettings(this);
261+
const defaultDomain = getSettingsDefaultDomain(this);
262+
if (defaultDomain) {
263+
setSettingsDefaultDomain(merged, defaultDomain);
264+
}
265+
return merged;
257266
}
258267
let timeout = this.timeout;
259268
let retry = this.retry;
@@ -364,7 +373,7 @@ export class CallSettings {
364373
signal = options.signal;
365374
}
366375

367-
return new CallSettings({
376+
const merged = new CallSettings({
368377
timeout,
369378
retry,
370379
bundleOptions: this.bundleOptions,
@@ -380,6 +389,17 @@ export class CallSettings {
380389
resumableUpload,
381390
signal,
382391
});
392+
const defaultDomain =
393+
(options instanceof CallSettings
394+
? getSettingsDefaultDomain(options)
395+
: undefined) ??
396+
(!('apiName' in options) || options.apiName === this.apiName
397+
? getSettingsDefaultDomain(this)
398+
: undefined);
399+
if (defaultDomain) {
400+
setSettingsDefaultDomain(merged, defaultDomain);
401+
}
402+
return merged;
383403
}
384404
}
385405

@@ -869,8 +889,8 @@ export interface ClientConfig {
869889
* @param {boolean} [enableTelemetryTracing] - Flag to enable telemetry tracing
870890
* when passed in client options (or overridden by `GOOGLE_SDK_NODE_ENABLE_TRACING`).
871891
* @param {unknown} [_legacyTelemetryInfo] - Unused legacy parameter kept for backward compatibility.
872-
* @param {boolean} [enableTelemetryLogging] - Flag to enable telemetry logging
873-
* when passed in client options (or when `GOOGLE_SDK_NODE_LOGGING` matches the service namespace).
892+
* @param {boolean} [enableTelemetryLogging] - Flag to enable or disable telemetry
893+
* logging for this client, taking precedence over `GOOGLE_SDK_NODE_LOGGING` when set.
874894
* @return {Object} A mapping from method name to CallSettings, or null if the
875895
* service is not found in the config.
876896
*/
@@ -905,15 +925,21 @@ export function constructSettings(
905925
env.GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING?.trim().toLowerCase();
906926
const isExperimentalLoggingEnabled =
907927
experimentalLogging === 'true' || experimentalLogging === '1';
908-
const isLoggingEnvEnabled = checkLoggingEnabled(
909-
new CallSettings({apiName: serviceName}),
910-
);
911-
928+
const isLoggingEnvEnabled =
929+
isExperimentalLoggingEnabled && enableTelemetryLogging === undefined
930+
? checkLoggingEnabled(new CallSettings({apiName: serviceName}))
931+
: false;
932+
const resolvedLogging =
933+
enableTelemetryLogging !== undefined
934+
? enableTelemetryLogging
935+
: isLoggingEnvEnabled
936+
? true
937+
: undefined;
912938
const loggingRequested =
913-
isExperimentalLoggingEnabled &&
914-
(isLoggingEnvEnabled || Boolean(enableTelemetryLogging));
939+
isExperimentalLoggingEnabled && Boolean(resolvedLogging);
915940

916941
otherArgs = otherArgs || {};
942+
917943
// eslint-disable-next-line @typescript-eslint/no-explicit-any
918944
const defaults: any = {};
919945

@@ -982,9 +1008,7 @@ export function constructSettings(
9821008
apiName,
9831009
enableTelemetryTracing: isEnvSet ? isEnvEnabled : enableTelemetryTracing,
9841010
enableTelemetryLogging: isExperimentalLoggingEnabled
985-
? isLoggingEnvEnabled
986-
? true
987-
: enableTelemetryLogging
1011+
? resolvedLogging
9881012
: enableTelemetryLogging !== undefined
9891013
? false
9901014
: undefined,

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import * as protobuf from 'protobufjs';
2727
import objectHash from 'object-hash';
2828

2929
import * as gax from './gax';
30+
import {setSettingsDefaultDomain} from './observability/contextUtil';
3031
import {ClientOptions} from '@grpc/grpc-js/build/src/client';
3132

3233
const googleProtoFilesDir = path.join(__dirname, '..', '..', 'build', 'protos');
@@ -67,9 +68,10 @@ export interface GrpcClientOptions extends GoogleAuthOptions {
6768
* Whether to enable telemetry logging for the client.
6869
*
6970
* Logging is only enabled when `GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING` is enabled
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.
71+
* AND either `enableTelemetryLogging: true` is passed here or, when unset, the
72+
* `GOOGLE_SDK_NODE_LOGGING` environment variable matches the client service
73+
* namespace. Explicitly setting `enableTelemetryLogging: false` disables logging
74+
* for this client even when `GOOGLE_SDK_NODE_LOGGING` matches.
7375
*/
7476
enableTelemetryLogging?: boolean;
7577
}
@@ -430,7 +432,7 @@ export class GrpcClient {
430432
if (this._port !== undefined) {
431433
otherArgs.port = this._port;
432434
}
433-
return gax.constructSettings(
435+
const defaults = gax.constructSettings(
434436
serviceName,
435437
clientConfig,
436438
configOverrides,
@@ -440,6 +442,17 @@ export class GrpcClient {
440442
_legacyTelemetryInfo,
441443
enableTelemetryLogging ?? this._enableTelemetryLogging,
442444
);
445+
const defaultServicePath = (this.auth as {defaultServicePath?: unknown})
446+
?.defaultServicePath;
447+
if (
448+
typeof defaultServicePath === 'string' &&
449+
defaultServicePath.includes('.')
450+
) {
451+
for (const methodName of Object.keys(defaults)) {
452+
setSettingsDefaultDomain(defaults[methodName], defaultServicePath);
453+
}
454+
}
455+
return defaults;
443456
}
444457

445458
/**

0 commit comments

Comments
 (0)