From a341eb8530d959edabac0282c52c3e928abf733d Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 30 Sep 2024 21:32:20 -0400 Subject: [PATCH] fix: support override monitoring endpoint (#2364) * fix: support override monitoring endpoint * format * update default * add nullable annotation * update * update --- .../clirr-ignored-differences.xml | 10 ++++++ .../data/v2/BigtableDataClientFactory.java | 3 +- .../data/v2/stub/EnhancedBigtableStub.java | 16 ++++++--- .../v2/stub/EnhancedBigtableStubSettings.java | 33 +++++++++++++++++++ .../BigtableCloudMonitoringExporter.java | 24 +++++++++----- .../v2/stub/metrics/BuiltinMetricsView.java | 23 +++++++++++-- .../stub/metrics/DefaultMetricsProvider.java | 6 ++-- .../EnhancedBigtableStubSettingsTest.java | 22 +++++++++---- 8 files changed, 113 insertions(+), 24 deletions(-) diff --git a/google-cloud-bigtable/clirr-ignored-differences.xml b/google-cloud-bigtable/clirr-ignored-differences.xml index 8ddcb6fdf0..a3dc564c44 100644 --- a/google-cloud-bigtable/clirr-ignored-differences.xml +++ b/google-cloud-bigtable/clirr-ignored-differences.xml @@ -265,4 +265,14 @@ com/google/cloud/bigtable/admin/v2/stub/EnhancedBigtableTableAdminStub * + + 7004 + com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter + * + + + 7004 + com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider + * + diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java index 9b2f2e345f..34ec77bdfc 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java @@ -90,7 +90,8 @@ public static BigtableDataClientFactory create(BigtableDataSettings defaultSetti EnhancedBigtableStub.getOpenTelemetry( defaultSettings.getProjectId(), defaultSettings.getMetricsProvider(), - sharedClientContext.getCredentials()); + sharedClientContext.getCredentials(), + defaultSettings.getStubSettings().getMetricsEndpoint()); } catch (Throwable t) { logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index 17aa382f96..91c63c2b85 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -223,7 +223,8 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) getOpenTelemetry( settings.getProjectId(), settings.getMetricsProvider(), - clientContext.getCredentials()); + clientContext.getCredentials(), + settings.getMetricsEndpoint()); } catch (Throwable t) { logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t); } @@ -268,7 +269,11 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set // We don't want client side metrics to crash the client, so catch any exception when getting // the OTEL instance and log the exception instead. openTelemetry = - getOpenTelemetry(settings.getProjectId(), settings.getMetricsProvider(), credentials); + getOpenTelemetry( + settings.getProjectId(), + settings.getMetricsProvider(), + credentials, + settings.getMetricsEndpoint()); } catch (Throwable t) { logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t); } @@ -378,7 +383,10 @@ public static ApiTracerFactory createBigtableTracerFactory( @Nullable public static OpenTelemetry getOpenTelemetry( - String projectId, MetricsProvider metricsProvider, @Nullable Credentials defaultCredentials) + String projectId, + MetricsProvider metricsProvider, + @Nullable Credentials defaultCredentials, + @Nullable String metricsEndpoint) throws IOException { if (metricsProvider instanceof CustomOpenTelemetryMetricsProvider) { CustomOpenTelemetryMetricsProvider customMetricsProvider = @@ -390,7 +398,7 @@ public static OpenTelemetry getOpenTelemetry( ? BigtableDataSettings.getMetricsCredentials() : defaultCredentials; DefaultMetricsProvider defaultMetricsProvider = (DefaultMetricsProvider) metricsProvider; - return defaultMetricsProvider.getOpenTelemetry(projectId, credentials); + return defaultMetricsProvider.getOpenTelemetry(projectId, metricsEndpoint, credentials); } else if (metricsProvider instanceof NoopMetricsProvider) { return null; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java index 46933c1690..2a3d0ddba4 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettings.java @@ -65,6 +65,7 @@ import java.util.Set; import java.util.logging.Logger; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.threeten.bp.Duration; /** @@ -250,6 +251,7 @@ public class EnhancedBigtableStubSettings extends StubSettings getJwtAudienceMapping() { return jwtAudienceMapping; @@ -1184,6 +1216,7 @@ public String toString() { .add("pingAndWarmSettings", pingAndWarmSettings) .add("executeQuerySettings", executeQuerySettings) .add("metricsProvider", metricsProvider) + .add("metricsEndpoint", metricsEndpoint) .add("parent", super.toString()) .toString(); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index f6a2527302..fd54313e8d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -39,7 +39,6 @@ import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.monitoring.v3.MetricServiceSettings; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -79,11 +78,12 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { Logger.getLogger(BigtableCloudMonitoringExporter.class.getName()); // This system property can be used to override the monitoring endpoint - // to a different environment. It's meant for internal testing only. - private static final String MONITORING_ENDPOINT = - MoreObjects.firstNonNull( - System.getProperty("bigtable.test-monitoring-endpoint"), - MetricServiceSettings.getDefaultEndpoint()); + // to a different environment. It's meant for internal testing only and + // will be removed in future versions. Use settings in EnhancedBigtableStubSettings + // to override the endpoint. + @Deprecated @Nullable + private static final String MONITORING_ENDPOINT_OVERRIDE_SYS_PROP = + System.getProperty("bigtable.test-monitoring-endpoint"); private static final String APPLICATION_RESOURCE_PROJECT_ID = "project_id"; @@ -126,14 +126,22 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { .collect(ImmutableList.toImmutableList()); public static BigtableCloudMonitoringExporter create( - String projectId, @Nullable Credentials credentials) throws IOException { + String projectId, @Nullable Credentials credentials, @Nullable String endpoint) + throws IOException { MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder(); CredentialsProvider credentialsProvider = Optional.ofNullable(credentials) .map(FixedCredentialsProvider::create) .orElse(NoCredentialsProvider.create()); settingsBuilder.setCredentialsProvider(credentialsProvider); - settingsBuilder.setEndpoint(MONITORING_ENDPOINT); + if (MONITORING_ENDPOINT_OVERRIDE_SYS_PROP != null) { + logger.warning( + "Setting the monitoring endpoint through system variable will be removed in future versions"); + settingsBuilder.setEndpoint(MONITORING_ENDPOINT_OVERRIDE_SYS_PROP); + } + if (endpoint != null) { + settingsBuilder.setEndpoint(endpoint); + } org.threeten.bp.Duration timeout = Duration.ofMinutes(1); // TODO: createServiceTimeSeries needs special handling if the request failed. Leaving diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java index 445160a146..ca52581a92 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java @@ -37,7 +37,7 @@ private BuiltinMetricsView() {} /** * Register built-in metrics on the {@link SdkMeterProviderBuilder} with application default - * credentials. + * credentials and default endpoint. */ public static void registerBuiltinMetrics(String projectId, SdkMeterProviderBuilder builder) throws IOException { @@ -45,11 +45,28 @@ public static void registerBuiltinMetrics(String projectId, SdkMeterProviderBuil projectId, GoogleCredentials.getApplicationDefault(), builder); } - /** Register built-in metrics on the {@link SdkMeterProviderBuilder} with credentials. */ + /** + * Register built-in metrics on the {@link SdkMeterProviderBuilder} with custom credentials and + * default endpoint. + */ public static void registerBuiltinMetrics( String projectId, @Nullable Credentials credentials, SdkMeterProviderBuilder builder) throws IOException { - MetricExporter metricExporter = BigtableCloudMonitoringExporter.create(projectId, credentials); + BuiltinMetricsView.registerBuiltinMetrics(projectId, credentials, builder, null); + } + + /** + * Register built-in metrics on the {@link SdkMeterProviderBuilder} with custom credentials and + * endpoint. + */ + public static void registerBuiltinMetrics( + String projectId, + @Nullable Credentials credentials, + SdkMeterProviderBuilder builder, + @Nullable String endpoint) + throws IOException { + MetricExporter metricExporter = + BigtableCloudMonitoringExporter.create(projectId, credentials, endpoint); for (Map.Entry entry : BuiltinMetricsConstants.getAllViews().entrySet()) { builder.registerView(entry.getKey(), entry.getValue()); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java index b8aad8c931..c6b0a80c76 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java @@ -42,12 +42,14 @@ public final class DefaultMetricsProvider implements MetricsProvider { private DefaultMetricsProvider() {} @InternalApi - public OpenTelemetry getOpenTelemetry(String projectId, @Nullable Credentials credentials) + public OpenTelemetry getOpenTelemetry( + String projectId, String metricsEndpoint, @Nullable Credentials credentials) throws IOException { this.projectId = projectId; if (openTelemetry == null) { SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder(); - BuiltinMetricsView.registerBuiltinMetrics(projectId, credentials, meterProvider); + BuiltinMetricsView.registerBuiltinMetrics( + projectId, credentials, meterProvider, metricsEndpoint); openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); } return openTelemetry; diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java index 4bcacab4c7..5280abe1fd 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubSettingsTest.java @@ -81,6 +81,7 @@ public void settingsAreNotLostTest() { Duration watchdogInterval = Duration.ofSeconds(12); boolean enableRoutingCookie = false; boolean enableRetryInfo = false; + String metricsEndpoint = "test-endpoint:443"; EnhancedBigtableStubSettings.Builder builder = EnhancedBigtableStubSettings.newBuilder() @@ -93,7 +94,8 @@ public void settingsAreNotLostTest() { .setStreamWatchdogProvider(watchdogProvider) .setStreamWatchdogCheckInterval(watchdogInterval) .setEnableRoutingCookie(enableRoutingCookie) - .setEnableRetryInfo(enableRetryInfo); + .setEnableRetryInfo(enableRetryInfo) + .setMetricsEndpoint(metricsEndpoint); verifyBuilder( builder, @@ -106,7 +108,8 @@ public void settingsAreNotLostTest() { watchdogProvider, watchdogInterval, enableRoutingCookie, - enableRetryInfo); + enableRetryInfo, + metricsEndpoint); verifySettings( builder.build(), projectId, @@ -118,7 +121,8 @@ public void settingsAreNotLostTest() { watchdogProvider, watchdogInterval, enableRoutingCookie, - enableRetryInfo); + enableRetryInfo, + metricsEndpoint); verifyBuilder( builder.build().toBuilder(), projectId, @@ -130,7 +134,8 @@ public void settingsAreNotLostTest() { watchdogProvider, watchdogInterval, enableRoutingCookie, - enableRetryInfo); + enableRetryInfo, + metricsEndpoint); } private void verifyBuilder( @@ -144,7 +149,8 @@ private void verifyBuilder( WatchdogProvider watchdogProvider, Duration watchdogInterval, boolean enableRoutingCookie, - boolean enableRetryInfo) { + boolean enableRetryInfo, + String metricsEndpoint) { assertThat(builder.getProjectId()).isEqualTo(projectId); assertThat(builder.getInstanceId()).isEqualTo(instanceId); assertThat(builder.getAppProfileId()).isEqualTo(appProfileId); @@ -155,6 +161,7 @@ private void verifyBuilder( assertThat(builder.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); assertThat(builder.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie); assertThat(builder.getEnableRetryInfo()).isEqualTo(enableRetryInfo); + assertThat(builder.getMetricsEndpoint()).isEqualTo(metricsEndpoint); } private void verifySettings( @@ -168,7 +175,8 @@ private void verifySettings( WatchdogProvider watchdogProvider, Duration watchdogInterval, boolean enableRoutingCookie, - boolean enableRetryInfo) { + boolean enableRetryInfo, + String metricsEndpoint) { assertThat(settings.getProjectId()).isEqualTo(projectId); assertThat(settings.getInstanceId()).isEqualTo(instanceId); assertThat(settings.getAppProfileId()).isEqualTo(appProfileId); @@ -179,6 +187,7 @@ private void verifySettings( assertThat(settings.getStreamWatchdogCheckInterval()).isEqualTo(watchdogInterval); assertThat(settings.getEnableRoutingCookie()).isEqualTo(enableRoutingCookie); assertThat(settings.getEnableRetryInfo()).isEqualTo(enableRetryInfo); + assertThat(settings.getMetricsEndpoint()).isEqualTo(metricsEndpoint); } @Test @@ -965,6 +974,7 @@ public void enableRetryInfoFalseValueTest() throws IOException { "pingAndWarmSettings", "executeQuerySettings", "metricsProvider", + "metricsEndpoint", }; @Test