Skip to content

fix(bigquery-jdbc): correct usage-telemetry counts and disable telemetry in integration tests - #14940

Open
Neenu1995 wants to merge 1 commit into
mainfrom
turn-off-telemetry
Open

Neenu1995 wants to merge 1 commit into
mainfrom
turn-off-telemetry

Conversation

@Neenu1995

@Neenu1995 Neenu1995 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Telemetry count fixes

  • BigQueryDriver.connect: fetch DatabaseMetaData once for the startup log line instead of
    four times, so a new connection no longer records 4× DRIVER_FEATURE_METADATA_RETRIEVAL.
  • BigQueryDriver.connect: resolve AuthenticationType immediately after URL parsing so failed
    ConnectionAttempt records carry the real auth type instead of AUTHENTICATION_TYPE_UNSPECIFIED.
  • BigQueryDriver.connect: throw BigQueryJdbcException on IOException instead of returning
    null, as JDBC requires for an accepted URL that fails. Behaviour change for callers that
    relied on the null return.
  • BigQueryStatement: cancel() now records DRIVER_FEATURE_STATEMENT_CANCEL and delegates to a
    new private cancelInternal(); close() calls cancelInternal() directly, so closing a
    statement is no longer counted as a user-initiated cancel.
  • BigQueryPooledConnection: remove the DRIVER_FEATURE_METADATA_RETRIEVAL record from the
    pooled wrapper's getMetaData(); the delegate already records it (was double-counted).
  • BigQueryParameterHandler.setParameter(int, Object, Class): record
    DRIVER_FEATURE_PARAMETER_BINDING like the other three overloads. This is the path behind
    PreparedStatement.setInt/setLong/setString/..., so the most common bindings were uncounted.

Build

  • pom.xml: set system property BIGQUERY_JDBC_TELEMETRY_ENABLED=false in the failsafe
    configuration so integration tests do not emit usage telemetry. Test-only; does not affect the
    published artifact or end users.

Tests

  • Add count-level assertions via mockStatic(TelemetryManager) for each fix above. Existing
    telemetry tests only asserted TelemetryManager.isInitialized(), which is why none of these
    were caught.
  • BigQueryDriverTest: introduce VALID_TOKEN_URL and reuse it in the four tests that inlined
    the same URL; cover the IOException path by stubbing BigQueryJdbcRootLogger.setLevel.
  • BigQueryStatementTest: add assertFeatureRecorded(action, feature, expectedCount) helper.
  • BigQueryPooledConnectionTest: assert the wrapper delegates and records nothing itself.

@Neenu1995
Neenu1995 requested review from a team as code owners November 6, 2026 15:40

@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 refactors and refines telemetry recording and error handling in the BigQuery JDBC driver. Key changes include caching DatabaseMetaData in BigQueryDriver to prevent redundant fetches, throwing a BigQueryJdbcException instead of returning null on connection failures, and introducing an internal cancel method to avoid logging telemetry during housekeeping statement closures. Additionally, telemetry tracking has been added for parameter binding and corrected in pooled connections to prevent double-counting. Comprehensive unit tests have been added to verify these behaviors. There are no review comments to assess, so I have no additional feedback to provide.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants