From dc20d62265d37798f0083768932af9c14aeec8d1 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Wed, 23 Sep 2026 10:31:15 +0000 Subject: [PATCH 01/10] feat(storage): add OpenTelemetry client metrics configuration and options [Generated-by: AI] --- .../cloud/storage/GrpcStorageOptions.java | 149 ++++++++++- .../cloud/storage/HttpStorageOptions.java | 133 +++++++++- .../cloud/storage/StorageMetricsConfig.java | 87 +++++++ .../google/cloud/storage/StorageOptions.java | 72 ++++++ .../cloud/storage/StorageOptionsTest.java | 242 ++++++++++++++++++ 5 files changed, 670 insertions(+), 13 deletions(-) create mode 100644 java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 1a6726b9c01b..02ac9ef28304 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -100,6 +100,7 @@ import io.grpc.StatusRuntimeException; import io.grpc.protobuf.ProtoUtils; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.metrics.MeterProvider; import java.io.Closeable; import java.io.IOException; import java.io.InputStream; @@ -122,6 +123,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.logging.Logger; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; /** * @since 2.14.0 @@ -140,7 +142,7 @@ public final class GrpcStorageOptions extends StorageOptions System.getProperty("com.google.cloud.storage.grpc.bound_token", "false")); private final GrpcRetryAlgorithmManager retryAlgorithmManager; - private final java.time.Duration terminationAwaitDuration; + private final Duration terminationAwaitDuration; private final boolean attemptDirectPath; private final boolean enableGrpcClientMetrics; @@ -148,6 +150,10 @@ public final class GrpcStorageOptions extends StorageOptions private final GrpcInterceptorProvider grpcInterceptorProvider; private final BlobWriteSessionConfig blobWriteSessionConfig; private transient OpenTelemetry openTelemetry; + private final boolean enableOtelMetrics; + private final boolean enableOtelDebugMetrics; + private transient MeterProvider meterProvider; + private final Duration metricInterval; private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) { super(builder, serviceDefaults); @@ -165,6 +171,16 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) this.grpcInterceptorProvider = builder.grpcInterceptorProvider; this.blobWriteSessionConfig = builder.blobWriteSessionConfig; this.openTelemetry = builder.openTelemetry; + this.enableOtelMetrics = + builder.enableOtelMetrics != null + ? builder.enableOtelMetrics + : StorageMetricsConfig.isEnableOtelMetrics(); + this.enableOtelDebugMetrics = + builder.enableOtelDebugMetrics != null + ? builder.enableOtelDebugMetrics + : StorageMetricsConfig.isEnableOtelDebugMetrics(); + this.meterProvider = builder.meterProvider; + this.metricInterval = builder.metricInterval; } @Override @@ -178,7 +194,7 @@ GrpcRetryAlgorithmManager getRetryAlgorithmManager() { } @InternalApi - java.time.Duration getTerminationAwaitDuration() { + Duration getTerminationAwaitDuration() { return terminationAwaitDuration; } @@ -195,6 +211,7 @@ GrpcInterceptorProvider getGrpcInterceptorProvider() { private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { in.defaultReadObject(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); + this.meterProvider = this.openTelemetry.getMeterProvider(); } /** @@ -363,9 +380,9 @@ private Tuple> resolveSettingsAndOpts() throw // seconds. // To allow read streams to have longer lifespans, crank up their timeouts, instead rely // on idleTimeout below. - .setLogicalTimeout(java.time.Duration.ofDays(28)) + .setLogicalTimeout(Duration.ofDays(28)) .build(); - java.time.Duration totalTimeout = baseRetrySettings.getTotalTimeoutDuration(); + Duration totalTimeout = baseRetrySettings.getTotalTimeoutDuration(); // retries for unary methods are generally handled at a different level, except // StartResumableWrite @@ -414,6 +431,45 @@ public OpenTelemetry getOpenTelemetry() { return openTelemetry; } + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public boolean isEnableOtelMetrics() { + return enableOtelMetrics; + } + + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public boolean isEnableOtelDebugMetrics() { + return enableOtelDebugMetrics; + } + + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public @Nullable MeterProvider getMeterProvider() { + if (meterProvider == null && openTelemetry != null) { + return openTelemetry.getMeterProvider(); + } + return meterProvider; + } + + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public Duration getMetricInterval() { + return metricInterval; + } + /** * @since 2.14.0 */ @@ -432,6 +488,10 @@ public int hashCode() { grpcInterceptorProvider, blobWriteSessionConfig, openTelemetry, + enableOtelMetrics, + enableOtelDebugMetrics, + meterProvider, + metricInterval, baseHashCode()); } @@ -446,11 +506,15 @@ public boolean equals(Object o) { GrpcStorageOptions that = (GrpcStorageOptions) o; return attemptDirectPath == that.attemptDirectPath && enableGrpcClientMetrics == that.enableGrpcClientMetrics + && enableOtelMetrics == that.enableOtelMetrics + && enableOtelDebugMetrics == that.enableOtelDebugMetrics && Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager) && Objects.equals(terminationAwaitDuration, that.terminationAwaitDuration) && Objects.equals(grpcInterceptorProvider, that.grpcInterceptorProvider) && Objects.equals(blobWriteSessionConfig, that.blobWriteSessionConfig) && Objects.equals(openTelemetry, that.openTelemetry) + && Objects.equals(meterProvider, that.meterProvider) + && Objects.equals(metricInterval, that.metricInterval) && this.baseEquals(that); } @@ -492,7 +556,7 @@ protected boolean shouldRefreshService(Storage cachedService) { public static final class Builder extends StorageOptions.Builder { private StorageRetryStrategy storageRetryStrategy; - private java.time.Duration terminationAwaitDuration; + private Duration terminationAwaitDuration; private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath(); private boolean enableGrpcClientMetrics = GrpcStorageDefaults.INSTANCE.isEnableGrpcClientMetrics(); @@ -501,6 +565,10 @@ public static final class Builder extends StorageOptions.Builder { private BlobWriteSessionConfig blobWriteSessionConfig = GrpcStorageDefaults.INSTANCE.getDefaultStorageWriterConfig(); private OpenTelemetry openTelemetry = GrpcStorageDefaults.INSTANCE.getDefaultOpenTelemetry(); + private Boolean enableOtelMetrics = null; + private Boolean enableOtelDebugMetrics = null; + private MeterProvider meterProvider = null; + private Duration metricInterval = Duration.ofSeconds(60); private boolean grpcMetricsManuallyEnabled = false; @@ -516,13 +584,16 @@ public static final class Builder extends StorageOptions.Builder { this.grpcInterceptorProvider = gso.grpcInterceptorProvider; this.blobWriteSessionConfig = gso.blobWriteSessionConfig; this.openTelemetry = gso.openTelemetry; + this.enableOtelMetrics = gso.isEnableOtelMetrics(); + this.enableOtelDebugMetrics = gso.isEnableOtelDebugMetrics(); + this.meterProvider = gso.getMeterProvider(); + this.metricInterval = gso.getMetricInterval(); } /** - * This method is obsolete. Use {@link #setTerminationAwaitJavaTimeDuration(java.time.Duration)} - * instead. + * This method is obsolete. Use {@link #setTerminationAwaitJavaTimeDuration(Duration)} instead. */ - @ObsoleteApi("Use setTerminationAwaitJavaTimeDuration(java.time.Duration) instead") + @ObsoleteApi("Use setTerminationAwaitJavaTimeDuration(Duration) instead") public Builder setTerminationAwaitDuration(org.threeten.bp.Duration terminationAwaitDuration) { return setTerminationAwaitJavaTimeDuration(toJavaTimeDuration(terminationAwaitDuration)); } @@ -535,8 +606,7 @@ public Builder setTerminationAwaitDuration(org.threeten.bp.Duration terminationA * @return the builder * @since 2.14.0 */ - public Builder setTerminationAwaitJavaTimeDuration( - java.time.Duration terminationAwaitDuration) { + public Builder setTerminationAwaitJavaTimeDuration(Duration terminationAwaitDuration) { this.terminationAwaitDuration = requireNonNull(terminationAwaitDuration, "terminationAwaitDuration must be non null"); return this; @@ -750,6 +820,61 @@ public GrpcStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) return this; } + /** + * Enable or disable OpenTelemetry client metrics. + * + * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public GrpcStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics) { + this.enableOtelMetrics = enableOtelMetrics; + return this; + } + + /** + * Enable or disable OpenTelemetry debug client metrics. + * + * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public GrpcStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDebugMetrics) { + this.enableOtelDebugMetrics = enableOtelDebugMetrics; + return this; + } + + /** + * Set a custom {@link MeterProvider} for recording client metrics. + * + * @param meterProvider custom MeterProvider to use + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public GrpcStorageOptions.Builder setMeterProvider(MeterProvider meterProvider) { + this.meterProvider = meterProvider; + return this; + } + + /** + * Set the metric export interval for periodic metric reading. + * + * @param metricInterval interval duration + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public GrpcStorageOptions.Builder setMetricInterval(Duration metricInterval) { + checkArgument( + metricInterval != null && !metricInterval.isNegative() && !metricInterval.isZero(), + "metricInterval must be positive"); + this.metricInterval = metricInterval; + return this; + } + /** * @since 2.14.0 */ @@ -817,8 +942,8 @@ public org.threeten.bp.Duration getTerminationAwaitDuration() { /** * @since 2.14.0 */ - public java.time.Duration getTerminationAwaitDurationJavaTime() { - return java.time.Duration.ofMinutes(1); + public Duration getTerminationAwaitDurationJavaTime() { + return Duration.ofMinutes(1); } /** diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index dac8a010cdfa..da131282881d 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -42,13 +42,16 @@ import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.metrics.MeterProvider; import java.io.IOException; import java.io.ObjectInputStream; import java.io.Serializable; import java.time.Clock; +import java.time.Duration; import java.util.Objects; import java.util.Set; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; /** * @since 2.14.0 @@ -66,6 +69,10 @@ public class HttpStorageOptions extends StorageOptions { private final BlobWriteSessionConfig blobWriteSessionConfig; private transient OpenTelemetry openTelemetry; + private final boolean enableOtelMetrics; + private final boolean enableOtelDebugMetrics; + private transient MeterProvider meterProvider; + private final Duration metricInterval; private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { super(builder, serviceDefaults); @@ -76,6 +83,16 @@ private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { retryDepsAdapter = new RetryDependenciesAdapter(); blobWriteSessionConfig = builder.blobWriteSessionConfig; openTelemetry = builder.openTelemetry; + this.enableOtelMetrics = + builder.enableOtelMetrics != null + ? builder.enableOtelMetrics + : StorageMetricsConfig.isEnableOtelMetrics(); + this.enableOtelDebugMetrics = + builder.enableOtelDebugMetrics != null + ? builder.enableOtelDebugMetrics + : StorageMetricsConfig.isEnableOtelDebugMetrics(); + this.meterProvider = builder.meterProvider; + this.metricInterval = builder.metricInterval; } @Override @@ -102,6 +119,45 @@ public OpenTelemetry getOpenTelemetry() { return openTelemetry; } + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public boolean isEnableOtelMetrics() { + return enableOtelMetrics; + } + + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public boolean isEnableOtelDebugMetrics() { + return enableOtelDebugMetrics; + } + + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public @Nullable MeterProvider getMeterProvider() { + if (meterProvider == null && openTelemetry != null) { + return openTelemetry.getMeterProvider(); + } + return meterProvider; + } + + /** + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public Duration getMetricInterval() { + return metricInterval; + } + @Override public HttpStorageOptions.Builder toBuilder() { return new HttpStorageOptions.Builder(this); @@ -110,7 +166,14 @@ public HttpStorageOptions.Builder toBuilder() { @Override public int hashCode() { return Objects.hash( - retryAlgorithmManager, blobWriteSessionConfig, openTelemetry, baseHashCode()); + retryAlgorithmManager, + blobWriteSessionConfig, + openTelemetry, + enableOtelMetrics, + enableOtelDebugMetrics, + meterProvider, + metricInterval, + baseHashCode()); } @Override @@ -125,6 +188,10 @@ public boolean equals(Object o) { return Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager) && Objects.equals(blobWriteSessionConfig, that.blobWriteSessionConfig) && Objects.equals(openTelemetry, that.openTelemetry) + && enableOtelMetrics == that.enableOtelMetrics + && enableOtelDebugMetrics == that.enableOtelDebugMetrics + && Objects.equals(meterProvider, that.meterProvider) + && Objects.equals(metricInterval, that.metricInterval) && this.baseEquals(that); } @@ -132,6 +199,7 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE in.defaultReadObject(); this.retryDepsAdapter = new RetryDependenciesAdapter(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); + this.meterProvider = this.openTelemetry.getMeterProvider(); } public static HttpStorageOptions.Builder newBuilder() { @@ -157,6 +225,10 @@ public static class Builder extends StorageOptions.Builder { private BlobWriteSessionConfig blobWriteSessionConfig = HttpStorageDefaults.INSTANCE.getDefaultStorageWriterConfig(); private OpenTelemetry openTelemetry = HttpStorageDefaults.INSTANCE.getDefaultOpenTelemetry(); + private Boolean enableOtelMetrics = null; + private Boolean enableOtelDebugMetrics = null; + private MeterProvider meterProvider = null; + private Duration metricInterval = Duration.ofSeconds(60); Builder() {} @@ -166,6 +238,10 @@ public static class Builder extends StorageOptions.Builder { this.storageRetryStrategy = hso.retryAlgorithmManager.retryStrategy; this.blobWriteSessionConfig = hso.blobWriteSessionConfig; this.openTelemetry = hso.getOpenTelemetry(); + this.enableOtelMetrics = hso.isEnableOtelMetrics(); + this.enableOtelDebugMetrics = hso.isEnableOtelDebugMetrics(); + this.meterProvider = hso.getMeterProvider(); + this.metricInterval = hso.getMetricInterval(); } @Override @@ -317,6 +393,61 @@ public HttpStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) this.openTelemetry = openTelemetry; return this; } + + /** + * Enable or disable OpenTelemetry client metrics. + * + * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public HttpStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics) { + this.enableOtelMetrics = enableOtelMetrics; + return this; + } + + /** + * Enable or disable OpenTelemetry debug client metrics. + * + * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public HttpStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDebugMetrics) { + this.enableOtelDebugMetrics = enableOtelDebugMetrics; + return this; + } + + /** + * Set a custom {@link MeterProvider} for recording client metrics. + * + * @param meterProvider custom MeterProvider to use + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public HttpStorageOptions.Builder setMeterProvider(MeterProvider meterProvider) { + this.meterProvider = meterProvider; + return this; + } + + /** + * Set the metric export interval for periodic metric reading. + * + * @param metricInterval interval duration + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + @Override + public HttpStorageOptions.Builder setMetricInterval(Duration metricInterval) { + checkArgument( + metricInterval != null && !metricInterval.isNegative() && !metricInterval.isZero(), + "metricInterval must be positive"); + this.metricInterval = metricInterval; + return this; + } } public static final class HttpStorageDefaults extends StorageDefaults { diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java new file mode 100644 index 000000000000..df1a505a9bf4 --- /dev/null +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java @@ -0,0 +1,87 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import com.google.common.annotations.VisibleForTesting; +import java.util.function.Function; + +final class StorageMetricsConfig { + static final String SYS_PROP_ENABLE_OTEL_METRICS = "com.google.cloud.storage.enable_otel_metrics"; + static final String SYS_PROP_ENABLE_OTEL_DEBUG_METRICS = + "com.google.cloud.storage.enable_otel_debug_metrics"; + static final String ENV_ENABLE_OTEL_METRICS_JAVA = "GCP_STORAGE_JAVA_ENABLE_OTEL_METRICS"; + static final String ENV_ENABLE_OTEL_METRICS_FALLBACK = "GCP_STORAGE_ENABLE_OTEL_METRICS"; + static final String ENV_ENABLE_OTEL_DEBUG_METRICS = "GCP_STORAGE_JAVA_ENABLE_OTEL_DEBUG_METRICS"; + + private static Function sysPropResolver = System::getProperty; + private static Function envResolver = System::getenv; + + private StorageMetricsConfig() {} + + static boolean isEnableOtelMetrics() { + return isEnableOtelMetrics(sysPropResolver, envResolver); + } + + static boolean isEnableOtelDebugMetrics() { + return isEnableOtelDebugMetrics(sysPropResolver, envResolver); + } + + @VisibleForTesting + static boolean isEnableOtelMetrics( + Function sysProps, Function env) { + String sysProp = sysProps.apply(SYS_PROP_ENABLE_OTEL_METRICS); + if (sysProp != null) { + return Boolean.parseBoolean(sysProp); + } + String javaEnv = env.apply(ENV_ENABLE_OTEL_METRICS_JAVA); + if (javaEnv != null) { + return Boolean.parseBoolean(javaEnv); + } + String fallbackEnv = env.apply(ENV_ENABLE_OTEL_METRICS_FALLBACK); + if (fallbackEnv != null) { + return Boolean.parseBoolean(fallbackEnv); + } + return false; + } + + @VisibleForTesting + static boolean isEnableOtelDebugMetrics( + Function sysProps, Function env) { + String sysProp = sysProps.apply(SYS_PROP_ENABLE_OTEL_DEBUG_METRICS); + if (sysProp != null) { + return Boolean.parseBoolean(sysProp); + } + String javaEnv = env.apply(ENV_ENABLE_OTEL_DEBUG_METRICS); + if (javaEnv != null) { + return Boolean.parseBoolean(javaEnv); + } + return false; + } + + @VisibleForTesting + static void setResolversForTesting( + Function testSysProps, Function testEnv) { + sysPropResolver = testSysProps != null ? testSysProps : System::getProperty; + envResolver = testEnv != null ? testEnv : System::getenv; + } + + @VisibleForTesting + static void resetResolversForTesting() { + sysPropResolver = System::getProperty; + envResolver = System::getenv; + } +} diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 97eecedaeef1..791c1b47f737 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -33,11 +33,14 @@ import com.google.cloud.storage.TransportCompatibility.Transport; import com.google.cloud.storage.spi.StorageRpcFactory; import io.opentelemetry.api.OpenTelemetry; +import io.opentelemetry.api.metrics.MeterProvider; import java.io.IOException; import java.io.InputStream; +import java.time.Duration; import java.util.Locale; import java.util.Properties; import org.checkerframework.checker.nullness.qual.NonNull; +import org.checkerframework.checker.nullness.qual.Nullable; public abstract class StorageOptions extends ServiceOptions { @@ -143,6 +146,43 @@ public abstract StorageOptions.Builder setBlobWriteSessionConfig( @BetaApi public abstract StorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry); + /** + * Enable or disable OpenTelemetry client metrics. + * + * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract StorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics); + + /** + * Enable or disable OpenTelemetry debug client metrics. + * + * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract StorageOptions.Builder setEnableOtelDebugMetrics( + boolean enableOtelDebugMetrics); + + /** + * Set a custom {@link MeterProvider} for recording client metrics. + * + * @param meterProvider custom MeterProvider to use + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract StorageOptions.Builder setMeterProvider(MeterProvider meterProvider); + + /** + * Set the metric export interval for periodic metric reading. + * + * @param metricInterval interval duration + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract StorageOptions.Builder setMetricInterval(Duration metricInterval); + @Override public abstract StorageOptions build(); } @@ -185,6 +225,38 @@ public static String version() { @BetaApi public abstract OpenTelemetry getOpenTelemetry(); + /** + * Whether OpenTelemetry client metrics are enabled. + * + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract boolean isEnableOtelMetrics(); + + /** + * Whether OpenTelemetry debug client metrics are enabled. + * + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract boolean isEnableOtelDebugMetrics(); + + /** + * The {@link MeterProvider} configured for recording client metrics. + * + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract @Nullable MeterProvider getMeterProvider(); + + /** + * The metric export interval configured for periodic metric reading. + * + * @since 2.50.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public abstract Duration getMetricInterval(); + @SuppressWarnings("unchecked") @Override public abstract StorageOptions.Builder toBuilder(); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java new file mode 100644 index 000000000000..f7e83d5c2158 --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -0,0 +1,242 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import io.opentelemetry.api.metrics.MeterProvider; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; +import java.time.Duration; +import java.util.HashMap; +import java.util.Map; +import org.junit.After; +import org.junit.Test; + +public final class StorageOptionsTest { + + @After + public void tearDown() { + System.clearProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS); + System.clearProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_DEBUG_METRICS); + StorageMetricsConfig.resetResolversForTesting(); + } + + @Test + public void defaultState_metricsDisabled() { + HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); + assertThat(httpOptions.isEnableOtelMetrics()).isFalse(); + assertThat(httpOptions.isEnableOtelDebugMetrics()).isFalse(); + assertThat(httpOptions.getMeterProvider()) + .isEqualTo(httpOptions.getOpenTelemetry().getMeterProvider()); + assertThat(httpOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + + GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); + assertThat(grpcOptions.isEnableOtelMetrics()).isFalse(); + assertThat(grpcOptions.isEnableOtelDebugMetrics()).isFalse(); + assertThat(grpcOptions.getMeterProvider()) + .isEqualTo(grpcOptions.getOpenTelemetry().getMeterProvider()); + assertThat(grpcOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + } + + @Test + public void builder_explicitEnabling() { + MeterProvider mockMeterProvider = SdkMeterProvider.builder().build(); + Duration interval = Duration.ofSeconds(30); + + HttpStorageOptions httpOptions = + HttpStorageOptions.http() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .setMeterProvider(mockMeterProvider) + .setMetricInterval(interval) + .build(); + assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); + assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); + assertThat(httpOptions.getMeterProvider()).isSameInstanceAs(mockMeterProvider); + assertThat(httpOptions.getMetricInterval()).isEqualTo(interval); + + HttpStorageOptions rebuiltHttp = httpOptions.toBuilder().build(); + assertThat(rebuiltHttp.isEnableOtelMetrics()).isTrue(); + assertThat(rebuiltHttp.isEnableOtelDebugMetrics()).isTrue(); + assertThat(rebuiltHttp.getMeterProvider()).isSameInstanceAs(mockMeterProvider); + assertThat(rebuiltHttp.getMetricInterval()).isEqualTo(interval); + + GrpcStorageOptions grpcOptions = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .setMeterProvider(mockMeterProvider) + .setMetricInterval(interval) + .build(); + assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); + assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); + assertThat(grpcOptions.getMeterProvider()).isSameInstanceAs(mockMeterProvider); + assertThat(grpcOptions.getMetricInterval()).isEqualTo(interval); + + GrpcStorageOptions rebuiltGrpc = grpcOptions.toBuilder().build(); + assertThat(rebuiltGrpc.isEnableOtelMetrics()).isTrue(); + assertThat(rebuiltGrpc.isEnableOtelDebugMetrics()).isTrue(); + assertThat(rebuiltGrpc.getMeterProvider()).isSameInstanceAs(mockMeterProvider); + assertThat(rebuiltGrpc.getMetricInterval()).isEqualTo(interval); + } + + @Test + public void builder_explicitDisablingOverridesGate() { + System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "true"); + + HttpStorageOptions httpOptions = HttpStorageOptions.http().setEnableOtelMetrics(false).build(); + assertThat(httpOptions.isEnableOtelMetrics()).isFalse(); + + GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().setEnableOtelMetrics(false).build(); + assertThat(grpcOptions.isEnableOtelMetrics()).isFalse(); + } + + @Test + public void developmentGate_systemProperty() { + System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "true"); + System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_DEBUG_METRICS, "true"); + + HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); + assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); + assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); + + GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); + assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); + assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); + } + + @Test + public void developmentGate_environmentVariableJava() { + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + StorageMetricsConfig.setResolversForTesting(k -> null, env::get); + + HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); + assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); + assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); + + GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); + assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); + assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); + } + + @Test + public void developmentGate_environmentVariableFallback() { + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_FALLBACK, "true"); + StorageMetricsConfig.setResolversForTesting(k -> null, env::get); + + HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); + assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); + + GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); + assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); + } + + @Test + public void developmentGate_systemPropertyTakesPrecedenceOverEnv() { + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + Map sysProps = new HashMap<>(); + sysProps.put(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "false"); + StorageMetricsConfig.setResolversForTesting(sysProps::get, env::get); + + HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); + assertThat(httpOptions.isEnableOtelMetrics()).isFalse(); + + GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); + assertThat(grpcOptions.isEnableOtelMetrics()).isFalse(); + } + + @Test + public void setMetricInterval_validation() { + assertThrows( + IllegalArgumentException.class, + () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(-1))); + assertThrows( + IllegalArgumentException.class, + () -> HttpStorageOptions.http().setMetricInterval(Duration.ZERO)); + assertThrows( + IllegalArgumentException.class, () -> HttpStorageOptions.http().setMetricInterval(null)); + + assertThrows( + IllegalArgumentException.class, + () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(-1))); + assertThrows( + IllegalArgumentException.class, + () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ZERO)); + assertThrows( + IllegalArgumentException.class, () -> GrpcStorageOptions.grpc().setMetricInterval(null)); + } + + @Test + public void serializationRoundTrip_preservesMeterProvider() throws Exception { + MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); + HttpStorageOptions httpOptions = + HttpStorageOptions.http() + .setEnableOtelMetrics(true) + .setMeterProvider(customMeterProvider) + .build(); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(httpOptions); + } + + HttpStorageOptions deserializedHttp; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserializedHttp = (HttpStorageOptions) ois.readObject(); + } + + assertThat(deserializedHttp).isNotNull(); + assertThat(deserializedHttp.isEnableOtelMetrics()).isTrue(); + // Transient meterProvider falls back to openTelemetry.getMeterProvider() after deserialization + assertThat(deserializedHttp.getMeterProvider()).isNotNull(); + assertThat(deserializedHttp.getMeterProvider()) + .isEqualTo(deserializedHttp.getOpenTelemetry().getMeterProvider()); + + GrpcStorageOptions grpcOptions = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(true) + .setMeterProvider(customMeterProvider) + .build(); + + baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(grpcOptions); + } + + GrpcStorageOptions deserializedGrpc; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserializedGrpc = (GrpcStorageOptions) ois.readObject(); + } + + assertThat(deserializedGrpc).isNotNull(); + assertThat(deserializedGrpc.isEnableOtelMetrics()).isTrue(); + assertThat(deserializedGrpc.getMeterProvider()).isNotNull(); + assertThat(deserializedGrpc.getMeterProvider()) + .isEqualTo(deserializedGrpc.getOpenTelemetry().getMeterProvider()); + } +} From 07dcbb1a20078c2394b4beacf558cdcd45d93c1d Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Wed, 23 Sep 2026 10:56:59 +0000 Subject: [PATCH 02/10] fix(storage): preserve meterProvider equality across toBuilder and serialization [Generated-by: AI] --- .../java/com/google/cloud/storage/GrpcStorageOptions.java | 7 +++---- .../java/com/google/cloud/storage/HttpStorageOptions.java | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 02ac9ef28304..db0a4575b518 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -211,7 +211,6 @@ GrpcInterceptorProvider getGrpcInterceptorProvider() { private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { in.defaultReadObject(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); - this.meterProvider = this.openTelemetry.getMeterProvider(); } /** @@ -490,7 +489,7 @@ public int hashCode() { openTelemetry, enableOtelMetrics, enableOtelDebugMetrics, - meterProvider, + getMeterProvider(), metricInterval, baseHashCode()); } @@ -513,7 +512,7 @@ public boolean equals(Object o) { && Objects.equals(grpcInterceptorProvider, that.grpcInterceptorProvider) && Objects.equals(blobWriteSessionConfig, that.blobWriteSessionConfig) && Objects.equals(openTelemetry, that.openTelemetry) - && Objects.equals(meterProvider, that.meterProvider) + && Objects.equals(getMeterProvider(), that.getMeterProvider()) && Objects.equals(metricInterval, that.metricInterval) && this.baseEquals(that); } @@ -586,7 +585,7 @@ public static final class Builder extends StorageOptions.Builder { this.openTelemetry = gso.openTelemetry; this.enableOtelMetrics = gso.isEnableOtelMetrics(); this.enableOtelDebugMetrics = gso.isEnableOtelDebugMetrics(); - this.meterProvider = gso.getMeterProvider(); + this.meterProvider = gso.meterProvider; this.metricInterval = gso.getMetricInterval(); } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index da131282881d..75d69d4d7b07 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -171,7 +171,7 @@ public int hashCode() { openTelemetry, enableOtelMetrics, enableOtelDebugMetrics, - meterProvider, + getMeterProvider(), metricInterval, baseHashCode()); } @@ -190,7 +190,7 @@ public boolean equals(Object o) { && Objects.equals(openTelemetry, that.openTelemetry) && enableOtelMetrics == that.enableOtelMetrics && enableOtelDebugMetrics == that.enableOtelDebugMetrics - && Objects.equals(meterProvider, that.meterProvider) + && Objects.equals(getMeterProvider(), that.getMeterProvider()) && Objects.equals(metricInterval, that.metricInterval) && this.baseEquals(that); } @@ -199,7 +199,6 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE in.defaultReadObject(); this.retryDepsAdapter = new RetryDependenciesAdapter(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); - this.meterProvider = this.openTelemetry.getMeterProvider(); } public static HttpStorageOptions.Builder newBuilder() { @@ -240,7 +239,7 @@ public static class Builder extends StorageOptions.Builder { this.openTelemetry = hso.getOpenTelemetry(); this.enableOtelMetrics = hso.isEnableOtelMetrics(); this.enableOtelDebugMetrics = hso.isEnableOtelDebugMetrics(); - this.meterProvider = hso.getMeterProvider(); + this.meterProvider = hso.meterProvider; this.metricInterval = hso.getMetricInterval(); } From 434948f6528de48d151f66acd67dab39ce8dd923 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Wed, 30 Sep 2026 06:43:50 +0000 Subject: [PATCH 03/10] fix(storage): add deserialization guard for metricInterval and update options - In GrpcStorageOptions and HttpStorageOptions: - Add null check in getMetricInterval() defaulting to 60 seconds - Use getMetricInterval() in equals() and hashCode() - Update Javadoc for setMeterProvider to document nullable/default behavior - In StorageOptions: - Update Javadoc for setMeterProvider on Builder - In StorageMetricsConfig: - Make sysPropResolver and envResolver volatile for thread safety - In StorageOptionsTest: - Add unit test verifying deserialization with null metricInterval defaults to 60s and equals/hashCode match - Verify explicit client options take precedence over env vars and sysprops [Generated-by: AI] --- .../cloud/storage/GrpcStorageOptions.java | 10 +-- .../cloud/storage/HttpStorageOptions.java | 10 +-- .../cloud/storage/StorageMetricsConfig.java | 4 +- .../google/cloud/storage/StorageOptions.java | 4 +- .../cloud/storage/StorageOptionsTest.java | 78 +++++++++++++++++-- 5 files changed, 87 insertions(+), 19 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index db0a4575b518..777500351133 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -466,7 +466,7 @@ public boolean isEnableOtelDebugMetrics() { @BetaApi @Override public Duration getMetricInterval() { - return metricInterval; + return metricInterval != null ? metricInterval : Duration.ofSeconds(60); } /** @@ -490,7 +490,7 @@ public int hashCode() { enableOtelMetrics, enableOtelDebugMetrics, getMeterProvider(), - metricInterval, + getMetricInterval(), baseHashCode()); } @@ -513,7 +513,7 @@ public boolean equals(Object o) { && Objects.equals(blobWriteSessionConfig, that.blobWriteSessionConfig) && Objects.equals(openTelemetry, that.openTelemetry) && Objects.equals(getMeterProvider(), that.getMeterProvider()) - && Objects.equals(metricInterval, that.metricInterval) + && Objects.equals(getMetricInterval(), that.getMetricInterval()) && this.baseEquals(that); } @@ -848,12 +848,12 @@ public GrpcStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDe /** * Set a custom {@link MeterProvider} for recording client metrics. * - * @param meterProvider custom MeterProvider to use + * @param meterProvider custom MeterProvider to use, or null to use the default * @since 2.50.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override - public GrpcStorageOptions.Builder setMeterProvider(MeterProvider meterProvider) { + public GrpcStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider) { this.meterProvider = meterProvider; return this; } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index 75d69d4d7b07..8a401e12e3e1 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -155,7 +155,7 @@ public boolean isEnableOtelDebugMetrics() { @BetaApi @Override public Duration getMetricInterval() { - return metricInterval; + return metricInterval != null ? metricInterval : Duration.ofSeconds(60); } @Override @@ -172,7 +172,7 @@ public int hashCode() { enableOtelMetrics, enableOtelDebugMetrics, getMeterProvider(), - metricInterval, + getMetricInterval(), baseHashCode()); } @@ -191,7 +191,7 @@ public boolean equals(Object o) { && enableOtelMetrics == that.enableOtelMetrics && enableOtelDebugMetrics == that.enableOtelDebugMetrics && Objects.equals(getMeterProvider(), that.getMeterProvider()) - && Objects.equals(metricInterval, that.metricInterval) + && Objects.equals(getMetricInterval(), that.getMetricInterval()) && this.baseEquals(that); } @@ -422,12 +422,12 @@ public HttpStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDe /** * Set a custom {@link MeterProvider} for recording client metrics. * - * @param meterProvider custom MeterProvider to use + * @param meterProvider custom MeterProvider to use, or null to use the default * @since 2.50.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override - public HttpStorageOptions.Builder setMeterProvider(MeterProvider meterProvider) { + public HttpStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider) { this.meterProvider = meterProvider; return this; } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java index df1a505a9bf4..2e7e5859a9a2 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java @@ -27,8 +27,8 @@ final class StorageMetricsConfig { static final String ENV_ENABLE_OTEL_METRICS_FALLBACK = "GCP_STORAGE_ENABLE_OTEL_METRICS"; static final String ENV_ENABLE_OTEL_DEBUG_METRICS = "GCP_STORAGE_JAVA_ENABLE_OTEL_DEBUG_METRICS"; - private static Function sysPropResolver = System::getProperty; - private static Function envResolver = System::getenv; + private static volatile Function sysPropResolver = System::getProperty; + private static volatile Function envResolver = System::getenv; private StorageMetricsConfig() {} diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 791c1b47f737..f578109b4f0d 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -168,11 +168,11 @@ public abstract StorageOptions.Builder setEnableOtelDebugMetrics( /** * Set a custom {@link MeterProvider} for recording client metrics. * - * @param meterProvider custom MeterProvider to use + * @param meterProvider custom MeterProvider to use, or null to use the default * @since 2.50.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract StorageOptions.Builder setMeterProvider(MeterProvider meterProvider); + public abstract StorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider); /** * Set the metric export interval for periodic metric reading. diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index f7e83d5c2158..46c934fda911 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -25,6 +25,7 @@ import java.io.ByteArrayOutputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.lang.reflect.Field; import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -100,14 +101,28 @@ public void builder_explicitEnabling() { } @Test - public void builder_explicitDisablingOverridesGate() { + public void builder_explicitOptionsTakePrecedenceOverGate() { + // When sysprop and env are set to true, explicit builder false takes precedence System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "true"); + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + StorageMetricsConfig.setResolversForTesting(System::getProperty, env::get); - HttpStorageOptions httpOptions = HttpStorageOptions.http().setEnableOtelMetrics(false).build(); - assertThat(httpOptions.isEnableOtelMetrics()).isFalse(); + HttpStorageOptions httpDisabled = HttpStorageOptions.http().setEnableOtelMetrics(false).build(); + assertThat(httpDisabled.isEnableOtelMetrics()).isFalse(); - GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().setEnableOtelMetrics(false).build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isFalse(); + GrpcStorageOptions grpcDisabled = GrpcStorageOptions.grpc().setEnableOtelMetrics(false).build(); + assertThat(grpcDisabled.isEnableOtelMetrics()).isFalse(); + + // When sysprop and env are set to false, explicit builder true takes precedence + System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "false"); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); + + HttpStorageOptions httpEnabled = HttpStorageOptions.http().setEnableOtelMetrics(true).build(); + assertThat(httpEnabled.isEnableOtelMetrics()).isTrue(); + + GrpcStorageOptions grpcEnabled = GrpcStorageOptions.grpc().setEnableOtelMetrics(true).build(); + assertThat(grpcEnabled.isEnableOtelMetrics()).isTrue(); } @Test @@ -239,4 +254,57 @@ public void serializationRoundTrip_preservesMeterProvider() throws Exception { assertThat(deserializedGrpc.getMeterProvider()) .isEqualTo(deserializedGrpc.getOpenTelemetry().getMeterProvider()); } + + @Test + public void deserializationWithNullMetricInterval_defaultsToSixtySeconds() throws Exception { + Field httpBuilderField = HttpStorageOptions.Builder.class.getDeclaredField("metricInterval"); + httpBuilderField.setAccessible(true); + HttpStorageOptions.Builder httpBuilder = HttpStorageOptions.http(); + httpBuilderField.set(httpBuilder, null); + HttpStorageOptions httpWithNullInterval = httpBuilder.build(); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(httpWithNullInterval); + } + + HttpStorageOptions deserializedHttp; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserializedHttp = (HttpStorageOptions) ois.readObject(); + } + + assertThat(deserializedHttp).isNotNull(); + // Verify NPE guard defaults to 60 seconds + assertThat(deserializedHttp.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + // Verify equals and hashCode match a default instance + HttpStorageOptions defaultHttp = HttpStorageOptions.http().build(); + assertThat(deserializedHttp).isEqualTo(defaultHttp); + assertThat(defaultHttp).isEqualTo(deserializedHttp); + assertThat(deserializedHttp.hashCode()).isEqualTo(defaultHttp.hashCode()); + + Field grpcBuilderField = GrpcStorageOptions.Builder.class.getDeclaredField("metricInterval"); + grpcBuilderField.setAccessible(true); + GrpcStorageOptions.Builder grpcBuilder = GrpcStorageOptions.grpc(); + grpcBuilderField.set(grpcBuilder, null); + GrpcStorageOptions grpcWithNullInterval = grpcBuilder.build(); + + baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(grpcWithNullInterval); + } + + GrpcStorageOptions deserializedGrpc; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserializedGrpc = (GrpcStorageOptions) ois.readObject(); + } + + assertThat(deserializedGrpc).isNotNull(); + assertThat(deserializedGrpc.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + GrpcStorageOptions defaultGrpc = GrpcStorageOptions.grpc().build(); + assertThat(deserializedGrpc).isEqualTo(defaultGrpc); + assertThat(defaultGrpc).isEqualTo(deserializedGrpc); + assertThat(deserializedGrpc.hashCode()).isEqualTo(defaultGrpc.hashCode()); + } } From 327b558fe25ca6bc126f266eb4ffc6159fb11aab Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 8 Oct 2026 06:59:01 +0000 Subject: [PATCH 04/10] fix(storage): address PR review comments for otel metrics config [Generated-by: AI] --- .../cloud/storage/GrpcStorageOptions.java | 34 ++- .../cloud/storage/HttpStorageOptions.java | 33 ++- .../cloud/storage/StorageMetricsConfig.java | 84 ++++--- .../google/cloud/storage/StorageOptions.java | 4 + .../cloud/storage/StorageOptionsTest.java | 208 ++++++++++++++---- 5 files changed, 270 insertions(+), 93 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 777500351133..19f075a9905e 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -133,6 +133,7 @@ public final class GrpcStorageOptions extends StorageOptions implements Retrying.RetryingDependencies { private static final long serialVersionUID = -4499446543857945349L; + private static final Logger LOGGER = Logger.getLogger(GrpcStorageOptions.class.getName()); private static final String GCS_SCOPE = "https://www.googleapis.com/auth/devstorage.full_control"; private static final Set SCOPES = ImmutableSet.of(GCS_SCOPE); private static final String DEFAULT_HOST = "https://storage.googleapis.com"; @@ -142,7 +143,7 @@ public final class GrpcStorageOptions extends StorageOptions System.getProperty("com.google.cloud.storage.grpc.bound_token", "false")); private final GrpcRetryAlgorithmManager retryAlgorithmManager; - private final Duration terminationAwaitDuration; + private final java.time.Duration terminationAwaitDuration; private final boolean attemptDirectPath; private final boolean enableGrpcClientMetrics; @@ -152,8 +153,11 @@ public final class GrpcStorageOptions extends StorageOptions private transient OpenTelemetry openTelemetry; private final boolean enableOtelMetrics; private final boolean enableOtelDebugMetrics; + @Nullable private final Boolean rawEnableOtelMetrics; + @Nullable private final Boolean rawEnableOtelDebugMetrics; private transient MeterProvider meterProvider; private final Duration metricInterval; + private final boolean customMeterProviderConfigured; private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) { super(builder, serviceDefaults); @@ -171,16 +175,20 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) this.grpcInterceptorProvider = builder.grpcInterceptorProvider; this.blobWriteSessionConfig = builder.blobWriteSessionConfig; this.openTelemetry = builder.openTelemetry; + this.rawEnableOtelMetrics = builder.enableOtelMetrics; + this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; this.enableOtelMetrics = - builder.enableOtelMetrics != null - ? builder.enableOtelMetrics - : StorageMetricsConfig.isEnableOtelMetrics(); + StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); this.enableOtelDebugMetrics = - builder.enableOtelDebugMetrics != null - ? builder.enableOtelDebugMetrics - : StorageMetricsConfig.isEnableOtelDebugMetrics(); + StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); this.meterProvider = builder.meterProvider; + this.customMeterProviderConfigured = builder.meterProvider != null; this.metricInterval = builder.metricInterval; + if (this.meterProvider == null && this.metricInterval != null) { + checkArgument( + this.metricInterval.compareTo(Duration.ofSeconds(60)) >= 0, + "Metric export interval cannot be less than 60 seconds when using Cloud Monitoring exporter"); + } } @Override @@ -211,6 +219,10 @@ GrpcInterceptorProvider getGrpcInterceptorProvider() { private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { in.defaultReadObject(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); + if (this.customMeterProviderConfigured || this.meterProvider != null) { + LOGGER.warning( + "Custom MeterProvider is transient and cannot be serialized across workers; falling back to default provider."); + } } /** @@ -583,8 +595,8 @@ public static final class Builder extends StorageOptions.Builder { this.grpcInterceptorProvider = gso.grpcInterceptorProvider; this.blobWriteSessionConfig = gso.blobWriteSessionConfig; this.openTelemetry = gso.openTelemetry; - this.enableOtelMetrics = gso.isEnableOtelMetrics(); - this.enableOtelDebugMetrics = gso.isEnableOtelDebugMetrics(); + this.enableOtelMetrics = gso.rawEnableOtelMetrics; + this.enableOtelDebugMetrics = gso.rawEnableOtelDebugMetrics; this.meterProvider = gso.meterProvider; this.metricInterval = gso.getMetricInterval(); } @@ -861,6 +873,10 @@ public GrpcStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter /** * Set the metric export interval for periodic metric reading. * + *

When using the default Cloud Monitoring exporter, the interval must be at least 60 + * seconds (1 minute). Intervals less than 60 seconds are permitted only when a custom + * {@link MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + * * @param metricInterval interval duration * @since 2.50.0 This new api is in preview and is subject to breaking changes. */ diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index 8a401e12e3e1..114a4c865f38 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -50,6 +50,7 @@ import java.time.Duration; import java.util.Objects; import java.util.Set; +import java.util.logging.Logger; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -61,6 +62,7 @@ public class HttpStorageOptions extends StorageOptions { private static final long serialVersionUID = -5302637952911052045L; + private static final Logger LOGGER = Logger.getLogger(HttpStorageOptions.class.getName()); private static final String API_SHORT_NAME = "Storage"; private static final String GCS_SCOPE = "https://www.googleapis.com/auth/devstorage.full_control"; private static final Set SCOPES = ImmutableSet.of(GCS_SCOPE); @@ -71,8 +73,11 @@ public class HttpStorageOptions extends StorageOptions { private transient OpenTelemetry openTelemetry; private final boolean enableOtelMetrics; private final boolean enableOtelDebugMetrics; + @Nullable private final Boolean rawEnableOtelMetrics; + @Nullable private final Boolean rawEnableOtelDebugMetrics; private transient MeterProvider meterProvider; private final Duration metricInterval; + private final boolean customMeterProviderConfigured; private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { super(builder, serviceDefaults); @@ -83,16 +88,20 @@ private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { retryDepsAdapter = new RetryDependenciesAdapter(); blobWriteSessionConfig = builder.blobWriteSessionConfig; openTelemetry = builder.openTelemetry; + this.rawEnableOtelMetrics = builder.enableOtelMetrics; + this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; this.enableOtelMetrics = - builder.enableOtelMetrics != null - ? builder.enableOtelMetrics - : StorageMetricsConfig.isEnableOtelMetrics(); + StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); this.enableOtelDebugMetrics = - builder.enableOtelDebugMetrics != null - ? builder.enableOtelDebugMetrics - : StorageMetricsConfig.isEnableOtelDebugMetrics(); + StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); this.meterProvider = builder.meterProvider; + this.customMeterProviderConfigured = builder.meterProvider != null; this.metricInterval = builder.metricInterval; + if (this.meterProvider == null && this.metricInterval != null) { + checkArgument( + this.metricInterval.compareTo(Duration.ofSeconds(60)) >= 0, + "Metric export interval cannot be less than 60 seconds when using Cloud Monitoring exporter"); + } } @Override @@ -199,6 +208,10 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE in.defaultReadObject(); this.retryDepsAdapter = new RetryDependenciesAdapter(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); + if (this.customMeterProviderConfigured || this.meterProvider != null) { + LOGGER.warning( + "Custom MeterProvider is transient and cannot be serialized across workers; falling back to default provider."); + } } public static HttpStorageOptions.Builder newBuilder() { @@ -237,8 +250,8 @@ public static class Builder extends StorageOptions.Builder { this.storageRetryStrategy = hso.retryAlgorithmManager.retryStrategy; this.blobWriteSessionConfig = hso.blobWriteSessionConfig; this.openTelemetry = hso.getOpenTelemetry(); - this.enableOtelMetrics = hso.isEnableOtelMetrics(); - this.enableOtelDebugMetrics = hso.isEnableOtelDebugMetrics(); + this.enableOtelMetrics = hso.rawEnableOtelMetrics; + this.enableOtelDebugMetrics = hso.rawEnableOtelDebugMetrics; this.meterProvider = hso.meterProvider; this.metricInterval = hso.getMetricInterval(); } @@ -435,6 +448,10 @@ public HttpStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter /** * Set the metric export interval for periodic metric reading. * + *

When using the default Cloud Monitoring exporter, the interval must be at least 60 + * seconds (1 minute). Intervals less than 60 seconds are permitted only when a custom + * {@link MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + * * @param metricInterval interval duration * @since 2.50.0 This new api is in preview and is subject to breaking changes. */ diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java index 2e7e5859a9a2..1fbc3801d903 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java @@ -17,71 +17,93 @@ package com.google.cloud.storage; import com.google.common.annotations.VisibleForTesting; +import java.util.Locale; import java.util.function.Function; +import java.util.logging.Logger; +import org.checkerframework.checker.nullness.qual.Nullable; final class StorageMetricsConfig { - static final String SYS_PROP_ENABLE_OTEL_METRICS = "com.google.cloud.storage.enable_otel_metrics"; - static final String SYS_PROP_ENABLE_OTEL_DEBUG_METRICS = - "com.google.cloud.storage.enable_otel_debug_metrics"; + private static final Logger LOGGER = Logger.getLogger(StorageMetricsConfig.class.getName()); + static final String ENV_ENABLE_OTEL_METRICS_JAVA = "GCP_STORAGE_JAVA_ENABLE_OTEL_METRICS"; - static final String ENV_ENABLE_OTEL_METRICS_FALLBACK = "GCP_STORAGE_ENABLE_OTEL_METRICS"; static final String ENV_ENABLE_OTEL_DEBUG_METRICS = "GCP_STORAGE_JAVA_ENABLE_OTEL_DEBUG_METRICS"; - private static volatile Function sysPropResolver = System::getProperty; private static volatile Function envResolver = System::getenv; private StorageMetricsConfig() {} static boolean isEnableOtelMetrics() { - return isEnableOtelMetrics(sysPropResolver, envResolver); + return isEnableOtelMetrics(null); } static boolean isEnableOtelDebugMetrics() { - return isEnableOtelDebugMetrics(sysPropResolver, envResolver); + return isEnableOtelDebugMetrics(null); + } + + static boolean isEnableOtelMetrics(@Nullable Boolean builderSetting) { + return isEnableOtelMetrics(envResolver, builderSetting); + } + + static boolean isEnableOtelDebugMetrics(@Nullable Boolean builderSetting) { + return isEnableOtelDebugMetrics(envResolver, builderSetting); } @VisibleForTesting static boolean isEnableOtelMetrics( - Function sysProps, Function env) { - String sysProp = sysProps.apply(SYS_PROP_ENABLE_OTEL_METRICS); - if (sysProp != null) { - return Boolean.parseBoolean(sysProp); - } + Function env, @Nullable Boolean builderSetting) { String javaEnv = env.apply(ENV_ENABLE_OTEL_METRICS_JAVA); - if (javaEnv != null) { - return Boolean.parseBoolean(javaEnv); - } - String fallbackEnv = env.apply(ENV_ENABLE_OTEL_METRICS_FALLBACK); - if (fallbackEnv != null) { - return Boolean.parseBoolean(fallbackEnv); + Boolean parsed = parseBooleanValue(javaEnv, ENV_ENABLE_OTEL_METRICS_JAVA); + if (parsed != null) { + return parsed; } - return false; + return builderSetting != null ? builderSetting : false; } @VisibleForTesting static boolean isEnableOtelDebugMetrics( - Function sysProps, Function env) { - String sysProp = sysProps.apply(SYS_PROP_ENABLE_OTEL_DEBUG_METRICS); - if (sysProp != null) { - return Boolean.parseBoolean(sysProp); - } + Function env, @Nullable Boolean builderSetting) { String javaEnv = env.apply(ENV_ENABLE_OTEL_DEBUG_METRICS); - if (javaEnv != null) { - return Boolean.parseBoolean(javaEnv); + Boolean parsed = parseBooleanValue(javaEnv, ENV_ENABLE_OTEL_DEBUG_METRICS); + if (parsed != null) { + return parsed; + } + return builderSetting != null ? builderSetting : false; + } + + @Nullable + @VisibleForTesting + static Boolean parseBooleanValue(@Nullable String val, String varName) { + if (val == null) { + return null; + } + String normalized = val.trim().toLowerCase(Locale.ENGLISH); + switch (normalized) { + case "1": + case "t": + case "true": + return Boolean.TRUE; + case "0": + case "f": + case "false": + return Boolean.FALSE; + default: + LOGGER.warning( + "Unrecognized boolean value '" + + val + + "' for " + + varName + + "; ignoring and falling back to builder/default"); + return null; } - return false; } @VisibleForTesting - static void setResolversForTesting( - Function testSysProps, Function testEnv) { - sysPropResolver = testSysProps != null ? testSysProps : System::getProperty; + static void setEnvResolverForTesting(Function testEnv) { envResolver = testEnv != null ? testEnv : System::getenv; } @VisibleForTesting static void resetResolversForTesting() { - sysPropResolver = System::getProperty; envResolver = System::getenv; } } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index f578109b4f0d..82ab77a5238b 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -177,6 +177,10 @@ public abstract StorageOptions.Builder setEnableOtelDebugMetrics( /** * Set the metric export interval for periodic metric reading. * + *

When using the default Cloud Monitoring exporter, the interval must be at least 60 + * seconds (1 minute). Intervals less than 60 seconds are permitted only when a custom + * {@link MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + * * @param metricInterval interval duration * @since 2.50.0 This new api is in preview and is subject to breaking changes. */ diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index 46c934fda911..f56c707ae745 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -36,8 +36,6 @@ public final class StorageOptionsTest { @After public void tearDown() { - System.clearProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS); - System.clearProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_DEBUG_METRICS); StorageMetricsConfig.resetResolversForTesting(); } @@ -101,90 +99,169 @@ public void builder_explicitEnabling() { } @Test - public void builder_explicitOptionsTakePrecedenceOverGate() { - // When sysprop and env are set to true, explicit builder false takes precedence - System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "true"); + public void precedence_envTakesPrecedenceOverBuilder() { Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); - StorageMetricsConfig.setResolversForTesting(System::getProperty, env::get); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + StorageMetricsConfig.setEnvResolverForTesting(env::get); - HttpStorageOptions httpDisabled = HttpStorageOptions.http().setEnableOtelMetrics(false).build(); - assertThat(httpDisabled.isEnableOtelMetrics()).isFalse(); + HttpStorageOptions httpDisabled = + HttpStorageOptions.http() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(false) + .build(); + assertThat(httpDisabled.isEnableOtelMetrics()).isTrue(); + assertThat(httpDisabled.isEnableOtelDebugMetrics()).isTrue(); - GrpcStorageOptions grpcDisabled = GrpcStorageOptions.grpc().setEnableOtelMetrics(false).build(); - assertThat(grpcDisabled.isEnableOtelMetrics()).isFalse(); + GrpcStorageOptions grpcDisabled = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(false) + .build(); + assertThat(grpcDisabled.isEnableOtelMetrics()).isTrue(); + assertThat(grpcDisabled.isEnableOtelDebugMetrics()).isTrue(); - // When sysprop and env are set to false, explicit builder true takes precedence - System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "false"); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "false"); + + HttpStorageOptions httpEnabled = + HttpStorageOptions.http() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(httpEnabled.isEnableOtelMetrics()).isFalse(); + assertThat(httpEnabled.isEnableOtelDebugMetrics()).isFalse(); - HttpStorageOptions httpEnabled = HttpStorageOptions.http().setEnableOtelMetrics(true).build(); + GrpcStorageOptions grpcEnabled = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(grpcEnabled.isEnableOtelMetrics()).isFalse(); + assertThat(grpcEnabled.isEnableOtelDebugMetrics()).isFalse(); + } + + @Test + public void precedence_builderTakesPrecedenceOverDefaultWhenEnvUnset() { + Map env = new HashMap<>(); + StorageMetricsConfig.setEnvResolverForTesting(env::get); + + HttpStorageOptions httpEnabled = + HttpStorageOptions.http() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); assertThat(httpEnabled.isEnableOtelMetrics()).isTrue(); + assertThat(httpEnabled.isEnableOtelDebugMetrics()).isTrue(); - GrpcStorageOptions grpcEnabled = GrpcStorageOptions.grpc().setEnableOtelMetrics(true).build(); + GrpcStorageOptions grpcEnabled = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); assertThat(grpcEnabled.isEnableOtelMetrics()).isTrue(); + assertThat(grpcEnabled.isEnableOtelDebugMetrics()).isTrue(); + + HttpStorageOptions httpDisabled = + HttpStorageOptions.http() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(false) + .build(); + assertThat(httpDisabled.isEnableOtelMetrics()).isFalse(); + assertThat(httpDisabled.isEnableOtelDebugMetrics()).isFalse(); + + GrpcStorageOptions grpcDisabled = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(false) + .build(); + assertThat(grpcDisabled.isEnableOtelMetrics()).isFalse(); + assertThat(grpcDisabled.isEnableOtelDebugMetrics()).isFalse(); } @Test - public void developmentGate_systemProperty() { - System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "true"); - System.setProperty(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_DEBUG_METRICS, "true"); + public void permissiveBooleanParsing() { + String[] trueValues = {"1", "t", "T", "true", "TRUE", "True", " true "}; + for (String val : trueValues) { + assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isTrue(); + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isTrue(); + } - HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); - assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); - assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); + String[] falseValues = {"0", "f", "F", "false", "FALSE", "False", " false "}; + for (String val : falseValues) { + assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isFalse(); + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isFalse(); + } - GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); - assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); + String[] invalidValues = {"invalid", "2", "yes", "no", "enabled", ""}; + for (String val : invalidValues) { + assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isNull(); + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); + // Unrecognized values fall back to builder setting + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); + // When builder setting is null, falls back to default false + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); + } + + assertThat(StorageMetricsConfig.parseBooleanValue(null, "TEST_VAR")).isNull(); } @Test - public void developmentGate_environmentVariableJava() { + public void toBuilder_doesNotBakeInEnvironmentVariable() { Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); - StorageMetricsConfig.setResolversForTesting(k -> null, env::get); + StorageMetricsConfig.setEnvResolverForTesting(env::get); HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); + // Clear environment variable and rebuild + env.clear(); + HttpStorageOptions rebuiltHttp = httpOptions.toBuilder().build(); + assertThat(rebuiltHttp.isEnableOtelMetrics()).isFalse(); + assertThat(rebuiltHttp.isEnableOtelDebugMetrics()).isFalse(); + + // Repeat for GrpcStorageOptions + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); - } - - @Test - public void developmentGate_environmentVariableFallback() { - Map env = new HashMap<>(); - env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_FALLBACK, "true"); - StorageMetricsConfig.setResolversForTesting(k -> null, env::get); - HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); - assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); - - GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); + env.clear(); + GrpcStorageOptions rebuiltGrpc = grpcOptions.toBuilder().build(); + assertThat(rebuiltGrpc.isEnableOtelMetrics()).isFalse(); + assertThat(rebuiltGrpc.isEnableOtelDebugMetrics()).isFalse(); } @Test - public void developmentGate_systemPropertyTakesPrecedenceOverEnv() { + public void environmentVariableJava() { Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); - Map sysProps = new HashMap<>(); - sysProps.put(StorageMetricsConfig.SYS_PROP_ENABLE_OTEL_METRICS, "false"); - StorageMetricsConfig.setResolversForTesting(sysProps::get, env::get); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + StorageMetricsConfig.setEnvResolverForTesting(env::get); HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); - assertThat(httpOptions.isEnableOtelMetrics()).isFalse(); + assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); + assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isFalse(); + assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); + assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); } @Test public void setMetricInterval_validation() { + MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); + assertThrows( IllegalArgumentException.class, () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(-1))); @@ -202,10 +279,49 @@ public void setMetricInterval_validation() { () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ZERO)); assertThrows( IllegalArgumentException.class, () -> GrpcStorageOptions.grpc().setMetricInterval(null)); + + // Reject intervals < 60s when meterProvider == null (SDK-owned Cloud Monitoring exporter) + assertThrows( + IllegalArgumentException.class, + () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(59)).build()); + assertThrows( + IllegalArgumentException.class, + () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(30)).build()); + + assertThrows( + IllegalArgumentException.class, + () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(59)).build()); + assertThrows( + IllegalArgumentException.class, + () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(30)).build()); + + // Accept intervals >= 60s when meterProvider == null + HttpStorageOptions http60 = + HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(60)).build(); + assertThat(http60.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + + GrpcStorageOptions grpc60 = + GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(60)).build(); + assertThat(grpc60.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + + // Accept intervals < 60s when custom meterProvider is configured + HttpStorageOptions httpCustom = + HttpStorageOptions.http() + .setMeterProvider(customMeterProvider) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(httpCustom.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + + GrpcStorageOptions grpcCustom = + GrpcStorageOptions.grpc() + .setMeterProvider(customMeterProvider) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(grpcCustom.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); } @Test - public void serializationRoundTrip_preservesMeterProvider() throws Exception { + public void serializationRoundTrip_customMeterProviderDoesNotCarryOver() throws Exception { MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); HttpStorageOptions httpOptions = HttpStorageOptions.http() @@ -226,8 +342,9 @@ public void serializationRoundTrip_preservesMeterProvider() throws Exception { assertThat(deserializedHttp).isNotNull(); assertThat(deserializedHttp.isEnableOtelMetrics()).isTrue(); - // Transient meterProvider falls back to openTelemetry.getMeterProvider() after deserialization + // Transient meterProvider does not carry over; falls back to openTelemetry.getMeterProvider() without NPE assertThat(deserializedHttp.getMeterProvider()).isNotNull(); + assertThat(deserializedHttp.getMeterProvider()).isNotSameInstanceAs(customMeterProvider); assertThat(deserializedHttp.getMeterProvider()) .isEqualTo(deserializedHttp.getOpenTelemetry().getMeterProvider()); @@ -251,6 +368,7 @@ public void serializationRoundTrip_preservesMeterProvider() throws Exception { assertThat(deserializedGrpc).isNotNull(); assertThat(deserializedGrpc.isEnableOtelMetrics()).isTrue(); assertThat(deserializedGrpc.getMeterProvider()).isNotNull(); + assertThat(deserializedGrpc.getMeterProvider()).isNotSameInstanceAs(customMeterProvider); assertThat(deserializedGrpc.getMeterProvider()) .isEqualTo(deserializedGrpc.getOpenTelemetry().getMeterProvider()); } From 517e61f4fb1423137852d50423f0169caf70d543 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 8 Oct 2026 07:22:12 +0000 Subject: [PATCH 05/10] style(storage): format StorageOptions classes with fmt-maven-plugin Format StorageOptions, GrpcStorageOptions, HttpStorageOptions, and StorageOptionsTest using com.spotify.fmt:fmt-maven-plugin:2.25:format under JDK 17 to resolve CI lint check failure. [Generated-by: AI] --- .../com/google/cloud/storage/GrpcStorageOptions.java | 9 ++++----- .../com/google/cloud/storage/HttpStorageOptions.java | 9 ++++----- .../java/com/google/cloud/storage/StorageOptions.java | 6 +++--- .../com/google/cloud/storage/StorageOptionsTest.java | 3 ++- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 19f075a9905e..1589b1b25cd6 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -177,8 +177,7 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) this.openTelemetry = builder.openTelemetry; this.rawEnableOtelMetrics = builder.enableOtelMetrics; this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; - this.enableOtelMetrics = - StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); + this.enableOtelMetrics = StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); this.enableOtelDebugMetrics = StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); this.meterProvider = builder.meterProvider; @@ -873,9 +872,9 @@ public GrpcStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter /** * Set the metric export interval for periodic metric reading. * - *

When using the default Cloud Monitoring exporter, the interval must be at least 60 - * seconds (1 minute). Intervals less than 60 seconds are permitted only when a custom - * {@link MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + *

When using the default Cloud Monitoring exporter, the interval must be at least 60 seconds + * (1 minute). Intervals less than 60 seconds are permitted only when a custom {@link + * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. * * @param metricInterval interval duration * @since 2.50.0 This new api is in preview and is subject to breaking changes. diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index 114a4c865f38..3dbde5029d5b 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -90,8 +90,7 @@ private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { openTelemetry = builder.openTelemetry; this.rawEnableOtelMetrics = builder.enableOtelMetrics; this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; - this.enableOtelMetrics = - StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); + this.enableOtelMetrics = StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); this.enableOtelDebugMetrics = StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); this.meterProvider = builder.meterProvider; @@ -448,9 +447,9 @@ public HttpStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter /** * Set the metric export interval for periodic metric reading. * - *

When using the default Cloud Monitoring exporter, the interval must be at least 60 - * seconds (1 minute). Intervals less than 60 seconds are permitted only when a custom - * {@link MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + *

When using the default Cloud Monitoring exporter, the interval must be at least 60 seconds + * (1 minute). Intervals less than 60 seconds are permitted only when a custom {@link + * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. * * @param metricInterval interval duration * @since 2.50.0 This new api is in preview and is subject to breaking changes. diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 82ab77a5238b..9de103e65f5f 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -177,9 +177,9 @@ public abstract StorageOptions.Builder setEnableOtelDebugMetrics( /** * Set the metric export interval for periodic metric reading. * - *

When using the default Cloud Monitoring exporter, the interval must be at least 60 - * seconds (1 minute). Intervals less than 60 seconds are permitted only when a custom - * {@link MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + *

When using the default Cloud Monitoring exporter, the interval must be at least 60 seconds + * (1 minute). Intervals less than 60 seconds are permitted only when a custom {@link + * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. * * @param metricInterval interval duration * @since 2.50.0 This new api is in preview and is subject to breaking changes. diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index f56c707ae745..b00710c08456 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -342,7 +342,8 @@ public void serializationRoundTrip_customMeterProviderDoesNotCarryOver() throws assertThat(deserializedHttp).isNotNull(); assertThat(deserializedHttp.isEnableOtelMetrics()).isTrue(); - // Transient meterProvider does not carry over; falls back to openTelemetry.getMeterProvider() without NPE + // Transient meterProvider does not carry over; falls back to openTelemetry.getMeterProvider() + // without NPE assertThat(deserializedHttp.getMeterProvider()).isNotNull(); assertThat(deserializedHttp.getMeterProvider()).isNotSameInstanceAs(customMeterProvider); assertThat(deserializedHttp.getMeterProvider()) From 7f624a23c72f7887d0732a0275a19d8aaff0bc97 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Thu, 15 Oct 2026 06:25:31 +0000 Subject: [PATCH 06/10] fix(storage): address review comments on otel metrics config [Generated-by: AI] --- .../cloud/storage/GrpcStorageOptions.java | 115 ++++-------------- .../cloud/storage/HttpStorageOptions.java | 96 ++------------- .../cloud/storage/StorageMetricsConfig.java | 10 +- .../google/cloud/storage/StorageOptions.java | 114 ++++++++++++++--- .../storage/StorageMetricsConfigTest.java | 111 +++++++++++++++++ .../cloud/storage/StorageOptionsTest.java | 75 +++++++----- 6 files changed, 295 insertions(+), 226 deletions(-) create mode 100644 java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 1589b1b25cd6..e2415c798154 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -133,7 +133,6 @@ public final class GrpcStorageOptions extends StorageOptions implements Retrying.RetryingDependencies { private static final long serialVersionUID = -4499446543857945349L; - private static final Logger LOGGER = Logger.getLogger(GrpcStorageOptions.class.getName()); private static final String GCS_SCOPE = "https://www.googleapis.com/auth/devstorage.full_control"; private static final Set SCOPES = ImmutableSet.of(GCS_SCOPE); private static final String DEFAULT_HOST = "https://storage.googleapis.com"; @@ -151,13 +150,6 @@ public final class GrpcStorageOptions extends StorageOptions private final GrpcInterceptorProvider grpcInterceptorProvider; private final BlobWriteSessionConfig blobWriteSessionConfig; private transient OpenTelemetry openTelemetry; - private final boolean enableOtelMetrics; - private final boolean enableOtelDebugMetrics; - @Nullable private final Boolean rawEnableOtelMetrics; - @Nullable private final Boolean rawEnableOtelDebugMetrics; - private transient MeterProvider meterProvider; - private final Duration metricInterval; - private final boolean customMeterProviderConfigured; private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) { super(builder, serviceDefaults); @@ -175,19 +167,6 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) this.grpcInterceptorProvider = builder.grpcInterceptorProvider; this.blobWriteSessionConfig = builder.blobWriteSessionConfig; this.openTelemetry = builder.openTelemetry; - this.rawEnableOtelMetrics = builder.enableOtelMetrics; - this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; - this.enableOtelMetrics = StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); - this.enableOtelDebugMetrics = - StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); - this.meterProvider = builder.meterProvider; - this.customMeterProviderConfigured = builder.meterProvider != null; - this.metricInterval = builder.metricInterval; - if (this.meterProvider == null && this.metricInterval != null) { - checkArgument( - this.metricInterval.compareTo(Duration.ofSeconds(60)) >= 0, - "Metric export interval cannot be less than 60 seconds when using Cloud Monitoring exporter"); - } } @Override @@ -201,7 +180,7 @@ GrpcRetryAlgorithmManager getRetryAlgorithmManager() { } @InternalApi - Duration getTerminationAwaitDuration() { + java.time.Duration getTerminationAwaitDuration() { return terminationAwaitDuration; } @@ -218,10 +197,6 @@ GrpcInterceptorProvider getGrpcInterceptorProvider() { private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { in.defaultReadObject(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); - if (this.customMeterProviderConfigured || this.meterProvider != null) { - LOGGER.warning( - "Custom MeterProvider is transient and cannot be serialized across workers; falling back to default provider."); - } } /** @@ -390,9 +365,9 @@ private Tuple> resolveSettingsAndOpts() throw // seconds. // To allow read streams to have longer lifespans, crank up their timeouts, instead rely // on idleTimeout below. - .setLogicalTimeout(Duration.ofDays(28)) + .setLogicalTimeout(java.time.Duration.ofDays(28)) .build(); - Duration totalTimeout = baseRetrySettings.getTotalTimeoutDuration(); + java.time.Duration totalTimeout = baseRetrySettings.getTotalTimeoutDuration(); // retries for unary methods are generally handled at a different level, except // StartResumableWrite @@ -441,45 +416,6 @@ public OpenTelemetry getOpenTelemetry() { return openTelemetry; } - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public boolean isEnableOtelMetrics() { - return enableOtelMetrics; - } - - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public boolean isEnableOtelDebugMetrics() { - return enableOtelDebugMetrics; - } - - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public @Nullable MeterProvider getMeterProvider() { - if (meterProvider == null && openTelemetry != null) { - return openTelemetry.getMeterProvider(); - } - return meterProvider; - } - - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public Duration getMetricInterval() { - return metricInterval != null ? metricInterval : Duration.ofSeconds(60); - } - /** * @since 2.14.0 */ @@ -566,7 +502,7 @@ protected boolean shouldRefreshService(Storage cachedService) { public static final class Builder extends StorageOptions.Builder { private StorageRetryStrategy storageRetryStrategy; - private Duration terminationAwaitDuration; + private java.time.Duration terminationAwaitDuration; private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath(); private boolean enableGrpcClientMetrics = GrpcStorageDefaults.INSTANCE.isEnableGrpcClientMetrics(); @@ -575,10 +511,6 @@ public static final class Builder extends StorageOptions.Builder { private BlobWriteSessionConfig blobWriteSessionConfig = GrpcStorageDefaults.INSTANCE.getDefaultStorageWriterConfig(); private OpenTelemetry openTelemetry = GrpcStorageDefaults.INSTANCE.getDefaultOpenTelemetry(); - private Boolean enableOtelMetrics = null; - private Boolean enableOtelDebugMetrics = null; - private MeterProvider meterProvider = null; - private Duration metricInterval = Duration.ofSeconds(60); private boolean grpcMetricsManuallyEnabled = false; @@ -594,16 +526,13 @@ public static final class Builder extends StorageOptions.Builder { this.grpcInterceptorProvider = gso.grpcInterceptorProvider; this.blobWriteSessionConfig = gso.blobWriteSessionConfig; this.openTelemetry = gso.openTelemetry; - this.enableOtelMetrics = gso.rawEnableOtelMetrics; - this.enableOtelDebugMetrics = gso.rawEnableOtelDebugMetrics; - this.meterProvider = gso.meterProvider; - this.metricInterval = gso.getMetricInterval(); } /** - * This method is obsolete. Use {@link #setTerminationAwaitJavaTimeDuration(Duration)} instead. + * This method is obsolete. Use {@link #setTerminationAwaitJavaTimeDuration(java.time.Duration)} + * instead. */ - @ObsoleteApi("Use setTerminationAwaitJavaTimeDuration(Duration) instead") + @ObsoleteApi("Use setTerminationAwaitJavaTimeDuration(java.time.Duration) instead") public Builder setTerminationAwaitDuration(org.threeten.bp.Duration terminationAwaitDuration) { return setTerminationAwaitJavaTimeDuration(toJavaTimeDuration(terminationAwaitDuration)); } @@ -616,7 +545,8 @@ public Builder setTerminationAwaitDuration(org.threeten.bp.Duration terminationA * @return the builder * @since 2.14.0 */ - public Builder setTerminationAwaitJavaTimeDuration(Duration terminationAwaitDuration) { + public Builder setTerminationAwaitJavaTimeDuration( + java.time.Duration terminationAwaitDuration) { this.terminationAwaitDuration = requireNonNull(terminationAwaitDuration, "terminationAwaitDuration must be non null"); return this; @@ -827,6 +757,7 @@ public GrpcStorageOptions.Builder setApiTracerFactory(ApiTracerFactory apiTracer public GrpcStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) { requireNonNull(openTelemetry, "openTelemetry must be non null"); this.openTelemetry = openTelemetry; + this.customOpenTelemetryConfigured = true; return this; } @@ -834,12 +765,12 @@ public GrpcStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) * Enable or disable OpenTelemetry client metrics. * * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public GrpcStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics) { - this.enableOtelMetrics = enableOtelMetrics; + super.setEnableOtelMetrics(enableOtelMetrics); return this; } @@ -847,12 +778,12 @@ public GrpcStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics * Enable or disable OpenTelemetry debug client metrics. * * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public GrpcStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDebugMetrics) { - this.enableOtelDebugMetrics = enableOtelDebugMetrics; + super.setEnableOtelDebugMetrics(enableOtelDebugMetrics); return this; } @@ -860,12 +791,12 @@ public GrpcStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDe * Set a custom {@link MeterProvider} for recording client metrics. * * @param meterProvider custom MeterProvider to use, or null to use the default - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public GrpcStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider) { - this.meterProvider = meterProvider; + super.setMeterProvider(meterProvider); return this; } @@ -874,18 +805,16 @@ public GrpcStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter * *

When using the default Cloud Monitoring exporter, the interval must be at least 60 seconds * (1 minute). Intervals less than 60 seconds are permitted only when a custom {@link - * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)} or explicit + * OpenTelemetry is configured via {@link #setOpenTelemetry(OpenTelemetry)}. * * @param metricInterval interval duration - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public GrpcStorageOptions.Builder setMetricInterval(Duration metricInterval) { - checkArgument( - metricInterval != null && !metricInterval.isNegative() && !metricInterval.isZero(), - "metricInterval must be positive"); - this.metricInterval = metricInterval; + super.setMetricInterval(metricInterval); return this; } @@ -956,8 +885,8 @@ public org.threeten.bp.Duration getTerminationAwaitDuration() { /** * @since 2.14.0 */ - public Duration getTerminationAwaitDurationJavaTime() { - return Duration.ofMinutes(1); + public java.time.Duration getTerminationAwaitDurationJavaTime() { + return java.time.Duration.ofMinutes(1); } /** diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index 3dbde5029d5b..ee3e73487464 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -50,7 +50,6 @@ import java.time.Duration; import java.util.Objects; import java.util.Set; -import java.util.logging.Logger; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -62,7 +61,6 @@ public class HttpStorageOptions extends StorageOptions { private static final long serialVersionUID = -5302637952911052045L; - private static final Logger LOGGER = Logger.getLogger(HttpStorageOptions.class.getName()); private static final String API_SHORT_NAME = "Storage"; private static final String GCS_SCOPE = "https://www.googleapis.com/auth/devstorage.full_control"; private static final Set SCOPES = ImmutableSet.of(GCS_SCOPE); @@ -71,13 +69,6 @@ public class HttpStorageOptions extends StorageOptions { private final BlobWriteSessionConfig blobWriteSessionConfig; private transient OpenTelemetry openTelemetry; - private final boolean enableOtelMetrics; - private final boolean enableOtelDebugMetrics; - @Nullable private final Boolean rawEnableOtelMetrics; - @Nullable private final Boolean rawEnableOtelDebugMetrics; - private transient MeterProvider meterProvider; - private final Duration metricInterval; - private final boolean customMeterProviderConfigured; private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { super(builder, serviceDefaults); @@ -88,19 +79,6 @@ private HttpStorageOptions(Builder builder, StorageDefaults serviceDefaults) { retryDepsAdapter = new RetryDependenciesAdapter(); blobWriteSessionConfig = builder.blobWriteSessionConfig; openTelemetry = builder.openTelemetry; - this.rawEnableOtelMetrics = builder.enableOtelMetrics; - this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; - this.enableOtelMetrics = StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); - this.enableOtelDebugMetrics = - StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); - this.meterProvider = builder.meterProvider; - this.customMeterProviderConfigured = builder.meterProvider != null; - this.metricInterval = builder.metricInterval; - if (this.meterProvider == null && this.metricInterval != null) { - checkArgument( - this.metricInterval.compareTo(Duration.ofSeconds(60)) >= 0, - "Metric export interval cannot be less than 60 seconds when using Cloud Monitoring exporter"); - } } @Override @@ -127,45 +105,6 @@ public OpenTelemetry getOpenTelemetry() { return openTelemetry; } - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public boolean isEnableOtelMetrics() { - return enableOtelMetrics; - } - - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public boolean isEnableOtelDebugMetrics() { - return enableOtelDebugMetrics; - } - - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public @Nullable MeterProvider getMeterProvider() { - if (meterProvider == null && openTelemetry != null) { - return openTelemetry.getMeterProvider(); - } - return meterProvider; - } - - /** - * @since 2.50.0 This new api is in preview and is subject to breaking changes. - */ - @BetaApi - @Override - public Duration getMetricInterval() { - return metricInterval != null ? metricInterval : Duration.ofSeconds(60); - } - @Override public HttpStorageOptions.Builder toBuilder() { return new HttpStorageOptions.Builder(this); @@ -207,10 +146,6 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE in.defaultReadObject(); this.retryDepsAdapter = new RetryDependenciesAdapter(); this.openTelemetry = HttpStorageOptions.getDefaultInstance().getOpenTelemetry(); - if (this.customMeterProviderConfigured || this.meterProvider != null) { - LOGGER.warning( - "Custom MeterProvider is transient and cannot be serialized across workers; falling back to default provider."); - } } public static HttpStorageOptions.Builder newBuilder() { @@ -236,10 +171,6 @@ public static class Builder extends StorageOptions.Builder { private BlobWriteSessionConfig blobWriteSessionConfig = HttpStorageDefaults.INSTANCE.getDefaultStorageWriterConfig(); private OpenTelemetry openTelemetry = HttpStorageDefaults.INSTANCE.getDefaultOpenTelemetry(); - private Boolean enableOtelMetrics = null; - private Boolean enableOtelDebugMetrics = null; - private MeterProvider meterProvider = null; - private Duration metricInterval = Duration.ofSeconds(60); Builder() {} @@ -249,10 +180,6 @@ public static class Builder extends StorageOptions.Builder { this.storageRetryStrategy = hso.retryAlgorithmManager.retryStrategy; this.blobWriteSessionConfig = hso.blobWriteSessionConfig; this.openTelemetry = hso.getOpenTelemetry(); - this.enableOtelMetrics = hso.rawEnableOtelMetrics; - this.enableOtelDebugMetrics = hso.rawEnableOtelDebugMetrics; - this.meterProvider = hso.meterProvider; - this.metricInterval = hso.getMetricInterval(); } @Override @@ -402,6 +329,7 @@ public HttpStorageOptions build() { public HttpStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) { requireNonNull(openTelemetry, "openTelemetry must be non null"); this.openTelemetry = openTelemetry; + this.customOpenTelemetryConfigured = true; return this; } @@ -409,12 +337,12 @@ public HttpStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) * Enable or disable OpenTelemetry client metrics. * * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public HttpStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics) { - this.enableOtelMetrics = enableOtelMetrics; + super.setEnableOtelMetrics(enableOtelMetrics); return this; } @@ -422,12 +350,12 @@ public HttpStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics * Enable or disable OpenTelemetry debug client metrics. * * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public HttpStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDebugMetrics) { - this.enableOtelDebugMetrics = enableOtelDebugMetrics; + super.setEnableOtelDebugMetrics(enableOtelDebugMetrics); return this; } @@ -435,12 +363,12 @@ public HttpStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDe * Set a custom {@link MeterProvider} for recording client metrics. * * @param meterProvider custom MeterProvider to use, or null to use the default - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public HttpStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider) { - this.meterProvider = meterProvider; + super.setMeterProvider(meterProvider); return this; } @@ -449,18 +377,16 @@ public HttpStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter * *

When using the default Cloud Monitoring exporter, the interval must be at least 60 seconds * (1 minute). Intervals less than 60 seconds are permitted only when a custom {@link - * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)} or explicit + * OpenTelemetry is configured via {@link #setOpenTelemetry(OpenTelemetry)}. * * @param metricInterval interval duration - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override public HttpStorageOptions.Builder setMetricInterval(Duration metricInterval) { - checkArgument( - metricInterval != null && !metricInterval.isNegative() && !metricInterval.isZero(), - "metricInterval must be positive"); - this.metricInterval = metricInterval; + super.setMetricInterval(metricInterval); return this; } } diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java index 1fbc3801d903..30be2ba85ef2 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java @@ -51,23 +51,29 @@ static boolean isEnableOtelDebugMetrics(@Nullable Boolean builderSetting) { @VisibleForTesting static boolean isEnableOtelMetrics( Function env, @Nullable Boolean builderSetting) { + if (builderSetting != null) { + return builderSetting; + } String javaEnv = env.apply(ENV_ENABLE_OTEL_METRICS_JAVA); Boolean parsed = parseBooleanValue(javaEnv, ENV_ENABLE_OTEL_METRICS_JAVA); if (parsed != null) { return parsed; } - return builderSetting != null ? builderSetting : false; + return false; } @VisibleForTesting static boolean isEnableOtelDebugMetrics( Function env, @Nullable Boolean builderSetting) { + if (builderSetting != null) { + return builderSetting; + } String javaEnv = env.apply(ENV_ENABLE_OTEL_DEBUG_METRICS); Boolean parsed = parseBooleanValue(javaEnv, ENV_ENABLE_OTEL_DEBUG_METRICS); if (parsed != null) { return parsed; } - return builderSetting != null ? builderSetting : false; + return false; } @Nullable diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 9de103e65f5f..2d123041ca20 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -16,6 +16,8 @@ package com.google.cloud.storage; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.cloud.NoCredentials; @@ -36,9 +38,11 @@ import io.opentelemetry.api.metrics.MeterProvider; import java.io.IOException; import java.io.InputStream; +import java.io.ObjectInputStream; import java.time.Duration; import java.util.Locale; import java.util.Properties; +import java.util.logging.Logger; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -115,12 +119,25 @@ public DefaultStorageRpcFactory() { public abstract static class Builder extends ServiceOptions.Builder { + @Nullable Boolean enableOtelMetrics = null; + @Nullable Boolean enableOtelDebugMetrics = null; + @Nullable MeterProvider meterProvider = null; + Duration metricInterval = Duration.ofSeconds(60); + boolean customOpenTelemetryConfigured = false; + boolean customMeterProviderConfigured = false; + Builder() { setUseJwtAccessWithScope(false); } Builder(StorageOptions options) { super(options); + this.enableOtelMetrics = options.rawEnableOtelMetrics; + this.enableOtelDebugMetrics = options.rawEnableOtelDebugMetrics; + this.meterProvider = options.meterProvider; + this.metricInterval = options.metricInterval; + this.customMeterProviderConfigured = options.customMeterProviderConfigured; + this.customOpenTelemetryConfigured = options.customOpenTelemetryConfigured; } public abstract Builder setStorageRetryStrategy(StorageRetryStrategy storageRetryStrategy); @@ -150,49 +167,102 @@ public abstract StorageOptions.Builder setBlobWriteSessionConfig( * Enable or disable OpenTelemetry client metrics. * * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract StorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics); + public StorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics) { + this.enableOtelMetrics = enableOtelMetrics; + return this; + } /** * Enable or disable OpenTelemetry debug client metrics. * * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract StorageOptions.Builder setEnableOtelDebugMetrics( - boolean enableOtelDebugMetrics); + public StorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDebugMetrics) { + this.enableOtelDebugMetrics = enableOtelDebugMetrics; + return this; + } /** * Set a custom {@link MeterProvider} for recording client metrics. * * @param meterProvider custom MeterProvider to use, or null to use the default - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract StorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider); + public StorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider) { + this.meterProvider = meterProvider; + this.customMeterProviderConfigured = meterProvider != null; + return this; + } /** * Set the metric export interval for periodic metric reading. * *

When using the default Cloud Monitoring exporter, the interval must be at least 60 seconds * (1 minute). Intervals less than 60 seconds are permitted only when a custom {@link - * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)}. + * MeterProvider} is configured via {@link #setMeterProvider(MeterProvider)} or explicit + * OpenTelemetry is configured via {@link #setOpenTelemetry(OpenTelemetry)}. * * @param metricInterval interval duration - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract StorageOptions.Builder setMetricInterval(Duration metricInterval); + public StorageOptions.Builder setMetricInterval(Duration metricInterval) { + checkArgument( + metricInterval != null && !metricInterval.isNegative() && !metricInterval.isZero(), + "metricInterval must be positive"); + this.metricInterval = metricInterval; + return this; + } @Override public abstract StorageOptions build(); } + private static final Logger LOGGER = Logger.getLogger(StorageOptions.class.getName()); + + final boolean enableOtelMetrics; + final boolean enableOtelDebugMetrics; + @Nullable final Boolean rawEnableOtelMetrics; + @Nullable final Boolean rawEnableOtelDebugMetrics; + transient MeterProvider meterProvider; + final Duration metricInterval; + final boolean customMeterProviderConfigured; + final boolean customOpenTelemetryConfigured; + StorageOptions(Builder builder, StorageDefaults serviceDefaults) { super(StorageFactory.class, StorageRpcFactory.class, builder, serviceDefaults); + this.rawEnableOtelMetrics = builder.enableOtelMetrics; + this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; + this.enableOtelMetrics = StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); + this.enableOtelDebugMetrics = + StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); + this.meterProvider = builder.meterProvider; + this.customMeterProviderConfigured = + builder.customMeterProviderConfigured || builder.meterProvider != null; + this.customOpenTelemetryConfigured = builder.customOpenTelemetryConfigured; + this.metricInterval = + builder.metricInterval != null ? builder.metricInterval : Duration.ofSeconds(60); + boolean hasCustomOtelOrMeterProvider = + this.customMeterProviderConfigured || this.customOpenTelemetryConfigured; + if (!hasCustomOtelOrMeterProvider && this.metricInterval != null) { + checkArgument( + this.metricInterval.compareTo(Duration.ofSeconds(60)) >= 0, + "Metric export interval cannot be less than 60 seconds when using Cloud Monitoring exporter"); + } + } + + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { + in.defaultReadObject(); + if (this.customMeterProviderConfigured || this.meterProvider != null) { + LOGGER.warning( + "Custom MeterProvider is transient and cannot be serialized across workers; falling back to default provider."); + } } abstract static class StorageDefaults implements ServiceDefaults {} @@ -232,34 +302,42 @@ public static String version() { /** * Whether OpenTelemetry client metrics are enabled. * - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract boolean isEnableOtelMetrics(); + public boolean isEnableOtelMetrics() { + return enableOtelMetrics; + } /** * Whether OpenTelemetry debug client metrics are enabled. * - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract boolean isEnableOtelDebugMetrics(); + public boolean isEnableOtelDebugMetrics() { + return enableOtelDebugMetrics; + } /** * The {@link MeterProvider} configured for recording client metrics. * - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract @Nullable MeterProvider getMeterProvider(); + public @Nullable MeterProvider getMeterProvider() { + return meterProvider; + } /** * The metric export interval configured for periodic metric reading. * - * @since 2.50.0 This new api is in preview and is subject to breaking changes. + * @since 2.74.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public abstract Duration getMetricInterval(); + public Duration getMetricInterval() { + return metricInterval != null ? metricInterval : Duration.ofSeconds(60); + } @SuppressWarnings("unchecked") @Override diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java new file mode 100644 index 000000000000..5cb4f3946490 --- /dev/null +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java @@ -0,0 +1,111 @@ +/* + * Copyright 2026 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.storage; + +import static com.google.common.truth.Truth.assertThat; + +import java.util.HashMap; +import java.util.Map; +import org.junit.After; +import org.junit.Test; + +public final class StorageMetricsConfigTest { + + @After + public void tearDown() { + StorageMetricsConfig.resetResolversForTesting(); + } + + @Test + public void isEnableOtelMetrics_precedenceOrder() { + Map env = new HashMap<>(); + + // 1. Explicit programmatic builder setting takes precedence over environment variable + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); + + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); + + // 2. When builder setting is null, environment variable is respected + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isTrue(); + + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); + + // 3. When both builder setting and env var are absent/null, default is false + env.clear(); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); + } + + @Test + public void isEnableOtelDebugMetrics_precedenceOrder() { + Map env = new HashMap<>(); + + // 1. Explicit programmatic builder setting takes precedence over environment variable + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, false)).isFalse(); + + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "false"); + assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, true)).isTrue(); + + // 2. When builder setting is null, environment variable is respected + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, null)).isTrue(); + + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "false"); + assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, null)).isFalse(); + + // 3. When both builder setting and env var are absent/null, default is false + env.clear(); + assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, null)).isFalse(); + } + + @Test + public void parseBooleanValue_handlesPermissiveValues() { + String[] trueValues = {"1", "t", "T", "true", "TRUE", "True", " true "}; + for (String val : trueValues) { + assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isTrue(); + } + + String[] falseValues = {"0", "f", "F", "false", "FALSE", "False", " false "}; + for (String val : falseValues) { + assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isFalse(); + } + + String[] invalidValues = {"invalid", "2", "yes", "no", "enabled", ""}; + for (String val : invalidValues) { + assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isNull(); + } + + assertThat(StorageMetricsConfig.parseBooleanValue(null, "TEST_VAR")).isNull(); + } + + @Test + public void setEnvResolverForTesting_resolution() { + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + StorageMetricsConfig.setEnvResolverForTesting(env::get); + + assertThat(StorageMetricsConfig.isEnableOtelMetrics()).isTrue(); + assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics()).isTrue(); + + StorageMetricsConfig.resetResolversForTesting(); + } +} diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index b00710c08456..a54b9a01dba3 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -44,15 +44,13 @@ public void defaultState_metricsDisabled() { HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); assertThat(httpOptions.isEnableOtelMetrics()).isFalse(); assertThat(httpOptions.isEnableOtelDebugMetrics()).isFalse(); - assertThat(httpOptions.getMeterProvider()) - .isEqualTo(httpOptions.getOpenTelemetry().getMeterProvider()); + assertThat(httpOptions.getMeterProvider()).isNull(); assertThat(httpOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); assertThat(grpcOptions.isEnableOtelMetrics()).isFalse(); assertThat(grpcOptions.isEnableOtelDebugMetrics()).isFalse(); - assertThat(grpcOptions.getMeterProvider()) - .isEqualTo(grpcOptions.getOpenTelemetry().getMeterProvider()); + assertThat(grpcOptions.getMeterProvider()).isNull(); assertThat(grpcOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); } @@ -99,7 +97,7 @@ public void builder_explicitEnabling() { } @Test - public void precedence_envTakesPrecedenceOverBuilder() { + public void precedence_builderTakesPrecedenceOverEnv() { Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); @@ -110,16 +108,16 @@ public void precedence_envTakesPrecedenceOverBuilder() { .setEnableOtelMetrics(false) .setEnableOtelDebugMetrics(false) .build(); - assertThat(httpDisabled.isEnableOtelMetrics()).isTrue(); - assertThat(httpDisabled.isEnableOtelDebugMetrics()).isTrue(); + assertThat(httpDisabled.isEnableOtelMetrics()).isFalse(); + assertThat(httpDisabled.isEnableOtelDebugMetrics()).isFalse(); GrpcStorageOptions grpcDisabled = GrpcStorageOptions.grpc() .setEnableOtelMetrics(false) .setEnableOtelDebugMetrics(false) .build(); - assertThat(grpcDisabled.isEnableOtelMetrics()).isTrue(); - assertThat(grpcDisabled.isEnableOtelDebugMetrics()).isTrue(); + assertThat(grpcDisabled.isEnableOtelMetrics()).isFalse(); + assertThat(grpcDisabled.isEnableOtelDebugMetrics()).isFalse(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "false"); @@ -129,16 +127,16 @@ public void precedence_envTakesPrecedenceOverBuilder() { .setEnableOtelMetrics(true) .setEnableOtelDebugMetrics(true) .build(); - assertThat(httpEnabled.isEnableOtelMetrics()).isFalse(); - assertThat(httpEnabled.isEnableOtelDebugMetrics()).isFalse(); + assertThat(httpEnabled.isEnableOtelMetrics()).isTrue(); + assertThat(httpEnabled.isEnableOtelDebugMetrics()).isTrue(); GrpcStorageOptions grpcEnabled = GrpcStorageOptions.grpc() .setEnableOtelMetrics(true) .setEnableOtelDebugMetrics(true) .build(); - assertThat(grpcEnabled.isEnableOtelMetrics()).isFalse(); - assertThat(grpcEnabled.isEnableOtelDebugMetrics()).isFalse(); + assertThat(grpcEnabled.isEnableOtelMetrics()).isTrue(); + assertThat(grpcEnabled.isEnableOtelDebugMetrics()).isTrue(); } @Test @@ -186,7 +184,11 @@ public void permissiveBooleanParsing() { assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isTrue(); Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isTrue(); + // When builderSetting is null, env var is parsed and respected + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isTrue(); + // When builderSetting is provided, builder takes precedence over env var + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); } String[] falseValues = {"0", "f", "F", "false", "FALSE", "False", " false "}; @@ -194,7 +196,11 @@ public void permissiveBooleanParsing() { assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isFalse(); Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isFalse(); + // When builderSetting is null, env var is parsed and respected + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); + // When builderSetting is provided, builder takes precedence over env var + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); } String[] invalidValues = {"invalid", "2", "yes", "no", "enabled", ""}; @@ -202,13 +208,16 @@ public void permissiveBooleanParsing() { assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isNull(); Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); - // Unrecognized values fall back to builder setting + // Unrecognized values fall back to builder setting if provided assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); // When builder setting is null, falls back to default false assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); } + // When both are absent/null, default is false + Map emptyEnv = new HashMap<>(); + assertThat(StorageMetricsConfig.isEnableOtelMetrics(emptyEnv::get, null)).isFalse(); assertThat(StorageMetricsConfig.parseBooleanValue(null, "TEST_VAR")).isNull(); } @@ -318,6 +327,22 @@ public void setMetricInterval_validation() { .setMetricInterval(Duration.ofSeconds(30)) .build(); assertThat(grpcCustom.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + + // Accept intervals < 60s when explicit openTelemetry is configured + io.opentelemetry.api.OpenTelemetry customOtel = io.opentelemetry.api.OpenTelemetry.noop(); + HttpStorageOptions httpCustomOtel = + HttpStorageOptions.http() + .setOpenTelemetry(customOtel) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(httpCustomOtel.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + + GrpcStorageOptions grpcCustomOtel = + GrpcStorageOptions.grpc() + .setOpenTelemetry(customOtel) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(grpcCustomOtel.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); } @Test @@ -342,12 +367,8 @@ public void serializationRoundTrip_customMeterProviderDoesNotCarryOver() throws assertThat(deserializedHttp).isNotNull(); assertThat(deserializedHttp.isEnableOtelMetrics()).isTrue(); - // Transient meterProvider does not carry over; falls back to openTelemetry.getMeterProvider() - // without NPE - assertThat(deserializedHttp.getMeterProvider()).isNotNull(); - assertThat(deserializedHttp.getMeterProvider()).isNotSameInstanceAs(customMeterProvider); - assertThat(deserializedHttp.getMeterProvider()) - .isEqualTo(deserializedHttp.getOpenTelemetry().getMeterProvider()); + // Transient meterProvider does not carry over; returns null + assertThat(deserializedHttp.getMeterProvider()).isNull(); GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc() @@ -368,15 +389,13 @@ public void serializationRoundTrip_customMeterProviderDoesNotCarryOver() throws assertThat(deserializedGrpc).isNotNull(); assertThat(deserializedGrpc.isEnableOtelMetrics()).isTrue(); - assertThat(deserializedGrpc.getMeterProvider()).isNotNull(); - assertThat(deserializedGrpc.getMeterProvider()).isNotSameInstanceAs(customMeterProvider); - assertThat(deserializedGrpc.getMeterProvider()) - .isEqualTo(deserializedGrpc.getOpenTelemetry().getMeterProvider()); + // Transient meterProvider does not carry over; returns null + assertThat(deserializedGrpc.getMeterProvider()).isNull(); } @Test public void deserializationWithNullMetricInterval_defaultsToSixtySeconds() throws Exception { - Field httpBuilderField = HttpStorageOptions.Builder.class.getDeclaredField("metricInterval"); + Field httpBuilderField = StorageOptions.Builder.class.getDeclaredField("metricInterval"); httpBuilderField.setAccessible(true); HttpStorageOptions.Builder httpBuilder = HttpStorageOptions.http(); httpBuilderField.set(httpBuilder, null); @@ -402,7 +421,7 @@ public void deserializationWithNullMetricInterval_defaultsToSixtySeconds() throw assertThat(defaultHttp).isEqualTo(deserializedHttp); assertThat(deserializedHttp.hashCode()).isEqualTo(defaultHttp.hashCode()); - Field grpcBuilderField = GrpcStorageOptions.Builder.class.getDeclaredField("metricInterval"); + Field grpcBuilderField = StorageOptions.Builder.class.getDeclaredField("metricInterval"); grpcBuilderField.setAccessible(true); GrpcStorageOptions.Builder grpcBuilder = GrpcStorageOptions.grpc(); grpcBuilderField.set(grpcBuilder, null); From 4f4f7266572d7e3772a59f8a2afd377adbbb8c25 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Mon, 19 Oct 2026 07:47:06 +0000 Subject: [PATCH 07/10] fix(storage): clamp metricInterval on deserialization and resolve review comments Clamp metric export interval to at least 60 seconds when meterProvider is null and no custom OpenTelemetry is configured, preventing invalid sub-60s intervals during deserialization when falling back to default Cloud Monitoring exporter. [Generated-by: AI] --- .../google/cloud/storage/StorageOptions.java | 8 ++- .../cloud/storage/StorageOptionsTest.java | 49 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 2d123041ca20..2b81b87a768b 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -336,7 +336,13 @@ public boolean isEnableOtelDebugMetrics() { */ @BetaApi public Duration getMetricInterval() { - return metricInterval != null ? metricInterval : Duration.ofSeconds(60); + Duration interval = metricInterval != null ? metricInterval : Duration.ofSeconds(60); + if (meterProvider == null && !customOpenTelemetryConfigured) { + if (interval.compareTo(Duration.ofSeconds(60)) < 0) { + return Duration.ofSeconds(60); + } + } + return interval; } @SuppressWarnings("unchecked") diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index a54b9a01dba3..222e82da7dd8 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -393,6 +393,55 @@ public void serializationRoundTrip_customMeterProviderDoesNotCarryOver() throws assertThat(deserializedGrpc.getMeterProvider()).isNull(); } + @Test + public void serializationRoundTrip_customMeterProviderWithSub60sInterval_clampsToSixtySeconds() + throws Exception { + MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); + HttpStorageOptions httpOptions = + HttpStorageOptions.http() + .setMeterProvider(customMeterProvider) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(httpOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(httpOptions); + } + + HttpStorageOptions deserializedHttp; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserializedHttp = (HttpStorageOptions) ois.readObject(); + } + + assertThat(deserializedHttp).isNotNull(); + assertThat(deserializedHttp.getMeterProvider()).isNull(); + assertThat(deserializedHttp.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + + GrpcStorageOptions grpcOptions = + GrpcStorageOptions.grpc() + .setMeterProvider(customMeterProvider) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(grpcOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + + baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(grpcOptions); + } + + GrpcStorageOptions deserializedGrpc; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserializedGrpc = (GrpcStorageOptions) ois.readObject(); + } + + assertThat(deserializedGrpc).isNotNull(); + assertThat(deserializedGrpc.getMeterProvider()).isNull(); + assertThat(deserializedGrpc.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + } + @Test public void deserializationWithNullMetricInterval_defaultsToSixtySeconds() throws Exception { Field httpBuilderField = StorageOptions.Builder.class.getDeclaredField("metricInterval"); From 9519e62bf75f61ea8981a167782880d126a5c57b Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Mon, 19 Oct 2026 09:58:41 +0000 Subject: [PATCH 08/10] refactor(storage): address PR review comments for client metrics config and tests Deduplicate flag resolution in StorageMetricsConfig by extracting resolveFlag helper, separate multi-behavior tests per go/tott/726, and add input diagnostic messages in looped assertions. [Generated-by: AI] --- .../cloud/storage/StorageMetricsConfig.java | 19 ++--- .../storage/StorageMetricsConfigTest.java | 43 ++++++---- .../cloud/storage/StorageOptionsTest.java | 83 ++++++++++++++----- 3 files changed, 99 insertions(+), 46 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java index 30be2ba85ef2..923d72d146b8 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageMetricsConfig.java @@ -51,25 +51,22 @@ static boolean isEnableOtelDebugMetrics(@Nullable Boolean builderSetting) { @VisibleForTesting static boolean isEnableOtelMetrics( Function env, @Nullable Boolean builderSetting) { - if (builderSetting != null) { - return builderSetting; - } - String javaEnv = env.apply(ENV_ENABLE_OTEL_METRICS_JAVA); - Boolean parsed = parseBooleanValue(javaEnv, ENV_ENABLE_OTEL_METRICS_JAVA); - if (parsed != null) { - return parsed; - } - return false; + return resolveFlag(env, builderSetting, ENV_ENABLE_OTEL_METRICS_JAVA); } @VisibleForTesting static boolean isEnableOtelDebugMetrics( Function env, @Nullable Boolean builderSetting) { + return resolveFlag(env, builderSetting, ENV_ENABLE_OTEL_DEBUG_METRICS); + } + + private static boolean resolveFlag( + Function env, @Nullable Boolean builderSetting, String envVarName) { if (builderSetting != null) { return builderSetting; } - String javaEnv = env.apply(ENV_ENABLE_OTEL_DEBUG_METRICS); - Boolean parsed = parseBooleanValue(javaEnv, ENV_ENABLE_OTEL_DEBUG_METRICS); + String javaEnv = env.apply(envVarName); + Boolean parsed = parseBooleanValue(javaEnv, envVarName); if (parsed != null) { return parsed; } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java index 5cb4f3946490..46e1f5494b32 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java @@ -17,6 +17,7 @@ package com.google.cloud.storage; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import java.util.HashMap; import java.util.Map; @@ -31,48 +32,54 @@ public void tearDown() { } @Test - public void isEnableOtelMetrics_precedenceOrder() { + public void isEnableOtelMetrics_explicitBuilderSettingTakesPrecedenceOverEnvVar() { Map env = new HashMap<>(); - - // 1. Explicit programmatic builder setting takes precedence over environment variable env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); + } - // 2. When builder setting is null, environment variable is respected + @Test + public void isEnableOtelMetrics_envVarRespectedWhenBuilderSettingNull() { + Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isTrue(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); + } - // 3. When both builder setting and env var are absent/null, default is false - env.clear(); + @Test + public void isEnableOtelMetrics_defaultsToFalseWhenUnset() { + Map env = new HashMap<>(); assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); } @Test - public void isEnableOtelDebugMetrics_precedenceOrder() { + public void isEnableOtelDebugMetrics_explicitBuilderSettingTakesPrecedenceOverEnvVar() { Map env = new HashMap<>(); - - // 1. Explicit programmatic builder setting takes precedence over environment variable env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, false)).isFalse(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "false"); assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, true)).isTrue(); + } - // 2. When builder setting is null, environment variable is respected + @Test + public void isEnableOtelDebugMetrics_envVarRespectedWhenBuilderSettingNull() { + Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, null)).isTrue(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "false"); assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, null)).isFalse(); + } - // 3. When both builder setting and env var are absent/null, default is false - env.clear(); + @Test + public void isEnableOtelDebugMetrics_defaultsToFalseWhenUnset() { + Map env = new HashMap<>(); assertThat(StorageMetricsConfig.isEnableOtelDebugMetrics(env::get, null)).isFalse(); } @@ -80,17 +87,23 @@ public void isEnableOtelDebugMetrics_precedenceOrder() { public void parseBooleanValue_handlesPermissiveValues() { String[] trueValues = {"1", "t", "T", "true", "TRUE", "True", " true "}; for (String val : trueValues) { - assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isTrue(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")) + .isTrue(); } String[] falseValues = {"0", "f", "F", "false", "FALSE", "False", " false "}; for (String val : falseValues) { - assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isFalse(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")) + .isFalse(); } String[] invalidValues = {"invalid", "2", "yes", "no", "enabled", ""}; for (String val : invalidValues) { - assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isNull(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")) + .isNull(); } assertThat(StorageMetricsConfig.parseBooleanValue(null, "TEST_VAR")).isNull(); diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index 222e82da7dd8..e6df708367c9 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -17,6 +17,7 @@ package com.google.cloud.storage; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import static org.junit.Assert.assertThrows; import io.opentelemetry.api.metrics.MeterProvider; @@ -178,44 +179,76 @@ public void precedence_builderTakesPrecedenceOverDefaultWhenEnvUnset() { } @Test - public void permissiveBooleanParsing() { + public void permissiveBooleanParsing_trueValues() { String[] trueValues = {"1", "t", "T", "true", "TRUE", "True", " true "}; for (String val : trueValues) { - assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isTrue(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")) + .isTrue(); Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); // When builderSetting is null, env var is parsed and respected - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isTrue(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)) + .isTrue(); // When builderSetting is provided, builder takes precedence over env var - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)) + .isFalse(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)) + .isTrue(); } + } + @Test + public void permissiveBooleanParsing_falseValues() { String[] falseValues = {"0", "f", "F", "false", "FALSE", "False", " false "}; for (String val : falseValues) { - assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isFalse(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")) + .isFalse(); Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); // When builderSetting is null, env var is parsed and respected - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)) + .isFalse(); // When builderSetting is provided, builder takes precedence over env var - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)) + .isTrue(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)) + .isFalse(); } + } + @Test + public void permissiveBooleanParsing_invalidValuesFallback() { String[] invalidValues = {"invalid", "2", "yes", "no", "enabled", ""}; for (String val : invalidValues) { - assertThat(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")).isNull(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.parseBooleanValue(val, "TEST_VAR")) + .isNull(); Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, val); // Unrecognized values fall back to builder setting if provided - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)).isTrue(); - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)).isFalse(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, true)) + .isTrue(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, false)) + .isFalse(); // When builder setting is null, falls back to default false - assertThat(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)).isFalse(); + assertWithMessage("Failed for input value: '%s'", val) + .that(StorageMetricsConfig.isEnableOtelMetrics(env::get, null)) + .isFalse(); } + } - // When both are absent/null, default is false + @Test + public void permissiveBooleanParsing_nullHandled() { Map emptyEnv = new HashMap<>(); assertThat(StorageMetricsConfig.isEnableOtelMetrics(emptyEnv::get, null)).isFalse(); assertThat(StorageMetricsConfig.parseBooleanValue(null, "TEST_VAR")).isNull(); @@ -268,9 +301,7 @@ public void environmentVariableJava() { } @Test - public void setMetricInterval_validation() { - MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); - + public void setMetricInterval_rejectsNullZeroOrNegative() { assertThrows( IllegalArgumentException.class, () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(-1))); @@ -288,7 +319,10 @@ public void setMetricInterval_validation() { () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ZERO)); assertThrows( IllegalArgumentException.class, () -> GrpcStorageOptions.grpc().setMetricInterval(null)); + } + @Test + public void setMetricInterval_rejectsSubSixtySecondsWithoutCustomProvider() { // Reject intervals < 60s when meterProvider == null (SDK-owned Cloud Monitoring exporter) assertThrows( IllegalArgumentException.class, @@ -303,8 +337,10 @@ public void setMetricInterval_validation() { assertThrows( IllegalArgumentException.class, () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(30)).build()); + } - // Accept intervals >= 60s when meterProvider == null + @Test + public void setMetricInterval_acceptsSixtySecondsOrGreaterWithDefaultProvider() { HttpStorageOptions http60 = HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(60)).build(); assertThat(http60.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); @@ -312,8 +348,12 @@ public void setMetricInterval_validation() { GrpcStorageOptions grpc60 = GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(60)).build(); assertThat(grpc60.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + } + + @Test + public void setMetricInterval_acceptsSubSixtySecondsWithCustomMeterProvider() { + MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); - // Accept intervals < 60s when custom meterProvider is configured HttpStorageOptions httpCustom = HttpStorageOptions.http() .setMeterProvider(customMeterProvider) @@ -327,9 +367,12 @@ public void setMetricInterval_validation() { .setMetricInterval(Duration.ofSeconds(30)) .build(); assertThat(grpcCustom.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + } - // Accept intervals < 60s when explicit openTelemetry is configured + @Test + public void setMetricInterval_acceptsSubSixtySecondsWithCustomOpenTelemetry() { io.opentelemetry.api.OpenTelemetry customOtel = io.opentelemetry.api.OpenTelemetry.noop(); + HttpStorageOptions httpCustomOtel = HttpStorageOptions.http() .setOpenTelemetry(customOtel) From 3d889a73369b7d4690f21c23918b09d6469351b7 Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Tue, 20 Oct 2026 07:14:07 +0000 Subject: [PATCH 09/10] fix(storage): address review comments on PR #14484 for metrics options and gating --- .../cloud/storage/GrpcStorageOptions.java | 18 +-- .../cloud/storage/HttpStorageOptions.java | 18 +-- .../google/cloud/storage/StorageOptions.java | 116 ++++++++++++------ .../storage/StorageMetricsConfigTest.java | 60 +++++++++ .../cloud/storage/StorageOptionsTest.java | 71 +++++++++++ 5 files changed, 230 insertions(+), 53 deletions(-) diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index e2415c798154..0793d4614223 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -434,8 +434,8 @@ public int hashCode() { grpcInterceptorProvider, blobWriteSessionConfig, openTelemetry, - enableOtelMetrics, - enableOtelDebugMetrics, + isEnableOtelMetrics(), + isEnableOtelDebugMetrics(), getMeterProvider(), getMetricInterval(), baseHashCode()); @@ -452,8 +452,8 @@ public boolean equals(Object o) { GrpcStorageOptions that = (GrpcStorageOptions) o; return attemptDirectPath == that.attemptDirectPath && enableGrpcClientMetrics == that.enableGrpcClientMetrics - && enableOtelMetrics == that.enableOtelMetrics - && enableOtelDebugMetrics == that.enableOtelDebugMetrics + && isEnableOtelMetrics() == that.isEnableOtelMetrics() + && isEnableOtelDebugMetrics() == that.isEnableOtelDebugMetrics() && Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager) && Objects.equals(terminationAwaitDuration, that.terminationAwaitDuration) && Objects.equals(grpcInterceptorProvider, that.grpcInterceptorProvider) @@ -757,7 +757,7 @@ public GrpcStorageOptions.Builder setApiTracerFactory(ApiTracerFactory apiTracer public GrpcStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) { requireNonNull(openTelemetry, "openTelemetry must be non null"); this.openTelemetry = openTelemetry; - this.customOpenTelemetryConfigured = true; + setCustomOpenTelemetryConfigured(true); return this; } @@ -765,7 +765,7 @@ public GrpcStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) * Enable or disable OpenTelemetry client metrics. * * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -778,7 +778,7 @@ public GrpcStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics * Enable or disable OpenTelemetry debug client metrics. * * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -791,7 +791,7 @@ public GrpcStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDe * Set a custom {@link MeterProvider} for recording client metrics. * * @param meterProvider custom MeterProvider to use, or null to use the default - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -809,7 +809,7 @@ public GrpcStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter * OpenTelemetry is configured via {@link #setOpenTelemetry(OpenTelemetry)}. * * @param metricInterval interval duration - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java index ee3e73487464..8dff1c8185c1 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/HttpStorageOptions.java @@ -116,8 +116,8 @@ public int hashCode() { retryAlgorithmManager, blobWriteSessionConfig, openTelemetry, - enableOtelMetrics, - enableOtelDebugMetrics, + isEnableOtelMetrics(), + isEnableOtelDebugMetrics(), getMeterProvider(), getMetricInterval(), baseHashCode()); @@ -135,8 +135,8 @@ public boolean equals(Object o) { return Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager) && Objects.equals(blobWriteSessionConfig, that.blobWriteSessionConfig) && Objects.equals(openTelemetry, that.openTelemetry) - && enableOtelMetrics == that.enableOtelMetrics - && enableOtelDebugMetrics == that.enableOtelDebugMetrics + && isEnableOtelMetrics() == that.isEnableOtelMetrics() + && isEnableOtelDebugMetrics() == that.isEnableOtelDebugMetrics() && Objects.equals(getMeterProvider(), that.getMeterProvider()) && Objects.equals(getMetricInterval(), that.getMetricInterval()) && this.baseEquals(that); @@ -329,7 +329,7 @@ public HttpStorageOptions build() { public HttpStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) { requireNonNull(openTelemetry, "openTelemetry must be non null"); this.openTelemetry = openTelemetry; - this.customOpenTelemetryConfigured = true; + setCustomOpenTelemetryConfigured(true); return this; } @@ -337,7 +337,7 @@ public HttpStorageOptions.Builder setOpenTelemetry(OpenTelemetry openTelemetry) * Enable or disable OpenTelemetry client metrics. * * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -350,7 +350,7 @@ public HttpStorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics * Enable or disable OpenTelemetry debug client metrics. * * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -363,7 +363,7 @@ public HttpStorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDe * Set a custom {@link MeterProvider} for recording client metrics. * * @param meterProvider custom MeterProvider to use, or null to use the default - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -381,7 +381,7 @@ public HttpStorageOptions.Builder setMeterProvider(@Nullable MeterProvider meter * OpenTelemetry is configured via {@link #setOpenTelemetry(OpenTelemetry)}. * * @param metricInterval interval duration - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override diff --git a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java index 2b81b87a768b..672c9ce34041 100644 --- a/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java +++ b/java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/StorageOptions.java @@ -119,12 +119,12 @@ public DefaultStorageRpcFactory() { public abstract static class Builder extends ServiceOptions.Builder { - @Nullable Boolean enableOtelMetrics = null; - @Nullable Boolean enableOtelDebugMetrics = null; - @Nullable MeterProvider meterProvider = null; - Duration metricInterval = Duration.ofSeconds(60); - boolean customOpenTelemetryConfigured = false; - boolean customMeterProviderConfigured = false; + private @Nullable Boolean enableOtelMetrics = null; + private @Nullable Boolean enableOtelDebugMetrics = null; + private @Nullable MeterProvider meterProvider = null; + private Duration metricInterval = Duration.ofSeconds(60); + private boolean customOpenTelemetryConfigured = false; + private boolean customMeterProviderConfigured = false; Builder() { setUseJwtAccessWithScope(false); @@ -132,12 +132,40 @@ public abstract static class Builder Builder(StorageOptions options) { super(options); - this.enableOtelMetrics = options.rawEnableOtelMetrics; - this.enableOtelDebugMetrics = options.rawEnableOtelDebugMetrics; - this.meterProvider = options.meterProvider; + this.enableOtelMetrics = options.getRawEnableOtelMetrics(); + this.enableOtelDebugMetrics = options.getRawEnableOtelDebugMetrics(); + this.meterProvider = options.getMeterProvider(); this.metricInterval = options.metricInterval; - this.customMeterProviderConfigured = options.customMeterProviderConfigured; - this.customOpenTelemetryConfigured = options.customOpenTelemetryConfigured; + this.customMeterProviderConfigured = options.isCustomMeterProviderConfigured(); + this.customOpenTelemetryConfigured = options.isCustomOpenTelemetryConfigured(); + } + + @Nullable Boolean getEnableOtelMetrics() { + return enableOtelMetrics; + } + + @Nullable Boolean getEnableOtelDebugMetrics() { + return enableOtelDebugMetrics; + } + + @Nullable MeterProvider getMeterProvider() { + return meterProvider; + } + + Duration getMetricInterval() { + return metricInterval; + } + + boolean isCustomOpenTelemetryConfigured() { + return customOpenTelemetryConfigured; + } + + void setCustomOpenTelemetryConfigured(boolean customOpenTelemetryConfigured) { + this.customOpenTelemetryConfigured = customOpenTelemetryConfigured; + } + + boolean isCustomMeterProviderConfigured() { + return customMeterProviderConfigured; } public abstract Builder setStorageRetryStrategy(StorageRetryStrategy storageRetryStrategy); @@ -167,7 +195,7 @@ public abstract StorageOptions.Builder setBlobWriteSessionConfig( * Enable or disable OpenTelemetry client metrics. * * @param enableOtelMetrics whether OpenTelemetry client metrics should be enabled - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public StorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics) { @@ -179,7 +207,7 @@ public StorageOptions.Builder setEnableOtelMetrics(boolean enableOtelMetrics) { * Enable or disable OpenTelemetry debug client metrics. * * @param enableOtelDebugMetrics whether OpenTelemetry debug client metrics should be enabled - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public StorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDebugMetrics) { @@ -191,7 +219,7 @@ public StorageOptions.Builder setEnableOtelDebugMetrics(boolean enableOtelDebugM * Set a custom {@link MeterProvider} for recording client metrics. * * @param meterProvider custom MeterProvider to use, or null to use the default - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public StorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProvider) { @@ -209,7 +237,7 @@ public StorageOptions.Builder setMeterProvider(@Nullable MeterProvider meterProv * OpenTelemetry is configured via {@link #setOpenTelemetry(OpenTelemetry)}. * * @param metricInterval interval duration - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public StorageOptions.Builder setMetricInterval(Duration metricInterval) { @@ -226,28 +254,30 @@ public StorageOptions.Builder setMetricInterval(Duration metricInterval) { private static final Logger LOGGER = Logger.getLogger(StorageOptions.class.getName()); - final boolean enableOtelMetrics; - final boolean enableOtelDebugMetrics; - @Nullable final Boolean rawEnableOtelMetrics; - @Nullable final Boolean rawEnableOtelDebugMetrics; - transient MeterProvider meterProvider; - final Duration metricInterval; - final boolean customMeterProviderConfigured; - final boolean customOpenTelemetryConfigured; + private final boolean enableOtelMetrics; + private final boolean enableOtelDebugMetrics; + @Nullable private final Boolean rawEnableOtelMetrics; + @Nullable private final Boolean rawEnableOtelDebugMetrics; + private transient MeterProvider meterProvider; + private final Duration metricInterval; + private final boolean customMeterProviderConfigured; + private final boolean customOpenTelemetryConfigured; StorageOptions(Builder builder, StorageDefaults serviceDefaults) { super(StorageFactory.class, StorageRpcFactory.class, builder, serviceDefaults); - this.rawEnableOtelMetrics = builder.enableOtelMetrics; - this.rawEnableOtelDebugMetrics = builder.enableOtelDebugMetrics; - this.enableOtelMetrics = StorageMetricsConfig.isEnableOtelMetrics(builder.enableOtelMetrics); + this.rawEnableOtelMetrics = builder.getEnableOtelMetrics(); + this.rawEnableOtelDebugMetrics = builder.getEnableOtelDebugMetrics(); + this.enableOtelMetrics = + StorageMetricsConfig.isEnableOtelMetrics(builder.getEnableOtelMetrics()); this.enableOtelDebugMetrics = - StorageMetricsConfig.isEnableOtelDebugMetrics(builder.enableOtelDebugMetrics); - this.meterProvider = builder.meterProvider; + this.enableOtelMetrics + && StorageMetricsConfig.isEnableOtelDebugMetrics(builder.getEnableOtelDebugMetrics()); + this.meterProvider = builder.getMeterProvider(); this.customMeterProviderConfigured = - builder.customMeterProviderConfigured || builder.meterProvider != null; - this.customOpenTelemetryConfigured = builder.customOpenTelemetryConfigured; + builder.isCustomMeterProviderConfigured() || builder.getMeterProvider() != null; + this.customOpenTelemetryConfigured = builder.isCustomOpenTelemetryConfigured(); this.metricInterval = - builder.metricInterval != null ? builder.metricInterval : Duration.ofSeconds(60); + builder.getMetricInterval() != null ? builder.getMetricInterval() : Duration.ofSeconds(60); boolean hasCustomOtelOrMeterProvider = this.customMeterProviderConfigured || this.customOpenTelemetryConfigured; if (!hasCustomOtelOrMeterProvider && this.metricInterval != null) { @@ -257,6 +287,22 @@ public StorageOptions.Builder setMetricInterval(Duration metricInterval) { } } + @Nullable Boolean getRawEnableOtelMetrics() { + return rawEnableOtelMetrics; + } + + @Nullable Boolean getRawEnableOtelDebugMetrics() { + return rawEnableOtelDebugMetrics; + } + + boolean isCustomMeterProviderConfigured() { + return customMeterProviderConfigured; + } + + boolean isCustomOpenTelemetryConfigured() { + return customOpenTelemetryConfigured; + } + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { in.defaultReadObject(); if (this.customMeterProviderConfigured || this.meterProvider != null) { @@ -302,7 +348,7 @@ public static String version() { /** * Whether OpenTelemetry client metrics are enabled. * - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public boolean isEnableOtelMetrics() { @@ -312,7 +358,7 @@ public boolean isEnableOtelMetrics() { /** * Whether OpenTelemetry debug client metrics are enabled. * - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public boolean isEnableOtelDebugMetrics() { @@ -322,7 +368,7 @@ public boolean isEnableOtelDebugMetrics() { /** * The {@link MeterProvider} configured for recording client metrics. * - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public @Nullable MeterProvider getMeterProvider() { @@ -332,7 +378,7 @@ public boolean isEnableOtelDebugMetrics() { /** * The metric export interval configured for periodic metric reading. * - * @since 2.74.0 This new api is in preview and is subject to breaking changes. + * @since 2.76.0 This new api is in preview and is subject to breaking changes. */ @BetaApi public Duration getMetricInterval() { diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java index 46e1f5494b32..3a44ef23dc10 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageMetricsConfigTest.java @@ -121,4 +121,64 @@ public void setEnvResolverForTesting_resolution() { StorageMetricsConfig.resetResolversForTesting(); } + + @Test + public void debugMetricsGatedOnBaseMetrics_inStorageOptions() { + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + StorageMetricsConfig.setEnvResolverForTesting(env::get); + + // Base metrics unset in env (defaults to false) -> options evaluate debug metrics to false + HttpStorageOptions httpEnvDebugOnly = HttpStorageOptions.http().build(); + assertThat(httpEnvDebugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(httpEnvDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + + GrpcStorageOptions grpcEnvDebugOnly = GrpcStorageOptions.grpc().build(); + assertThat(grpcEnvDebugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(grpcEnvDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + + // Base metrics set to true in env -> options evaluate debug metrics to true + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + HttpStorageOptions httpEnvBoth = HttpStorageOptions.http().build(); + assertThat(httpEnvBoth.isEnableOtelMetrics()).isTrue(); + assertThat(httpEnvBoth.isEnableOtelDebugMetrics()).isTrue(); + + GrpcStorageOptions grpcEnvBoth = GrpcStorageOptions.grpc().build(); + assertThat(grpcEnvBoth.isEnableOtelMetrics()).isTrue(); + assertThat(grpcEnvBoth.isEnableOtelDebugMetrics()).isTrue(); + + // Explicit builder disabling base metrics overrides env and keeps debug metrics false + HttpStorageOptions httpBuilderGated = + HttpStorageOptions.http() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(httpBuilderGated.isEnableOtelMetrics()).isFalse(); + assertThat(httpBuilderGated.isEnableOtelDebugMetrics()).isFalse(); + + GrpcStorageOptions grpcBuilderGated = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(grpcBuilderGated.isEnableOtelMetrics()).isFalse(); + assertThat(grpcBuilderGated.isEnableOtelDebugMetrics()).isFalse(); + + // Explicit builder enabling both evaluates debug metrics to true + HttpStorageOptions httpBuilderBoth = + HttpStorageOptions.http() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(httpBuilderBoth.isEnableOtelMetrics()).isTrue(); + assertThat(httpBuilderBoth.isEnableOtelDebugMetrics()).isTrue(); + + GrpcStorageOptions grpcBuilderBoth = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(grpcBuilderBoth.isEnableOtelMetrics()).isTrue(); + assertThat(grpcBuilderBoth.isEnableOtelDebugMetrics()).isTrue(); + } } diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index e6df708367c9..366c2d17cfdc 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -178,6 +178,77 @@ public void precedence_builderTakesPrecedenceOverDefaultWhenEnvUnset() { assertThat(grpcDisabled.isEnableOtelDebugMetrics()).isFalse(); } + @Test + public void debugMetricsGatedOnBaseMetrics_builder() { + HttpStorageOptions httpDebugOnly = + HttpStorageOptions.http() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(httpDebugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(httpDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + + GrpcStorageOptions grpcDebugOnly = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(false) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(grpcDebugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(grpcDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + + HttpStorageOptions httpBoth = + HttpStorageOptions.http() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(httpBoth.isEnableOtelMetrics()).isTrue(); + assertThat(httpBoth.isEnableOtelDebugMetrics()).isTrue(); + + GrpcStorageOptions grpcBoth = + GrpcStorageOptions.grpc() + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .build(); + assertThat(grpcBoth.isEnableOtelMetrics()).isTrue(); + assertThat(grpcBoth.isEnableOtelDebugMetrics()).isTrue(); + } + + @Test + public void debugMetricsGatedOnBaseMetrics_env() { + Map env = new HashMap<>(); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + StorageMetricsConfig.setEnvResolverForTesting(env::get); + + // Base metrics unset (defaults to false) -> debug metrics should evaluate to false + HttpStorageOptions httpEnvDebugOnly = HttpStorageOptions.http().build(); + assertThat(httpEnvDebugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(httpEnvDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + + GrpcStorageOptions grpcEnvDebugOnly = GrpcStorageOptions.grpc().build(); + assertThat(grpcEnvDebugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(grpcEnvDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + + // Base metrics explicitly false in env -> debug metrics should evaluate to false + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); + HttpStorageOptions httpEnvExplicitFalse = HttpStorageOptions.http().build(); + assertThat(httpEnvExplicitFalse.isEnableOtelMetrics()).isFalse(); + assertThat(httpEnvExplicitFalse.isEnableOtelDebugMetrics()).isFalse(); + + GrpcStorageOptions grpcEnvExplicitFalse = GrpcStorageOptions.grpc().build(); + assertThat(grpcEnvExplicitFalse.isEnableOtelMetrics()).isFalse(); + assertThat(grpcEnvExplicitFalse.isEnableOtelDebugMetrics()).isFalse(); + + // Base metrics enabled in env -> debug metrics should evaluate to true + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + HttpStorageOptions httpEnvBoth = HttpStorageOptions.http().build(); + assertThat(httpEnvBoth.isEnableOtelMetrics()).isTrue(); + assertThat(httpEnvBoth.isEnableOtelDebugMetrics()).isTrue(); + + GrpcStorageOptions grpcEnvBoth = GrpcStorageOptions.grpc().build(); + assertThat(grpcEnvBoth.isEnableOtelMetrics()).isTrue(); + assertThat(grpcEnvBoth.isEnableOtelDebugMetrics()).isTrue(); + } + @Test public void permissiveBooleanParsing_trueValues() { String[] trueValues = {"1", "t", "T", "true", "TRUE", "True", " true "}; From e7a358831a38b839cf3bf489ca46654eaa77786b Mon Sep 17 00:00:00 2001 From: Nidhi Nandwani Date: Tue, 20 Oct 2026 09:03:03 +0000 Subject: [PATCH 10/10] test(storage): deduplicate HTTP and gRPC test cases in StorageOptionsTest [Generated-by: AI] --- .../cloud/storage/StorageOptionsTest.java | 648 +++++++----------- 1 file changed, 249 insertions(+), 399 deletions(-) diff --git a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java index 366c2d17cfdc..f2dc6f3fda8d 100644 --- a/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java +++ b/java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/StorageOptionsTest.java @@ -35,218 +35,159 @@ public final class StorageOptionsTest { + @FunctionalInterface + private interface BuilderConsumer { + void accept(StorageOptions.Builder builder) throws Exception; + } + + private static void forEachTransport(BuilderConsumer consumer) throws Exception { + consumer.accept(HttpStorageOptions.http()); + consumer.accept(GrpcStorageOptions.grpc()); + } + @After public void tearDown() { StorageMetricsConfig.resetResolversForTesting(); } @Test - public void defaultState_metricsDisabled() { - HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); - assertThat(httpOptions.isEnableOtelMetrics()).isFalse(); - assertThat(httpOptions.isEnableOtelDebugMetrics()).isFalse(); - assertThat(httpOptions.getMeterProvider()).isNull(); - assertThat(httpOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); - - GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isFalse(); - assertThat(grpcOptions.isEnableOtelDebugMetrics()).isFalse(); - assertThat(grpcOptions.getMeterProvider()).isNull(); - assertThat(grpcOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + public void defaultState_metricsDisabled() throws Exception { + forEachTransport( + builder -> { + StorageOptions options = builder.build(); + assertThat(options.isEnableOtelMetrics()).isFalse(); + assertThat(options.isEnableOtelDebugMetrics()).isFalse(); + assertThat(options.getMeterProvider()).isNull(); + assertThat(options.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + }); } @Test - public void builder_explicitEnabling() { + public void builder_explicitEnabling() throws Exception { MeterProvider mockMeterProvider = SdkMeterProvider.builder().build(); Duration interval = Duration.ofSeconds(30); - HttpStorageOptions httpOptions = - HttpStorageOptions.http() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .setMeterProvider(mockMeterProvider) - .setMetricInterval(interval) - .build(); - assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); - assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); - assertThat(httpOptions.getMeterProvider()).isSameInstanceAs(mockMeterProvider); - assertThat(httpOptions.getMetricInterval()).isEqualTo(interval); - - HttpStorageOptions rebuiltHttp = httpOptions.toBuilder().build(); - assertThat(rebuiltHttp.isEnableOtelMetrics()).isTrue(); - assertThat(rebuiltHttp.isEnableOtelDebugMetrics()).isTrue(); - assertThat(rebuiltHttp.getMeterProvider()).isSameInstanceAs(mockMeterProvider); - assertThat(rebuiltHttp.getMetricInterval()).isEqualTo(interval); - - GrpcStorageOptions grpcOptions = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .setMeterProvider(mockMeterProvider) - .setMetricInterval(interval) - .build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); - assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); - assertThat(grpcOptions.getMeterProvider()).isSameInstanceAs(mockMeterProvider); - assertThat(grpcOptions.getMetricInterval()).isEqualTo(interval); - - GrpcStorageOptions rebuiltGrpc = grpcOptions.toBuilder().build(); - assertThat(rebuiltGrpc.isEnableOtelMetrics()).isTrue(); - assertThat(rebuiltGrpc.isEnableOtelDebugMetrics()).isTrue(); - assertThat(rebuiltGrpc.getMeterProvider()).isSameInstanceAs(mockMeterProvider); - assertThat(rebuiltGrpc.getMetricInterval()).isEqualTo(interval); + forEachTransport( + builder -> { + StorageOptions options = + builder + .setEnableOtelMetrics(true) + .setEnableOtelDebugMetrics(true) + .setMeterProvider(mockMeterProvider) + .setMetricInterval(interval) + .build(); + assertThat(options.isEnableOtelMetrics()).isTrue(); + assertThat(options.isEnableOtelDebugMetrics()).isTrue(); + assertThat(options.getMeterProvider()).isSameInstanceAs(mockMeterProvider); + assertThat(options.getMetricInterval()).isEqualTo(interval); + + StorageOptions rebuilt = options.toBuilder().build(); + assertThat(rebuilt.isEnableOtelMetrics()).isTrue(); + assertThat(rebuilt.isEnableOtelDebugMetrics()).isTrue(); + assertThat(rebuilt.getMeterProvider()).isSameInstanceAs(mockMeterProvider); + assertThat(rebuilt.getMetricInterval()).isEqualTo(interval); + }); } @Test - public void precedence_builderTakesPrecedenceOverEnv() { + public void precedence_builderTakesPrecedenceOverEnv() throws Exception { Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); StorageMetricsConfig.setEnvResolverForTesting(env::get); - HttpStorageOptions httpDisabled = - HttpStorageOptions.http() - .setEnableOtelMetrics(false) - .setEnableOtelDebugMetrics(false) - .build(); - assertThat(httpDisabled.isEnableOtelMetrics()).isFalse(); - assertThat(httpDisabled.isEnableOtelDebugMetrics()).isFalse(); - - GrpcStorageOptions grpcDisabled = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(false) - .setEnableOtelDebugMetrics(false) - .build(); - assertThat(grpcDisabled.isEnableOtelMetrics()).isFalse(); - assertThat(grpcDisabled.isEnableOtelDebugMetrics()).isFalse(); + forEachTransport( + builder -> { + StorageOptions disabled = + builder.setEnableOtelMetrics(false).setEnableOtelDebugMetrics(false).build(); + assertThat(disabled.isEnableOtelMetrics()).isFalse(); + assertThat(disabled.isEnableOtelDebugMetrics()).isFalse(); + }); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "false"); - HttpStorageOptions httpEnabled = - HttpStorageOptions.http() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(httpEnabled.isEnableOtelMetrics()).isTrue(); - assertThat(httpEnabled.isEnableOtelDebugMetrics()).isTrue(); - - GrpcStorageOptions grpcEnabled = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(grpcEnabled.isEnableOtelMetrics()).isTrue(); - assertThat(grpcEnabled.isEnableOtelDebugMetrics()).isTrue(); + forEachTransport( + builder -> { + StorageOptions enabled = + builder.setEnableOtelMetrics(true).setEnableOtelDebugMetrics(true).build(); + assertThat(enabled.isEnableOtelMetrics()).isTrue(); + assertThat(enabled.isEnableOtelDebugMetrics()).isTrue(); + }); } @Test - public void precedence_builderTakesPrecedenceOverDefaultWhenEnvUnset() { + public void precedence_builderTakesPrecedenceOverDefaultWhenEnvUnset() throws Exception { Map env = new HashMap<>(); StorageMetricsConfig.setEnvResolverForTesting(env::get); - HttpStorageOptions httpEnabled = - HttpStorageOptions.http() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(httpEnabled.isEnableOtelMetrics()).isTrue(); - assertThat(httpEnabled.isEnableOtelDebugMetrics()).isTrue(); - - GrpcStorageOptions grpcEnabled = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(grpcEnabled.isEnableOtelMetrics()).isTrue(); - assertThat(grpcEnabled.isEnableOtelDebugMetrics()).isTrue(); - - HttpStorageOptions httpDisabled = - HttpStorageOptions.http() - .setEnableOtelMetrics(false) - .setEnableOtelDebugMetrics(false) - .build(); - assertThat(httpDisabled.isEnableOtelMetrics()).isFalse(); - assertThat(httpDisabled.isEnableOtelDebugMetrics()).isFalse(); - - GrpcStorageOptions grpcDisabled = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(false) - .setEnableOtelDebugMetrics(false) - .build(); - assertThat(grpcDisabled.isEnableOtelMetrics()).isFalse(); - assertThat(grpcDisabled.isEnableOtelDebugMetrics()).isFalse(); + forEachTransport( + builder -> { + StorageOptions enabled = + builder.setEnableOtelMetrics(true).setEnableOtelDebugMetrics(true).build(); + assertThat(enabled.isEnableOtelMetrics()).isTrue(); + assertThat(enabled.isEnableOtelDebugMetrics()).isTrue(); + }); + + forEachTransport( + builder -> { + StorageOptions disabled = + builder.setEnableOtelMetrics(false).setEnableOtelDebugMetrics(false).build(); + assertThat(disabled.isEnableOtelMetrics()).isFalse(); + assertThat(disabled.isEnableOtelDebugMetrics()).isFalse(); + }); } @Test - public void debugMetricsGatedOnBaseMetrics_builder() { - HttpStorageOptions httpDebugOnly = - HttpStorageOptions.http() - .setEnableOtelMetrics(false) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(httpDebugOnly.isEnableOtelMetrics()).isFalse(); - assertThat(httpDebugOnly.isEnableOtelDebugMetrics()).isFalse(); - - GrpcStorageOptions grpcDebugOnly = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(false) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(grpcDebugOnly.isEnableOtelMetrics()).isFalse(); - assertThat(grpcDebugOnly.isEnableOtelDebugMetrics()).isFalse(); - - HttpStorageOptions httpBoth = - HttpStorageOptions.http() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(httpBoth.isEnableOtelMetrics()).isTrue(); - assertThat(httpBoth.isEnableOtelDebugMetrics()).isTrue(); - - GrpcStorageOptions grpcBoth = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(true) - .setEnableOtelDebugMetrics(true) - .build(); - assertThat(grpcBoth.isEnableOtelMetrics()).isTrue(); - assertThat(grpcBoth.isEnableOtelDebugMetrics()).isTrue(); + public void debugMetricsGatedOnBaseMetrics_builder() throws Exception { + forEachTransport( + builder -> { + StorageOptions debugOnly = + builder.setEnableOtelMetrics(false).setEnableOtelDebugMetrics(true).build(); + assertThat(debugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(debugOnly.isEnableOtelDebugMetrics()).isFalse(); + }); + + forEachTransport( + builder -> { + StorageOptions both = + builder.setEnableOtelMetrics(true).setEnableOtelDebugMetrics(true).build(); + assertThat(both.isEnableOtelMetrics()).isTrue(); + assertThat(both.isEnableOtelDebugMetrics()).isTrue(); + }); } @Test - public void debugMetricsGatedOnBaseMetrics_env() { + public void debugMetricsGatedOnBaseMetrics_env() throws Exception { Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); StorageMetricsConfig.setEnvResolverForTesting(env::get); // Base metrics unset (defaults to false) -> debug metrics should evaluate to false - HttpStorageOptions httpEnvDebugOnly = HttpStorageOptions.http().build(); - assertThat(httpEnvDebugOnly.isEnableOtelMetrics()).isFalse(); - assertThat(httpEnvDebugOnly.isEnableOtelDebugMetrics()).isFalse(); - - GrpcStorageOptions grpcEnvDebugOnly = GrpcStorageOptions.grpc().build(); - assertThat(grpcEnvDebugOnly.isEnableOtelMetrics()).isFalse(); - assertThat(grpcEnvDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + forEachTransport( + builder -> { + StorageOptions envDebugOnly = builder.build(); + assertThat(envDebugOnly.isEnableOtelMetrics()).isFalse(); + assertThat(envDebugOnly.isEnableOtelDebugMetrics()).isFalse(); + }); // Base metrics explicitly false in env -> debug metrics should evaluate to false env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "false"); - HttpStorageOptions httpEnvExplicitFalse = HttpStorageOptions.http().build(); - assertThat(httpEnvExplicitFalse.isEnableOtelMetrics()).isFalse(); - assertThat(httpEnvExplicitFalse.isEnableOtelDebugMetrics()).isFalse(); - - GrpcStorageOptions grpcEnvExplicitFalse = GrpcStorageOptions.grpc().build(); - assertThat(grpcEnvExplicitFalse.isEnableOtelMetrics()).isFalse(); - assertThat(grpcEnvExplicitFalse.isEnableOtelDebugMetrics()).isFalse(); + forEachTransport( + builder -> { + StorageOptions envExplicitFalse = builder.build(); + assertThat(envExplicitFalse.isEnableOtelMetrics()).isFalse(); + assertThat(envExplicitFalse.isEnableOtelDebugMetrics()).isFalse(); + }); // Base metrics enabled in env -> debug metrics should evaluate to true env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); - HttpStorageOptions httpEnvBoth = HttpStorageOptions.http().build(); - assertThat(httpEnvBoth.isEnableOtelMetrics()).isTrue(); - assertThat(httpEnvBoth.isEnableOtelDebugMetrics()).isTrue(); - - GrpcStorageOptions grpcEnvBoth = GrpcStorageOptions.grpc().build(); - assertThat(grpcEnvBoth.isEnableOtelMetrics()).isTrue(); - assertThat(grpcEnvBoth.isEnableOtelDebugMetrics()).isTrue(); + forEachTransport( + builder -> { + StorageOptions envBoth = builder.build(); + assertThat(envBoth.isEnableOtelMetrics()).isTrue(); + assertThat(envBoth.isEnableOtelDebugMetrics()).isTrue(); + }); } @Test @@ -326,286 +267,195 @@ public void permissiveBooleanParsing_nullHandled() { } @Test - public void toBuilder_doesNotBakeInEnvironmentVariable() { + public void toBuilder_doesNotBakeInEnvironmentVariable() throws Exception { Map env = new HashMap<>(); - env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); - env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); - StorageMetricsConfig.setEnvResolverForTesting(env::get); - - HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); - assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); - assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); - // Clear environment variable and rebuild - env.clear(); - HttpStorageOptions rebuiltHttp = httpOptions.toBuilder().build(); - assertThat(rebuiltHttp.isEnableOtelMetrics()).isFalse(); - assertThat(rebuiltHttp.isEnableOtelDebugMetrics()).isFalse(); - - // Repeat for GrpcStorageOptions - env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); - env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); - GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); - assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); - - env.clear(); - GrpcStorageOptions rebuiltGrpc = grpcOptions.toBuilder().build(); - assertThat(rebuiltGrpc.isEnableOtelMetrics()).isFalse(); - assertThat(rebuiltGrpc.isEnableOtelDebugMetrics()).isFalse(); + forEachTransport( + builder -> { + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); + env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); + StorageMetricsConfig.setEnvResolverForTesting(env::get); + + StorageOptions options = builder.build(); + assertThat(options.isEnableOtelMetrics()).isTrue(); + assertThat(options.isEnableOtelDebugMetrics()).isTrue(); + + // Clear environment variable and rebuild + env.clear(); + StorageOptions rebuilt = options.toBuilder().build(); + assertThat(rebuilt.isEnableOtelMetrics()).isFalse(); + assertThat(rebuilt.isEnableOtelDebugMetrics()).isFalse(); + }); } @Test - public void environmentVariableJava() { + public void environmentVariableJava() throws Exception { Map env = new HashMap<>(); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_METRICS_JAVA, "true"); env.put(StorageMetricsConfig.ENV_ENABLE_OTEL_DEBUG_METRICS, "true"); StorageMetricsConfig.setEnvResolverForTesting(env::get); - HttpStorageOptions httpOptions = HttpStorageOptions.http().build(); - assertThat(httpOptions.isEnableOtelMetrics()).isTrue(); - assertThat(httpOptions.isEnableOtelDebugMetrics()).isTrue(); - - GrpcStorageOptions grpcOptions = GrpcStorageOptions.grpc().build(); - assertThat(grpcOptions.isEnableOtelMetrics()).isTrue(); - assertThat(grpcOptions.isEnableOtelDebugMetrics()).isTrue(); + forEachTransport( + builder -> { + StorageOptions options = builder.build(); + assertThat(options.isEnableOtelMetrics()).isTrue(); + assertThat(options.isEnableOtelDebugMetrics()).isTrue(); + }); } @Test - public void setMetricInterval_rejectsNullZeroOrNegative() { - assertThrows( - IllegalArgumentException.class, - () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(-1))); - assertThrows( - IllegalArgumentException.class, - () -> HttpStorageOptions.http().setMetricInterval(Duration.ZERO)); - assertThrows( - IllegalArgumentException.class, () -> HttpStorageOptions.http().setMetricInterval(null)); - - assertThrows( - IllegalArgumentException.class, - () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(-1))); - assertThrows( - IllegalArgumentException.class, - () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ZERO)); - assertThrows( - IllegalArgumentException.class, () -> GrpcStorageOptions.grpc().setMetricInterval(null)); + public void setMetricInterval_rejectsNullZeroOrNegative() throws Exception { + forEachTransport( + builder -> { + assertThrows( + IllegalArgumentException.class, + () -> builder.setMetricInterval(Duration.ofSeconds(-1))); + assertThrows( + IllegalArgumentException.class, () -> builder.setMetricInterval(Duration.ZERO)); + assertThrows(IllegalArgumentException.class, () -> builder.setMetricInterval(null)); + }); } @Test - public void setMetricInterval_rejectsSubSixtySecondsWithoutCustomProvider() { + public void setMetricInterval_rejectsSubSixtySecondsWithoutCustomProvider() throws Exception { // Reject intervals < 60s when meterProvider == null (SDK-owned Cloud Monitoring exporter) - assertThrows( - IllegalArgumentException.class, - () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(59)).build()); - assertThrows( - IllegalArgumentException.class, - () -> HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(30)).build()); - - assertThrows( - IllegalArgumentException.class, - () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(59)).build()); - assertThrows( - IllegalArgumentException.class, - () -> GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(30)).build()); + forEachTransport( + builder -> { + assertThrows( + IllegalArgumentException.class, + () -> builder.setMetricInterval(Duration.ofSeconds(59)).build()); + assertThrows( + IllegalArgumentException.class, + () -> builder.setMetricInterval(Duration.ofSeconds(30)).build()); + }); } @Test - public void setMetricInterval_acceptsSixtySecondsOrGreaterWithDefaultProvider() { - HttpStorageOptions http60 = - HttpStorageOptions.http().setMetricInterval(Duration.ofSeconds(60)).build(); - assertThat(http60.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); - - GrpcStorageOptions grpc60 = - GrpcStorageOptions.grpc().setMetricInterval(Duration.ofSeconds(60)).build(); - assertThat(grpc60.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + public void setMetricInterval_acceptsSixtySecondsOrGreaterWithDefaultProvider() throws Exception { + forEachTransport( + builder -> { + StorageOptions options = builder.setMetricInterval(Duration.ofSeconds(60)).build(); + assertThat(options.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + }); } @Test - public void setMetricInterval_acceptsSubSixtySecondsWithCustomMeterProvider() { + public void setMetricInterval_acceptsSubSixtySecondsWithCustomMeterProvider() throws Exception { MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); - HttpStorageOptions httpCustom = - HttpStorageOptions.http() - .setMeterProvider(customMeterProvider) - .setMetricInterval(Duration.ofSeconds(30)) - .build(); - assertThat(httpCustom.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); - - GrpcStorageOptions grpcCustom = - GrpcStorageOptions.grpc() - .setMeterProvider(customMeterProvider) - .setMetricInterval(Duration.ofSeconds(30)) - .build(); - assertThat(grpcCustom.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + forEachTransport( + builder -> { + StorageOptions customOptions = + builder + .setMeterProvider(customMeterProvider) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(customOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + }); } @Test - public void setMetricInterval_acceptsSubSixtySecondsWithCustomOpenTelemetry() { + public void setMetricInterval_acceptsSubSixtySecondsWithCustomOpenTelemetry() throws Exception { io.opentelemetry.api.OpenTelemetry customOtel = io.opentelemetry.api.OpenTelemetry.noop(); - HttpStorageOptions httpCustomOtel = - HttpStorageOptions.http() - .setOpenTelemetry(customOtel) - .setMetricInterval(Duration.ofSeconds(30)) - .build(); - assertThat(httpCustomOtel.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); - - GrpcStorageOptions grpcCustomOtel = - GrpcStorageOptions.grpc() - .setOpenTelemetry(customOtel) - .setMetricInterval(Duration.ofSeconds(30)) - .build(); - assertThat(grpcCustomOtel.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + forEachTransport( + builder -> { + StorageOptions customOptionsOtel = + builder + .setOpenTelemetry(customOtel) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(customOptionsOtel.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + }); } @Test public void serializationRoundTrip_customMeterProviderDoesNotCarryOver() throws Exception { MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); - HttpStorageOptions httpOptions = - HttpStorageOptions.http() - .setEnableOtelMetrics(true) - .setMeterProvider(customMeterProvider) - .build(); - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { - oos.writeObject(httpOptions); - } - HttpStorageOptions deserializedHttp; - try (ObjectInputStream ois = - new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { - deserializedHttp = (HttpStorageOptions) ois.readObject(); - } - - assertThat(deserializedHttp).isNotNull(); - assertThat(deserializedHttp.isEnableOtelMetrics()).isTrue(); - // Transient meterProvider does not carry over; returns null - assertThat(deserializedHttp.getMeterProvider()).isNull(); - - GrpcStorageOptions grpcOptions = - GrpcStorageOptions.grpc() - .setEnableOtelMetrics(true) - .setMeterProvider(customMeterProvider) - .build(); - - baos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { - oos.writeObject(grpcOptions); - } - - GrpcStorageOptions deserializedGrpc; - try (ObjectInputStream ois = - new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { - deserializedGrpc = (GrpcStorageOptions) ois.readObject(); - } - - assertThat(deserializedGrpc).isNotNull(); - assertThat(deserializedGrpc.isEnableOtelMetrics()).isTrue(); - // Transient meterProvider does not carry over; returns null - assertThat(deserializedGrpc.getMeterProvider()).isNull(); + forEachTransport( + builder -> { + StorageOptions options = + builder.setEnableOtelMetrics(true).setMeterProvider(customMeterProvider).build(); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(options); + } + + StorageOptions deserialized; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserialized = (StorageOptions) ois.readObject(); + } + + assertThat(deserialized).isNotNull(); + assertThat(deserialized.isEnableOtelMetrics()).isTrue(); + // Transient meterProvider does not carry over; returns null + assertThat(deserialized.getMeterProvider()).isNull(); + }); } @Test public void serializationRoundTrip_customMeterProviderWithSub60sInterval_clampsToSixtySeconds() throws Exception { MeterProvider customMeterProvider = SdkMeterProvider.builder().build(); - HttpStorageOptions httpOptions = - HttpStorageOptions.http() - .setMeterProvider(customMeterProvider) - .setMetricInterval(Duration.ofSeconds(30)) - .build(); - assertThat(httpOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { - oos.writeObject(httpOptions); - } - - HttpStorageOptions deserializedHttp; - try (ObjectInputStream ois = - new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { - deserializedHttp = (HttpStorageOptions) ois.readObject(); - } - - assertThat(deserializedHttp).isNotNull(); - assertThat(deserializedHttp.getMeterProvider()).isNull(); - assertThat(deserializedHttp.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); - - GrpcStorageOptions grpcOptions = - GrpcStorageOptions.grpc() - .setMeterProvider(customMeterProvider) - .setMetricInterval(Duration.ofSeconds(30)) - .build(); - assertThat(grpcOptions.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); - - baos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { - oos.writeObject(grpcOptions); - } - GrpcStorageOptions deserializedGrpc; - try (ObjectInputStream ois = - new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { - deserializedGrpc = (GrpcStorageOptions) ois.readObject(); - } - - assertThat(deserializedGrpc).isNotNull(); - assertThat(deserializedGrpc.getMeterProvider()).isNull(); - assertThat(deserializedGrpc.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + forEachTransport( + builder -> { + StorageOptions options = + builder + .setMeterProvider(customMeterProvider) + .setMetricInterval(Duration.ofSeconds(30)) + .build(); + assertThat(options.getMetricInterval()).isEqualTo(Duration.ofSeconds(30)); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(options); + } + + StorageOptions deserialized; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserialized = (StorageOptions) ois.readObject(); + } + + assertThat(deserialized).isNotNull(); + assertThat(deserialized.getMeterProvider()).isNull(); + assertThat(deserialized.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + }); } @Test public void deserializationWithNullMetricInterval_defaultsToSixtySeconds() throws Exception { - Field httpBuilderField = StorageOptions.Builder.class.getDeclaredField("metricInterval"); - httpBuilderField.setAccessible(true); - HttpStorageOptions.Builder httpBuilder = HttpStorageOptions.http(); - httpBuilderField.set(httpBuilder, null); - HttpStorageOptions httpWithNullInterval = httpBuilder.build(); - - ByteArrayOutputStream baos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { - oos.writeObject(httpWithNullInterval); - } - - HttpStorageOptions deserializedHttp; - try (ObjectInputStream ois = - new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { - deserializedHttp = (HttpStorageOptions) ois.readObject(); - } - - assertThat(deserializedHttp).isNotNull(); - // Verify NPE guard defaults to 60 seconds - assertThat(deserializedHttp.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); - // Verify equals and hashCode match a default instance - HttpStorageOptions defaultHttp = HttpStorageOptions.http().build(); - assertThat(deserializedHttp).isEqualTo(defaultHttp); - assertThat(defaultHttp).isEqualTo(deserializedHttp); - assertThat(deserializedHttp.hashCode()).isEqualTo(defaultHttp.hashCode()); - - Field grpcBuilderField = StorageOptions.Builder.class.getDeclaredField("metricInterval"); - grpcBuilderField.setAccessible(true); - GrpcStorageOptions.Builder grpcBuilder = GrpcStorageOptions.grpc(); - grpcBuilderField.set(grpcBuilder, null); - GrpcStorageOptions grpcWithNullInterval = grpcBuilder.build(); - - baos = new ByteArrayOutputStream(); - try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { - oos.writeObject(grpcWithNullInterval); - } - - GrpcStorageOptions deserializedGrpc; - try (ObjectInputStream ois = - new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { - deserializedGrpc = (GrpcStorageOptions) ois.readObject(); - } - - assertThat(deserializedGrpc).isNotNull(); - assertThat(deserializedGrpc.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); - GrpcStorageOptions defaultGrpc = GrpcStorageOptions.grpc().build(); - assertThat(deserializedGrpc).isEqualTo(defaultGrpc); - assertThat(defaultGrpc).isEqualTo(deserializedGrpc); - assertThat(deserializedGrpc.hashCode()).isEqualTo(defaultGrpc.hashCode()); + Field builderField = StorageOptions.Builder.class.getDeclaredField("metricInterval"); + builderField.setAccessible(true); + + forEachTransport( + builder -> { + StorageOptions defaultOptions = builder.build(); + builderField.set(builder, null); + StorageOptions optionsWithNullInterval = builder.build(); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(optionsWithNullInterval); + } + + StorageOptions deserialized; + try (ObjectInputStream ois = + new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + deserialized = (StorageOptions) ois.readObject(); + } + + assertThat(deserialized).isNotNull(); + // Verify NPE guard defaults to 60 seconds + assertThat(deserialized.getMetricInterval()).isEqualTo(Duration.ofSeconds(60)); + // Verify equals and hashCode match a default instance + assertThat(deserialized).isEqualTo(defaultOptions); + assertThat(defaultOptions).isEqualTo(deserialized); + assertThat(deserialized.hashCode()).isEqualTo(defaultOptions.hashCode()); + }); } }