Skip to content

feat(gax): add observability logging flag - #9656

Open
shivanee-p wants to merge 4 commits into
mainfrom
shivaneep-o11y-logging-flag
Open

shivanee-p wants to merge 4 commits into
mainfrom
shivaneep-o11y-logging-flag

Conversation

@shivanee-p

@shivanee-p shivanee-p commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

This adds an enableTelemetryLogging client option to gax, alongside the existing enableTelemetryTracing. Logging is gated behind an experimental env var, and the resolved tracing and logging flags are passed to each call through CallSettings.otherArgs

Key Changes

  • Renamed checkTelemetryEnabled to checkTracingEnabled to make room for a logging check. Updated the callers in createApiCall.ts and the export in index.ts
  • Added checkLoggingEnabled(settings?). Logging is enabled only when:
    • GOOGLE_SDK_NODE_EXPERIMENTAL_LOGGING is true/1, and
    • either GOOGLE_SDK_NODE_ENABLE_LOGGING is true/1, or, if that env var is unset, enableTelemetryLogging: true is set on the settings or on otherArgs.
    • GOOGLE_SDK_NODE_ENABLE_LOGGING always overrides the client option when it is set.
  • Added enableTelemetryLogging?: boolean to ClientOptions, GrpcClientOptions, CallOptions and CallSettings
  • When tracing or logging is requested, otherArgs now includes internalMethodName, enableTelemetryTracing and enableTelemetryLogging

@shivanee-p

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread core/packages/gax/src/gax.ts
Comment thread core/packages/gax/test/unit/gax.ts

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread core/packages/gax/src/fallback.ts
Comment thread core/packages/gax/src/grpc.ts
@shivanee-p
shivanee-p force-pushed the shivaneep-o11y-logging-flag branch from 36a8e1d to 5ee484f Compare October 19, 2026 21:39
@shivanee-p shivanee-p changed the title chore(gax): add observability logging flag feat(gax): add observability logging flag Oct 19, 2026
@shivanee-p
shivanee-p marked this pull request as ready for review October 19, 2026 22:05
@shivanee-p
shivanee-p requested a review from a team as a code owner October 19, 2026 22:05
@shivanee-p

Copy link
Copy Markdown
Contributor Author

Note: the error from the breaking-change-check is a false positive and not related to the changes in this PR

@shivanee-p shivanee-p added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Oct 19, 2026
@github-actions
github-actions Bot requested a review from danieljbruce October 19, 2026 22:25

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant