From b40a98b29aafc1d6ac3feaf2283c9ea8a56a6f40 Mon Sep 17 00:00:00 2001 From: Frank Natividad Date: Fri, 17 May 2024 10:47:26 -0700 Subject: [PATCH 01/18] feat: prototype of grpc metrics enablement --- google-cloud-storage/pom.xml | 63 +++++++++++++- .../cloud/storage/GrpcStorageOptions.java | 84 +++++++++++++++++++ .../google/cloud/storage/it/ITGrpcTest.java | 20 +++-- pom.xml | 7 ++ 4 files changed, 161 insertions(+), 13 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index b6bc155e3d..91aa49f34f 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -124,6 +124,58 @@ com.google.api.grpc gapic-google-cloud-storage-v2 + + + io.opentelemetry + opentelemetry-api + 1.37.0 + + + io.opentelemetry + opentelemetry-sdk + 1.37.0 + + + io.grpc + grpc-opentelemetry + 1.64.0-SNAPSHOT + + + io.opentelemetry + opentelemetry-sdk-metrics + 1.37.0 + + + io.opentelemetry + opentelemetry-sdk-common + 1.37.0 + + + io.opentelemetry + opentelemetry-sdk-extension-autoconfigure + 1.37.0 + + + + com.google.cloud.opentelemetry + detector-resources + 0.27.0-alpha + + + com.google.cloud.opentelemetry + exporter-metrics + 0.28.0 + + + io.opentelemetry.instrumentation + opentelemetry-resources + 2.3.0-alpha + + + io.opentelemetry.contrib + opentelemetry-gcp-resources + 1.34.0-alpha + @@ -159,10 +211,7 @@ grpc-googleapis runtime - - org.checkerframework - checker-qual - + - - io.opentelemetry - opentelemetry-api - 1.37.0 - io.opentelemetry opentelemetry-sdk @@ -138,8 +133,14 @@ io.grpc grpc-opentelemetry - 1.64.0-SNAPSHOT + 1.64.0 + + io.opentelemetry + opentelemetry-api + 1.37.0 + + io.opentelemetry opentelemetry-sdk-metrics @@ -164,7 +165,7 @@ com.google.cloud.opentelemetry exporter-metrics - 0.28.0 + 0.29.0 io.opentelemetry.instrumentation @@ -318,6 +319,16 @@ + + + + io.opentelemetry.semconv + opentelemetry-semconv + 1.25.0-alpha + + + + @@ -393,6 +404,38 @@ + + org.apache.maven.plugins + maven-shade-plugin + 3.5.3 + + + package + + shade + + + + + *:* + + META-INF/*.SF + META-INF/*.DSA + META-INF/*.RSA + + + + + + com.google.cloud.storage.Main + + + true + shaded + + + + diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 3ff26d1706..56e7f7d9c7 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -41,6 +41,7 @@ import com.google.api.gax.rpc.internal.QuotaProjectIdHidingCredentials; import com.google.api.pathtemplate.PathTemplate; import com.google.auth.Credentials; +import com.google.cloud.MonitoredResource; import com.google.cloud.NoCredentials; import com.google.cloud.ServiceFactory; import com.google.cloud.ServiceOptions; @@ -50,6 +51,7 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.opentelemetry.metric.GoogleCloudMetricExporter; import com.google.cloud.opentelemetry.metric.MetricConfiguration; +import com.google.cloud.opentelemetry.metric.MonitoredResourceDescription; import com.google.cloud.spi.ServiceRpcFactory; import com.google.cloud.storage.Storage.BlobWriteOption; import com.google.cloud.storage.TransportCompatibility.Transport; @@ -96,6 +98,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.IdentityHashMap; import java.util.List; import java.util.Locale; @@ -103,7 +106,9 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.Set; +import java.util.UUID; +import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; import io.opentelemetry.instrumentation.resources.OsResource; @@ -303,13 +308,25 @@ private Tuple> resolveSettingsAndOpts() throw if (scheme.equals("http")) { channelProviderBuilder.setChannelConfigurator(ManagedChannelBuilder::usePlaintext); } + + + GCPResourceProvider resourceProvider = new GCPResourceProvider(); + Attributes detectedAttributes = resourceProvider.getAttributes(); + + MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription("generic_task", + ImmutableSet.of("project_id", "location", "namespace", "job", "task_id")); + // Uncomment when gcs_client MR is available: + //new MonitoredResourceDescription( + // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", "instance_id", "api")); + MetricExporter cloudMonitoringExporter = GoogleCloudMetricExporter.createWithConfiguration(MetricConfiguration.builder() - // TODO: Blocked by support in exporter project - // .setPrefix("storage.googleapis.com/") - // .setUseServiceTimeSeries(true) + .setMonitoredResourceDescription(monitoredResourceDescription) + //.setUseServiceTimeSeries(true) .build()); + + // Now set up PeriodicMetricReader to use this Exporter SdkMeterProvider provider = SdkMeterProvider.builder() .registerMetricReader( @@ -320,26 +337,24 @@ private Tuple> resolveSettingsAndOpts() throw PeriodicMetricReader.builder(cloudMonitoringExporter) .setInterval(java.time.Duration.ofSeconds(20)) .build()) - .addResource(Resource.getDefault() - .merge(OsResource.get()) - .merge(Resource.create(Attributes.builder() - .put("namespace", "gcs_client_instance") - .put("job", "") - .put("task_id", "") - .build()))).build(); - -// monitored_resource.set_type("generic_task"); -// auto& labels = *monitored_resource.mutable_labels(); -// // project_id untouched -// // location untouched -// labels["namespace"] = "gcs_client_instance"; -// labels["job"] = labels["host_id"]; -// labels["task_id"] = labels["instance_id"]; -// labels.erase("cloud_platform"); -// labels.erase("host_id"); -// labels.erase("api"); - - OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().setMeterProvider(provider).build(); + .setResource(Resource.create(Attributes.builder() + .put("gcp.resource_type", "generic_task") + .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put("task_id", detectedAttributes.get(AttributeKey.stringKey("gcp.gce.instance.hostname"))) + .put("namespace", "gcs_client_instance") + .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) + .put("project_id", detectedAttributes.get(AttributeKey.stringKey("cloud.account.id"))) + + /** Uncomment when gcs_client MR is available + .put("cloud_platform", detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) + .put("host_id", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put("instance_id", UUID.randomUUID().toString()) + .put("api", "grpc") + **/ + .build())) + .build(); + + OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); // TODO: add openTelemetrySdk.shutdown(); support GrpcOpenTelemetry grpcOpenTelemetry = GrpcOpenTelemetry.newBuilder().sdk(openTelemetrySdk) .enableMetrics(Arrays.asList( diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Main.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Main.java new file mode 100644 index 0000000000..1af60f8154 --- /dev/null +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/Main.java @@ -0,0 +1,38 @@ +package com.google.cloud.storage; + +import com.google.api.gax.paging.Page; +import com.google.common.collect.ImmutableList; + +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.UUID; +import java.util.stream.IntStream; +import java.util.stream.StreamSupport; + +public class Main { + public static void main(String[] args) throws InterruptedException { + Storage storage = StorageOptions.grpc().build().getService(); + System.out.println("created"); + BucketInfo bucketInfo = BucketInfo.of("gcs-grpc-team-jl-"+UUID.randomUUID()); + storage.create(bucketInfo); + + byte[] content = "Hello, World!".getBytes(StandardCharsets.UTF_8); + String prefix = UUID.randomUUID().toString(); + List blobs = + IntStream.rangeClosed(1, 10) + .mapToObj(i -> String.format("%s/%02d", prefix, i)) + .map(n -> BlobInfo.newBuilder(bucketInfo, n).build()) + .map(info -> storage.create(info, content, Storage.BlobTargetOption.doesNotExist())) + .collect(ImmutableList.toImmutableList()); + + while(true) { + Page list = storage.list(bucketInfo.getName(), Storage.BlobListOption.prefix(prefix)); + Thread.sleep(1000); + } +// ImmutableList actual = +// StreamSupport.stream(list.iterateAll().spliterator(), false) +// .map(Blob::getName) +// .collect(ImmutableList.toImmutableList()); +// assertThat(actual).isEqualTo(expected); + } +} diff --git a/pom.xml b/pom.xml index e8eb521fed..43630521de 100644 --- a/pom.xml +++ b/pom.xml @@ -66,6 +66,11 @@ pom import + + com.google.cloud.opentelemetry + exporter-metrics + 0.29.0 + com.google.cloud google-cloud-shared-dependencies From 06de28aacbd39a2524cbd68ecb44176ab00f7d01 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Fri, 14 Jun 2024 15:41:17 -0700 Subject: [PATCH 03/18] add histogram boundaries and universe domain configuration --- google-cloud-storage/pom.xml | 32 -- .../cloud/storage/GrpcStorageOptions.java | 295 +++++++++++++----- .../java/com/google/cloud/storage/Main.java | 38 --- .../cloud/storage/ITGrpcMetricsTest.java | 70 +++++ 4 files changed, 282 insertions(+), 153 deletions(-) delete mode 100644 google-cloud-storage/src/main/java/com/google/cloud/storage/Main.java create mode 100644 google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index cd2758be24..b2409a8853 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -404,38 +404,6 @@ - - org.apache.maven.plugins - maven-shade-plugin - 3.5.3 - - - package - - shade - - - - - *:* - - META-INF/*.SF - META-INF/*.DSA - META-INF/*.RSA - - - - - - com.google.cloud.storage.Main - - - true - shaded - - - - diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 56e7f7d9c7..e1238b9553 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -41,7 +41,6 @@ import com.google.api.gax.rpc.internal.QuotaProjectIdHidingCredentials; import com.google.api.pathtemplate.PathTemplate; import com.google.auth.Credentials; -import com.google.cloud.MonitoredResource; import com.google.cloud.NoCredentials; import com.google.cloud.ServiceFactory; import com.google.cloud.ServiceOptions; @@ -59,7 +58,6 @@ import com.google.cloud.storage.UnifiedOpts.UserProject; import com.google.cloud.storage.spi.StorageRpcFactory; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Function; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; @@ -91,6 +89,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.Serializable; +import java.math.BigDecimal; +import java.math.MathContext; import java.net.URI; import java.nio.ByteBuffer; import java.time.Clock; @@ -98,7 +98,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.IdentityHashMap; import java.util.List; import java.util.Locale; @@ -106,19 +105,22 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.Set; -import java.util.UUID; +import java.util.stream.Collectors; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; -import io.opentelemetry.instrumentation.resources.OsResource; import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.autoconfigure.ResourceConfiguration; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentSelector; +import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.View; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; import io.opentelemetry.sdk.resources.Resource; -import io.opentelemetry.semconv.ResourceAttributes; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import org.threeten.bp.Duration; @@ -137,6 +139,7 @@ public final class GrpcStorageOptions extends StorageOptions private final GrpcRetryAlgorithmManager retryAlgorithmManager; private final Duration terminationAwaitDuration; private final boolean attemptDirectPath; + private final boolean enableMetrics; private final GrpcInterceptorProvider grpcInterceptorProvider; private final BlobWriteSessionConfig blobWriteSessionConfig; @@ -150,6 +153,7 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) MoreObjects.firstNonNull( builder.terminationAwaitDuration, serviceDefaults.getTerminationAwaitDuration()); this.attemptDirectPath = builder.attemptDirectPath; + this.enableMetrics = builder.enableMetrics; this.grpcInterceptorProvider = builder.grpcInterceptorProvider; this.blobWriteSessionConfig = builder.blobWriteSessionConfig; } @@ -309,82 +313,9 @@ private Tuple> resolveSettingsAndOpts() throw channelProviderBuilder.setChannelConfigurator(ManagedChannelBuilder::usePlaintext); } - - GCPResourceProvider resourceProvider = new GCPResourceProvider(); - Attributes detectedAttributes = resourceProvider.getAttributes(); - - MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription("generic_task", - ImmutableSet.of("project_id", "location", "namespace", "job", "task_id")); - // Uncomment when gcs_client MR is available: - //new MonitoredResourceDescription( - // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", "instance_id", "api")); - - MetricExporter cloudMonitoringExporter = - GoogleCloudMetricExporter.createWithConfiguration(MetricConfiguration.builder() - .setMonitoredResourceDescription(monitoredResourceDescription) - //.setUseServiceTimeSeries(true) - .build()); - - - - // Now set up PeriodicMetricReader to use this Exporter - SdkMeterProvider provider = SdkMeterProvider.builder() - .registerMetricReader( - // Set collection interval to 20 seconds. - // See https://cloud.google.com/monitoring/quotas#custom_metrics_quotas - // Rate at which data can be written to a single time series: one point each 10 - // seconds. - PeriodicMetricReader.builder(cloudMonitoringExporter) - .setInterval(java.time.Duration.ofSeconds(20)) - .build()) - .setResource(Resource.create(Attributes.builder() - .put("gcp.resource_type", "generic_task") - .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) - .put("task_id", detectedAttributes.get(AttributeKey.stringKey("gcp.gce.instance.hostname"))) - .put("namespace", "gcs_client_instance") - .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", detectedAttributes.get(AttributeKey.stringKey("cloud.account.id"))) - - /** Uncomment when gcs_client MR is available - .put("cloud_platform", detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) - .put("host_id", detectedAttributes.get(AttributeKey.stringKey("host.id"))) - .put("instance_id", UUID.randomUUID().toString()) - .put("api", "grpc") - **/ - .build())) - .build(); - - OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); - // TODO: add openTelemetrySdk.shutdown(); support - GrpcOpenTelemetry grpcOpenTelemetry = GrpcOpenTelemetry.newBuilder().sdk(openTelemetrySdk) - .enableMetrics(Arrays.asList( - "grpc.lb.wrr.rr_fallback", - "grpc.lb.wrr.endpoint_weight_not_yet_usable", - "grpc.lb.wrr.endpoint_weight_stale", - "grpc.lb.wrr.endpoint_weights", - "grpc.lb.rls.cache_entries", - "grpc.lb.rls.cache_size", - "grpc.lb.rls.default_target_picks", - "grpc.lb.rls.target_picks", - "grpc.lb.rls.failed_picks" - // These don't exist yet, but you'll want them in a later release -// "grpc.xds_client.connected", -// "grpc.xds_client.server_failure", -// "grpc.xds_client.resource_updates_valid", -// "grpc.xds_client.resource_updates_invalid", -// "grpc.xds_client.resources" - )) - .build(); - // TODO: Is there another way of doing this? - ApiFunction channelConfigurator = channelProviderBuilder.getChannelConfigurator(); - channelProviderBuilder.setChannelConfigurator(b -> { - System.out.println("Called Managed Channel Builder"); - grpcOpenTelemetry.configureChannelBuilder(b); - if (channelConfigurator != null) { - return channelConfigurator.apply(b); - } - return b; - }); + if(enableMetrics) { + enableGrpcMetrics(channelProviderBuilder, endpoint); + } builder.setTransportChannelProvider(channelProviderBuilder.build()); RetrySettings baseRetrySettings = getRetrySettings(); @@ -436,6 +367,182 @@ private Tuple> resolveSettingsAndOpts() throw return Tuple.of(builder.build(), defaultOpts); } + private void enableGrpcMetrics(InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint) { + String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint); + SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint); + + OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); + GrpcOpenTelemetry grpcOpenTelemetry = GrpcOpenTelemetry.newBuilder().sdk(openTelemetrySdk) + .enableMetrics(Arrays.asList( + "grpc.lb.wrr.rr_fallback", + "grpc.lb.wrr.endpoint_weight_not_yet_usable", + "grpc.lb.wrr.endpoint_weight_stale", + "grpc.lb.wrr.endpoint_weights", + "grpc.lb.rls.cache_entries", + "grpc.lb.rls.cache_size", + "grpc.lb.rls.default_target_picks", + "grpc.lb.rls.target_picks", + "grpc.lb.rls.failed_picks", + "grpc.xds_client.connected", + "grpc.xds_client.server_failure", + "grpc.xds_client.resource_updates_valid", + "grpc.xds_client.resource_updates_invalid", + "grpc.xds_client.resources" + )) + .build(); + ApiFunction channelConfigurator = channelProviderBuilder.getChannelConfigurator(); + channelProviderBuilder.setChannelConfigurator(b -> { + grpcOpenTelemetry.configureChannelBuilder(b); + if (channelConfigurator != null) { + return channelConfigurator.apply(b); + } + return b; + }); + } + + @VisibleForTesting + String getCloudMonitoringEndpoint(String endpoint) { + String metricServiceEndpoint = "monitoring.googleapis.com"; + + String universeDomain = this.getUniverseDomain(); + + // use contains instead of equals because endpoint has a port in it + if(universeDomain != null && endpoint.contains("storage." + universeDomain)) { + metricServiceEndpoint = "monitoring." + universeDomain; + } + else if(!endpoint.contains("storage.googleapis.com")) { + String canonicalEndpoint = "storage.googleapis.com"; + String privateEndpoint = "private.googleapis.com"; + String restrictedEndpoint = "restricted.googleapis.com"; + if(universeDomain != null) { + canonicalEndpoint = "storage." + universeDomain; + privateEndpoint = "private." + universeDomain; + restrictedEndpoint = "restricted." + universeDomain; + } + String match = ImmutableList.of(canonicalEndpoint, privateEndpoint, restrictedEndpoint) + .stream() + .filter(s -> endpoint.contains(s) || endpoint.contains("google-c2p:///" + s)) + .collect(Collectors.joining()); + if(!StringUtils.isNullOrEmpty(match)) { + metricServiceEndpoint = match; + } + } + return metricServiceEndpoint + ":" + endpoint.split(":")[1]; + } + + @VisibleForTesting + SdkMeterProvider createMeterProvider(String metricServiceEndpoint) { + GCPResourceProvider resourceProvider = new GCPResourceProvider(); + Attributes detectedAttributes = resourceProvider.getAttributes(); + + MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription("generic_task", + ImmutableSet.of("project_id", "location", "namespace", "job", "task_id")); + // When the gcs_client MR is available, do this instead: + //new MonitoredResourceDescription( + // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", "instance_id", "api")); + + + MetricExporter cloudMonitoringExporter = + GoogleCloudMetricExporter.createWithConfiguration(MetricConfiguration.builder() + .setMonitoredResourceDescription(monitoredResourceDescription) + .setMetricServiceEndpoint(metricServiceEndpoint ) + //.setUseServiceTimeSeries(true) + .build()); + + String projectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); + SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder() + .registerMetricReader( + // Set collection interval to 20 seconds. + // See https://cloud.google.com/monitoring/quotas#custom_metrics_quotas + // Rate at which data can be written to a single time series: one point each 10 + // seconds. + PeriodicMetricReader.builder(cloudMonitoringExporter) + .setInterval(java.time.Duration.ofSeconds(20)) + .build()) + .setResource(Resource.create(Attributes.builder() + .put("gcp.resource_type", "generic_task") + .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put("task_id", detectedAttributes.get(AttributeKey.stringKey("gcp.gce.instance.hostname"))) + .put("namespace", "gcs_client_instance") + .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) + .put("project_id", projectId == null ? this.getProjectId() : projectId) + + /** Uncomment when gcs_client MR is available + .put("cloud_platform", detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) + .put("host_id", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put("instance_id", UUID.randomUUID().toString()) + .put("api", "grpc") + **/ + .build())); + + addHistogramView(providerBuilder, latencyHistogramBoundaries(), "grpc.client.attempt.duration", "s"); + addHistogramView(providerBuilder, sizeHistogramBoundaries(), "grpc.client.attempt.rcvd_total_compressed_message_size", "By"); + addHistogramView(providerBuilder, sizeHistogramBoundaries(), "grpc.client.attempt.sent_total_compressed_message_size", "By"); + + return providerBuilder.build(); + } + + private void addHistogramView(SdkMeterProviderBuilder provider, List boundaries, String name, String unit) { + InstrumentSelector instrumentSelector = InstrumentSelector.builder() + .setType(InstrumentType.HISTOGRAM) + .setUnit(unit) + .setName(name) + .setMeterName("grpc-java") + .setMeterSchemaUrl("") + .build(); + View view = View.builder() + .setName(name) + .setDescription("A view of " + name + " with histogram boundaries more appropriate for Google Cloud Storage RPCs") + .setAggregation(Aggregation.explicitBucketHistogram(boundaries)) + .build(); + provider.registerView(instrumentSelector, view); + } + + private List latencyHistogramBoundaries() { + List boundaries = new ArrayList<>(); + BigDecimal boundary = new BigDecimal(0, MathContext.UNLIMITED); + BigDecimal increment = new BigDecimal("0.002", MathContext.UNLIMITED); // 2ms + + // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads in the + // 100 KiB range + for(int i = 0; i != 50; i++) { + boundaries.add(boundary.doubleValue()); + boundary = boundary.add(increment); + } + + // For the remaining buckets do 10 10ms, 10 20ms, and so on, up until 5 minutes + increment = new BigDecimal("0.01", MathContext.UNLIMITED); //10 ms + for(int i = 0; i != 150 && boundary.compareTo(new BigDecimal(300)) < 1; i++) { + boundaries.add(boundary.doubleValue()); + if(i != 0 && i % 10 == 0) { + increment = increment.multiply(new BigDecimal(2)); + } + boundary = boundary.add(increment); + } + + return boundaries; + } + + private List sizeHistogramBoundaries() { + long kb = 1024; + long mb = 1024 * kb; + long gb = 1024 * mb; + + List boundaries = new ArrayList<>(); + long boundary = 0; + long increment = 128 * kb; + + // 128 KiB increments up to 4MiB, then exponential growth + while (boundaries.size() < 200 && boundary <= 16 * gb) { + boundaries.add((double)boundary); + boundary += increment; + if(boundary >= 4 * mb) { + increment *= 2; + } + } + return boundaries; + } + /** @since 2.14.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -449,6 +556,7 @@ public int hashCode() { retryAlgorithmManager, terminationAwaitDuration, attemptDirectPath, + enableMetrics, grpcInterceptorProvider, blobWriteSessionConfig, baseHashCode()); @@ -464,6 +572,7 @@ public boolean equals(Object o) { } GrpcStorageOptions that = (GrpcStorageOptions) o; return attemptDirectPath == that.attemptDirectPath + && enableMetrics == that.enableMetrics && Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager) && Objects.equals(terminationAwaitDuration, that.terminationAwaitDuration) && Objects.equals(grpcInterceptorProvider, that.grpcInterceptorProvider) @@ -507,6 +616,7 @@ public static final class Builder extends StorageOptions.Builder { private StorageRetryStrategy storageRetryStrategy; private Duration terminationAwaitDuration; private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath(); + private boolean enableMetrics = GrpcStorageDefaults.INSTANCE.isEnableMetrics(); private GrpcInterceptorProvider grpcInterceptorProvider = GrpcStorageDefaults.INSTANCE.grpcInterceptorProvider(); private BlobWriteSessionConfig blobWriteSessionConfig = @@ -520,6 +630,7 @@ public static final class Builder extends StorageOptions.Builder { this.storageRetryStrategy = gso.getRetryAlgorithmManager().retryStrategy; this.terminationAwaitDuration = gso.getTerminationAwaitDuration(); this.attemptDirectPath = gso.attemptDirectPath; + this.enableMetrics = gso.enableMetrics; this.grpcInterceptorProvider = gso.grpcInterceptorProvider; this.blobWriteSessionConfig = gso.blobWriteSessionConfig; } @@ -553,6 +664,18 @@ public GrpcStorageOptions.Builder setAttemptDirectPath(boolean attemptDirectPath this.attemptDirectPath = attemptDirectPath; return this; } + /** + * Option for whether this client should emit metrics to Cloud Monitoring. + * Enabled by default. Emitting metrics is free and requires minimal CPU + * and memory, but can be disabled by setting this to false. + * + * @since 2.41.0 This new api is in preview and is subject to breaking changes. + */ + @BetaApi + public GrpcStorageOptions.Builder setEnableMetrics(boolean enableMetrics) { + this.enableMetrics = enableMetrics; + return this; + } /** @since 2.14.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @@ -759,6 +882,12 @@ public boolean isAttemptDirectPath() { return false; } + /** @since 2.41.0 This new api is in preview and is subject to breaking changes. */ + @BetaApi + public boolean isEnableMetrics() { + return true; + } + /** @since 2.22.3 This new api is in preview and is subject to breaking changes. */ @BetaApi public GrpcInterceptorProvider grpcInterceptorProvider() { diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/Main.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/Main.java deleted file mode 100644 index 1af60f8154..0000000000 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/Main.java +++ /dev/null @@ -1,38 +0,0 @@ -package com.google.cloud.storage; - -import com.google.api.gax.paging.Page; -import com.google.common.collect.ImmutableList; - -import java.nio.charset.StandardCharsets; -import java.util.List; -import java.util.UUID; -import java.util.stream.IntStream; -import java.util.stream.StreamSupport; - -public class Main { - public static void main(String[] args) throws InterruptedException { - Storage storage = StorageOptions.grpc().build().getService(); - System.out.println("created"); - BucketInfo bucketInfo = BucketInfo.of("gcs-grpc-team-jl-"+UUID.randomUUID()); - storage.create(bucketInfo); - - byte[] content = "Hello, World!".getBytes(StandardCharsets.UTF_8); - String prefix = UUID.randomUUID().toString(); - List blobs = - IntStream.rangeClosed(1, 10) - .mapToObj(i -> String.format("%s/%02d", prefix, i)) - .map(n -> BlobInfo.newBuilder(bucketInfo, n).build()) - .map(info -> storage.create(info, content, Storage.BlobTargetOption.doesNotExist())) - .collect(ImmutableList.toImmutableList()); - - while(true) { - Page list = storage.list(bucketInfo.getName(), Storage.BlobListOption.prefix(prefix)); - Thread.sleep(1000); - } -// ImmutableList actual = -// StreamSupport.stream(list.iterateAll().spliterator(), false) -// .map(Blob::getName) -// .collect(ImmutableList.toImmutableList()); -// assertThat(actual).isEqualTo(expected); - } -} diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java new file mode 100644 index 0000000000..21b1e41686 --- /dev/null +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -0,0 +1,70 @@ +package com.google.cloud.storage; + +import com.google.cloud.storage.it.runner.StorageITRunner; +import com.google.cloud.storage.it.runner.annotations.Backend; +import com.google.cloud.storage.it.runner.annotations.CrossRun; +import io.opentelemetry.api.GlobalOpenTelemetry; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(StorageITRunner.class) +@CrossRun( + backends = {Backend.PROD}, + transports = {TransportCompatibility.Transport.GRPC}) +public class ITGrpcMetricsTest { + @Test + public void testGrpcMetrics() { + GlobalOpenTelemetry.resetForTest(); // avoids problems with "GlobalOpenTelemetry.set has already been called" + Storage storage = StorageOptions.grpc().build().getService(); + Assert.assertEquals("monitoring.googleapis.com:443", + ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("storage.googleapis.com:443")); + SdkMeterProvider provider = ((GrpcStorageOptions)storage.getOptions()) + .createMeterProvider("monitoring.googleapis.com:443"); + + + /** SDKMeterProvider doesn't expose the relevant fields we want to test, but they are present + * in the String representation, so we'll check that instead. + * Most of the resources are auto-set, and will depend on environment, which could cause flakes to check. + * We're only responsible for setting the project ID, endpoint, and Histogram boundaries, + * so we'll just check those + **/ + String result = provider.toString(); + + Assert.assertTrue(result.contains(storage.getOptions().getProjectId())); + + // This is the check for the Seconds histogram boundary. We can't practically check for every boundary, + // but if *any* are present, that means they're different from the results and we successfully set them + Assert.assertTrue(result.contains("1.2")); + + // This is the check for the Size boundary + Assert.assertTrue(result.contains("131072")); + } + + @Test + public void testGrpcMetrics_universeDomain() { + GlobalOpenTelemetry.resetForTest(); + Storage storage = StorageOptions.grpc().setUniverseDomain("my-universe-domain.com").build().getService(); + Assert.assertEquals("monitoring.my-universe-domain.com:443", + ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("storage.my-universe-domain.com:443")); + } + + @Test + public void testGrpcMetrics_private() { + GlobalOpenTelemetry.resetForTest(); + Storage storage = StorageOptions.grpc().setHost("https://private.googleapis.com").build().getService(); + + Assert.assertEquals("private.googleapis.com:443", + ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("private.googleapis.com:443")); + } + + @Test + public void testGrpcMetrics_restricted() { + GlobalOpenTelemetry.resetForTest(); + Storage storage = StorageOptions.grpc().setHost("https://restricted.googleapis.com").build().getService(); + + Assert.assertEquals("restricted.googleapis.com:443", + ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("restricted.googleapis.com:443")); + } +} From 88f6782f1f3f81c87001aa47e69ee476a94d8c36 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 17 Jun 2024 15:58:15 -0700 Subject: [PATCH 04/18] lint --- .../cloud/storage/GrpcStorageOptions.java | 206 ++++++++++-------- .../cloud/storage/ITGrpcMetricsTest.java | 106 +++++---- .../google/cloud/storage/it/ITGrpcTest.java | 18 +- 3 files changed, 187 insertions(+), 143 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index e1238b9553..39a112e31e 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -85,6 +85,20 @@ import io.grpc.Status; import io.grpc.opentelemetry.GrpcOpenTelemetry; import io.grpc.protobuf.ProtoUtils; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.internal.StringUtils; +import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentSelector; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.View; +import io.opentelemetry.sdk.metrics.export.MetricExporter; +import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; +import io.opentelemetry.sdk.resources.Resource; import java.io.Closeable; import java.io.IOException; import java.io.InputStream; @@ -106,21 +120,6 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; - -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.internal.StringUtils; -import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.metrics.Aggregation; -import io.opentelemetry.sdk.metrics.InstrumentSelector; -import io.opentelemetry.sdk.metrics.InstrumentType; -import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; -import io.opentelemetry.sdk.metrics.View; -import io.opentelemetry.sdk.metrics.export.MetricExporter; -import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; -import io.opentelemetry.sdk.resources.Resource; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import org.threeten.bp.Duration; @@ -313,7 +312,7 @@ private Tuple> resolveSettingsAndOpts() throw channelProviderBuilder.setChannelConfigurator(ManagedChannelBuilder::usePlaintext); } - if(enableMetrics) { + if (enableMetrics) { enableGrpcMetrics(channelProviderBuilder, endpoint); } @@ -367,13 +366,18 @@ private Tuple> resolveSettingsAndOpts() throw return Tuple.of(builder.build(), defaultOpts); } - private void enableGrpcMetrics(InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint) { + private void enableGrpcMetrics( + InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint) { String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint); SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint); - OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); - GrpcOpenTelemetry grpcOpenTelemetry = GrpcOpenTelemetry.newBuilder().sdk(openTelemetrySdk) - .enableMetrics(Arrays.asList( + OpenTelemetrySdk openTelemetrySdk = + OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); + GrpcOpenTelemetry grpcOpenTelemetry = + GrpcOpenTelemetry.newBuilder() + .sdk(openTelemetrySdk) + .enableMetrics( + Arrays.asList( "grpc.lb.wrr.rr_fallback", "grpc.lb.wrr.endpoint_weight_not_yet_usable", "grpc.lb.wrr.endpoint_weight_stale", @@ -387,17 +391,18 @@ private void enableGrpcMetrics(InstantiatingGrpcChannelProvider.Builder channelP "grpc.xds_client.server_failure", "grpc.xds_client.resource_updates_valid", "grpc.xds_client.resource_updates_invalid", - "grpc.xds_client.resources" - )) + "grpc.xds_client.resources")) .build(); - ApiFunction channelConfigurator = channelProviderBuilder.getChannelConfigurator(); - channelProviderBuilder.setChannelConfigurator(b -> { - grpcOpenTelemetry.configureChannelBuilder(b); - if (channelConfigurator != null) { - return channelConfigurator.apply(b); - } - return b; - }); + ApiFunction channelConfigurator = + channelProviderBuilder.getChannelConfigurator(); + channelProviderBuilder.setChannelConfigurator( + b -> { + grpcOpenTelemetry.configureChannelBuilder(b); + if (channelConfigurator != null) { + return channelConfigurator.apply(b); + } + return b; + }); } @VisibleForTesting @@ -407,23 +412,22 @@ String getCloudMonitoringEndpoint(String endpoint) { String universeDomain = this.getUniverseDomain(); // use contains instead of equals because endpoint has a port in it - if(universeDomain != null && endpoint.contains("storage." + universeDomain)) { + if (universeDomain != null && endpoint.contains("storage." + universeDomain)) { metricServiceEndpoint = "monitoring." + universeDomain; - } - else if(!endpoint.contains("storage.googleapis.com")) { + } else if (!endpoint.contains("storage.googleapis.com")) { String canonicalEndpoint = "storage.googleapis.com"; String privateEndpoint = "private.googleapis.com"; String restrictedEndpoint = "restricted.googleapis.com"; - if(universeDomain != null) { + if (universeDomain != null) { canonicalEndpoint = "storage." + universeDomain; privateEndpoint = "private." + universeDomain; restrictedEndpoint = "restricted." + universeDomain; } - String match = ImmutableList.of(canonicalEndpoint, privateEndpoint, restrictedEndpoint) - .stream() + String match = + ImmutableList.of(canonicalEndpoint, privateEndpoint, restrictedEndpoint).stream() .filter(s -> endpoint.contains(s) || endpoint.contains("google-c2p:///" + s)) .collect(Collectors.joining()); - if(!StringUtils.isNullOrEmpty(match)) { + if (!StringUtils.isNullOrEmpty(match)) { metricServiceEndpoint = match; } } @@ -435,64 +439,91 @@ SdkMeterProvider createMeterProvider(String metricServiceEndpoint) { GCPResourceProvider resourceProvider = new GCPResourceProvider(); Attributes detectedAttributes = resourceProvider.getAttributes(); - MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription("generic_task", + MonitoredResourceDescription monitoredResourceDescription = + new MonitoredResourceDescription( + "generic_task", ImmutableSet.of("project_id", "location", "namespace", "job", "task_id")); // When the gcs_client MR is available, do this instead: - //new MonitoredResourceDescription( - // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", "instance_id", "api")); - + // new MonitoredResourceDescription( + // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", + // "instance_id", "api")); MetricExporter cloudMonitoringExporter = - GoogleCloudMetricExporter.createWithConfiguration(MetricConfiguration.builder() - .setMonitoredResourceDescription(monitoredResourceDescription) - .setMetricServiceEndpoint(metricServiceEndpoint ) - //.setUseServiceTimeSeries(true) - .build()); + GoogleCloudMetricExporter.createWithConfiguration( + MetricConfiguration.builder() + .setMonitoredResourceDescription(monitoredResourceDescription) + .setMetricServiceEndpoint(metricServiceEndpoint) + // .setUseServiceTimeSeries(true) + .build()); String projectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); - SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder() + SdkMeterProviderBuilder providerBuilder = + SdkMeterProvider.builder() .registerMetricReader( - // Set collection interval to 20 seconds. - // See https://cloud.google.com/monitoring/quotas#custom_metrics_quotas - // Rate at which data can be written to a single time series: one point each 10 - // seconds. - PeriodicMetricReader.builder(cloudMonitoringExporter) - .setInterval(java.time.Duration.ofSeconds(20)) - .build()) - .setResource(Resource.create(Attributes.builder() - .put("gcp.resource_type", "generic_task") - .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) - .put("task_id", detectedAttributes.get(AttributeKey.stringKey("gcp.gce.instance.hostname"))) - .put("namespace", "gcs_client_instance") - .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", projectId == null ? this.getProjectId() : projectId) - - /** Uncomment when gcs_client MR is available - .put("cloud_platform", detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) - .put("host_id", detectedAttributes.get(AttributeKey.stringKey("host.id"))) - .put("instance_id", UUID.randomUUID().toString()) - .put("api", "grpc") - **/ - .build())); - - addHistogramView(providerBuilder, latencyHistogramBoundaries(), "grpc.client.attempt.duration", "s"); - addHistogramView(providerBuilder, sizeHistogramBoundaries(), "grpc.client.attempt.rcvd_total_compressed_message_size", "By"); - addHistogramView(providerBuilder, sizeHistogramBoundaries(), "grpc.client.attempt.sent_total_compressed_message_size", "By"); + // Set collection interval to 20 seconds. + // See https://cloud.google.com/monitoring/quotas#custom_metrics_quotas + // Rate at which data can be written to a single time series: one point each 10 + // seconds. + PeriodicMetricReader.builder(cloudMonitoringExporter) + .setInterval(java.time.Duration.ofSeconds(20)) + .build()) + .setResource( + Resource.create( + Attributes.builder() + .put("gcp.resource_type", "generic_task") + .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put( + "task_id", + detectedAttributes.get( + AttributeKey.stringKey("gcp.gce.instance.hostname"))) + .put("namespace", "gcs_client_instance") + .put( + "location", + detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) + .put("project_id", projectId == null ? this.getProjectId() : projectId) + + /** + * Uncomment when gcs_client MR is available .put("cloud_platform", + * detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) + * .put("host_id", + * detectedAttributes.get(AttributeKey.stringKey("host.id"))) + * .put("instance_id", UUID.randomUUID().toString()) .put("api", "grpc") + */ + .build())); + + addHistogramView( + providerBuilder, latencyHistogramBoundaries(), "grpc.client.attempt.duration", "s"); + addHistogramView( + providerBuilder, + sizeHistogramBoundaries(), + "grpc.client.attempt.rcvd_total_compressed_message_size", + "By"); + addHistogramView( + providerBuilder, + sizeHistogramBoundaries(), + "grpc.client.attempt.sent_total_compressed_message_size", + "By"); return providerBuilder.build(); } - private void addHistogramView(SdkMeterProviderBuilder provider, List boundaries, String name, String unit) { - InstrumentSelector instrumentSelector = InstrumentSelector.builder() + private void addHistogramView( + SdkMeterProviderBuilder provider, List boundaries, String name, String unit) { + InstrumentSelector instrumentSelector = + InstrumentSelector.builder() .setType(InstrumentType.HISTOGRAM) .setUnit(unit) .setName(name) .setMeterName("grpc-java") .setMeterSchemaUrl("") .build(); - View view = View.builder() + View view = + View.builder() .setName(name) - .setDescription("A view of " + name + " with histogram boundaries more appropriate for Google Cloud Storage RPCs") + .setDescription( + "A view of " + + name + + " with histogram boundaries more appropriate for Google Cloud Storage RPCs") .setAggregation(Aggregation.explicitBucketHistogram(boundaries)) .build(); provider.registerView(instrumentSelector, view); @@ -503,18 +534,19 @@ private List latencyHistogramBoundaries() { BigDecimal boundary = new BigDecimal(0, MathContext.UNLIMITED); BigDecimal increment = new BigDecimal("0.002", MathContext.UNLIMITED); // 2ms - // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads in the + // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads + // in the // 100 KiB range - for(int i = 0; i != 50; i++) { + for (int i = 0; i != 50; i++) { boundaries.add(boundary.doubleValue()); boundary = boundary.add(increment); } // For the remaining buckets do 10 10ms, 10 20ms, and so on, up until 5 minutes - increment = new BigDecimal("0.01", MathContext.UNLIMITED); //10 ms - for(int i = 0; i != 150 && boundary.compareTo(new BigDecimal(300)) < 1; i++) { + increment = new BigDecimal("0.01", MathContext.UNLIMITED); // 10 ms + for (int i = 0; i != 150 && boundary.compareTo(new BigDecimal(300)) < 1; i++) { boundaries.add(boundary.doubleValue()); - if(i != 0 && i % 10 == 0) { + if (i != 0 && i % 10 == 0) { increment = increment.multiply(new BigDecimal(2)); } boundary = boundary.add(increment); @@ -534,9 +566,9 @@ private List sizeHistogramBoundaries() { // 128 KiB increments up to 4MiB, then exponential growth while (boundaries.size() < 200 && boundary <= 16 * gb) { - boundaries.add((double)boundary); + boundaries.add((double) boundary); boundary += increment; - if(boundary >= 4 * mb) { + if (boundary >= 4 * mb) { increment *= 2; } } @@ -665,9 +697,9 @@ public GrpcStorageOptions.Builder setAttemptDirectPath(boolean attemptDirectPath return this; } /** - * Option for whether this client should emit metrics to Cloud Monitoring. - * Enabled by default. Emitting metrics is free and requires minimal CPU - * and memory, but can be disabled by setting this to false. + * Option for whether this client should emit metrics to Cloud Monitoring. Enabled by default. + * Emitting metrics is free and requires minimal CPU and memory, but can be disabled by setting + * this to false. * * @since 2.41.0 This new api is in preview and is subject to breaking changes. */ diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index 21b1e41686..75504d657b 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -11,60 +11,74 @@ @RunWith(StorageITRunner.class) @CrossRun( - backends = {Backend.PROD}, - transports = {TransportCompatibility.Transport.GRPC}) + backends = {Backend.PROD}, + transports = {TransportCompatibility.Transport.GRPC}) public class ITGrpcMetricsTest { - @Test - public void testGrpcMetrics() { - GlobalOpenTelemetry.resetForTest(); // avoids problems with "GlobalOpenTelemetry.set has already been called" - Storage storage = StorageOptions.grpc().build().getService(); - Assert.assertEquals("monitoring.googleapis.com:443", - ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("storage.googleapis.com:443")); - SdkMeterProvider provider = ((GrpcStorageOptions)storage.getOptions()) - .createMeterProvider("monitoring.googleapis.com:443"); + @Test + public void testGrpcMetrics() { + GlobalOpenTelemetry + .resetForTest(); // avoids problems with "GlobalOpenTelemetry.set has already been called" + Storage storage = StorageOptions.grpc().build().getService(); + Assert.assertEquals( + "monitoring.googleapis.com:443", + ((GrpcStorageOptions) storage.getOptions()) + .getCloudMonitoringEndpoint("storage.googleapis.com:443")); + SdkMeterProvider provider = + ((GrpcStorageOptions) storage.getOptions()) + .createMeterProvider("monitoring.googleapis.com:443"); + /** + * SDKMeterProvider doesn't expose the relevant fields we want to test, but they are present in + * the String representation, so we'll check that instead. Most of the resources are auto-set, + * and will depend on environment, which could cause flakes to check. We're only responsible for + * setting the project ID, endpoint, and Histogram boundaries, so we'll just check those + */ + String result = provider.toString(); - /** SDKMeterProvider doesn't expose the relevant fields we want to test, but they are present - * in the String representation, so we'll check that instead. - * Most of the resources are auto-set, and will depend on environment, which could cause flakes to check. - * We're only responsible for setting the project ID, endpoint, and Histogram boundaries, - * so we'll just check those - **/ - String result = provider.toString(); + Assert.assertTrue(result.contains(storage.getOptions().getProjectId())); - Assert.assertTrue(result.contains(storage.getOptions().getProjectId())); + // This is the check for the Seconds histogram boundary. We can't practically check for every + // boundary, + // but if *any* are present, that means they're different from the results and we successfully + // set them + Assert.assertTrue(result.contains("1.2")); - // This is the check for the Seconds histogram boundary. We can't practically check for every boundary, - // but if *any* are present, that means they're different from the results and we successfully set them - Assert.assertTrue(result.contains("1.2")); + // This is the check for the Size boundary + Assert.assertTrue(result.contains("131072")); + } - // This is the check for the Size boundary - Assert.assertTrue(result.contains("131072")); - } + @Test + public void testGrpcMetrics_universeDomain() { + GlobalOpenTelemetry.resetForTest(); + Storage storage = + StorageOptions.grpc().setUniverseDomain("my-universe-domain.com").build().getService(); + Assert.assertEquals( + "monitoring.my-universe-domain.com:443", + ((GrpcStorageOptions) storage.getOptions()) + .getCloudMonitoringEndpoint("storage.my-universe-domain.com:443")); + } - @Test - public void testGrpcMetrics_universeDomain() { - GlobalOpenTelemetry.resetForTest(); - Storage storage = StorageOptions.grpc().setUniverseDomain("my-universe-domain.com").build().getService(); - Assert.assertEquals("monitoring.my-universe-domain.com:443", - ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("storage.my-universe-domain.com:443")); - } + @Test + public void testGrpcMetrics_private() { + GlobalOpenTelemetry.resetForTest(); + Storage storage = + StorageOptions.grpc().setHost("https://private.googleapis.com").build().getService(); - @Test - public void testGrpcMetrics_private() { - GlobalOpenTelemetry.resetForTest(); - Storage storage = StorageOptions.grpc().setHost("https://private.googleapis.com").build().getService(); + Assert.assertEquals( + "private.googleapis.com:443", + ((GrpcStorageOptions) storage.getOptions()) + .getCloudMonitoringEndpoint("private.googleapis.com:443")); + } - Assert.assertEquals("private.googleapis.com:443", - ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("private.googleapis.com:443")); - } + @Test + public void testGrpcMetrics_restricted() { + GlobalOpenTelemetry.resetForTest(); + Storage storage = + StorageOptions.grpc().setHost("https://restricted.googleapis.com").build().getService(); - @Test - public void testGrpcMetrics_restricted() { - GlobalOpenTelemetry.resetForTest(); - Storage storage = StorageOptions.grpc().setHost("https://restricted.googleapis.com").build().getService(); - - Assert.assertEquals("restricted.googleapis.com:443", - ((GrpcStorageOptions) storage.getOptions()).getCloudMonitoringEndpoint("restricted.googleapis.com:443")); - } + Assert.assertEquals( + "restricted.googleapis.com:443", + ((GrpcStorageOptions) storage.getOptions()) + .getCloudMonitoringEndpoint("restricted.googleapis.com:443")); + } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java index c8d2938d61..741fd8c3e4 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java @@ -77,7 +77,7 @@ public void testCreateBucket() { } @Test - public void listBlobs() throws InterruptedException { + public void listBlobs() { byte[] content = "Hello, World!".getBytes(StandardCharsets.UTF_8); String prefix = generator.randomObjectName(); List blobs = @@ -90,15 +90,13 @@ public void listBlobs() throws InterruptedException { List expected = blobs.stream().map(Blob::getName).collect(ImmutableList.toImmutableList()); - while(true) { - Page list = storage.list(bucketInfo.getName(), BlobListOption.prefix(prefix)); - Thread.sleep(1000); - } -// ImmutableList actual = -// StreamSupport.stream(list.iterateAll().spliterator(), false) -// .map(Blob::getName) -// .collect(ImmutableList.toImmutableList()); -// assertThat(actual).isEqualTo(expected); + Page list = storage.list(bucketInfo.getName(), BlobListOption.prefix(prefix)); + ImmutableList actual = + StreamSupport.stream(list.iterateAll().spliterator(), false) + .map(Blob::getName) + .collect(ImmutableList.toImmutableList()); + + assertThat(actual).isEqualTo(expected); } @Test From f42efece0de85ad93b788c68f4e6e4df6e3849a3 Mon Sep 17 00:00:00 2001 From: JesseLovelace <43148100+JesseLovelace@users.noreply.github.com> Date: Tue, 18 Jun 2024 13:47:41 -0700 Subject: [PATCH 05/18] Apply suggestions from code review Co-authored-by: BenWhitehead --- google-cloud-storage/pom.xml | 14 +++++----- .../cloud/storage/ITGrpcMetricsTest.java | 27 +++++++++---------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index b2409a8853..cf4e4144e4 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -128,7 +128,7 @@ io.opentelemetry opentelemetry-sdk - 1.37.0 + 1.38.0 io.grpc @@ -138,23 +138,23 @@ io.opentelemetry opentelemetry-api - 1.37.0 + 1.38.0 io.opentelemetry opentelemetry-sdk-metrics - 1.37.0 + 1.38.0 io.opentelemetry opentelemetry-sdk-common - 1.37.0 + 1.38.0 io.opentelemetry opentelemetry-sdk-extension-autoconfigure - 1.37.0 + 1.38.0 @@ -170,12 +170,12 @@ io.opentelemetry.instrumentation opentelemetry-resources - 2.3.0-alpha + 2.5.0-alpha io.opentelemetry.contrib opentelemetry-gcp-resources - 1.34.0-alpha + 1.36.0-alpha diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index 75504d657b..5b0a6156ab 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -1,5 +1,7 @@ package com.google.cloud.storage; +import static com.google.common.truth.Truth.assertThat; + import com.google.cloud.storage.it.runner.StorageITRunner; import com.google.cloud.storage.it.runner.annotations.Backend; import com.google.cloud.storage.it.runner.annotations.CrossRun; @@ -10,24 +12,21 @@ import org.junit.runner.RunWith; @RunWith(StorageITRunner.class) -@CrossRun( - backends = {Backend.PROD}, - transports = {TransportCompatibility.Transport.GRPC}) +@SingleBackend(Backend.PROD) public class ITGrpcMetricsTest { @Test public void testGrpcMetrics() { GlobalOpenTelemetry .resetForTest(); // avoids problems with "GlobalOpenTelemetry.set has already been called" - Storage storage = StorageOptions.grpc().build().getService(); - Assert.assertEquals( - "monitoring.googleapis.com:443", - ((GrpcStorageOptions) storage.getOptions()) - .getCloudMonitoringEndpoint("storage.googleapis.com:443")); + GrpcStorageOptions grpcStorageOptions = StorageOptions.grpc().build(); + assertThat(grpcStorageOptions.getCloudMonitoringEndpoint("storage.googleapis.com:443")) + .isEqualTo("monitoring.googleapis.com:443"); + SdkMeterProvider provider = - ((GrpcStorageOptions) storage.getOptions()) - .createMeterProvider("monitoring.googleapis.com:443"); + grpcStorageOptions.createMeterProvider("monitoring.googleapis.com:443"); + - /** + /* * SDKMeterProvider doesn't expose the relevant fields we want to test, but they are present in * the String representation, so we'll check that instead. Most of the resources are auto-set, * and will depend on environment, which could cause flakes to check. We're only responsible for @@ -35,16 +34,16 @@ public void testGrpcMetrics() { */ String result = provider.toString(); - Assert.assertTrue(result.contains(storage.getOptions().getProjectId())); + assertThat(result).contains(grpcStorageOptions.getProjectId()); // This is the check for the Seconds histogram boundary. We can't practically check for every // boundary, // but if *any* are present, that means they're different from the results and we successfully // set them - Assert.assertTrue(result.contains("1.2")); + assertThat(result).contains("1.2"); // This is the check for the Size boundary - Assert.assertTrue(result.contains("131072")); + assertThat(result).contains("131072"); } @Test From e9422c1c5391ce3db7a6a640a50f3b66d85f11eb Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Tue, 18 Jun 2024 15:42:37 -0700 Subject: [PATCH 06/18] review comments --- google-cloud-storage/pom.xml | 10 - .../cloud/storage/GrpcStorageOptions.java | 240 +--------------- .../OpenTelemetryBootstrappingUtils.java | 256 ++++++++++++++++++ .../cloud/storage/ITGrpcMetricsTest.java | 59 ++-- .../google/cloud/storage/it/ITGrpcTest.java | 2 +- renovate.json | 7 + 6 files changed, 297 insertions(+), 277 deletions(-) create mode 100644 google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index cf4e4144e4..93c0752552 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -319,16 +319,6 @@ - - - - io.opentelemetry.semconv - opentelemetry-semconv - 1.25.0-alpha - - - - diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 39a112e31e..bbaf9cc77b 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -20,7 +20,6 @@ import static java.util.Objects.requireNonNull; import com.google.api.core.ApiClock; -import com.google.api.core.ApiFunction; import com.google.api.core.BetaApi; import com.google.api.core.InternalApi; import com.google.api.gax.core.CredentialsProvider; @@ -48,9 +47,6 @@ import com.google.cloud.TransportOptions; import com.google.cloud.Tuple; import com.google.cloud.grpc.GrpcTransportOptions; -import com.google.cloud.opentelemetry.metric.GoogleCloudMetricExporter; -import com.google.cloud.opentelemetry.metric.MetricConfiguration; -import com.google.cloud.opentelemetry.metric.MonitoredResourceDescription; import com.google.cloud.spi.ServiceRpcFactory; import com.google.cloud.storage.Storage.BlobWriteOption; import com.google.cloud.storage.TransportCompatibility.Transport; @@ -83,28 +79,11 @@ import io.grpc.ManagedChannelBuilder; import io.grpc.MethodDescriptor; import io.grpc.Status; -import io.grpc.opentelemetry.GrpcOpenTelemetry; import io.grpc.protobuf.ProtoUtils; -import io.opentelemetry.api.common.AttributeKey; -import io.opentelemetry.api.common.Attributes; -import io.opentelemetry.api.internal.StringUtils; -import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; -import io.opentelemetry.sdk.OpenTelemetrySdk; -import io.opentelemetry.sdk.metrics.Aggregation; -import io.opentelemetry.sdk.metrics.InstrumentSelector; -import io.opentelemetry.sdk.metrics.InstrumentType; -import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; -import io.opentelemetry.sdk.metrics.View; -import io.opentelemetry.sdk.metrics.export.MetricExporter; -import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; -import io.opentelemetry.sdk.resources.Resource; import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.io.Serializable; -import java.math.BigDecimal; -import java.math.MathContext; import java.net.URI; import java.nio.ByteBuffer; import java.time.Clock; @@ -119,7 +98,6 @@ import java.util.Map.Entry; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; import org.threeten.bp.Duration; @@ -313,7 +291,7 @@ private Tuple> resolveSettingsAndOpts() throw } if (enableMetrics) { - enableGrpcMetrics(channelProviderBuilder, endpoint); + OpenTelemetryBootstrappingUtils.enableGrpcMetrics(channelProviderBuilder, endpoint, this.getProjectId(), this.getUniverseDomain()); } builder.setTransportChannelProvider(channelProviderBuilder.build()); @@ -366,215 +344,6 @@ private Tuple> resolveSettingsAndOpts() throw return Tuple.of(builder.build(), defaultOpts); } - private void enableGrpcMetrics( - InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint) { - String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint); - SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint); - - OpenTelemetrySdk openTelemetrySdk = - OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); - GrpcOpenTelemetry grpcOpenTelemetry = - GrpcOpenTelemetry.newBuilder() - .sdk(openTelemetrySdk) - .enableMetrics( - Arrays.asList( - "grpc.lb.wrr.rr_fallback", - "grpc.lb.wrr.endpoint_weight_not_yet_usable", - "grpc.lb.wrr.endpoint_weight_stale", - "grpc.lb.wrr.endpoint_weights", - "grpc.lb.rls.cache_entries", - "grpc.lb.rls.cache_size", - "grpc.lb.rls.default_target_picks", - "grpc.lb.rls.target_picks", - "grpc.lb.rls.failed_picks", - "grpc.xds_client.connected", - "grpc.xds_client.server_failure", - "grpc.xds_client.resource_updates_valid", - "grpc.xds_client.resource_updates_invalid", - "grpc.xds_client.resources")) - .build(); - ApiFunction channelConfigurator = - channelProviderBuilder.getChannelConfigurator(); - channelProviderBuilder.setChannelConfigurator( - b -> { - grpcOpenTelemetry.configureChannelBuilder(b); - if (channelConfigurator != null) { - return channelConfigurator.apply(b); - } - return b; - }); - } - - @VisibleForTesting - String getCloudMonitoringEndpoint(String endpoint) { - String metricServiceEndpoint = "monitoring.googleapis.com"; - - String universeDomain = this.getUniverseDomain(); - - // use contains instead of equals because endpoint has a port in it - if (universeDomain != null && endpoint.contains("storage." + universeDomain)) { - metricServiceEndpoint = "monitoring." + universeDomain; - } else if (!endpoint.contains("storage.googleapis.com")) { - String canonicalEndpoint = "storage.googleapis.com"; - String privateEndpoint = "private.googleapis.com"; - String restrictedEndpoint = "restricted.googleapis.com"; - if (universeDomain != null) { - canonicalEndpoint = "storage." + universeDomain; - privateEndpoint = "private." + universeDomain; - restrictedEndpoint = "restricted." + universeDomain; - } - String match = - ImmutableList.of(canonicalEndpoint, privateEndpoint, restrictedEndpoint).stream() - .filter(s -> endpoint.contains(s) || endpoint.contains("google-c2p:///" + s)) - .collect(Collectors.joining()); - if (!StringUtils.isNullOrEmpty(match)) { - metricServiceEndpoint = match; - } - } - return metricServiceEndpoint + ":" + endpoint.split(":")[1]; - } - - @VisibleForTesting - SdkMeterProvider createMeterProvider(String metricServiceEndpoint) { - GCPResourceProvider resourceProvider = new GCPResourceProvider(); - Attributes detectedAttributes = resourceProvider.getAttributes(); - - MonitoredResourceDescription monitoredResourceDescription = - new MonitoredResourceDescription( - "generic_task", - ImmutableSet.of("project_id", "location", "namespace", "job", "task_id")); - // When the gcs_client MR is available, do this instead: - // new MonitoredResourceDescription( - // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", - // "instance_id", "api")); - - MetricExporter cloudMonitoringExporter = - GoogleCloudMetricExporter.createWithConfiguration( - MetricConfiguration.builder() - .setMonitoredResourceDescription(monitoredResourceDescription) - .setMetricServiceEndpoint(metricServiceEndpoint) - // .setUseServiceTimeSeries(true) - .build()); - - String projectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); - SdkMeterProviderBuilder providerBuilder = - SdkMeterProvider.builder() - .registerMetricReader( - // Set collection interval to 20 seconds. - // See https://cloud.google.com/monitoring/quotas#custom_metrics_quotas - // Rate at which data can be written to a single time series: one point each 10 - // seconds. - PeriodicMetricReader.builder(cloudMonitoringExporter) - .setInterval(java.time.Duration.ofSeconds(20)) - .build()) - .setResource( - Resource.create( - Attributes.builder() - .put("gcp.resource_type", "generic_task") - .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) - .put( - "task_id", - detectedAttributes.get( - AttributeKey.stringKey("gcp.gce.instance.hostname"))) - .put("namespace", "gcs_client_instance") - .put( - "location", - detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", projectId == null ? this.getProjectId() : projectId) - - /** - * Uncomment when gcs_client MR is available .put("cloud_platform", - * detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) - * .put("host_id", - * detectedAttributes.get(AttributeKey.stringKey("host.id"))) - * .put("instance_id", UUID.randomUUID().toString()) .put("api", "grpc") - */ - .build())); - - addHistogramView( - providerBuilder, latencyHistogramBoundaries(), "grpc.client.attempt.duration", "s"); - addHistogramView( - providerBuilder, - sizeHistogramBoundaries(), - "grpc.client.attempt.rcvd_total_compressed_message_size", - "By"); - addHistogramView( - providerBuilder, - sizeHistogramBoundaries(), - "grpc.client.attempt.sent_total_compressed_message_size", - "By"); - - return providerBuilder.build(); - } - - private void addHistogramView( - SdkMeterProviderBuilder provider, List boundaries, String name, String unit) { - InstrumentSelector instrumentSelector = - InstrumentSelector.builder() - .setType(InstrumentType.HISTOGRAM) - .setUnit(unit) - .setName(name) - .setMeterName("grpc-java") - .setMeterSchemaUrl("") - .build(); - View view = - View.builder() - .setName(name) - .setDescription( - "A view of " - + name - + " with histogram boundaries more appropriate for Google Cloud Storage RPCs") - .setAggregation(Aggregation.explicitBucketHistogram(boundaries)) - .build(); - provider.registerView(instrumentSelector, view); - } - - private List latencyHistogramBoundaries() { - List boundaries = new ArrayList<>(); - BigDecimal boundary = new BigDecimal(0, MathContext.UNLIMITED); - BigDecimal increment = new BigDecimal("0.002", MathContext.UNLIMITED); // 2ms - - // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads - // in the - // 100 KiB range - for (int i = 0; i != 50; i++) { - boundaries.add(boundary.doubleValue()); - boundary = boundary.add(increment); - } - - // For the remaining buckets do 10 10ms, 10 20ms, and so on, up until 5 minutes - increment = new BigDecimal("0.01", MathContext.UNLIMITED); // 10 ms - for (int i = 0; i != 150 && boundary.compareTo(new BigDecimal(300)) < 1; i++) { - boundaries.add(boundary.doubleValue()); - if (i != 0 && i % 10 == 0) { - increment = increment.multiply(new BigDecimal(2)); - } - boundary = boundary.add(increment); - } - - return boundaries; - } - - private List sizeHistogramBoundaries() { - long kb = 1024; - long mb = 1024 * kb; - long gb = 1024 * mb; - - List boundaries = new ArrayList<>(); - long boundary = 0; - long increment = 128 * kb; - - // 128 KiB increments up to 4MiB, then exponential growth - while (boundaries.size() < 200 && boundary <= 16 * gb) { - boundaries.add((double) boundary); - boundary += increment; - if (boundary >= 4 * mb) { - increment *= 2; - } - } - return boundaries; - } - /** @since 2.14.0 This new api is in preview and is subject to breaking changes. */ @BetaApi @Override @@ -697,9 +466,10 @@ public GrpcStorageOptions.Builder setAttemptDirectPath(boolean attemptDirectPath return this; } /** - * Option for whether this client should emit metrics to Cloud Monitoring. Enabled by default. - * Emitting metrics is free and requires minimal CPU and memory, but can be disabled by setting - * this to false. + * Option for whether this client should emit internal gRPC client internal metrics to Cloud Monitoring. + * To disable metric reporting, set this to false. + * True by default. + * Emitting metrics is free and requires minimal CPU and memory. * * @since 2.41.0 This new api is in preview and is subject to breaking changes. */ diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java new file mode 100644 index 0000000000..bec7a5ef97 --- /dev/null +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -0,0 +1,256 @@ +package com.google.cloud.storage;/* + * Copyright 2024 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. + */ + +import com.google.api.core.ApiFunction; +import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; +import com.google.cloud.opentelemetry.metric.GoogleCloudMetricExporter; +import com.google.cloud.opentelemetry.metric.MetricConfiguration; +import com.google.cloud.opentelemetry.metric.MonitoredResourceDescription; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import io.grpc.ManagedChannelBuilder; +import io.grpc.opentelemetry.GrpcOpenTelemetry; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.internal.StringUtils; +import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; +import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.metrics.Aggregation; +import io.opentelemetry.sdk.metrics.InstrumentSelector; +import io.opentelemetry.sdk.metrics.InstrumentType; +import io.opentelemetry.sdk.metrics.SdkMeterProvider; +import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; +import io.opentelemetry.sdk.metrics.View; +import io.opentelemetry.sdk.metrics.export.MetricExporter; +import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; +import io.opentelemetry.sdk.resources.Resource; + +import java.math.BigDecimal; +import java.math.MathContext; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +public class OpenTelemetryBootstrappingUtils { + + static void enableGrpcMetrics( + InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint, String projectId, String universeDomain) { + String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint, universeDomain); + SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint, projectId); + + OpenTelemetrySdk openTelemetrySdk = + OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); + GrpcOpenTelemetry grpcOpenTelemetry = + GrpcOpenTelemetry.newBuilder() + .sdk(openTelemetrySdk) + .enableMetrics( + ImmutableList.of( + "grpc.lb.wrr.rr_fallback", + "grpc.lb.wrr.endpoint_weight_not_yet_usable", + "grpc.lb.wrr.endpoint_weight_stale", + "grpc.lb.wrr.endpoint_weights", + "grpc.lb.rls.cache_entries", + "grpc.lb.rls.cache_size", + "grpc.lb.rls.default_target_picks", + "grpc.lb.rls.target_picks", + "grpc.lb.rls.failed_picks", + "grpc.xds_client.connected", + "grpc.xds_client.server_failure", + "grpc.xds_client.resource_updates_valid", + "grpc.xds_client.resource_updates_invalid", + "grpc.xds_client.resources")) + .build(); + ApiFunction channelConfigurator = + channelProviderBuilder.getChannelConfigurator(); + channelProviderBuilder.setChannelConfigurator( + b -> { + grpcOpenTelemetry.configureChannelBuilder(b); + if (channelConfigurator != null) { + return channelConfigurator.apply(b); + } + return b; + }); + } + + @VisibleForTesting + static String getCloudMonitoringEndpoint(String endpoint, String universeDomain) { + String metricServiceEndpoint = "monitoring.googleapis.com"; + + // use contains instead of equals because endpoint has a port in it + if (universeDomain != null && endpoint.contains("storage." + universeDomain)) { + metricServiceEndpoint = "monitoring." + universeDomain; + } else if (!endpoint.contains("storage.googleapis.com")) { + String canonicalEndpoint = "storage.googleapis.com"; + String privateEndpoint = "private.googleapis.com"; + String restrictedEndpoint = "restricted.googleapis.com"; + if (universeDomain != null) { + canonicalEndpoint = "storage." + universeDomain; + privateEndpoint = "private." + universeDomain; + restrictedEndpoint = "restricted." + universeDomain; + } + String match = + ImmutableList.of(canonicalEndpoint, privateEndpoint, restrictedEndpoint).stream() + .filter(s -> endpoint.contains(s) || endpoint.contains("google-c2p:///" + s)) + .collect(Collectors.joining()); + if (!StringUtils.isNullOrEmpty(match)) { + metricServiceEndpoint = match; + } + } + return metricServiceEndpoint + ":" + endpoint.split(":")[1]; + } + + @VisibleForTesting + static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String projectId) { + GCPResourceProvider resourceProvider = new GCPResourceProvider(); + Attributes detectedAttributes = resourceProvider.getAttributes(); + + MonitoredResourceDescription monitoredResourceDescription = + new MonitoredResourceDescription( + "generic_task", + ImmutableSet.of("project_id", "location", "namespace", "job", "task_id")); + // When the gcs_client MR is available, do this instead: + // new MonitoredResourceDescription( + // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", + // "instance_id", "api")); + + MetricExporter cloudMonitoringExporter = + GoogleCloudMetricExporter.createWithConfiguration( + MetricConfiguration.builder() + .setMonitoredResourceDescription(monitoredResourceDescription) + .setMetricServiceEndpoint(metricServiceEndpoint) + // .setUseServiceTimeSeries(true) + .build()); + + String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); + SdkMeterProviderBuilder providerBuilder = + SdkMeterProvider.builder() + .registerMetricReader( + // Set collection interval to 20 seconds. + // See https://cloud.google.com/monitoring/quotas#custom_metrics_quotas + // Rate at which data can be written to a single time series: one point each 10 + // seconds. + PeriodicMetricReader.builder(cloudMonitoringExporter) + .setInterval(java.time.Duration.ofSeconds(20)) + .build()) + .setResource( + Resource.create( + Attributes.builder() + .put("gcp.resource_type", "generic_task") + .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put( + "task_id", + detectedAttributes.get( + AttributeKey.stringKey("gcp.gce.instance.hostname"))) + .put("namespace", "gcs_client_instance") + .put( + "location", + detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) + .put("project_id", detectedProjectId == null ? projectId : detectedProjectId) + + /** + * Uncomment when gcs_client MR is available .put("cloud_platform", + * detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) + * .put("host_id", + * detectedAttributes.get(AttributeKey.stringKey("host.id"))) + * .put("instance_id", UUID.randomUUID().toString()) .put("api", "grpc") + */ + .build())); + + addHistogramView( + providerBuilder, latencyHistogramBoundaries(), "grpc.client.attempt.duration", "s"); + addHistogramView( + providerBuilder, + sizeHistogramBoundaries(), + "grpc.client.attempt.rcvd_total_compressed_message_size", + "By"); + addHistogramView( + providerBuilder, + sizeHistogramBoundaries(), + "grpc.client.attempt.sent_total_compressed_message_size", + "By"); + + return providerBuilder.build(); + } + + private static void addHistogramView( + SdkMeterProviderBuilder provider, List boundaries, String name, String unit) { + InstrumentSelector instrumentSelector = + InstrumentSelector.builder() + .setType(InstrumentType.HISTOGRAM) + .setUnit(unit) + .setName(name) + .setMeterName("grpc-java") + .setMeterSchemaUrl("") + .build(); + View view = + View.builder() + .setName(name) + .setDescription( + "A view of " + + name + + " with histogram boundaries more appropriate for Google Cloud Storage RPCs") + .setAggregation(Aggregation.explicitBucketHistogram(boundaries)) + .build(); + provider.registerView(instrumentSelector, view); + } + + private static List latencyHistogramBoundaries() { + List boundaries = new ArrayList<>(); + BigDecimal boundary = new BigDecimal(0, MathContext.UNLIMITED); + BigDecimal increment = new BigDecimal("0.002", MathContext.UNLIMITED); // 2ms + + // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads + // in the + // 100 KiB range + for (int i = 0; i != 50; i++) { + boundaries.add(boundary.doubleValue()); + boundary = boundary.add(increment); + } + + // For the remaining buckets do 10 10ms, 10 20ms, and so on, up until 5 minutes + increment = new BigDecimal("0.01", MathContext.UNLIMITED); // 10 ms + for (int i = 0; i != 150 && boundary.compareTo(new BigDecimal(300)) < 1; i++) { + boundaries.add(boundary.doubleValue()); + if (i != 0 && i % 10 == 0) { + increment = increment.multiply(new BigDecimal(2)); + } + boundary = boundary.add(increment); + } + + return boundaries; + } + + private static List sizeHistogramBoundaries() { + long kb = 1024; + long mb = 1024 * kb; + long gb = 1024 * mb; + + List boundaries = new ArrayList<>(); + long boundary = 0; + long increment = 128 * kb; + + // 128 KiB increments up to 4MiB, then exponential growth + while (boundaries.size() < 200 && boundary <= 16 * gb) { + boundaries.add((double) boundary); + boundary += increment; + if (boundary >= 4 * mb) { + increment *= 2; + } + } + return boundaries; + } +} diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index 5b0a6156ab..a9e1aa89c0 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -1,13 +1,27 @@ +/* + * Copyright 2024 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 com.google.cloud.storage.it.runner.StorageITRunner; import com.google.cloud.storage.it.runner.annotations.Backend; -import com.google.cloud.storage.it.runner.annotations.CrossRun; -import io.opentelemetry.api.GlobalOpenTelemetry; +import com.google.cloud.storage.it.runner.annotations.SingleBackend; import io.opentelemetry.sdk.metrics.SdkMeterProvider; -import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -16,14 +30,11 @@ public class ITGrpcMetricsTest { @Test public void testGrpcMetrics() { - GlobalOpenTelemetry - .resetForTest(); // avoids problems with "GlobalOpenTelemetry.set has already been called" GrpcStorageOptions grpcStorageOptions = StorageOptions.grpc().build(); - assertThat(grpcStorageOptions.getCloudMonitoringEndpoint("storage.googleapis.com:443")) + assertThat(OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("storage.googleapis.com:443", "storage.googleapis.com")) .isEqualTo("monitoring.googleapis.com:443"); - SdkMeterProvider provider = - grpcStorageOptions.createMeterProvider("monitoring.googleapis.com:443"); + SdkMeterProvider provider = OpenTelemetryBootstrappingUtils.createMeterProvider("monitoring.googleapis.com:443", grpcStorageOptions.getProjectId()); /* @@ -48,36 +59,22 @@ public void testGrpcMetrics() { @Test public void testGrpcMetrics_universeDomain() { - GlobalOpenTelemetry.resetForTest(); - Storage storage = - StorageOptions.grpc().setUniverseDomain("my-universe-domain.com").build().getService(); - Assert.assertEquals( - "monitoring.my-universe-domain.com:443", - ((GrpcStorageOptions) storage.getOptions()) - .getCloudMonitoringEndpoint("storage.my-universe-domain.com:443")); + assertThat( + "monitoring.my-universe-domain.com:443").isEqualTo( + OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("storage.my-universe-domain.com:443", "my-universe-domain.com")); } @Test public void testGrpcMetrics_private() { - GlobalOpenTelemetry.resetForTest(); - Storage storage = - StorageOptions.grpc().setHost("https://private.googleapis.com").build().getService(); - - Assert.assertEquals( - "private.googleapis.com:443", - ((GrpcStorageOptions) storage.getOptions()) - .getCloudMonitoringEndpoint("private.googleapis.com:443")); + assertThat( + "private.googleapis.com:443").isEqualTo( + OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("private.googleapis.com:443", null)); } @Test public void testGrpcMetrics_restricted() { - GlobalOpenTelemetry.resetForTest(); - Storage storage = - StorageOptions.grpc().setHost("https://restricted.googleapis.com").build().getService(); - - Assert.assertEquals( - "restricted.googleapis.com:443", - ((GrpcStorageOptions) storage.getOptions()) - .getCloudMonitoringEndpoint("restricted.googleapis.com:443")); + assertThat( + "restricted.googleapis.com:443").isEqualTo( + OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("restricted.googleapis.com:443", null)); } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java index 741fd8c3e4..3ac87cad0f 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/it/ITGrpcTest.java @@ -59,7 +59,7 @@ @RunWith(StorageITRunner.class) @CrossRun( - backends = {Backend.PROD}, + backends = {Backend.TEST_BENCH}, transports = {Transport.GRPC}) public final class ITGrpcTest { diff --git a/renovate.json b/renovate.json index c7467c6b72..eafe605eaf 100644 --- a/renovate.json +++ b/renovate.json @@ -111,6 +111,13 @@ ], "semanticCommitType": "test", "semanticCommitScope": "deps" + }, + { + "packagePatterns": [ + "^io.grpc.opentelemetry:", + "^io.grpc:grpc-opentelemetry:", + "^com.google.cloud.opentelemetry:" + ] } ], "semanticCommits": true, From bc3815c4d291b21d4b3d63068be0cc6e1c728ea3 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Fri, 26 Jul 2024 11:39:39 -0700 Subject: [PATCH 07/18] fix metric format --- google-cloud-storage/pom.xml | 41 +++++- .../OpenTelemetryBootstrappingUtils.java | 125 +++++++++--------- 2 files changed, 99 insertions(+), 67 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index 93c0752552..3064436fab 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -128,7 +128,7 @@ io.opentelemetry opentelemetry-sdk - 1.38.0 + 1.40.0 io.grpc @@ -138,23 +138,43 @@ io.opentelemetry opentelemetry-api - 1.38.0 + 1.40.0 io.opentelemetry opentelemetry-sdk-metrics - 1.38.0 + 1.40.0 io.opentelemetry opentelemetry-sdk-common - 1.38.0 + 1.40.0 io.opentelemetry opentelemetry-sdk-extension-autoconfigure - 1.38.0 + 1.40.0 + + + io.opentelemetry + opentelemetry-sdk-extension-autoconfigure-spi + 1.40.0 + + + io.opentelemetry + opentelemetry-sdk-logs + 1.40.0 + + + io.opentelemetry + opentelemetry-context + 1.40.0 + + + io.opentelemetry + opentelemetry-sdk-trace + 1.40.0 @@ -165,7 +185,7 @@ com.google.cloud.opentelemetry exporter-metrics - 0.29.0 + 0.31.0 io.opentelemetry.instrumentation @@ -318,6 +338,15 @@ + + + + io.opentelemetry.semconv + opentelemetry-semconv + 1.25.0-alpha + + + diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index bec7a5ef97..eba69995ce 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -1,4 +1,4 @@ -package com.google.cloud.storage;/* +/* * Copyright 2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -14,6 +14,8 @@ * limitations under the License. */ +package com.google.cloud.storage; + import com.google.api.core.ApiFunction; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.cloud.opentelemetry.metric.GoogleCloudMetricExporter; @@ -22,6 +24,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import io.grpc.ManagedChannelBuilder; import io.grpc.opentelemetry.GrpcOpenTelemetry; import io.opentelemetry.api.common.AttributeKey; @@ -42,11 +45,37 @@ import java.math.BigDecimal; import java.math.MathContext; import java.util.ArrayList; +import java.util.Collection; import java.util.List; +import java.util.UUID; import java.util.stream.Collectors; public class OpenTelemetryBootstrappingUtils { + private static final Collection METRICS_TO_ENABLE = ImmutableList.of( + "grpc.lb.wrr.rr_fallback", + "grpc.lb.wrr.endpoint_weight_not_yet_usable", + "grpc.lb.wrr.endpoint_weight_stale", + "grpc.lb.wrr.endpoint_weights", + "grpc.lb.rls.cache_entries", + "grpc.lb.rls.cache_size", + "grpc.lb.rls.default_target_picks", + "grpc.lb.rls.target_picks", + "grpc.lb.rls.failed_picks", + "grpc.xds_client.connected", + "grpc.xds_client.server_failure", + "grpc.xds_client.resource_updates_valid", + "grpc.xds_client.resource_updates_invalid", + "grpc.xds_client.resources"); + + private static final Collection METRICS_ENABLED_BY_DEFAULT = ImmutableList.of( + "grpc.client.attempt.sent_total_compressed_message_size", + "grpc.client.attempt.rcvd_total_compressed_message_size", + "grpc.client.attempt.started", + "grpc.client.attempt.duration", + "grpc.client.call.duration" + ); + static void enableGrpcMetrics( InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint, String projectId, String universeDomain) { String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint, universeDomain); @@ -57,22 +86,7 @@ static void enableGrpcMetrics( GrpcOpenTelemetry grpcOpenTelemetry = GrpcOpenTelemetry.newBuilder() .sdk(openTelemetrySdk) - .enableMetrics( - ImmutableList.of( - "grpc.lb.wrr.rr_fallback", - "grpc.lb.wrr.endpoint_weight_not_yet_usable", - "grpc.lb.wrr.endpoint_weight_stale", - "grpc.lb.wrr.endpoint_weights", - "grpc.lb.rls.cache_entries", - "grpc.lb.rls.cache_size", - "grpc.lb.rls.default_target_picks", - "grpc.lb.rls.target_picks", - "grpc.lb.rls.failed_picks", - "grpc.xds_client.connected", - "grpc.xds_client.server_failure", - "grpc.xds_client.resource_updates_valid", - "grpc.xds_client.resource_updates_invalid", - "grpc.xds_client.resources")) + .enableMetrics(METRICS_TO_ENABLE) .build(); ApiFunction channelConfigurator = channelProviderBuilder.getChannelConfigurator(); @@ -119,68 +133,58 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String Attributes detectedAttributes = resourceProvider.getAttributes(); MonitoredResourceDescription monitoredResourceDescription = - new MonitoredResourceDescription( - "generic_task", - ImmutableSet.of("project_id", "location", "namespace", "job", "task_id")); - // When the gcs_client MR is available, do this instead: - // new MonitoredResourceDescription( - // "gcs_client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", - // "instance_id", "api")); + new MonitoredResourceDescription( + "storage.googleapis.com/Client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", + "instance_id", "api")); MetricExporter cloudMonitoringExporter = GoogleCloudMetricExporter.createWithConfiguration( MetricConfiguration.builder() .setMonitoredResourceDescription(monitoredResourceDescription) + .setInstrumentationLibraryLabelsEnabled(false) .setMetricServiceEndpoint(metricServiceEndpoint) - // .setUseServiceTimeSeries(true) + .setPrefix("storage.googleapis.com/client") + .setUseServiceTimeSeries(true) .build()); String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); - SdkMeterProviderBuilder providerBuilder = - SdkMeterProvider.builder() - .registerMetricReader( - // Set collection interval to 20 seconds. - // See https://cloud.google.com/monitoring/quotas#custom_metrics_quotas - // Rate at which data can be written to a single time series: one point each 10 - // seconds. + SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder(); + + // This replaces the dots with slashes in each metric, which is the format needed for this monitored resource + for (String metric: ImmutableList.copyOf(Iterables.concat(METRICS_TO_ENABLE, METRICS_ENABLED_BY_DEFAULT))) { + providerBuilder.registerView(InstrumentSelector.builder().setName(metric).build(), + View.builder().setName(metric.replace(".", "/")).build()); + } + providerBuilder.registerMetricReader( PeriodicMetricReader.builder(cloudMonitoringExporter) - .setInterval(java.time.Duration.ofSeconds(20)) + .setInterval(java.time.Duration.ofSeconds(60)) .build()) - .setResource( - Resource.create( - Attributes.builder() - .put("gcp.resource_type", "generic_task") - .put("job", detectedAttributes.get(AttributeKey.stringKey("host.id"))) - .put( - "task_id", - detectedAttributes.get( - AttributeKey.stringKey("gcp.gce.instance.hostname"))) - .put("namespace", "gcs_client_instance") - .put( - "location", - detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", detectedProjectId == null ? projectId : detectedProjectId) - - /** - * Uncomment when gcs_client MR is available .put("cloud_platform", - * detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) - * .put("host_id", - * detectedAttributes.get(AttributeKey.stringKey("host.id"))) - * .put("instance_id", UUID.randomUUID().toString()) .put("api", "grpc") - */ - .build())); + .setResource( + Resource.create( + Attributes.builder() + .put("gcp.resource_type", "storage.googleapis.com/Client") + .put( + "location", + detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) + .put("project_id", detectedProjectId == null ? projectId : detectedProjectId) + .put("cloud_platform", + detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) + .put("host_id", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put("instance_id", UUID.randomUUID().toString()) + .put("api", "grpc") + .build())); addHistogramView( - providerBuilder, latencyHistogramBoundaries(), "grpc.client.attempt.duration", "s"); + providerBuilder, latencyHistogramBoundaries(), "grpc/client/attempt/duration", "s"); addHistogramView( providerBuilder, sizeHistogramBoundaries(), - "grpc.client.attempt.rcvd_total_compressed_message_size", + "grpc/client/attempt/rcvd_total_compressed_message_size", "By"); addHistogramView( providerBuilder, sizeHistogramBoundaries(), - "grpc.client.attempt.sent_total_compressed_message_size", + "grpc/client/attempt/sent_total_compressed_message_size", "By"); return providerBuilder.build(); @@ -214,8 +218,7 @@ private static List latencyHistogramBoundaries() { BigDecimal increment = new BigDecimal("0.002", MathContext.UNLIMITED); // 2ms // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads - // in the - // 100 KiB range + // in the 100 KiB range for (int i = 0; i != 50; i++) { boundaries.add(boundary.doubleValue()); boundary = boundary.add(increment); From 01859a05581ed26fac05419e8354703e08f44e72 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 29 Jul 2024 10:12:59 -0700 Subject: [PATCH 08/18] use build instead of buildAndRegisterGlobal --- .../google/cloud/storage/OpenTelemetryBootstrappingUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index eba69995ce..cefd44405a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -50,7 +50,7 @@ import java.util.UUID; import java.util.stream.Collectors; -public class OpenTelemetryBootstrappingUtils { +final class OpenTelemetryBootstrappingUtils { private static final Collection METRICS_TO_ENABLE = ImmutableList.of( "grpc.lb.wrr.rr_fallback", @@ -82,7 +82,7 @@ static void enableGrpcMetrics( SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint, projectId); OpenTelemetrySdk openTelemetrySdk = - OpenTelemetrySdk.builder().setMeterProvider(provider).buildAndRegisterGlobal(); + OpenTelemetrySdk.builder().setMeterProvider(provider).build(); GrpcOpenTelemetry grpcOpenTelemetry = GrpcOpenTelemetry.newBuilder() .sdk(openTelemetrySdk) From c8252a82c5024546f24fc394693f543817f452e8 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 29 Jul 2024 14:21:51 -0400 Subject: [PATCH 09/18] chore: mvn-fmt --- .../cloud/storage/GrpcStorageOptions.java | 10 +- .../OpenTelemetryBootstrappingUtils.java | 371 +++++++++--------- .../cloud/storage/ITGrpcMetricsTest.java | 30 +- 3 files changed, 212 insertions(+), 199 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index bbaf9cc77b..dc579f2a80 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -291,7 +291,8 @@ private Tuple> resolveSettingsAndOpts() throw } if (enableMetrics) { - OpenTelemetryBootstrappingUtils.enableGrpcMetrics(channelProviderBuilder, endpoint, this.getProjectId(), this.getUniverseDomain()); + OpenTelemetryBootstrappingUtils.enableGrpcMetrics( + channelProviderBuilder, endpoint, this.getProjectId(), this.getUniverseDomain()); } builder.setTransportChannelProvider(channelProviderBuilder.build()); @@ -466,10 +467,9 @@ public GrpcStorageOptions.Builder setAttemptDirectPath(boolean attemptDirectPath return this; } /** - * Option for whether this client should emit internal gRPC client internal metrics to Cloud Monitoring. - * To disable metric reporting, set this to false. - * True by default. - * Emitting metrics is free and requires minimal CPU and memory. + * Option for whether this client should emit internal gRPC client internal metrics to Cloud + * Monitoring. To disable metric reporting, set this to false. True by default. Emitting metrics + * is free and requires minimal CPU and memory. * * @since 2.41.0 This new api is in preview and is subject to breaking changes. */ diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index cefd44405a..0907dfa0f1 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -41,7 +41,6 @@ import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; import io.opentelemetry.sdk.resources.Resource; - import java.math.BigDecimal; import java.math.MathContext; import java.util.ArrayList; @@ -52,208 +51,216 @@ final class OpenTelemetryBootstrappingUtils { - private static final Collection METRICS_TO_ENABLE = ImmutableList.of( - "grpc.lb.wrr.rr_fallback", - "grpc.lb.wrr.endpoint_weight_not_yet_usable", - "grpc.lb.wrr.endpoint_weight_stale", - "grpc.lb.wrr.endpoint_weights", - "grpc.lb.rls.cache_entries", - "grpc.lb.rls.cache_size", - "grpc.lb.rls.default_target_picks", - "grpc.lb.rls.target_picks", - "grpc.lb.rls.failed_picks", - "grpc.xds_client.connected", - "grpc.xds_client.server_failure", - "grpc.xds_client.resource_updates_valid", - "grpc.xds_client.resource_updates_invalid", - "grpc.xds_client.resources"); + private static final Collection METRICS_TO_ENABLE = + ImmutableList.of( + "grpc.lb.wrr.rr_fallback", + "grpc.lb.wrr.endpoint_weight_not_yet_usable", + "grpc.lb.wrr.endpoint_weight_stale", + "grpc.lb.wrr.endpoint_weights", + "grpc.lb.rls.cache_entries", + "grpc.lb.rls.cache_size", + "grpc.lb.rls.default_target_picks", + "grpc.lb.rls.target_picks", + "grpc.lb.rls.failed_picks", + "grpc.xds_client.connected", + "grpc.xds_client.server_failure", + "grpc.xds_client.resource_updates_valid", + "grpc.xds_client.resource_updates_invalid", + "grpc.xds_client.resources"); - private static final Collection METRICS_ENABLED_BY_DEFAULT = ImmutableList.of( - "grpc.client.attempt.sent_total_compressed_message_size", - "grpc.client.attempt.rcvd_total_compressed_message_size", - "grpc.client.attempt.started", - "grpc.client.attempt.duration", - "grpc.client.call.duration" - ); + private static final Collection METRICS_ENABLED_BY_DEFAULT = + ImmutableList.of( + "grpc.client.attempt.sent_total_compressed_message_size", + "grpc.client.attempt.rcvd_total_compressed_message_size", + "grpc.client.attempt.started", + "grpc.client.attempt.duration", + "grpc.client.call.duration"); - static void enableGrpcMetrics( - InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint, String projectId, String universeDomain) { - String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint, universeDomain); - SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint, projectId); + static void enableGrpcMetrics( + InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, + String endpoint, + String projectId, + String universeDomain) { + String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint, universeDomain); + SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint, projectId); - OpenTelemetrySdk openTelemetrySdk = - OpenTelemetrySdk.builder().setMeterProvider(provider).build(); - GrpcOpenTelemetry grpcOpenTelemetry = - GrpcOpenTelemetry.newBuilder() - .sdk(openTelemetrySdk) - .enableMetrics(METRICS_TO_ENABLE) - .build(); - ApiFunction channelConfigurator = - channelProviderBuilder.getChannelConfigurator(); - channelProviderBuilder.setChannelConfigurator( - b -> { - grpcOpenTelemetry.configureChannelBuilder(b); - if (channelConfigurator != null) { - return channelConfigurator.apply(b); - } - return b; - }); - } + OpenTelemetrySdk openTelemetrySdk = + OpenTelemetrySdk.builder().setMeterProvider(provider).build(); + GrpcOpenTelemetry grpcOpenTelemetry = + GrpcOpenTelemetry.newBuilder() + .sdk(openTelemetrySdk) + .enableMetrics(METRICS_TO_ENABLE) + .build(); + ApiFunction channelConfigurator = + channelProviderBuilder.getChannelConfigurator(); + channelProviderBuilder.setChannelConfigurator( + b -> { + grpcOpenTelemetry.configureChannelBuilder(b); + if (channelConfigurator != null) { + return channelConfigurator.apply(b); + } + return b; + }); + } - @VisibleForTesting - static String getCloudMonitoringEndpoint(String endpoint, String universeDomain) { - String metricServiceEndpoint = "monitoring.googleapis.com"; + @VisibleForTesting + static String getCloudMonitoringEndpoint(String endpoint, String universeDomain) { + String metricServiceEndpoint = "monitoring.googleapis.com"; - // use contains instead of equals because endpoint has a port in it - if (universeDomain != null && endpoint.contains("storage." + universeDomain)) { - metricServiceEndpoint = "monitoring." + universeDomain; - } else if (!endpoint.contains("storage.googleapis.com")) { - String canonicalEndpoint = "storage.googleapis.com"; - String privateEndpoint = "private.googleapis.com"; - String restrictedEndpoint = "restricted.googleapis.com"; - if (universeDomain != null) { - canonicalEndpoint = "storage." + universeDomain; - privateEndpoint = "private." + universeDomain; - restrictedEndpoint = "restricted." + universeDomain; - } - String match = - ImmutableList.of(canonicalEndpoint, privateEndpoint, restrictedEndpoint).stream() - .filter(s -> endpoint.contains(s) || endpoint.contains("google-c2p:///" + s)) - .collect(Collectors.joining()); - if (!StringUtils.isNullOrEmpty(match)) { - metricServiceEndpoint = match; - } - } - return metricServiceEndpoint + ":" + endpoint.split(":")[1]; + // use contains instead of equals because endpoint has a port in it + if (universeDomain != null && endpoint.contains("storage." + universeDomain)) { + metricServiceEndpoint = "monitoring." + universeDomain; + } else if (!endpoint.contains("storage.googleapis.com")) { + String canonicalEndpoint = "storage.googleapis.com"; + String privateEndpoint = "private.googleapis.com"; + String restrictedEndpoint = "restricted.googleapis.com"; + if (universeDomain != null) { + canonicalEndpoint = "storage." + universeDomain; + privateEndpoint = "private." + universeDomain; + restrictedEndpoint = "restricted." + universeDomain; + } + String match = + ImmutableList.of(canonicalEndpoint, privateEndpoint, restrictedEndpoint).stream() + .filter(s -> endpoint.contains(s) || endpoint.contains("google-c2p:///" + s)) + .collect(Collectors.joining()); + if (!StringUtils.isNullOrEmpty(match)) { + metricServiceEndpoint = match; + } } + return metricServiceEndpoint + ":" + endpoint.split(":")[1]; + } - @VisibleForTesting - static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String projectId) { - GCPResourceProvider resourceProvider = new GCPResourceProvider(); - Attributes detectedAttributes = resourceProvider.getAttributes(); + @VisibleForTesting + static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String projectId) { + GCPResourceProvider resourceProvider = new GCPResourceProvider(); + Attributes detectedAttributes = resourceProvider.getAttributes(); - MonitoredResourceDescription monitoredResourceDescription = + MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription( - "storage.googleapis.com/Client", ImmutableSet.of("project_id", "location", "cloud_platform", "host_id", - "instance_id", "api")); - - MetricExporter cloudMonitoringExporter = - GoogleCloudMetricExporter.createWithConfiguration( - MetricConfiguration.builder() - .setMonitoredResourceDescription(monitoredResourceDescription) - .setInstrumentationLibraryLabelsEnabled(false) - .setMetricServiceEndpoint(metricServiceEndpoint) - .setPrefix("storage.googleapis.com/client") - .setUseServiceTimeSeries(true) - .build()); - - String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); - SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder(); + "storage.googleapis.com/Client", + ImmutableSet.of( + "project_id", "location", "cloud_platform", "host_id", "instance_id", "api")); - // This replaces the dots with slashes in each metric, which is the format needed for this monitored resource - for (String metric: ImmutableList.copyOf(Iterables.concat(METRICS_TO_ENABLE, METRICS_ENABLED_BY_DEFAULT))) { - providerBuilder.registerView(InstrumentSelector.builder().setName(metric).build(), - View.builder().setName(metric.replace(".", "/")).build()); - } - providerBuilder.registerMetricReader( - PeriodicMetricReader.builder(cloudMonitoringExporter) - .setInterval(java.time.Duration.ofSeconds(60)) - .build()) - .setResource( - Resource.create( - Attributes.builder() - .put("gcp.resource_type", "storage.googleapis.com/Client") - .put( - "location", - detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", detectedProjectId == null ? projectId : detectedProjectId) - .put("cloud_platform", - detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) - .put("host_id", detectedAttributes.get(AttributeKey.stringKey("host.id"))) - .put("instance_id", UUID.randomUUID().toString()) - .put("api", "grpc") - .build())); + MetricExporter cloudMonitoringExporter = + GoogleCloudMetricExporter.createWithConfiguration( + MetricConfiguration.builder() + .setMonitoredResourceDescription(monitoredResourceDescription) + .setInstrumentationLibraryLabelsEnabled(false) + .setMetricServiceEndpoint(metricServiceEndpoint) + .setPrefix("storage.googleapis.com/client") + .setUseServiceTimeSeries(true) + .build()); - addHistogramView( - providerBuilder, latencyHistogramBoundaries(), "grpc/client/attempt/duration", "s"); - addHistogramView( - providerBuilder, - sizeHistogramBoundaries(), - "grpc/client/attempt/rcvd_total_compressed_message_size", - "By"); - addHistogramView( - providerBuilder, - sizeHistogramBoundaries(), - "grpc/client/attempt/sent_total_compressed_message_size", - "By"); + String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); + SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder(); - return providerBuilder.build(); + // This replaces the dots with slashes in each metric, which is the format needed for this + // monitored resource + for (String metric : + ImmutableList.copyOf(Iterables.concat(METRICS_TO_ENABLE, METRICS_ENABLED_BY_DEFAULT))) { + providerBuilder.registerView( + InstrumentSelector.builder().setName(metric).build(), + View.builder().setName(metric.replace(".", "/")).build()); } + providerBuilder + .registerMetricReader( + PeriodicMetricReader.builder(cloudMonitoringExporter) + .setInterval(java.time.Duration.ofSeconds(60)) + .build()) + .setResource( + Resource.create( + Attributes.builder() + .put("gcp.resource_type", "storage.googleapis.com/Client") + .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) + .put("project_id", detectedProjectId == null ? projectId : detectedProjectId) + .put( + "cloud_platform", + detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) + .put("host_id", detectedAttributes.get(AttributeKey.stringKey("host.id"))) + .put("instance_id", UUID.randomUUID().toString()) + .put("api", "grpc") + .build())); - private static void addHistogramView( - SdkMeterProviderBuilder provider, List boundaries, String name, String unit) { - InstrumentSelector instrumentSelector = - InstrumentSelector.builder() - .setType(InstrumentType.HISTOGRAM) - .setUnit(unit) - .setName(name) - .setMeterName("grpc-java") - .setMeterSchemaUrl("") - .build(); - View view = - View.builder() - .setName(name) - .setDescription( - "A view of " - + name - + " with histogram boundaries more appropriate for Google Cloud Storage RPCs") - .setAggregation(Aggregation.explicitBucketHistogram(boundaries)) - .build(); - provider.registerView(instrumentSelector, view); - } + addHistogramView( + providerBuilder, latencyHistogramBoundaries(), "grpc/client/attempt/duration", "s"); + addHistogramView( + providerBuilder, + sizeHistogramBoundaries(), + "grpc/client/attempt/rcvd_total_compressed_message_size", + "By"); + addHistogramView( + providerBuilder, + sizeHistogramBoundaries(), + "grpc/client/attempt/sent_total_compressed_message_size", + "By"); + + return providerBuilder.build(); + } - private static List latencyHistogramBoundaries() { - List boundaries = new ArrayList<>(); - BigDecimal boundary = new BigDecimal(0, MathContext.UNLIMITED); - BigDecimal increment = new BigDecimal("0.002", MathContext.UNLIMITED); // 2ms + private static void addHistogramView( + SdkMeterProviderBuilder provider, List boundaries, String name, String unit) { + InstrumentSelector instrumentSelector = + InstrumentSelector.builder() + .setType(InstrumentType.HISTOGRAM) + .setUnit(unit) + .setName(name) + .setMeterName("grpc-java") + .setMeterSchemaUrl("") + .build(); + View view = + View.builder() + .setName(name) + .setDescription( + "A view of " + + name + + " with histogram boundaries more appropriate for Google Cloud Storage RPCs") + .setAggregation(Aggregation.explicitBucketHistogram(boundaries)) + .build(); + provider.registerView(instrumentSelector, view); + } - // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads - // in the 100 KiB range - for (int i = 0; i != 50; i++) { - boundaries.add(boundary.doubleValue()); - boundary = boundary.add(increment); - } + private static List latencyHistogramBoundaries() { + List boundaries = new ArrayList<>(); + BigDecimal boundary = new BigDecimal(0, MathContext.UNLIMITED); + BigDecimal increment = new BigDecimal("0.002", MathContext.UNLIMITED); // 2ms - // For the remaining buckets do 10 10ms, 10 20ms, and so on, up until 5 minutes - increment = new BigDecimal("0.01", MathContext.UNLIMITED); // 10 ms - for (int i = 0; i != 150 && boundary.compareTo(new BigDecimal(300)) < 1; i++) { - boundaries.add(boundary.doubleValue()); - if (i != 0 && i % 10 == 0) { - increment = increment.multiply(new BigDecimal(2)); - } - boundary = boundary.add(increment); - } + // 2ms buckets for the first 100ms, so we can have higher resolution for uploads and downloads + // in the 100 KiB range + for (int i = 0; i != 50; i++) { + boundaries.add(boundary.doubleValue()); + boundary = boundary.add(increment); + } - return boundaries; + // For the remaining buckets do 10 10ms, 10 20ms, and so on, up until 5 minutes + increment = new BigDecimal("0.01", MathContext.UNLIMITED); // 10 ms + for (int i = 0; i != 150 && boundary.compareTo(new BigDecimal(300)) < 1; i++) { + boundaries.add(boundary.doubleValue()); + if (i != 0 && i % 10 == 0) { + increment = increment.multiply(new BigDecimal(2)); + } + boundary = boundary.add(increment); } - private static List sizeHistogramBoundaries() { - long kb = 1024; - long mb = 1024 * kb; - long gb = 1024 * mb; + return boundaries; + } + + private static List sizeHistogramBoundaries() { + long kb = 1024; + long mb = 1024 * kb; + long gb = 1024 * mb; - List boundaries = new ArrayList<>(); - long boundary = 0; - long increment = 128 * kb; + List boundaries = new ArrayList<>(); + long boundary = 0; + long increment = 128 * kb; - // 128 KiB increments up to 4MiB, then exponential growth - while (boundaries.size() < 200 && boundary <= 16 * gb) { - boundaries.add((double) boundary); - boundary += increment; - if (boundary >= 4 * mb) { - increment *= 2; - } - } - return boundaries; + // 128 KiB increments up to 4MiB, then exponential growth + while (boundaries.size() < 200 && boundary <= 16 * gb) { + boundaries.add((double) boundary); + boundary += increment; + if (boundary >= 4 * mb) { + increment *= 2; + } } + return boundaries; + } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index a9e1aa89c0..675835ca37 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -31,11 +31,14 @@ public class ITGrpcMetricsTest { @Test public void testGrpcMetrics() { GrpcStorageOptions grpcStorageOptions = StorageOptions.grpc().build(); - assertThat(OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("storage.googleapis.com:443", "storage.googleapis.com")) + assertThat( + OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint( + "storage.googleapis.com:443", "storage.googleapis.com")) .isEqualTo("monitoring.googleapis.com:443"); - SdkMeterProvider provider = OpenTelemetryBootstrappingUtils.createMeterProvider("monitoring.googleapis.com:443", grpcStorageOptions.getProjectId()); - + SdkMeterProvider provider = + OpenTelemetryBootstrappingUtils.createMeterProvider( + "monitoring.googleapis.com:443", grpcStorageOptions.getProjectId()); /* * SDKMeterProvider doesn't expose the relevant fields we want to test, but they are present in @@ -59,22 +62,25 @@ public void testGrpcMetrics() { @Test public void testGrpcMetrics_universeDomain() { - assertThat( - "monitoring.my-universe-domain.com:443").isEqualTo( - OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("storage.my-universe-domain.com:443", "my-universe-domain.com")); + assertThat("monitoring.my-universe-domain.com:443") + .isEqualTo( + OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint( + "storage.my-universe-domain.com:443", "my-universe-domain.com")); } @Test public void testGrpcMetrics_private() { - assertThat( - "private.googleapis.com:443").isEqualTo( - OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("private.googleapis.com:443", null)); + assertThat("private.googleapis.com:443") + .isEqualTo( + OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint( + "private.googleapis.com:443", null)); } @Test public void testGrpcMetrics_restricted() { - assertThat( - "restricted.googleapis.com:443").isEqualTo( - OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint("restricted.googleapis.com:443", null)); + assertThat("restricted.googleapis.com:443") + .isEqualTo( + OpenTelemetryBootstrappingUtils.getCloudMonitoringEndpoint( + "restricted.googleapis.com:443", null)); } } From 25c6469d271303c9ddeb3477b35146bfd2e786d8 Mon Sep 17 00:00:00 2001 From: BenWhitehead Date: Mon, 29 Jul 2024 13:56:46 -0400 Subject: [PATCH 10/18] deps: resolve dependency diamond --- google-cloud-storage/pom.xml | 23 ------------- pom.xml | 63 +++++++++++++++++++++++++++++------- renovate.json | 6 ++-- 3 files changed, 55 insertions(+), 37 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index 3064436fab..fe9bb3fedf 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -128,74 +128,60 @@ io.opentelemetry opentelemetry-sdk - 1.40.0 io.grpc grpc-opentelemetry - 1.64.0 io.opentelemetry opentelemetry-api - 1.40.0 io.opentelemetry opentelemetry-sdk-metrics - 1.40.0 io.opentelemetry opentelemetry-sdk-common - 1.40.0 io.opentelemetry opentelemetry-sdk-extension-autoconfigure - 1.40.0 io.opentelemetry opentelemetry-sdk-extension-autoconfigure-spi - 1.40.0 io.opentelemetry opentelemetry-sdk-logs - 1.40.0 io.opentelemetry opentelemetry-context - 1.40.0 io.opentelemetry opentelemetry-sdk-trace - 1.40.0 com.google.cloud.opentelemetry detector-resources - 0.27.0-alpha com.google.cloud.opentelemetry exporter-metrics - 0.31.0 io.opentelemetry.instrumentation opentelemetry-resources - 2.5.0-alpha io.opentelemetry.contrib opentelemetry-gcp-resources - 1.36.0-alpha @@ -338,15 +324,6 @@ - - - - io.opentelemetry.semconv - opentelemetry-semconv - 1.25.0-alpha - - - diff --git a/pom.xml b/pom.xml index 43630521de..4778468747 100644 --- a/pom.xml +++ b/pom.xml @@ -54,33 +54,72 @@ UTF-8 github google-cloud-storage-parent - 3.25.0 + 3.31.0 - io.grpc - grpc-bom - 1.64.0 + com.google.cloud + google-cloud-shared-dependencies + ${google-cloud-shared-dependencies.version} pom import + + + io.opentelemetry.semconv + opentelemetry-semconv + 1.25.0-alpha + com.google.cloud.opentelemetry exporter-metrics - 0.29.0 + 0.31.0 + + + io.opentelemetry.semconv + opentelemetry-semconv + + - com.google.cloud - google-cloud-shared-dependencies - ${google-cloud-shared-dependencies.version} - pom - import + com.google.cloud.opentelemetry + detector-resources + 0.27.0-alpha + + + io.opentelemetry.semconv + opentelemetry-semconv + + + + + io.opentelemetry.instrumentation + opentelemetry-resources + 2.6.0-alpha + + + io.opentelemetry.semconv + opentelemetry-semconv + + + + + io.opentelemetry.contrib + opentelemetry-gcp-resources + 1.37.0-alpha - io.grpc - * + io.opentelemetry.semconv + opentelemetry-semconv diff --git a/renovate.json b/renovate.json index eafe605eaf..d053e12ae7 100644 --- a/renovate.json +++ b/renovate.json @@ -113,9 +113,11 @@ "semanticCommitScope": "deps" }, { + "groupName": "OpenTelemetry extended dependencies", "packagePatterns": [ - "^io.grpc.opentelemetry:", - "^io.grpc:grpc-opentelemetry:", + "^io.opentelemetry.semconv:", + "^io.opentelemetry.instrumentation:", + "^io.opentelemetry.contrib:", "^com.google.cloud.opentelemetry:" ] } From 5611442b771c47ec37791998e074f93e14232393 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 29 Jul 2024 12:04:03 -0700 Subject: [PATCH 11/18] switch project ID defaults --- google-cloud-storage/pom.xml | 33 ++----------------- .../OpenTelemetryBootstrappingUtils.java | 2 +- 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index fe9bb3fedf..90f207a4c1 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -146,39 +146,17 @@ io.opentelemetry opentelemetry-sdk-common - - io.opentelemetry - opentelemetry-sdk-extension-autoconfigure - + io.opentelemetry opentelemetry-sdk-extension-autoconfigure-spi - - io.opentelemetry - opentelemetry-sdk-logs - - - io.opentelemetry - opentelemetry-context - - - io.opentelemetry - opentelemetry-sdk-trace - - - com.google.cloud.opentelemetry - detector-resources - com.google.cloud.opentelemetry exporter-metrics - - io.opentelemetry.instrumentation - opentelemetry-resources - + io.opentelemetry.contrib opentelemetry-gcp-resources @@ -240,12 +218,7 @@ 0.140.0 test - - io.opentelemetry - opentelemetry-sdk-testing - 1.37.0 - test - + com.google.cloud google-cloud-kms diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index 0907dfa0f1..4efcb5c24f 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -172,7 +172,7 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String Attributes.builder() .put("gcp.resource_type", "storage.googleapis.com/Client") .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", detectedProjectId == null ? projectId : detectedProjectId) + .put("project_id", projectId != null ? projectId : detectedProjectId) .put( "cloud_platform", detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) From f4f36fff1ad2843b7456bc6cf6bc6a4a17c990ab Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 29 Jul 2024 13:20:32 -0700 Subject: [PATCH 12/18] make sure project ID matches in request --- google-cloud-storage/pom.xml | 10 ++++++++++ .../cloud/storage/OpenTelemetryBootstrappingUtils.java | 8 ++++++-- .../com/google/cloud/storage/ITGrpcMetricsTest.java | 3 ++- renovate.json | 3 ++- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index 90f207a4c1..4fc6acbe36 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -162,6 +162,11 @@ opentelemetry-gcp-resources + + org.checkerframework + checker-qual + + com.fasterxml.jackson.core @@ -342,6 +347,11 @@ Add in vintage engine so that both JUnit4 and JUnit5 tests are run by the JUnit5 runner. --> org.junit.vintage:junit-vintage-engine + + + io.opentelemetry:>opentelemetry-sdk-extension-autoconfigure-spi diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index 4efcb5c24f..182be7fd62 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -135,6 +135,9 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String GCPResourceProvider resourceProvider = new GCPResourceProvider(); Attributes detectedAttributes = resourceProvider.getAttributes(); + String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); + String projectIdToUse = detectedProjectId == null ? projectId : detectedProjectId; + MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription( "storage.googleapis.com/Client", @@ -149,9 +152,10 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String .setMetricServiceEndpoint(metricServiceEndpoint) .setPrefix("storage.googleapis.com/client") .setUseServiceTimeSeries(true) + .setProjectId(projectIdToUse) .build()); - String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); + SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder(); // This replaces the dots with slashes in each metric, which is the format needed for this @@ -172,7 +176,7 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String Attributes.builder() .put("gcp.resource_type", "storage.googleapis.com/Client") .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", projectId != null ? projectId : detectedProjectId) + .put("project_id", projectIdToUse) .put( "cloud_platform", detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index 675835ca37..2fa772b1c7 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -17,6 +17,7 @@ package com.google.cloud.storage; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertTrue; import com.google.cloud.storage.it.runner.StorageITRunner; import com.google.cloud.storage.it.runner.annotations.Backend; @@ -48,7 +49,7 @@ public void testGrpcMetrics() { */ String result = provider.toString(); - assertThat(result).contains(grpcStorageOptions.getProjectId()); + assertTrue(result.contains(grpcStorageOptions.getProjectId()) || result.contains(System.getenv("GOOGLE_CLOUD_PROJECT"))); // This is the check for the Seconds histogram boundary. We can't practically check for every // boundary, diff --git a/renovate.json b/renovate.json index d053e12ae7..42380a2b69 100644 --- a/renovate.json +++ b/renovate.json @@ -118,7 +118,8 @@ "^io.opentelemetry.semconv:", "^io.opentelemetry.instrumentation:", "^io.opentelemetry.contrib:", - "^com.google.cloud.opentelemetry:" + "^com.google.cloud.opentelemetry:", + "$com.google.cloud.opentelemetry:shared-resourcemapping" ] } ], From bd1688e92ebb7a046122d7b157410e4c93ab2f23 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 29 Jul 2024 14:34:46 -0700 Subject: [PATCH 13/18] fix dependency issues --- google-cloud-storage/pom.xml | 16 ++++++++++++++++ .../storage/OpenTelemetryBootstrappingUtils.java | 8 ++++++-- .../google/cloud/storage/ITGrpcMetricsTest.java | 3 ++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index 90f207a4c1..fd824ffd61 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -152,6 +152,11 @@ opentelemetry-sdk-extension-autoconfigure-spi + + io.opentelemetry.semconv + opentelemetry-semconv + + com.google.cloud.opentelemetry exporter-metrics @@ -162,6 +167,11 @@ opentelemetry-gcp-resources + + org.checkerframework + checker-qual + + com.fasterxml.jackson.core @@ -342,6 +352,12 @@ Add in vintage engine so that both JUnit4 and JUnit5 tests are run by the JUnit5 runner. --> org.junit.vintage:junit-vintage-engine + + + io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi + io.opentelemetry.semconv:opentelemetry-semconv diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index 4efcb5c24f..182be7fd62 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -135,6 +135,9 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String GCPResourceProvider resourceProvider = new GCPResourceProvider(); Attributes detectedAttributes = resourceProvider.getAttributes(); + String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); + String projectIdToUse = detectedProjectId == null ? projectId : detectedProjectId; + MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription( "storage.googleapis.com/Client", @@ -149,9 +152,10 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String .setMetricServiceEndpoint(metricServiceEndpoint) .setPrefix("storage.googleapis.com/client") .setUseServiceTimeSeries(true) + .setProjectId(projectIdToUse) .build()); - String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); + SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder(); // This replaces the dots with slashes in each metric, which is the format needed for this @@ -172,7 +176,7 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String Attributes.builder() .put("gcp.resource_type", "storage.googleapis.com/Client") .put("location", detectedAttributes.get(AttributeKey.stringKey("cloud.region"))) - .put("project_id", projectId != null ? projectId : detectedProjectId) + .put("project_id", projectIdToUse) .put( "cloud_platform", detectedAttributes.get(AttributeKey.stringKey("cloud.platform"))) diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index 675835ca37..2fa772b1c7 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -17,6 +17,7 @@ package com.google.cloud.storage; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertTrue; import com.google.cloud.storage.it.runner.StorageITRunner; import com.google.cloud.storage.it.runner.annotations.Backend; @@ -48,7 +49,7 @@ public void testGrpcMetrics() { */ String result = provider.toString(); - assertThat(result).contains(grpcStorageOptions.getProjectId()); + assertTrue(result.contains(grpcStorageOptions.getProjectId()) || result.contains(System.getenv("GOOGLE_CLOUD_PROJECT"))); // This is the check for the Seconds histogram boundary. We can't practically check for every // boundary, From f61c36946dcd554d33d58e05fdfb17e2c28634c4 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Mon, 29 Jul 2024 14:44:16 -0700 Subject: [PATCH 14/18] rename enableMetrics to enableGrpcClientMetrics --- .../com/google/cloud/storage/GrpcStorageOptions.java | 10 +++++----- .../cloud/storage/OpenTelemetryBootstrappingUtils.java | 1 - .../com/google/cloud/storage/ITGrpcMetricsTest.java | 4 +++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index dc579f2a80..a8bf25f54a 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -130,7 +130,7 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) MoreObjects.firstNonNull( builder.terminationAwaitDuration, serviceDefaults.getTerminationAwaitDuration()); this.attemptDirectPath = builder.attemptDirectPath; - this.enableMetrics = builder.enableMetrics; + this.enableMetrics = builder.enableGrpcClientMetrics; this.grpcInterceptorProvider = builder.grpcInterceptorProvider; this.blobWriteSessionConfig = builder.blobWriteSessionConfig; } @@ -418,7 +418,7 @@ public static final class Builder extends StorageOptions.Builder { private StorageRetryStrategy storageRetryStrategy; private Duration terminationAwaitDuration; private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath(); - private boolean enableMetrics = GrpcStorageDefaults.INSTANCE.isEnableMetrics(); + private boolean enableGrpcClientMetrics = GrpcStorageDefaults.INSTANCE.isEnableMetrics(); private GrpcInterceptorProvider grpcInterceptorProvider = GrpcStorageDefaults.INSTANCE.grpcInterceptorProvider(); private BlobWriteSessionConfig blobWriteSessionConfig = @@ -432,7 +432,7 @@ public static final class Builder extends StorageOptions.Builder { this.storageRetryStrategy = gso.getRetryAlgorithmManager().retryStrategy; this.terminationAwaitDuration = gso.getTerminationAwaitDuration(); this.attemptDirectPath = gso.attemptDirectPath; - this.enableMetrics = gso.enableMetrics; + this.enableGrpcClientMetrics = gso.enableMetrics; this.grpcInterceptorProvider = gso.grpcInterceptorProvider; this.blobWriteSessionConfig = gso.blobWriteSessionConfig; } @@ -474,8 +474,8 @@ public GrpcStorageOptions.Builder setAttemptDirectPath(boolean attemptDirectPath * @since 2.41.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public GrpcStorageOptions.Builder setEnableMetrics(boolean enableMetrics) { - this.enableMetrics = enableMetrics; + public GrpcStorageOptions.Builder setEnableGrpcClientMetrics(boolean enableGrpcClientMetrics) { + this.enableGrpcClientMetrics = enableGrpcClientMetrics; return this; } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index 182be7fd62..96b98b7b0e 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -155,7 +155,6 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String .setProjectId(projectIdToUse) .build()); - SdkMeterProviderBuilder providerBuilder = SdkMeterProvider.builder(); // This replaces the dots with slashes in each metric, which is the format needed for this diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index 2fa772b1c7..3cd8fba37c 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -49,7 +49,9 @@ public void testGrpcMetrics() { */ String result = provider.toString(); - assertTrue(result.contains(grpcStorageOptions.getProjectId()) || result.contains(System.getenv("GOOGLE_CLOUD_PROJECT"))); + assertTrue( + result.contains(grpcStorageOptions.getProjectId()) + || result.contains(System.getenv("GOOGLE_CLOUD_PROJECT"))); // This is the check for the Seconds histogram boundary. We can't practically check for every // boundary, From ef47206dadca02a5df02fe254bdd78a31020abcc Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Tue, 30 Jul 2024 23:43:39 -0700 Subject: [PATCH 15/18] Suppress logging exceptions by default --- .../cloud/storage/GrpcStorageOptions.java | 26 +++- .../OpenTelemetryBootstrappingUtils.java | 124 +++++++++++++++++- .../cloud/storage/ITGrpcMetricsTest.java | 11 +- 3 files changed, 144 insertions(+), 17 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index a8bf25f54a..582797c37c 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -116,7 +116,9 @@ public final class GrpcStorageOptions extends StorageOptions private final GrpcRetryAlgorithmManager retryAlgorithmManager; private final Duration terminationAwaitDuration; private final boolean attemptDirectPath; - private final boolean enableMetrics; + private final boolean enableGrpcClientMetrics; + + private final boolean grpcClientMetricsManuallyEnabled; private final GrpcInterceptorProvider grpcInterceptorProvider; private final BlobWriteSessionConfig blobWriteSessionConfig; @@ -130,7 +132,8 @@ private GrpcStorageOptions(Builder builder, GrpcStorageDefaults serviceDefaults) MoreObjects.firstNonNull( builder.terminationAwaitDuration, serviceDefaults.getTerminationAwaitDuration()); this.attemptDirectPath = builder.attemptDirectPath; - this.enableMetrics = builder.enableGrpcClientMetrics; + this.enableGrpcClientMetrics = builder.enableGrpcClientMetrics; + this.grpcClientMetricsManuallyEnabled = builder.grpcMetricsManuallyEnabled; this.grpcInterceptorProvider = builder.grpcInterceptorProvider; this.blobWriteSessionConfig = builder.blobWriteSessionConfig; } @@ -290,9 +293,13 @@ private Tuple> resolveSettingsAndOpts() throw channelProviderBuilder.setChannelConfigurator(ManagedChannelBuilder::usePlaintext); } - if (enableMetrics) { + if (enableGrpcClientMetrics) { OpenTelemetryBootstrappingUtils.enableGrpcMetrics( - channelProviderBuilder, endpoint, this.getProjectId(), this.getUniverseDomain()); + channelProviderBuilder, + endpoint, + this.getProjectId(), + this.getUniverseDomain(), + !grpcClientMetricsManuallyEnabled); } builder.setTransportChannelProvider(channelProviderBuilder.build()); @@ -358,7 +365,7 @@ public int hashCode() { retryAlgorithmManager, terminationAwaitDuration, attemptDirectPath, - enableMetrics, + enableGrpcClientMetrics, grpcInterceptorProvider, blobWriteSessionConfig, baseHashCode()); @@ -374,7 +381,7 @@ public boolean equals(Object o) { } GrpcStorageOptions that = (GrpcStorageOptions) o; return attemptDirectPath == that.attemptDirectPath - && enableMetrics == that.enableMetrics + && enableGrpcClientMetrics == that.enableGrpcClientMetrics && Objects.equals(retryAlgorithmManager, that.retryAlgorithmManager) && Objects.equals(terminationAwaitDuration, that.terminationAwaitDuration) && Objects.equals(grpcInterceptorProvider, that.grpcInterceptorProvider) @@ -424,6 +431,8 @@ public static final class Builder extends StorageOptions.Builder { private BlobWriteSessionConfig blobWriteSessionConfig = GrpcStorageDefaults.INSTANCE.getDefaultStorageWriterConfig(); + private boolean grpcMetricsManuallyEnabled = false; + Builder() {} Builder(StorageOptions options) { @@ -432,7 +441,7 @@ public static final class Builder extends StorageOptions.Builder { this.storageRetryStrategy = gso.getRetryAlgorithmManager().retryStrategy; this.terminationAwaitDuration = gso.getTerminationAwaitDuration(); this.attemptDirectPath = gso.attemptDirectPath; - this.enableGrpcClientMetrics = gso.enableMetrics; + this.enableGrpcClientMetrics = gso.enableGrpcClientMetrics; this.grpcInterceptorProvider = gso.grpcInterceptorProvider; this.blobWriteSessionConfig = gso.blobWriteSessionConfig; } @@ -476,6 +485,9 @@ public GrpcStorageOptions.Builder setAttemptDirectPath(boolean attemptDirectPath @BetaApi public GrpcStorageOptions.Builder setEnableGrpcClientMetrics(boolean enableGrpcClientMetrics) { this.enableGrpcClientMetrics = enableGrpcClientMetrics; + if (enableGrpcClientMetrics) { + grpcMetricsManuallyEnabled = true; + } return this; } diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index 96b98b7b0e..fc73e12aa9 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -18,6 +18,8 @@ import com.google.api.core.ApiFunction; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; +import com.google.api.gax.rpc.PermissionDeniedException; +import com.google.api.gax.rpc.UnavailableException; import com.google.cloud.opentelemetry.metric.GoogleCloudMetricExporter; import com.google.cloud.opentelemetry.metric.MetricConfiguration; import com.google.cloud.opentelemetry.metric.MonitoredResourceDescription; @@ -32,25 +34,32 @@ import io.opentelemetry.api.internal.StringUtils; import io.opentelemetry.contrib.gcp.resource.GCPResourceProvider; import io.opentelemetry.sdk.OpenTelemetrySdk; +import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.common.export.MemoryMode; import io.opentelemetry.sdk.metrics.Aggregation; import io.opentelemetry.sdk.metrics.InstrumentSelector; import io.opentelemetry.sdk.metrics.InstrumentType; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; import io.opentelemetry.sdk.metrics.View; +import io.opentelemetry.sdk.metrics.data.AggregationTemporality; +import io.opentelemetry.sdk.metrics.data.MetricData; +import io.opentelemetry.sdk.metrics.export.DefaultAggregationSelector; import io.opentelemetry.sdk.metrics.export.MetricExporter; import io.opentelemetry.sdk.metrics.export.PeriodicMetricReader; import io.opentelemetry.sdk.resources.Resource; import java.math.BigDecimal; import java.math.MathContext; +import java.net.NoRouteToHostException; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Logger; import java.util.stream.Collectors; final class OpenTelemetryBootstrappingUtils { - private static final Collection METRICS_TO_ENABLE = ImmutableList.of( "grpc.lb.wrr.rr_fallback", @@ -76,13 +85,17 @@ final class OpenTelemetryBootstrappingUtils { "grpc.client.attempt.duration", "grpc.client.call.duration"); + static final Logger log = Logger.getLogger(OpenTelemetryBootstrappingUtils.class.getName()); + static void enableGrpcMetrics( InstantiatingGrpcChannelProvider.Builder channelProviderBuilder, String endpoint, String projectId, - String universeDomain) { + String universeDomain, + boolean shouldSuppressExceptions) { String metricServiceEndpoint = getCloudMonitoringEndpoint(endpoint, universeDomain); - SdkMeterProvider provider = createMeterProvider(metricServiceEndpoint, projectId); + SdkMeterProvider provider = + createMeterProvider(metricServiceEndpoint, projectId, shouldSuppressExceptions); OpenTelemetrySdk openTelemetrySdk = OpenTelemetrySdk.builder().setMeterProvider(provider).build(); @@ -131,13 +144,27 @@ static String getCloudMonitoringEndpoint(String endpoint, String universeDomain) } @VisibleForTesting - static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String projectId) { + static SdkMeterProvider createMeterProvider( + String metricServiceEndpoint, String projectId, boolean shouldSuppressExceptions) { GCPResourceProvider resourceProvider = new GCPResourceProvider(); Attributes detectedAttributes = resourceProvider.getAttributes(); String detectedProjectId = detectedAttributes.get(AttributeKey.stringKey("cloud.account.id")); String projectIdToUse = detectedProjectId == null ? projectId : detectedProjectId; + if (!projectIdToUse.equals(projectId)) { + log.warning( + "The Project ID configured for metrics is " + + projectIdToUse + + ", but the Project ID of the storage " + + "client is " + + projectId + + ". Make sure that the service account in use has the required metric writing role " + + "(roles/monitoring.metricWriter) in the project " + + projectIdToUse + + ", or metrics will not be written."); + } + MonitoredResourceDescription monitoredResourceDescription = new MonitoredResourceDescription( "storage.googleapis.com/Client", @@ -165,9 +192,13 @@ static SdkMeterProvider createMeterProvider(String metricServiceEndpoint, String InstrumentSelector.builder().setName(metric).build(), View.builder().setName(metric.replace(".", "/")).build()); } + MetricExporter exporter = + shouldSuppressExceptions + ? new PermissionDeniedSingleReportMetricsExporter(cloudMonitoringExporter) + : cloudMonitoringExporter; providerBuilder .registerMetricReader( - PeriodicMetricReader.builder(cloudMonitoringExporter) + PeriodicMetricReader.builder(exporter) .setInterval(java.time.Duration.ofSeconds(60)) .build()) .setResource( @@ -266,4 +297,87 @@ private static List sizeHistogramBoundaries() { } return boundaries; } + + private static final class PermissionDeniedSingleReportMetricsExporter implements MetricExporter { + private final MetricExporter delegate; + private final AtomicBoolean seenPermissionDenied = new AtomicBoolean(false); + private final AtomicBoolean seenNoRouteToHost = new AtomicBoolean(false); + + private PermissionDeniedSingleReportMetricsExporter(MetricExporter delegate) { + this.delegate = delegate; + } + + @Override + public CompletableResultCode export(Collection metrics) { + if (seenPermissionDenied.get() && seenNoRouteToHost.get()) { + return CompletableResultCode.ofFailure(); + } + + try { + return delegate.export(metrics); + } catch (PermissionDeniedException e) { + if (!seenPermissionDenied.get()) { + seenPermissionDenied.set(true); + throw e; + } + return CompletableResultCode.ofFailure(); + } catch (UnavailableException e) { + if (seenPermissionDenied.get() + && !seenNoRouteToHost.get() + && ultimateCause(e, NoRouteToHostException.class)) { + seenNoRouteToHost.set(true); + throw e; + } + return CompletableResultCode.ofFailure(); + } + } + + @Override + public Aggregation getDefaultAggregation(InstrumentType instrumentType) { + return delegate.getDefaultAggregation(instrumentType); + } + + @Override + public MemoryMode getMemoryMode() { + return delegate.getMemoryMode(); + } + + @Override + public CompletableResultCode flush() { + return delegate.flush(); + } + + @Override + public CompletableResultCode shutdown() { + return delegate.shutdown(); + } + + @Override + public void close() { + delegate.close(); + } + + @Override + public AggregationTemporality getAggregationTemporality(InstrumentType instrumentType) { + return delegate.getAggregationTemporality(instrumentType); + } + + @Override + public DefaultAggregationSelector with(InstrumentType instrumentType, Aggregation aggregation) { + return delegate.with(instrumentType, aggregation); + } + + private static boolean ultimateCause(Throwable t, Class c) { + if (t == null) { + return false; + } + + Throwable cause = t.getCause(); + if (cause != null && c.isAssignableFrom(cause.getClass())) { + return true; + } else { + return ultimateCause(cause, c); + } + } + } } diff --git a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java index 3cd8fba37c..f1a406ca6c 100644 --- a/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java +++ b/google-cloud-storage/src/test/java/com/google/cloud/storage/ITGrpcMetricsTest.java @@ -17,7 +17,6 @@ package com.google.cloud.storage; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertTrue; import com.google.cloud.storage.it.runner.StorageITRunner; import com.google.cloud.storage.it.runner.annotations.Backend; @@ -39,7 +38,7 @@ public void testGrpcMetrics() { SdkMeterProvider provider = OpenTelemetryBootstrappingUtils.createMeterProvider( - "monitoring.googleapis.com:443", grpcStorageOptions.getProjectId()); + "monitoring.googleapis.com:443", grpcStorageOptions.getProjectId(), false); /* * SDKMeterProvider doesn't expose the relevant fields we want to test, but they are present in @@ -49,9 +48,11 @@ public void testGrpcMetrics() { */ String result = provider.toString(); - assertTrue( - result.contains(grpcStorageOptions.getProjectId()) - || result.contains(System.getenv("GOOGLE_CLOUD_PROJECT"))); + // What the project ID will be will depend on the environment, so we just make sure it's present + // and not null/empty + assertThat(result.contains("project_id")); + assertThat(result).doesNotContain("project_id=\"\""); + assertThat(result).doesNotContain("project_id=null"); // This is the check for the Seconds histogram boundary. We can't practically check for every // boundary, From 2ffa2f39f83454329e4d7a141a94c5de26aba665 Mon Sep 17 00:00:00 2001 From: JesseLovelace <43148100+JesseLovelace@users.noreply.github.com> Date: Wed, 31 Jul 2024 09:19:34 -0700 Subject: [PATCH 16/18] Apply suggestions from code review Co-authored-by: BenWhitehead --- google-cloud-storage/pom.xml | 2 +- .../main/java/com/google/cloud/storage/GrpcStorageOptions.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-storage/pom.xml b/google-cloud-storage/pom.xml index fd824ffd61..839cc39559 100644 --- a/google-cloud-storage/pom.xml +++ b/google-cloud-storage/pom.xml @@ -354,7 +354,7 @@ org.junit.vintage:junit-vintage-engine io.opentelemetry:opentelemetry-sdk-extension-autoconfigure-spi io.opentelemetry.semconv:opentelemetry-semconv diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 582797c37c..37add2bab2 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -698,7 +698,7 @@ public boolean isAttemptDirectPath() { /** @since 2.41.0 This new api is in preview and is subject to breaking changes. */ @BetaApi - public boolean isEnableMetrics() { + public boolean isEnableGrpcClientMetrics() { return true; } From 287a85cb4ade80df10f685baa547d7d4fd16d838 Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Wed, 31 Jul 2024 09:31:33 -0700 Subject: [PATCH 17/18] fix isEnableGrpcMetrics --- .../main/java/com/google/cloud/storage/GrpcStorageOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 37add2bab2..41d6cf183f 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -425,7 +425,7 @@ public static final class Builder extends StorageOptions.Builder { private StorageRetryStrategy storageRetryStrategy; private Duration terminationAwaitDuration; private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath(); - private boolean enableGrpcClientMetrics = GrpcStorageDefaults.INSTANCE.isEnableMetrics(); + private boolean enableGrpcClientMetrics = GrpcStorageDefaults.INSTANCE.isEnableGrpcClientMetrics(); private GrpcInterceptorProvider grpcInterceptorProvider = GrpcStorageDefaults.INSTANCE.grpcInterceptorProvider(); private BlobWriteSessionConfig blobWriteSessionConfig = From 7f66e9f365b86db0d41f8f319a0e6632e9ee63ea Mon Sep 17 00:00:00 2001 From: Jesse Lovelace Date: Wed, 31 Jul 2024 09:47:38 -0700 Subject: [PATCH 18/18] lint --- .../main/java/com/google/cloud/storage/GrpcStorageOptions.java | 3 ++- .../google/cloud/storage/OpenTelemetryBootstrappingUtils.java | 3 +-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java index 41d6cf183f..64376cc107 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageOptions.java @@ -425,7 +425,8 @@ public static final class Builder extends StorageOptions.Builder { private StorageRetryStrategy storageRetryStrategy; private Duration terminationAwaitDuration; private boolean attemptDirectPath = GrpcStorageDefaults.INSTANCE.isAttemptDirectPath(); - private boolean enableGrpcClientMetrics = GrpcStorageDefaults.INSTANCE.isEnableGrpcClientMetrics(); + private boolean enableGrpcClientMetrics = + GrpcStorageDefaults.INSTANCE.isEnableGrpcClientMetrics(); private GrpcInterceptorProvider grpcInterceptorProvider = GrpcStorageDefaults.INSTANCE.grpcInterceptorProvider(); private BlobWriteSessionConfig blobWriteSessionConfig = diff --git a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java index fc73e12aa9..ed0ba544e5 100644 --- a/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java +++ b/google-cloud-storage/src/main/java/com/google/cloud/storage/OpenTelemetryBootstrappingUtils.java @@ -156,8 +156,7 @@ static SdkMeterProvider createMeterProvider( log.warning( "The Project ID configured for metrics is " + projectIdToUse - + ", but the Project ID of the storage " - + "client is " + + ", but the Project ID of the storage client is " + projectId + ". Make sure that the service account in use has the required metric writing role " + "(roles/monitoring.metricWriter) in the project "