-
Notifications
You must be signed in to change notification settings - Fork 182
feat(gcp-auth-extension): Support custom credentials via configuration properties #2767
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: main
Are you sure you want to change the base?
Changes from all commits
a28b0be
a25296c
ef176b5
c5cafe2
d228216
e96eb35
a63594d
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 |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| import static java.util.stream.Collectors.toMap; | ||
|
|
||
| import com.google.auth.oauth2.GoogleCredentials; | ||
| import com.google.auth.oauth2.ServiceAccountCredentials; | ||
| import com.google.auto.service.AutoService; | ||
| import io.opentelemetry.api.common.Attributes; | ||
| import io.opentelemetry.contrib.gcp.auth.GoogleAuthException.Reason; | ||
|
|
@@ -29,11 +30,17 @@ | |
| import io.opentelemetry.sdk.metrics.export.MetricExporter; | ||
| import io.opentelemetry.sdk.resources.Resource; | ||
| import io.opentelemetry.sdk.trace.export.SpanExporter; | ||
| import java.io.ByteArrayInputStream; | ||
| import java.io.File; | ||
| import java.io.FileInputStream; | ||
| import java.io.IOException; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.WeakHashMap; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.Logger; | ||
| import javax.annotation.Nonnull; | ||
|
|
@@ -43,8 +50,9 @@ | |
| * integration. | ||
| * | ||
| * <p>This class is registered as a service provider using {@link AutoService} and is responsible | ||
| * for customizing the OpenTelemetry configuration for GCP specific behavior. It retrieves Google | ||
| * Application Default Credentials (ADC) and adds them as authorization headers to the configured | ||
| * for customizing the OpenTelemetry configuration for GCP specific behavior. It retrieves | ||
| * credentials (either explicit service account keys from configuration or falling back to | ||
| * Application Default Credentials (ADC)) and adds them as authorization headers to the configured | ||
| * {@link SpanExporter}. It also sets default properties and resource attributes for GCP | ||
| * integration. | ||
| * | ||
|
|
@@ -57,6 +65,8 @@ public class GcpAuthAutoConfigurationCustomizerProvider | |
|
|
||
| private static final Logger logger = | ||
| Logger.getLogger(GcpAuthAutoConfigurationCustomizerProvider.class.getName()); | ||
| private static final Map<ConfigProperties, GoogleCredentials> credentialsCache = | ||
|
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. Does this need to be a map? The map implicates that there could be multiple IIUC, the same
Author
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. You are correct that within a single auto-configuration pass, the same I used a map (specifically a However, if you disagree and prefer to simplify it to a single cached instance, I am happy to change it back
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. I don't think the case of multiple SDKs apply here. The extension is designed to be used with autoconfigure module which uses the If the user does end up using multiple Also, could you clarify the use case around unit tests here? I don't quite get it. |
||
| Collections.synchronizedMap(new WeakHashMap<>()); | ||
| private static final String SIGNAL_TARGET_WARNING_FIX_SUGGESTION = | ||
| String.format( | ||
| "You may safely ignore this warning if it is intentional, otherwise please configure the '%s' by exporting valid values to environment variable: %s or by setting valid values in system property: %s.", | ||
|
|
@@ -75,8 +85,8 @@ public class GcpAuthAutoConfigurationCustomizerProvider | |
| * Customizes the provided {@link AutoConfigurationCustomizer} such that authenticated exports to | ||
| * GCP Telemetry API are possible from the configured OTLP exporter. | ||
| * | ||
| * <p>This method attempts to retrieve Google Application Default Credentials (ADC) and performs | ||
| * the following: | ||
| * <p>This method attempts to retrieve credentials (either from user-specified configuration or | ||
| * falling back to ADC) and performs the following: | ||
| * | ||
| * <ul> | ||
| * <li>Verifies whether the configured OTLP endpoint (base or signal specific) is a known GCP | ||
|
|
@@ -99,22 +109,71 @@ public class GcpAuthAutoConfigurationCustomizerProvider | |
| */ | ||
| @Override | ||
| 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)) | ||
| customizeSpanExporter( | ||
| spanExporter, resolveCredentials(configProperties), configProperties)) | ||
| .addMetricExporterCustomizer( | ||
| (metricExporter, configProperties) -> | ||
| customizeMetricExporter(metricExporter, credentials, configProperties)) | ||
| customizeMetricExporter( | ||
| metricExporter, resolveCredentials(configProperties), configProperties)) | ||
| .addResourceCustomizer( | ||
| (resource, configProperties) -> | ||
| customizeResource(resource, credentials, configProperties)); | ||
| customizeResource( | ||
| resource, resolveCredentials(configProperties), configProperties)); | ||
| } | ||
|
|
||
| private static GoogleCredentials resolveCredentials(ConfigProperties configProperties) { | ||
| synchronized (credentialsCache) { | ||
| GoogleCredentials cachedCredentials = credentialsCache.get(configProperties); | ||
| if (cachedCredentials != null) { | ||
| return cachedCredentials; | ||
| } | ||
|
|
||
| GoogleCredentials resolvedCredentials = loadCredentials(configProperties); | ||
| credentialsCache.put(configProperties, resolvedCredentials); | ||
| return resolvedCredentials; | ||
| } | ||
| } | ||
|
|
||
| private static GoogleCredentials loadCredentials(ConfigProperties configProperties) { | ||
| Optional<String> credsPath = | ||
| ConfigurableOption.GOOGLE_CLOUD_CREDENTIALS_PATH.getConfiguredValueAsOptional( | ||
| configProperties); | ||
| if (credsPath.isPresent()) { | ||
| File file = new File(credsPath.get()); | ||
| if (!file.exists()) { | ||
| throw new ConfigurationException("Credentials file not found: " + file.getName()); | ||
| } | ||
| try (FileInputStream fis = new FileInputStream(file)) { | ||
| return ServiceAccountCredentials.fromStream(fis) | ||
| .createScoped( | ||
| Collections.singletonList("https://www.googleapis.com/auth/cloud-platform")); | ||
| } catch (IOException e) { | ||
| throw new GoogleAuthException(Reason.FAILED_CREDENTIAL_CREATION, e); | ||
| } | ||
| } | ||
|
|
||
| Optional<String> credsJson = | ||
| ConfigurableOption.GOOGLE_CLOUD_CREDENTIALS_JSON.getConfiguredValueAsOptional( | ||
| configProperties); | ||
| if (credsJson.isPresent()) { | ||
| try (ByteArrayInputStream bais = | ||
| new ByteArrayInputStream(credsJson.get().getBytes(StandardCharsets.UTF_8))) { | ||
| return ServiceAccountCredentials.fromStream(bais) | ||
| .createScoped( | ||
| Collections.singletonList("https://www.googleapis.com/auth/cloud-platform")); | ||
| } catch (IOException e) { | ||
| throw new GoogleAuthException(Reason.FAILED_CREDENTIAL_CREATION, e); | ||
| } | ||
| } | ||
|
|
||
| try { | ||
| return GoogleCredentials.getApplicationDefault(); | ||
| } catch (IOException e) { | ||
| throw new GoogleAuthException(Reason.FAILED_ADC_RETRIEVAL, e); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
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.
To improve clarity, the documentation should indicate if the path should be relative or absolute.
Current documentation comments simply state - "Represents the Google Cloud Credentials Path option", but does not explain what these options are.
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.
Updated the comments