[OpenTelemetry] Add gcp otel auth extension#38773
Conversation
|
R: @kennknowles |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new extension module for Apache Beam that enables GCP authentication for OpenTelemetry OTLP exporters. By leveraging Google Application Default Credentials (ADC), this extension automatically configures exporters to include the necessary authorization headers when communicating with GCP telemetry endpoints, facilitating seamless integration with Google Cloud monitoring and tracing services. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new extension module, opentelemetry-gcp-auth-extension, to provide GCP authentication support for OpenTelemetry OTLP exporters in Apache Beam. The review feedback highlights several critical improvements: lazily retrieving Google Application Default Credentials (ADC) to prevent application startup crashes when ADC is not configured and GCP is not targeted; avoiding resource leaks by ensuring the original exporters are shut down when new ones are built; preventing potential NullPointerExceptions during header flattening; and refining endpoint checks to avoid noisy warnings when non-GCP endpoints are used.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| public void customize(@Nonnull AutoConfigurationCustomizer autoConfiguration) { | ||
| GoogleCredentials credentials; | ||
| try { | ||
| credentials = GoogleCredentials.getApplicationDefault(); | ||
| } catch (IOException e) { | ||
| throw new GoogleAuthException(Reason.FAILED_ADC_RETRIEVAL, e); | ||
| } | ||
| autoConfiguration | ||
| .addSpanExporterCustomizer( | ||
| (spanExporter, configProperties) -> | ||
| customizeSpanExporter(spanExporter, credentials, configProperties)) | ||
| .addMetricExporterCustomizer( | ||
| (metricExporter, configProperties) -> | ||
| customizeMetricExporter(metricExporter, credentials, configProperties)) | ||
| .addResourceCustomizer( | ||
| (resource, configProperties) -> | ||
| customizeResource(resource, credentials, configProperties)); | ||
| } |
There was a problem hiding this comment.
Unconditionally calling GoogleCredentials.getApplicationDefault() during autoconfiguration initialization will throw a GoogleAuthException and crash the application startup if Application Default Credentials (ADC) are not configured, even if the OTLP exporter is not targeting GCP. To prevent this, retrieve the credentials lazily using a Supplier and only evaluate it when a GCP endpoint is actually targeted.
@Override
public void customize(@Nonnull AutoConfigurationCustomizer autoConfiguration) {
java.util.function.Supplier<GoogleCredentials> credentialsSupplier =
new java.util.function.Supplier<GoogleCredentials>() {
private GoogleCredentials credentials;
@Override
public synchronized GoogleCredentials get() {
if (credentials == null) {
try {
credentials = GoogleCredentials.getApplicationDefault();
} catch (IOException e) {
throw new GoogleAuthException(Reason.FAILED_ADC_RETRIEVAL, e);
}
}
return credentials;
}
};
autoConfiguration
.addSpanExporterCustomizer(
(spanExporter, configProperties) ->
customizeSpanExporter(spanExporter, credentialsSupplier, configProperties))
.addMetricExporterCustomizer(
(metricExporter, configProperties) ->
customizeMetricExporter(metricExporter, credentialsSupplier, configProperties))
.addResourceCustomizer(
(resource, configProperties) ->
customizeResource(resource, credentialsSupplier, configProperties));
}| private static SpanExporter customizeSpanExporter( | ||
| SpanExporter exporter, GoogleCredentials credentials, ConfigProperties configProperties) { | ||
| if (isSignalTargeted(SIGNAL_TYPE_TRACES, configProperties)) { | ||
| return addAuthorizationHeaders(exporter, credentials, configProperties); | ||
| } else { | ||
| String[] params = {SIGNAL_TYPE_TRACES, SIGNAL_TARGET_WARNING_FIX_SUGGESTION}; | ||
| logger.log( | ||
| Level.WARNING, | ||
| "GCP Authentication Extension is not configured for signal type: {0}. {1}", | ||
| params); | ||
| } | ||
| return exporter; | ||
| } |
There was a problem hiding this comment.
Update customizeSpanExporter to accept a Supplier<GoogleCredentials> and only check/customize if the endpoint is actually a GCP endpoint. This avoids logging noisy warnings when GCP is not targeted.
private static SpanExporter customizeSpanExporter(
SpanExporter exporter, java.util.function.Supplier<GoogleCredentials> credentialsSupplier, ConfigProperties configProperties) {
if (!isGcpEndpoint(SIGNAL_TYPE_TRACES, configProperties)) {
return exporter;
}
if (isSignalTargeted(SIGNAL_TYPE_TRACES, configProperties)) {
return addAuthorizationHeaders(exporter, credentialsSupplier.get(), configProperties);
} else {
String[] params = {SIGNAL_TYPE_TRACES, SIGNAL_TARGET_WARNING_FIX_SUGGESTION};
logger.log(
Level.WARNING,
"GCP Authentication Extension is not configured for signal type: {0}. {1}",
params);
}
return exporter;
}| private static MetricExporter customizeMetricExporter( | ||
| MetricExporter exporter, GoogleCredentials credentials, ConfigProperties configProperties) { | ||
| if (isSignalTargeted(SIGNAL_TYPE_METRICS, configProperties)) { | ||
| return addAuthorizationHeaders(exporter, credentials, configProperties); | ||
| } else { | ||
| String[] params = {SIGNAL_TYPE_METRICS, SIGNAL_TARGET_WARNING_FIX_SUGGESTION}; | ||
| logger.log( | ||
| Level.WARNING, | ||
| "GCP Authentication Extension is not configured for signal type: {0}. {1}", | ||
| params); | ||
| } | ||
| return exporter; | ||
| } |
There was a problem hiding this comment.
Update customizeMetricExporter to accept a Supplier<GoogleCredentials> and only check/customize if the endpoint is actually a GCP endpoint. This avoids logging noisy warnings when GCP is not targeted.
private static MetricExporter customizeMetricExporter(
MetricExporter exporter, java.util.function.Supplier<GoogleCredentials> credentialsSupplier, ConfigProperties configProperties) {
if (!isGcpEndpoint(SIGNAL_TYPE_METRICS, configProperties)) {
return exporter;
}
if (isSignalTargeted(SIGNAL_TYPE_METRICS, configProperties)) {
return addAuthorizationHeaders(exporter, credentialsSupplier.get(), configProperties);
} else {
String[] params = {SIGNAL_TYPE_METRICS, SIGNAL_TARGET_WARNING_FIX_SUGGESTION};
logger.log(
Level.WARNING,
"GCP Authentication Extension is not configured for signal type: {0}. {1}",
params);
}
return exporter;
}| private static boolean isSignalTargeted(String checkSignal, ConfigProperties configProperties) { | ||
| String endpoint = configProperties.getString("otel.exporter.otlp." + checkSignal + ".endpoint"); | ||
| if (endpoint == null) { | ||
| endpoint = configProperties.getString("otel.exporter.otlp.endpoint"); | ||
| } | ||
| if (endpoint == null | ||
| || (!endpoint.startsWith("https://telemetry.googleapis.com") | ||
| && !endpoint.startsWith("https://telemetry.mtls.googleapis.com"))) { | ||
| return false; | ||
| } | ||
| String userSpecifiedTargetedSignals = | ||
| ConfigurableOption.GOOGLE_OTEL_AUTH_TARGET_SIGNALS.getConfiguredValueWithFallback( | ||
| configProperties, () -> SIGNAL_TYPE_ALL); | ||
| return stream(userSpecifiedTargetedSignals.split(",")) | ||
| .map(String::trim) | ||
| .anyMatch( | ||
| targetedSignal -> | ||
| targetedSignal.equals(checkSignal) || targetedSignal.equals(SIGNAL_TYPE_ALL)); | ||
| } |
There was a problem hiding this comment.
Separate the check for a GCP endpoint from the check for whether a signal is targeted. This prevents logging warnings for non-GCP endpoints.
private static boolean isGcpEndpoint(String checkSignal, ConfigProperties configProperties) {
String endpoint = configProperties.getString("otel.exporter.otlp." + checkSignal + ".endpoint");
if (endpoint == null) {
endpoint = configProperties.getString("otel.exporter.otlp.endpoint");
}
return endpoint != null
&& (endpoint.startsWith("https://telemetry.googleapis.com")
|| endpoint.startsWith("https://telemetry.mtls.googleapis.com"));
}
// Checks if the auth extension is configured to target the passed signal for authentication.
private static boolean isSignalTargeted(String checkSignal, ConfigProperties configProperties) {
if (!isGcpEndpoint(checkSignal, configProperties)) {
return false;
}
String userSpecifiedTargetedSignals =
ConfigurableOption.GOOGLE_OTEL_AUTH_TARGET_SIGNALS.getConfiguredValueWithFallback(
configProperties, () -> SIGNAL_TYPE_ALL);
return stream(userSpecifiedTargetedSignals.split(","))
.map(String::trim)
.anyMatch(
targetedSignal ->
targetedSignal.equals(checkSignal) || targetedSignal.equals(SIGNAL_TYPE_ALL));
}| private static SpanExporter addAuthorizationHeaders( | ||
| SpanExporter exporter, GoogleCredentials credentials, ConfigProperties configProperties) { | ||
| if (exporter instanceof OtlpHttpSpanExporter) { | ||
| OtlpHttpSpanExporterBuilder builder = | ||
| ((OtlpHttpSpanExporter) exporter) | ||
| .toBuilder() | ||
| .setHeaders(() -> getRequiredHeaderMap(credentials, configProperties)); | ||
| return builder.build(); | ||
| } else if (exporter instanceof OtlpGrpcSpanExporter) { | ||
| OtlpGrpcSpanExporterBuilder builder = | ||
| ((OtlpGrpcSpanExporter) exporter) | ||
| .toBuilder() | ||
| .setHeaders(() -> getRequiredHeaderMap(credentials, configProperties)); | ||
| return builder.build(); | ||
| } | ||
| return exporter; | ||
| } |
There was a problem hiding this comment.
When customizing the SpanExporter via toBuilder().build(), a new exporter instance is created with its own resources (like HTTP clients or gRPC channels). The original exporter instance is discarded without being shut down, which leaks these resources. Call exporter.shutdown() on the original exporter before returning the new one.
private static SpanExporter addAuthorizationHeaders(
SpanExporter exporter, GoogleCredentials credentials, ConfigProperties configProperties) {
if (exporter instanceof OtlpHttpSpanExporter) {
OtlpHttpSpanExporterBuilder builder =
((OtlpHttpSpanExporter) exporter)
.toBuilder()
.setHeaders(() -> getRequiredHeaderMap(credentials, configProperties));
SpanExporter newExporter = builder.build();
exporter.shutdown();
return newExporter;
} else if (exporter instanceof OtlpGrpcSpanExporter) {
OtlpGrpcSpanExporterBuilder builder =
((OtlpGrpcSpanExporter) exporter)
.toBuilder()
.setHeaders(() -> getRequiredHeaderMap(credentials, configProperties));
SpanExporter newExporter = builder.build();
exporter.shutdown();
return newExporter;
}
return exporter;
}| private static MetricExporter addAuthorizationHeaders( | ||
| MetricExporter exporter, GoogleCredentials credentials, ConfigProperties configProperties) { | ||
| if (exporter instanceof OtlpHttpMetricExporter) { | ||
| OtlpHttpMetricExporterBuilder builder = | ||
| ((OtlpHttpMetricExporter) exporter) | ||
| .toBuilder() | ||
| .setHeaders(() -> getRequiredHeaderMap(credentials, configProperties)); | ||
| return builder.build(); | ||
| } else if (exporter instanceof OtlpGrpcMetricExporter) { | ||
| OtlpGrpcMetricExporterBuilder builder = | ||
| ((OtlpGrpcMetricExporter) exporter) | ||
| .toBuilder() | ||
| .setHeaders(() -> getRequiredHeaderMap(credentials, configProperties)); | ||
| return builder.build(); | ||
| } | ||
| return exporter; | ||
| } |
There was a problem hiding this comment.
When customizing the MetricExporter via toBuilder().build(), a new exporter instance is created with its own resources. The original exporter instance is discarded without being shut down, which leaks these resources. Call exporter.shutdown() on the original exporter before returning the new one.
private static MetricExporter addAuthorizationHeaders(
MetricExporter exporter, GoogleCredentials credentials, ConfigProperties configProperties) {
if (exporter instanceof OtlpHttpMetricExporter) {
OtlpHttpMetricExporterBuilder builder =
((OtlpHttpMetricExporter) exporter)
.toBuilder()
.setHeaders(() -> getRequiredHeaderMap(credentials, configProperties));
MetricExporter newExporter = builder.build();
exporter.shutdown();
return newExporter;
} else if (exporter instanceof OtlpGrpcMetricExporter) {
OtlpGrpcMetricExporterBuilder builder =
((OtlpGrpcMetricExporter) exporter)
.toBuilder()
.setHeaders(() -> getRequiredHeaderMap(credentials, configProperties));
MetricExporter newExporter = builder.build();
exporter.shutdown();
return newExporter;
}
return exporter;
}| private static Resource customizeResource( | ||
| Resource resource, GoogleCredentials credentials, ConfigProperties configProperties) { | ||
| String gcpProjectId; | ||
| try { | ||
| gcpProjectId = ConfigurableOption.GOOGLE_CLOUD_PROJECT.getConfiguredValue(configProperties); | ||
| } catch (ConfigurationException e) { | ||
| gcpProjectId = credentials.getProjectId(); | ||
| if (gcpProjectId == null || gcpProjectId.isEmpty()) { | ||
| throw e; | ||
| } | ||
| } | ||
| Resource res = Resource.create(Attributes.of(stringKey(GCP_USER_PROJECT_ID_KEY), gcpProjectId)); | ||
| return resource.merge(res); | ||
| } |
There was a problem hiding this comment.
Update customizeResource to accept a Supplier<GoogleCredentials> and only customize the resource if at least one of the signals is targeting a GCP endpoint.
// Updates the current resource with the attributes required for ingesting OTLP data on GCP.
private static Resource customizeResource(
Resource resource, java.util.function.Supplier<GoogleCredentials> credentialsSupplier, ConfigProperties configProperties) {
if (!isGcpEndpoint(SIGNAL_TYPE_TRACES, configProperties)
&& !isGcpEndpoint(SIGNAL_TYPE_METRICS, configProperties)) {
return resource;
}
String gcpProjectId;
try {
gcpProjectId = ConfigurableOption.GOOGLE_CLOUD_PROJECT.getConfiguredValue(configProperties);
} catch (ConfigurationException e) {
gcpProjectId = credentialsSupplier.get().getProjectId();
if (gcpProjectId == null || gcpProjectId.isEmpty()) {
throw e;
}
}
Resource res = Resource.create(Attributes.of(stringKey(GCP_USER_PROJECT_ID_KEY), gcpProjectId));
return resource.merge(res);
}| Map<String, String> flattenedHeaders = | ||
| gcpHeaders.entrySet().stream() | ||
| .collect( | ||
| toMap( | ||
| Map.Entry::getKey, | ||
| entry -> | ||
| entry.getValue().stream() | ||
| .filter(Objects::nonNull) // Filter nulls | ||
| .filter(s -> !s.isEmpty()) // Filter empty strings | ||
| .collect(joining(",")))); |
There was a problem hiding this comment.
If gcpHeaders contains any entry with a null value, calling entry.getValue().stream() will throw a NullPointerException. Add a null check for entry.getValue() to make the header flattening more robust.
| Map<String, String> flattenedHeaders = | |
| gcpHeaders.entrySet().stream() | |
| .collect( | |
| toMap( | |
| Map.Entry::getKey, | |
| entry -> | |
| entry.getValue().stream() | |
| .filter(Objects::nonNull) // Filter nulls | |
| .filter(s -> !s.isEmpty()) // Filter empty strings | |
| .collect(joining(",")))); | |
| Map<String, String> flattenedHeaders = | |
| gcpHeaders.entrySet().stream() | |
| .collect( | |
| toMap( | |
| Map.Entry::getKey, | |
| entry -> | |
| entry.getValue() == null | |
| ? "" | |
| : entry.getValue().stream() | |
| .filter(Objects::nonNull) // Filter nulls | |
| .filter(s -> !s.isEmpty()) // Filter empty strings | |
| .collect(joining(",")))); |
stankiewicz
force-pushed
the
otel_gcp_auth
branch
from
17050ef to
dc69a2a
Compare
stankiewicz
force-pushed
the
otel_gcp_auth
branch
from
dc69a2a to
5d3ee09
Compare
stankiewicz
force-pushed
the
otel_gcp_auth
branch
from
5d3ee09 to
8c6b2a1
Compare
1 participant
Forks https://github.com/open-telemetry/opentelemetry-java-contrib/blob/main/gcp-auth-extension/src/main/java/io/opentelemetry/
Why it is needed - gcp-auth-extension in otel adds auth to all exporters, once fixed, we will be able to remove this module.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.