From 8ad2163238817312c10d2abadada34f55be1c9bb Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Fri, 29 Nov 2024 16:13:32 +0530 Subject: [PATCH 1/4] feat: Add Metrics host for built in metrics --- .../BuiltInOpenTelemetryMetricsProvider.java | 6 +++-- .../SpannerCloudMonitoringExporter.java | 15 ++++------- .../google/cloud/spanner/SpannerOptions.java | 27 ++++++++++++++++++- .../SpannerCloudMonitoringExporterTest.java | 4 +-- .../cloud/spanner/SpannerOptionsTest.java | 13 +++++++++ 5 files changed, 50 insertions(+), 15 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java index 9367c45b636..21675ff228c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java @@ -59,12 +59,14 @@ final class BuiltInOpenTelemetryMetricsProvider { private BuiltInOpenTelemetryMetricsProvider() {} - OpenTelemetry getOrCreateOpenTelemetry(String projectId, @Nullable Credentials credentials) { + OpenTelemetry getOrCreateOpenTelemetry( + String projectId, @Nullable Credentials credentials, @Nullable String metricsHost) { try { if (this.openTelemetry == null) { SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( - SpannerCloudMonitoringExporter.create(projectId, credentials), sdkMeterProviderBuilder); + SpannerCloudMonitoringExporter.create(projectId, credentials, metricsHost), + sdkMeterProviderBuilder); this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProviderBuilder.build()).build(); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index 3577c9f7b45..319d7ff3548 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -29,7 +29,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.Iterables; import com.google.common.util.concurrent.MoreExecutors; import com.google.monitoring.v3.CreateTimeSeriesRequest; @@ -63,13 +62,6 @@ class SpannerCloudMonitoringExporter implements MetricExporter { private static final Logger logger = Logger.getLogger(SpannerCloudMonitoringExporter.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("spanner.test-monitoring-endpoint"), - MetricServiceSettings.getDefaultEndpoint()); - // This the quota limit from Cloud Monitoring. More details in // https://cloud.google.com/monitoring/quotas#custom_metrics_quotas. private static final int EXPORT_BATCH_SIZE_LIMIT = 200; @@ -78,7 +70,8 @@ class SpannerCloudMonitoringExporter implements MetricExporter { private final MetricServiceClient client; private final String spannerProjectId; - static SpannerCloudMonitoringExporter create(String projectId, @Nullable Credentials credentials) + static SpannerCloudMonitoringExporter create( + String projectId, @Nullable Credentials credentials, @Nullable String metricsHost) throws IOException { MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder(); CredentialsProvider credentialsProvider; @@ -88,7 +81,9 @@ static SpannerCloudMonitoringExporter create(String projectId, @Nullable Credent credentialsProvider = FixedCredentialsProvider.create(credentials); } settingsBuilder.setCredentialsProvider(credentialsProvider); - settingsBuilder.setEndpoint(MONITORING_ENDPOINT); + if (metricsHost != null) { + settingsBuilder.setEndpoint(metricsHost); + } org.threeten.bp.Duration timeout = Duration.ofMinutes(1); // TODO: createServiceTimeSeries needs special handling if the request failed. Leaving diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index af54515e7c6..65e6782f0f0 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -164,6 +164,7 @@ public class SpannerOptions extends ServiceOptions { private final boolean enableBuiltInMetrics; private final boolean enableExtendedTracing; private final boolean enableEndToEndTracing; + private final String metricsHost; enum TracingFramework { OPEN_CENSUS, @@ -672,6 +673,7 @@ protected SpannerOptions(Builder builder) { enableExtendedTracing = builder.enableExtendedTracing; enableBuiltInMetrics = builder.enableBuiltInMetrics; enableEndToEndTracing = builder.enableEndToEndTracing; + metricsHost = builder.metricsHost; } /** @@ -712,6 +714,10 @@ default boolean isEnableBuiltInMetrics() { default boolean isEnableEndToEndTracing() { return false; } + + default String getMetricsHost() { + return null; + } } /** @@ -728,6 +734,7 @@ private static class SpannerEnvironmentImpl implements SpannerEnvironment { private static final String SPANNER_ENABLE_END_TO_END_TRACING = "SPANNER_ENABLE_END_TO_END_TRACING"; private static final String SPANNER_DISABLE_BUILTIN_METRICS = "SPANNER_DISABLE_BUILTIN_METRICS"; + private static final String SPANNER_METRICS_HOST = "SPANNER_METRICS_HOST"; private SpannerEnvironmentImpl() {} @@ -763,6 +770,11 @@ public boolean isEnableBuiltInMetrics() { public boolean isEnableEndToEndTracing() { return Boolean.parseBoolean(System.getenv(SPANNER_ENABLE_END_TO_END_TRACING)); } + + @Override + public String getMetricsHost() { + return System.getenv(SPANNER_METRICS_HOST); + } } /** Builder for {@link SpannerOptions} instances. */ @@ -828,6 +840,7 @@ public static class Builder private boolean enableExtendedTracing = SpannerOptions.environment.isEnableExtendedTracing(); private boolean enableEndToEndTracing = SpannerOptions.environment.isEnableEndToEndTracing(); private boolean enableBuiltInMetrics = SpannerOptions.environment.isEnableBuiltInMetrics(); + private String metricsHost = SpannerOptions.environment.getMetricsHost(); private static String createCustomClientLibToken(String token) { return token + " " + ServiceOptions.getGoogApiClientLibName(); @@ -895,6 +908,7 @@ protected Builder() { this.enableExtendedTracing = options.enableExtendedTracing; this.enableBuiltInMetrics = options.enableBuiltInMetrics; this.enableEndToEndTracing = options.enableEndToEndTracing; + this.metricsHost = options.metricsHost; } @Override @@ -1417,6 +1431,12 @@ public Builder setBuiltInMetricsEnabled(boolean enableBuiltInMetrics) { return this; } + /** Sets the metrics host to be used for Built-in client side metrics */ + public Builder setMetricsHost(String metricsHost) { + this.metricsHost = metricsHost; + return this; + } + /** * Sets whether to enable extended OpenTelemetry tracing. Enabling this option will add the * following additional attributes to the traces that are generated by the client: @@ -1727,7 +1747,7 @@ private ApiTracerFactory getDefaultApiTracerFactory() { private ApiTracerFactory createMetricsApiTracerFactory() { OpenTelemetry openTelemetry = this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( - this.getProjectId(), getCredentials()); + this.getProjectId(), getCredentials(), this.metricsHost); return openTelemetry != null ? new MetricsTracerFactory( @@ -1754,6 +1774,11 @@ public boolean isEnableBuiltInMetrics() { return enableBuiltInMetrics; } + /** Returns the override metrics Host. */ + String getMetricsHost() { + return metricsHost; + } + @BetaApi public boolean isUseVirtualThreads() { return useVirtualThreads; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java index acb7ae9fa1e..ab30de1ade0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerCloudMonitoringExporterTest.java @@ -337,7 +337,7 @@ public void testExportingSumDataInBatches() { @Test public void getAggregationTemporality() throws IOException { SpannerCloudMonitoringExporter actualExporter = - SpannerCloudMonitoringExporter.create(projectId, null); + SpannerCloudMonitoringExporter.create(projectId, null, null); assertThat(actualExporter.getAggregationTemporality(InstrumentType.COUNTER)) .isEqualTo(AggregationTemporality.CUMULATIVE); } @@ -348,7 +348,7 @@ public void testSkipExportingDataIfMissingInstanceId() throws IOException { Attributes.builder().putAll(attributes).remove(INSTANCE_ID_KEY).build(); SpannerCloudMonitoringExporter actualExporter = - SpannerCloudMonitoringExporter.create(projectId, null); + SpannerCloudMonitoringExporter.create(projectId, null, null); assertThat(actualExporter.getAggregationTemporality(InstrumentType.COUNTER)) .isEqualTo(AggregationTemporality.CUMULATIVE); ArgumentCaptor argumentCaptor = diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index e8421cd235c..2bb16833b85 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -754,6 +754,19 @@ public void testEndToEndTracingEnablement() { .isEndToEndTracingEnabled()); } + @Test + public void testMetricsHost() { + String metricsEndpoint = "test-endpoint:443"; + assertNull(SpannerOptions.newBuilder().setProjectId("p").build().getMetricsHost()); + assertThat( + SpannerOptions.newBuilder() + .setProjectId("p") + .setMetricsHost(metricsEndpoint) + .build() + .getMetricsHost()) + .isEqualTo(metricsEndpoint); + } + @Test public void testSetDirectedReadOptions() { final DirectedReadOptions directedReadOptions = From f434c1dd02d7ddc97d48885faee3b0773f5bd21a Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 5 Dec 2024 16:53:51 +0530 Subject: [PATCH 2/4] changed the host env variable --- .../BuiltInOpenTelemetryMetricsProvider.java | 4 +-- .../SpannerCloudMonitoringExporter.java | 6 ++-- .../google/cloud/spanner/SpannerOptions.java | 28 +++++++++---------- .../cloud/spanner/SpannerOptionsTest.java | 8 +++--- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java index 21675ff228c..0d30ff1d5f0 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java @@ -60,12 +60,12 @@ final class BuiltInOpenTelemetryMetricsProvider { private BuiltInOpenTelemetryMetricsProvider() {} OpenTelemetry getOrCreateOpenTelemetry( - String projectId, @Nullable Credentials credentials, @Nullable String metricsHost) { + String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost) { try { if (this.openTelemetry == null) { SdkMeterProviderBuilder sdkMeterProviderBuilder = SdkMeterProvider.builder(); BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( - SpannerCloudMonitoringExporter.create(projectId, credentials, metricsHost), + SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), sdkMeterProviderBuilder); this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProviderBuilder.build()).build(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java index 319d7ff3548..fc101fbcfc3 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerCloudMonitoringExporter.java @@ -71,7 +71,7 @@ class SpannerCloudMonitoringExporter implements MetricExporter { private final String spannerProjectId; static SpannerCloudMonitoringExporter create( - String projectId, @Nullable Credentials credentials, @Nullable String metricsHost) + String projectId, @Nullable Credentials credentials, @Nullable String monitoringHost) throws IOException { MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder(); CredentialsProvider credentialsProvider; @@ -81,8 +81,8 @@ static SpannerCloudMonitoringExporter create( credentialsProvider = FixedCredentialsProvider.create(credentials); } settingsBuilder.setCredentialsProvider(credentialsProvider); - if (metricsHost != null) { - settingsBuilder.setEndpoint(metricsHost); + if (monitoringHost != null) { + settingsBuilder.setEndpoint(monitoringHost); } org.threeten.bp.Duration timeout = Duration.ofMinutes(1); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 65e6782f0f0..d85b39c1253 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -164,7 +164,7 @@ public class SpannerOptions extends ServiceOptions { private final boolean enableBuiltInMetrics; private final boolean enableExtendedTracing; private final boolean enableEndToEndTracing; - private final String metricsHost; + private final String monitoringHost; enum TracingFramework { OPEN_CENSUS, @@ -673,7 +673,7 @@ protected SpannerOptions(Builder builder) { enableExtendedTracing = builder.enableExtendedTracing; enableBuiltInMetrics = builder.enableBuiltInMetrics; enableEndToEndTracing = builder.enableEndToEndTracing; - metricsHost = builder.metricsHost; + monitoringHost = builder.monitoringHost; } /** @@ -715,7 +715,7 @@ default boolean isEnableEndToEndTracing() { return false; } - default String getMetricsHost() { + default String getMonitoringHost() { return null; } } @@ -734,7 +734,7 @@ private static class SpannerEnvironmentImpl implements SpannerEnvironment { private static final String SPANNER_ENABLE_END_TO_END_TRACING = "SPANNER_ENABLE_END_TO_END_TRACING"; private static final String SPANNER_DISABLE_BUILTIN_METRICS = "SPANNER_DISABLE_BUILTIN_METRICS"; - private static final String SPANNER_METRICS_HOST = "SPANNER_METRICS_HOST"; + private static final String SPANNER_MONITORING_HOST = "SPANNER_MONITORING_HOST"; private SpannerEnvironmentImpl() {} @@ -772,8 +772,8 @@ public boolean isEnableEndToEndTracing() { } @Override - public String getMetricsHost() { - return System.getenv(SPANNER_METRICS_HOST); + public String getMonitoringHost() { + return System.getenv(SPANNER_MONITORING_HOST); } } @@ -840,7 +840,7 @@ public static class Builder private boolean enableExtendedTracing = SpannerOptions.environment.isEnableExtendedTracing(); private boolean enableEndToEndTracing = SpannerOptions.environment.isEnableEndToEndTracing(); private boolean enableBuiltInMetrics = SpannerOptions.environment.isEnableBuiltInMetrics(); - private String metricsHost = SpannerOptions.environment.getMetricsHost(); + private String monitoringHost = SpannerOptions.environment.getMonitoringHost(); private static String createCustomClientLibToken(String token) { return token + " " + ServiceOptions.getGoogApiClientLibName(); @@ -908,7 +908,7 @@ protected Builder() { this.enableExtendedTracing = options.enableExtendedTracing; this.enableBuiltInMetrics = options.enableBuiltInMetrics; this.enableEndToEndTracing = options.enableEndToEndTracing; - this.metricsHost = options.metricsHost; + this.monitoringHost = options.monitoringHost; } @Override @@ -1431,9 +1431,9 @@ public Builder setBuiltInMetricsEnabled(boolean enableBuiltInMetrics) { return this; } - /** Sets the metrics host to be used for Built-in client side metrics */ - public Builder setMetricsHost(String metricsHost) { - this.metricsHost = metricsHost; + /** Sets the monitoring host to be used for Built-in client side metrics */ + public Builder setMonitoringHost(String monitoringHost) { + this.monitoringHost = monitoringHost; return this; } @@ -1747,7 +1747,7 @@ private ApiTracerFactory getDefaultApiTracerFactory() { private ApiTracerFactory createMetricsApiTracerFactory() { OpenTelemetry openTelemetry = this.builtInOpenTelemetryMetricsProvider.getOrCreateOpenTelemetry( - this.getProjectId(), getCredentials(), this.metricsHost); + this.getProjectId(), getCredentials(), this.monitoringHost); return openTelemetry != null ? new MetricsTracerFactory( @@ -1775,8 +1775,8 @@ public boolean isEnableBuiltInMetrics() { } /** Returns the override metrics Host. */ - String getMetricsHost() { - return metricsHost; + String getMonitoringHost() { + return monitoringHost; } @BetaApi diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 2bb16833b85..7b891fdb7a9 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -755,15 +755,15 @@ public void testEndToEndTracingEnablement() { } @Test - public void testMetricsHost() { + public void testmonitoringHost() { String metricsEndpoint = "test-endpoint:443"; - assertNull(SpannerOptions.newBuilder().setProjectId("p").build().getMetricsHost()); + assertNull(SpannerOptions.newBuilder().setProjectId("p").build().getMonitoringHost()); assertThat( SpannerOptions.newBuilder() .setProjectId("p") - .setMetricsHost(metricsEndpoint) + .setMonitoringHost(metricsEndpoint) .build() - .getMetricsHost()) + .getMonitoringHost()) .isEqualTo(metricsEndpoint); } From aca9d30e95efcae9e0a07f7184139da4eb40b69f Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 5 Dec 2024 17:04:52 +0530 Subject: [PATCH 3/4] conflict fix --- .../cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java index ba172acbcad..4aeb98987d1 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInOpenTelemetryMetricsProvider.java @@ -67,8 +67,8 @@ OpenTelemetry getOrCreateOpenTelemetry( BuiltInOpenTelemetryMetricsView.registerBuiltinMetrics( SpannerCloudMonitoringExporter.create(projectId, credentials, monitoringHost), sdkMeterProviderBuilder); - this.openTelemetry = - OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProviderBuilder.build()).build(); + SdkMeterProvider sdkMeterProvider = sdkMeterProviderBuilder.build(); + this.openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(sdkMeterProvider).build(); Runtime.getRuntime().addShutdownHook(new Thread(sdkMeterProvider::close)); } return this.openTelemetry; From ac1ddbc00a96f33d5c8f34f21762b28783eca27f Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Thu, 5 Dec 2024 17:14:55 +0530 Subject: [PATCH 4/4] clirr check --- google-cloud-spanner/clirr-ignored-differences.xml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index bae308f2b50..5b84cb4ebc3 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -709,6 +709,13 @@ boolean isEnableBuiltInMetrics() + + + 7012 + com/google/cloud/spanner/SpannerOptions$SpannerEnvironment + java.lang.String getMonitoringHost() + + 7012