feat(bigquery-jdbc): add globalOtel support#13282
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new configuration option, useGlobalOpenTelemetry, allowing the BigQuery JDBC driver to utilize the global OpenTelemetry instance. The changes integrate this option across the connection, data source, and utility classes, and add comprehensive unit tests to verify the precedence hierarchy of OpenTelemetry configurations. The review feedback highlights an unused variable hasCustomOtel left in BigQueryConnection and suggests stubbing getTracer on the mocked OpenTelemetry instances in the new tests to prevent potential NullPointerExceptions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for using the global OpenTelemetry instance in the BigQuery JDBC driver. It adds a new configuration property useGlobalOpenTelemetry to the connection properties, data source, and connection logic, allowing the driver to fall back to GlobalOpenTelemetry.get() when enabled. Additionally, comprehensive unit tests have been added to verify the precedence hierarchy of OpenTelemetry configurations. One review comment suggests wrapping the BigQueryConnection in a try-with-resources block within the new test to ensure exception-safe resource cleanup and prevent potential resource leaks.
b/517124136
This PR introduces support for opting into the global OpenTelemetry instance via connection properties in the BigQuery JDBC driver. It also establishes a clear hierarchy of precedence for various OpenTelemetry configuration flags to ensure predictable behavior
Key Changes
1. Global OpenTelemetry Support
useGlobalOpenTelemetry(default:false).true, the driver will explicitly use the globally registered OpenTelemetry instance (GlobalOpenTelemetry.get()) for both tracing and logging.2. Precedence Rules for Telemetry Flags
Implemented the following hierarchy of precedence in
BigQueryConnection:customOpenTelemetry(Provided viaDataSource): Highest precedence. Overrides everything else.useGlobalOpenTelemetry: Second precedence. Overrides driver-managed GCP fallback flags.enableGcpTraceExporter/enableGcpLogExporter: Lowest precedence. Used only if above are false/null.3. Code Modifications
BigQueryJdbcUrlUtility.java: Added property definition and added it to valid properties.DataSource.java: Added field, getter/setter, and URL parsing support for the new property.BigQueryConnection.java: Updated constructor to read the new property and implemented the precedence logic for both tracer initialization and logging client registration.BigQueryJdbcOpenTelemetry.java: UpdatedgetOpenTelemetry()to handle theuseGlobalOpenTelemetryflag.4. Testing
BigQueryJdbcOpenTelemetryTest.java: Added unit test to verify thatuseGlobalOpenTelemetryreturns the global instance.BigQueryConnectionTest.java: Added a comprehensive parameterized test (testOpenTelemetryPrecedenceHierarchy) covering all 8 combinations of the 4 flags to verify the correct behavior ofOpenTelemetryresolution and logging client creation.