chore(gax): add observability logging flag

[shivanee-p]

No description provided.

[shivanee-p]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces telemetry logging capabilities to the Google API Extensions (gax) library, controlled by the GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING and GOOGLE_SDK_NODE_ENABLE_LOGGING environment variables or the enableTelemetryLogging client option. It also renames existing tracing-related telemetry functions to distinguish them from the new logging features. Feedback on the changes highlights a critical issue in gax.ts where the parameter-shifting logic in constructSettings was not updated to handle the new enableTelemetryLogging and _legacyTelemetryInfo parameters when status is omitted, which will cause incorrect parameter mapping and test failures.

[gemini-code-assist]

The shifting logic inside constructSettings (located in the function body) must be updated to support the new enableTelemetryLogging parameter. Currently, when status is omitted (e.g., in direct calls or unit tests where otherArgs is passed as the 5th argument), the shifting logic only shifts enableTelemetryTracing and otherArgs. It does not shift _legacyTelemetryInfo or enableTelemetryLogging. This causes enableTelemetryLogging to be mapped incorrectly to _legacyTelemetryInfo or lost entirely.

Please update the shifting logic inside the function body to also shift _legacyTelemetryInfo and enableTelemetryLogging:

  if (
    status &&
    !Object.prototype.hasOwnProperty.call(status, 'OK')
  ) {
    enableTelemetryLogging = _legacyTelemetryInfo as boolean;
    _legacyTelemetryInfo = enableTelemetryTracing;
    enableTelemetryTracing = otherArgs as boolean;
    otherArgs = status;
    status = undefined;
  }

[gemini-code-assist]

This test calls gax.constructSettings directly with 8 arguments, omitting the status parameter. Because the shifting logic inside constructSettings in gax.ts has not been updated to shift the new enableTelemetryLogging parameter, the 8th argument true is incorrectly mapped to _legacyTelemetryInfo (the 8th parameter), and enableTelemetryLogging (the 9th parameter) remains undefined. This test will fail unless the shifting logic in gax.ts is updated to shift all parameters correctly.

[gemini-code-assist]

Code Review

This pull request introduces telemetry logging capabilities to the GAX client, adding enableTelemetryLogging configuration options across client options, call settings, and environment variables. However, critical parameter mismatches were identified in both fallback.ts and grpc.ts when calling the imported constructSettings function. Because the apiName parameter was omitted, subsequent arguments like enableTelemetryTracing and enableTelemetryLogging are misaligned and mapped incorrectly.

[gemini-code-assist]

There is a parameter mismatch when calling the imported constructSettings function from gax.ts. In gax.ts, the 6th parameter is apiName?: string, and enableTelemetryTracing is the 7th parameter.

By passing enableTelemetryTracing ?? this._enableTelemetryTracing as the 6th argument, it is mapped to apiName in gax.ts. Consequently, enableTelemetryTracing in gax.ts receives _legacyTelemetryInfo (the 7th argument), and enableTelemetryLogging receives undefined because it is not passed as the 9th argument.

You should explicitly pass undefined for the apiName parameter to align the arguments correctly.

    return constructSettings(
      this._servicePath!,
      clientConfig,
      configCommons,
      Status,
      otherArgs,
      undefined,
      enableTelemetryTracing ?? this._enableTelemetryTracing,
      _legacyTelemetryInfo,
      enableTelemetryLogging ?? this._enableTelemetryLogging,
    );

[gemini-code-assist]

There is a parameter mismatch when calling the imported constructSettings function from gax.ts. In gax.ts, the 6th parameter is apiName?: string, and enableTelemetryTracing is the 7th parameter.

By passing enableTelemetryTracing ?? this._enableTelemetryTracing as the 6th argument, it is mapped to apiName in gax.ts. Consequently, enableTelemetryTracing in gax.ts receives _legacyTelemetryInfo (the 7th argument), and enableTelemetryLogging receives undefined because it is not passed as the 9th argument.

You should explicitly pass undefined for the apiName parameter to align the arguments correctly.

      this.grpc.status,
      otherArgs,
      undefined,
      enableTelemetryTracing ?? this._enableTelemetryTracing,
      _legacyTelemetryInfo,
      enableTelemetryLogging ?? this._enableTelemetryLogging,
    );