feat(bqjdbc): Per connection logs - Add BigQueryJdbcMdc#12748
feat(bqjdbc): Per connection logs - Add BigQueryJdbcMdc#12748
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces BigQueryJdbcMdc, a lightweight MDC implementation for the BigQuery JDBC driver using InheritableThreadLocal. Several critical architectural issues were identified: the dynamic creation of InheritableThreadLocal instances per connection is a significant anti-pattern that can lead to memory exhaustion; the static maps hold strong references to BigQueryConnection objects without a removal mechanism, causing memory leaks; and the global getConnectionId method suffers from O(N) complexity and non-deterministic behavior when multiple connections are present on the same thread.
...google-cloud-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryJdbcMdc.java
Show resolved
Hide resolved
...google-cloud-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryJdbcMdc.java
Show resolved
Hide resolved
...google-cloud-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryJdbcMdc.java
Show resolved
Hide resolved
| } | ||
|
|
||
| /** Retrieves the connection ID mapped to a specific BigQueryConnection instance. */ | ||
| public static String getConnectionId(BigQueryConnection connection) { |
There was a problem hiding this comment.
Why using MDC if BigQueryConnection object is available? We can add public connectionId property to the BigQueryConnection instead.
There was a problem hiding this comment.
But then we don't have a way to track different connection IDs in multiple threads.
The MDC here keeps track of different connection instances and it's corresponding connection ID and thread. This is cross verified when a LOG message comes in and redirected to the appropriate log file.
Implements BigQueryJdbcMdc using an InheritableThreadLocal design and corresponding unit tests for verfication.