Skip to content

feat(bigquery-jdbc): implement custom OTel SpanExporter credentials injector and authentication bypass#13263

Draft
keshavdandeva wants to merge 2 commits into
jdbc/e2e-test-otelfrom
jdbc/bypass-auth-lib-for-trace-export
Draft

feat(bigquery-jdbc): implement custom OTel SpanExporter credentials injector and authentication bypass#13263
keshavdandeva wants to merge 2 commits into
jdbc/e2e-test-otelfrom
jdbc/bypass-auth-lib-for-trace-export

Conversation

@keshavdandeva
Copy link
Copy Markdown
Contributor

b/516416076

This PR introduces a custom credentials resolver and authentication injector in the JDBC driver's OpenTelemetry engine (BigQueryJdbcOpenTelemetry.java).

The Problem

Previously, the Google Cloud OpenTelemetry exporter relied on the official opentelemetry-gcp-auth-extension to authenticate tracing exports. However, that extension only supports Application Default Credentials (ADC) out-of-the-box. If a client wanted to configure explicit Service Account credentials (via a raw JSON string or a specific credentials file path) using the driver's gcpTelemetryCredentials property, the exporter was unable to authenticate, preventing tracing egress in custom deployment sandboxes.

The Solution

We bypassed the official GCP auth extension's auto-configuration entirely for custom credentials. Instead, we implemented our own custom authentication injection pipeline:

  1. Credentials Resolution: Dynamically parses and instantiates Google Credentials from either a raw Service Account JSON string or a local file path using our core oauth utility:
  2. OTEL Exporter Customizer Interception: Utilizes OTel's addSpanExporterCustomizer block to intercept the instantiated span exporters at startup:
    • We inspect if the active exporter is an HTTP (OtlpHttpSpanExporter) or gRPC (OtlpGrpcSpanExporter) transport engine.
    • We programmatically extract the raw OAuth2 Access Token/Refresh Token headers by calling credentials.getRequestMetadata(...) against the OTLP endpoint (https://telemetry.googleapis.com:443).
    • We dynamically inject these authorization headers directly into the OTel exporter's builder header supplier:

This custom-header injection bypasses the gcp-auth-extension entirely, unlocking full OTel E2E tracing support under custom Service Accounts for all JDBC clients.

Copy link
Copy Markdown
Contributor

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

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 implements custom credential injection for OpenTelemetry OTLP exporters within the BigQuery JDBC driver, enabling authenticated trace exports via both HTTP and gRPC. The changes introduce a utility to extract authentication headers from credentials and update the OpenTelemetry SDK configuration to use a span exporter customizer. Review feedback points out that the newly added integration tests are currently empty placeholders and should be fully implemented. Additionally, it is recommended to handle exceptions more gracefully during header retrieval to avoid crashing background exporter threads and to refactor the SDK builder logic to reduce code duplication.

Comment on lines +167 to +219
@Test
public void testExecute_withExplicitCredentialsJson() throws Exception {
// Goal: Verify that passing a raw JSON string in gcpTelemetryCredentials works and invokes our
// customizer.
// How to test:
// If you have a test service account JSON, you can read it as a string and set it in
// props/DataSource:
// ds.setGcpTelemetryCredentials(saJsonString);
// Verify that traces are still successfully delivered to Cloud Trace.
}

@Test
public void testExecute_withExplicitCredentialsFilePath() throws Exception {
// Goal: Verify that passing a file path works.
// How to test:
// Save the test service account JSON to a temporary file.
// Set gcpTelemetryCredentials to the tempFilePath:
// ds.setGcpTelemetryCredentials(tempFilePath);
// Verify trace delivery.
}

@Test
public void testExecute_withMultiTenancySdkCaching() throws Exception {
// Goal: Verify that the driver correctly creates and caches separate SDK instances for
// different configurations.
// How to test:
// Create Connection A with gcpTelemetryProjectId = "project-a".
// Create Connection B with gcpTelemetryProjectId = "project-b".
// Even if project-b doesn't exist or fails to export, you can verify that the driver doesn't
// crash and that it attempts to create two separate pipelines.
// To be rigorous, we could add a package-private method in BigQueryJdbcOpenTelemetry to return
// the size of the sdkCache and assert that it is 2 after creating these connections.
}

@Test
public void testExecute_withExplicitCredentials_HTTP() throws Exception {
// Scenario A: Explicit Credentials + HTTP
// Goal: Verify that our customizer works for OtlpHttpSpanExporter.
// How to test:
// Set gcpTelemetryCredentials (JSON string or path).
// Set EnableHighThroughputAPI = 0 (to force HTTP).
// Verify that traces are delivered.
}

@Test
public void testExecute_withExplicitCredentials_gRPC() throws Exception {
// Scenario B: Explicit Credentials + gRPC
// Goal: Verify that our customizer works for OtlpGrpcSpanExporter.
// How to test:
// Set gcpTelemetryCredentials (JSON string or path).
// Set EnableHighThroughputAPI = 1 (to force gRPC).
// Verify that traces are delivered.
}
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.

high

The newly added test methods are empty and only contain comments describing the test goals. These tests should be implemented to verify the custom credentials injection and authentication bypass logic introduced in this PR. Submitting a feature without functional tests is discouraged as it increases the risk of regressions. If the implementation is not ready, consider removing these placeholder comments to avoid stale documentation.

References
  1. Remove comments that refer to stale implementations instead of updating them.

});
return headers;
} catch (IOException e) {
throw new RuntimeException("Failed to get auth headers", e);
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.

medium

Throwing a RuntimeException inside the header supplier may cause the background exporter thread to terminate unexpectedly if an error occurs during credential metadata retrieval (e.g., a network failure during token refresh). It is generally safer to log the error and return an empty map, allowing the exporter to handle the failure through standard OTLP response codes (like 401 Unauthorized) rather than crashing the thread.

Comment on lines +327 to +350
final Credentials finalCreds = credentials;
AutoConfiguredOpenTelemetrySdk autoConfigured;

if (finalCreds != null) {
autoConfigured =
AutoConfiguredOpenTelemetrySdk.builder()
.addPropertiesSupplier(() -> props)
.addSpanExporterCustomizer(
(spanExporter, configProperties) -> {
if (spanExporter instanceof OtlpHttpSpanExporter) {
return ((OtlpHttpSpanExporter) spanExporter)
.toBuilder().setHeaders(() -> getAuthHeaders(finalCreds)).build();
}
if (spanExporter instanceof OtlpGrpcSpanExporter) {
return ((OtlpGrpcSpanExporter) spanExporter)
.toBuilder().setHeaders(() -> getAuthHeaders(finalCreds)).build();
}
return spanExporter;
})
.build();
} else {
autoConfigured =
AutoConfiguredOpenTelemetrySdk.builder().addPropertiesSupplier(() -> props).build();
}
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.

medium

The AutoConfiguredOpenTelemetrySdk builder logic is duplicated between the if and else blocks. This can be refactored to use a single builder chain, which improves maintainability and reduces code duplication. When refactoring, ensure that any null checks or validation steps are not redundant with preceding logic.

          final Credentials finalCreds = credentials;
          AutoConfiguredOpenTelemetrySdk autoConfigured =
              AutoConfiguredOpenTelemetrySdk.builder()
                  .addPropertiesSupplier(() -> props)
                  .addSpanExporterCustomizer(
                      (spanExporter, configProperties) -> {
                        if (finalCreds != null) {
                          if (spanExporter instanceof OtlpHttpSpanExporter) {
                            return ((OtlpHttpSpanExporter) spanExporter)
                                .toBuilder().setHeaders(() -> getAuthHeaders(finalCreds)).build();
                          }
                          if (spanExporter instanceof OtlpGrpcSpanExporter) {
                            return ((OtlpGrpcSpanExporter) spanExporter)
                                .toBuilder().setHeaders(() -> getAuthHeaders(finalCreds)).build();
                          }
                        }
                        return spanExporter;
                      })
                  .build();
References
  1. When implementing property parsing or validation logic, ensure that null checks and validation steps are not redundant with checks already performed by upstream callers or preceding logic in the same method.

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.

1 participant