From b47de350e6507417627a453e8aebf985a77ac5ac Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 1 Apr 2024 21:34:06 +0000 Subject: [PATCH 1/5] Update export logic for createServiceTimeSeries --- .../metric/CloudMetricClient.java | 18 ++++-- .../metric/CloudMetricClientImpl.java | 5 ++ .../metric/InternalMetricExporter.java | 62 +++++++++++++------ .../metric/MetricConfiguration.java | 23 +++++++ .../metric/GoogleCloudMetricExporterTest.java | 12 ++-- 5 files changed, 94 insertions(+), 26 deletions(-) diff --git a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClient.java b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClient.java index ecaa0e5b..4073c422 100644 --- a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClient.java +++ b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClient.java @@ -31,14 +31,24 @@ public interface CloudMetricClient { MetricDescriptor createMetricDescriptor(CreateMetricDescriptorRequest request); /** - * Send a timeseries to Cloud Monitoring. + * Send a time series to Cloud Monitoring. * - * @param name The name of the project where we write the timeseries. - * @param timeSeries The list of timeseries to write. - *

Note: This can only take one point at per timeseries. + * @param name The name of the project where we write the time series. + * @param timeSeries The list of time series to write. + *

Note: This can only take one point at per time series. */ void createTimeSeries(ProjectName name, List timeSeries); + /** + * Send a service time series to Cloud Monitoring. A service time series is a time series for a + * metric from a Google Cloud service. This method should not be used for sending custom metrics. + * + * @param name The name of the project where we write the time series. + * @param timeSeries The list of time series to write. + *

Note: This can only take one point at per time series. + */ + void createServiceTimeSeries(ProjectName name, List timeSeries); + /** Shutdown this client, cleaning up any resources. */ void shutdown(); } diff --git a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClientImpl.java b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClientImpl.java index ef5c26f7..7f510409 100644 --- a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClientImpl.java +++ b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/CloudMetricClientImpl.java @@ -40,6 +40,11 @@ public void createTimeSeries(ProjectName name, List timeSeries) { this.metricServiceClient.createTimeSeries(name, timeSeries); } + @Override + public void createServiceTimeSeries(ProjectName name, List timeSeries) { + this.metricServiceClient.createServiceTimeSeries(name, timeSeries); + } + @Override public void shutdown() { this.metricServiceClient.shutdown(); diff --git a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java index 8c0c90a7..ab93dd05 100644 --- a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java +++ b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java @@ -45,6 +45,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.function.Function; import java.util.function.Predicate; import javax.annotation.Nonnull; import org.slf4j.Logger; @@ -66,18 +67,21 @@ class InternalMetricExporter implements MetricExporter { private final String prefix; private final MetricDescriptorStrategy metricDescriptorStrategy; private final Predicate> resourceAttributesFilter; + private final boolean useCreateServiceTimeSeries; InternalMetricExporter( String projectId, String prefix, CloudMetricClient client, MetricDescriptorStrategy descriptorStrategy, - Predicate> resourceAttributesFilter) { + Predicate> resourceAttributesFilter, + boolean useCreateServiceTimeSeries) { this.projectId = projectId; this.prefix = prefix; this.metricServiceClient = client; this.metricDescriptorStrategy = descriptorStrategy; this.resourceAttributesFilter = resourceAttributesFilter; + this.useCreateServiceTimeSeries = useCreateServiceTimeSeries; } static InternalMetricExporter createWithConfiguration(MetricConfiguration configuration) @@ -115,7 +119,8 @@ static InternalMetricExporter createWithConfiguration(MetricConfiguration config prefix, new CloudMetricClientImpl(MetricServiceClient.create(builder.build())), configuration.getDescriptorStrategy(), - configuration.getResourceAttributesFilter()); + configuration.getResourceAttributesFilter(), + configuration.getUseServiceTimeSeries()); } @VisibleForTesting @@ -124,9 +129,15 @@ static InternalMetricExporter createWithClient( String prefix, CloudMetricClient metricServiceClient, MetricDescriptorStrategy descriptorStrategy, - Predicate> resourceAttributesFilter) { + Predicate> resourceAttributesFilter, + boolean useCreateServiceTimeSeries) { return new InternalMetricExporter( - projectId, prefix, metricServiceClient, descriptorStrategy, resourceAttributesFilter); + projectId, + prefix, + metricServiceClient, + descriptorStrategy, + resourceAttributesFilter, + useCreateServiceTimeSeries); } private void exportDescriptor(MetricDescriptor descriptor) { @@ -197,17 +208,19 @@ public CompletableResultCode export(Collection metrics) { // } } // Update metric descriptors based on configured strategy. - try { - Collection descriptors = builder.getDescriptors(); - if (!descriptors.isEmpty()) { - metricDescriptorStrategy.exportDescriptors(descriptors, this::exportDescriptor); - } - } catch (Exception e) { - logger.warn("Failed to create metric descriptors", e); - } + exportDescriptors(builder); List series = builder.getTimeSeries(); - createTimeSeriesBatch(metricServiceClient, ProjectName.of(projectId), series); + Function, Void> timeSeriesGenerator = + timeSeries -> { + if (useCreateServiceTimeSeries) { + metricServiceClient.createServiceTimeSeries(ProjectName.of(projectId), timeSeries); + } else { + metricServiceClient.createTimeSeries(ProjectName.of(projectId), timeSeries); + } + return null; + }; + createTimeSeriesBatch(series, timeSeriesGenerator); // TODO: better error reporting. if (series.size() < metrics.size()) { return CompletableResultCode.ofFailure(); @@ -215,14 +228,27 @@ public CompletableResultCode export(Collection metrics) { return CompletableResultCode.ofSuccess(); } + private void exportDescriptors(MetricTimeSeriesBuilder timeSeriesBuilder) { + if (useCreateServiceTimeSeries) { + // do not export metric descriptors when using createServiceTimeSeries + return; + } + try { + Collection descriptors = timeSeriesBuilder.getDescriptors(); + if (!descriptors.isEmpty()) { + metricDescriptorStrategy.exportDescriptors(descriptors, this::exportDescriptor); + } + } catch (Exception e) { + logger.warn("Failed to create metric descriptors", e); + } + } + // Fragment metrics into batches and send to GCM. - private static void createTimeSeriesBatch( - CloudMetricClient metricServiceClient, - ProjectName projectName, - List allTimesSeries) { + private void createTimeSeriesBatch( + List allTimesSeries, Function, Void> timeSeriesGenerator) { List> batches = Lists.partition(allTimesSeries, MAX_BATCH_SIZE); for (List timeSeries : batches) { - metricServiceClient.createTimeSeries(projectName, new ArrayList<>(timeSeries)); + timeSeriesGenerator.apply(new ArrayList<>(timeSeries)); } } diff --git a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java index f073253f..7a7a365f 100644 --- a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java +++ b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java @@ -28,6 +28,7 @@ import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.semconv.ResourceAttributes; import java.time.Duration; +import java.util.List; import java.util.function.Predicate; import java.util.function.Supplier; import javax.annotation.Nullable; @@ -141,6 +142,15 @@ public final String getProjectId() { */ public abstract Predicate> getResourceAttributesFilter(); + /** + * Returns a boolean indicating if the {@link MetricConfiguration} is configured to write to a + * metric generated from a Google Cloud Service. + * + * @return true if the {@link MetricConfiguration} is configured to write to a metric generated + * from a Google Cloud Service, false otherwise. + */ + public abstract boolean getUseServiceTimeSeries(); + @VisibleForTesting abstract boolean getInsecureEndpoint(); @@ -164,6 +174,7 @@ public static Builder builder() { .setDeadline(DEFAULT_DEADLINE) .setDescriptorStrategy(MetricDescriptorStrategy.SEND_ONCE) .setInsecureEndpoint(false) + .setUseServiceTimeSeries(false) .setResourceAttributesFilter(DEFAULT_RESOURCE_ATTRIBUTES_FILTER) .setMetricServiceEndpoint(MetricServiceStubSettings.getDefaultEndpoint()); } @@ -231,6 +242,18 @@ public final Builder setProjectId(String projectId) { @VisibleForTesting abstract Builder setInsecureEndpoint(boolean value); + /** + * Sets the {@link MetricConfiguration} to configure the exporter to write metrics via {@link + * com.google.cloud.monitoring.v3.MetricServiceClient#createServiceTimeSeries(String, List)} + * method. By default, this is false. + * + * @param useServiceTimeSeries a boolean indicating whether to use {@link + * com.google.cloud.monitoring.v3.MetricServiceClient#createServiceTimeSeries(String, List)} + * method for writing metrics to Google Cloud Monitoring. + * @return this + */ + abstract Builder setUseServiceTimeSeries(boolean useServiceTimeSeries); + abstract MetricConfiguration autoBuild(); /** diff --git a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java index cb5a30e1..5b82d9c0 100644 --- a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java +++ b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java @@ -132,7 +132,8 @@ public void testExportSendsAllDescriptorsOnce() { DEFAULT_PREFIX, mockClient, MetricDescriptorStrategy.SEND_ONCE, - DEFAULT_RESOURCE_ATTRIBUTES_FILTER); + DEFAULT_RESOURCE_ATTRIBUTES_FILTER, + false); CompletableResultCode result = exporter.export(ImmutableList.of(aMetricData, aHistogram)); assertTrue(result.isSuccess()); CompletableResultCode result2 = exporter.export(ImmutableList.of(aMetricData, aHistogram)); @@ -241,7 +242,8 @@ public void testExportSucceeds() { DEFAULT_PREFIX, mockClient, MetricDescriptorStrategy.ALWAYS_SEND, - DEFAULT_RESOURCE_ATTRIBUTES_FILTER); + DEFAULT_RESOURCE_ATTRIBUTES_FILTER, + false); CompletableResultCode result = exporter.export(ImmutableList.of(aMetricData)); verify(mockClient, times(1)).createMetricDescriptor(metricDescriptorCaptor.capture()); @@ -362,7 +364,8 @@ public void testExportWithHistogram_Succeeds() { DEFAULT_PREFIX, mockClient, MetricDescriptorStrategy.ALWAYS_SEND, - DEFAULT_RESOURCE_ATTRIBUTES_FILTER); + DEFAULT_RESOURCE_ATTRIBUTES_FILTER, + false); CompletableResultCode result = exporter.export(ImmutableList.of(aHistogram)); verify(mockClient, times(1)).createMetricDescriptor(metricDescriptorCaptor.capture()); verify(mockClient, times(1)) @@ -382,7 +385,8 @@ public void testExportWithNonSupportedMetricTypeReturnsFailure() { DEFAULT_PREFIX, mockClient, MetricDescriptorStrategy.ALWAYS_SEND, - NO_RESOURCE_ATTRIBUTES); + NO_RESOURCE_ATTRIBUTES, + false); MetricData metricData = ImmutableMetricData.createDoubleSummary( From 82a62dcc0285201f190bb965647a0b5ff5c0b2de Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Mon, 1 Apr 2024 23:15:51 +0000 Subject: [PATCH 2/5] Add test case for use with CreateServiceTimeSeries --- .../cloud/opentelemetry/metric/FakeData.java | 10 +++++++++ .../metric/GoogleCloudMetricExporterTest.java | 21 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/FakeData.java b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/FakeData.java index dbdbd9ff..44b99600 100644 --- a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/FakeData.java +++ b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/FakeData.java @@ -119,6 +119,16 @@ public class FakeData { ImmutableSumData.create( true, AggregationTemporality.CUMULATIVE, ImmutableList.of(aLongPoint))); + static final MetricData googleComputeServiceMetricData = + ImmutableMetricData.createLongSum( + aGceResource, + anInstrumentationLibraryInfo, + "guest/disk/io_time", + "description", + "ns", + ImmutableSumData.create( + true, AggregationTemporality.CUMULATIVE, ImmutableList.of(aLongPoint))); + static final String aTraceId = "00000000000000000000000000000001"; static final String aSpanId = "0000000000000002"; static final SpanContext aSpanContext = diff --git a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java index 5b82d9c0..9484d4c0 100644 --- a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java +++ b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/GoogleCloudMetricExporterTest.java @@ -30,6 +30,7 @@ import static com.google.cloud.opentelemetry.metric.FakeData.aSpanId; import static com.google.cloud.opentelemetry.metric.FakeData.aTraceId; import static com.google.cloud.opentelemetry.metric.FakeData.anInstrumentationLibraryInfo; +import static com.google.cloud.opentelemetry.metric.FakeData.googleComputeServiceMetricData; import static com.google.cloud.opentelemetry.metric.MetricConfiguration.DEFAULT_PREFIX; import static com.google.cloud.opentelemetry.metric.MetricConfiguration.DEFAULT_RESOURCE_ATTRIBUTES_FILTER; import static com.google.cloud.opentelemetry.metric.MetricConfiguration.NO_RESOURCE_ATTRIBUTES; @@ -456,6 +457,26 @@ public void verifyExporterCreationErrorDoesNotBreakMetricExporter() { } } + @Test + public void verifyExporterExportGoogleServiceMetrics() { + MetricExporter exporter = + InternalMetricExporter.createWithClient( + aProjectId, + "compute.googleapis.com", + mockClient, + MetricDescriptorStrategy.ALWAYS_SEND, + NO_RESOURCE_ATTRIBUTES, + true); + + CompletableResultCode result = + exporter.export(ImmutableList.of(googleComputeServiceMetricData)); + verify(mockClient, times(0)).createMetricDescriptor(any()); + verify(mockClient, times(0)).createTimeSeries(any(ProjectName.class), any()); + verify(mockClient, times(1)).createServiceTimeSeries(any(ProjectName.class), any()); + + assertTrue(result.isSuccess()); + } + private void generateOpenTelemetryUsingGoogleCloudMetricExporter(MetricExporter metricExporter) { SdkMeterProvider meterProvider = SdkMeterProvider.builder() From 153ee56b740876cc7504426710d78b3be0722915 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Tue, 2 Apr 2024 15:57:54 +0000 Subject: [PATCH 3/5] Make serviceTimeSeries builder config public --- .../metric/MetricConfiguration.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java index 7a7a365f..fd525c16 100644 --- a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java +++ b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/MetricConfiguration.java @@ -226,6 +226,18 @@ public final Builder setProjectId(String projectId) { /** Sets the endpoint where to write Metrics. Defaults to monitoring.googleapis.com:443. */ public abstract Builder setMetricServiceEndpoint(String endpoint); + /** + * Sets the {@link MetricConfiguration} to configure the exporter to write metrics via {@link + * com.google.cloud.monitoring.v3.MetricServiceClient#createServiceTimeSeries(String, List)} + * method. By default, this is false. + * + * @param useServiceTimeSeries a boolean indicating whether to use {@link + * com.google.cloud.monitoring.v3.MetricServiceClient#createServiceTimeSeries(String, List)} + * method for writing metrics to Google Cloud Monitoring. + * @return this + */ + public abstract Builder setUseServiceTimeSeries(boolean useServiceTimeSeries); + /** * Set a filter to determine which resource attributes to add to metrics as metric labels. By * default, it adds service.name, service.namespace, and service.instance.id. This is @@ -242,18 +254,6 @@ public final Builder setProjectId(String projectId) { @VisibleForTesting abstract Builder setInsecureEndpoint(boolean value); - /** - * Sets the {@link MetricConfiguration} to configure the exporter to write metrics via {@link - * com.google.cloud.monitoring.v3.MetricServiceClient#createServiceTimeSeries(String, List)} - * method. By default, this is false. - * - * @param useServiceTimeSeries a boolean indicating whether to use {@link - * com.google.cloud.monitoring.v3.MetricServiceClient#createServiceTimeSeries(String, List)} - * method for writing metrics to Google Cloud Monitoring. - * @return this - */ - abstract Builder setUseServiceTimeSeries(boolean useServiceTimeSeries); - abstract MetricConfiguration autoBuild(); /** From d85c0a3ecb0551493d1833c16e77e90ddff81b0d Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Tue, 2 Apr 2024 19:30:54 +0000 Subject: [PATCH 4/5] Update configuration test --- .../cloud/opentelemetry/metric/MetricConfigurationTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/MetricConfigurationTest.java b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/MetricConfigurationTest.java index dcef7eca..ab8119f2 100644 --- a/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/MetricConfigurationTest.java +++ b/exporters/metrics/src/test/java/com/google/cloud/opentelemetry/metric/MetricConfigurationTest.java @@ -16,8 +16,10 @@ package com.google.cloud.opentelemetry.metric; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import com.google.auth.Credentials; import com.google.auth.oauth2.AccessToken; @@ -48,6 +50,7 @@ public void testDefaultConfigurationSucceeds() { assertNull(configuration.getCredentials()); assertEquals(PROJECT_ID, configuration.getProjectId()); + assertFalse(configuration.getUseServiceTimeSeries()); } @Test @@ -58,11 +61,13 @@ public void testSetAllConfigurationFieldsSucceeds() { .setProjectId(PROJECT_ID) .setCredentials(FAKE_CREDENTIALS) .setResourceAttributesFilter(allowAllPredicate) + .setUseServiceTimeSeries(true) .build(); assertEquals(FAKE_CREDENTIALS, configuration.getCredentials()); assertEquals(PROJECT_ID, configuration.getProjectId()); assertEquals(allowAllPredicate, configuration.getResourceAttributesFilter()); + assertTrue(configuration.getUseServiceTimeSeries()); } @Test From 79bd4fca642c2c9fec8fa86152418fab125ad410 Mon Sep 17 00:00:00 2001 From: Pranav Sharma Date: Tue, 2 Apr 2024 20:07:23 +0000 Subject: [PATCH 5/5] Switch functional interface to Consumer --- .../opentelemetry/metric/InternalMetricExporter.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java index ab93dd05..bbd65029 100644 --- a/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java +++ b/exporters/metrics/src/main/java/com/google/cloud/opentelemetry/metric/InternalMetricExporter.java @@ -45,7 +45,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import java.util.function.Function; +import java.util.function.Consumer; import java.util.function.Predicate; import javax.annotation.Nonnull; import org.slf4j.Logger; @@ -211,14 +211,13 @@ public CompletableResultCode export(Collection metrics) { exportDescriptors(builder); List series = builder.getTimeSeries(); - Function, Void> timeSeriesGenerator = + Consumer> timeSeriesGenerator = timeSeries -> { if (useCreateServiceTimeSeries) { metricServiceClient.createServiceTimeSeries(ProjectName.of(projectId), timeSeries); } else { metricServiceClient.createTimeSeries(ProjectName.of(projectId), timeSeries); } - return null; }; createTimeSeriesBatch(series, timeSeriesGenerator); // TODO: better error reporting. @@ -245,10 +244,10 @@ private void exportDescriptors(MetricTimeSeriesBuilder timeSeriesBuilder) { // Fragment metrics into batches and send to GCM. private void createTimeSeriesBatch( - List allTimesSeries, Function, Void> timeSeriesGenerator) { + List allTimesSeries, Consumer> timeSeriesGenerator) { List> batches = Lists.partition(allTimesSeries, MAX_BATCH_SIZE); for (List timeSeries : batches) { - timeSeriesGenerator.apply(new ArrayList<>(timeSeries)); + timeSeriesGenerator.accept(new ArrayList<>(timeSeries)); } }