-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(bigquery-jdbc): implement custom OTel SpanExporter credentials injector and authentication bypass #13263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: jdbc/e2e-test-otel
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,12 +29,17 @@ | |
| import io.opentelemetry.api.trace.Tracer; | ||
| import io.opentelemetry.context.Context; | ||
| import io.opentelemetry.context.Scope; | ||
| import io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter; | ||
| import io.opentelemetry.exporter.otlp.trace.OtlpGrpcSpanExporter; | ||
| import io.opentelemetry.sdk.OpenTelemetrySdk; | ||
| import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; | ||
| import java.io.IOException; | ||
| import java.net.URI; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.sql.SQLException; | ||
| import java.util.Collection; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.concurrent.Callable; | ||
|
|
@@ -221,6 +226,23 @@ private static Credentials resolveCredentialsFromString(String credsString) { | |
| BigQueryJdbcOpenTelemetry.class.getName()); | ||
| } | ||
|
|
||
| private static Map<String, String> getAuthHeaders(Credentials credentials) { | ||
| try { | ||
| Map<String, List<String>> metadata = | ||
| credentials.getRequestMetadata(URI.create(OTLP_ENDPOINT_VALUE)); | ||
| Map<String, String> headers = new HashMap<>(); | ||
| metadata.forEach( | ||
| (headerKey, headerValues) -> { | ||
| if (!headerValues.isEmpty()) { | ||
| headers.put(headerKey, headerValues.get(0)); | ||
| } | ||
| }); | ||
| return headers; | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("Failed to get auth headers", e); | ||
| } | ||
| } | ||
|
|
||
| public static TelemetryConfig getConnectionConfig(String connectionId) { | ||
| return connectionConfigs.get(connectionId); | ||
| } | ||
|
|
@@ -270,13 +292,9 @@ public static OpenTelemetry getOpenTelemetry( | |
| key, | ||
| k -> { | ||
| Map<String, String> props = new HashMap<>(); | ||
| Credentials credentials = null; | ||
| if (gcpTelemetryCredentials != null) { | ||
| byte[] credsBytes = gcpTelemetryCredentials.getBytes(StandardCharsets.UTF_8); | ||
| if (BigQueryJdbcOAuthUtility.isJson(credsBytes)) { | ||
| props.put(CREDENTIALS_JSON, gcpTelemetryCredentials); | ||
| } else { | ||
| props.put(CREDENTIALS_PATH, gcpTelemetryCredentials); | ||
| } | ||
| credentials = resolveCredentialsFromString(gcpTelemetryCredentials); | ||
| } | ||
|
|
||
| if (enableGcpTraceExporter) { | ||
|
|
@@ -306,8 +324,30 @@ public static OpenTelemetry getOpenTelemetry( | |
| props.put(OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT, DEFAULT_ATTRIBUTE_LENGTH_LIMIT); | ||
| } | ||
|
|
||
| AutoConfiguredOpenTelemetrySdk autoConfigured = | ||
| AutoConfiguredOpenTelemetrySdk.builder().addPropertiesSupplier(() -> props).build(); | ||
| 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(); | ||
| } | ||
|
Comment on lines
+327
to
+350
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
|
||
|
|
||
| OpenTelemetrySdk sdk = autoConfigured.getOpenTelemetrySdk(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -164,6 +164,60 @@ public void testExecute_withErrorCorrelation() throws Exception { | |
| "Traces must contain JDBC parent span 'BigQueryStatement.executeQuery'"); | ||
| } | ||
|
|
||
| @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. | ||
| } | ||
|
Comment on lines
+167
to
+219
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
|
||
|
|
||
| private String verifyAndFetchLogs(String connectionUuid) throws Exception { | ||
| try (Logging logging = | ||
| LoggingOptions.newBuilder().setProjectId(PROJECT_ID).build().getService()) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.