From 39f38b72c7b05b7947cb7a4bdc8d89240f960de8 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 16 Oct 2024 09:15:00 -0400 Subject: [PATCH 1/6] fix: export metrics to different projects --- .../BigtableCloudMonitoringExporter.java | 82 ++++++++----------- .../stub/metrics/BigtableExporterUtils.java | 16 ++-- .../v2/stub/metrics/BuiltinMetricsView.java | 3 +- .../BigtableCloudMonitoringExporterTest.java | 3 +- 4 files changed, 48 insertions(+), 56 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index 8aa53fa19861..8aec408e960c 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -58,6 +58,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Level; @@ -94,7 +95,6 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { private final MetricServiceClient client; - private final String bigtableProjectId; private final String taskId; // The resource the client application is running on @@ -128,8 +128,7 @@ public final class BigtableCloudMonitoringExporter implements MetricExporter { .collect(ImmutableList.toImmutableList()); public static BigtableCloudMonitoringExporter create( - String projectId, @Nullable Credentials credentials, @Nullable String endpoint) - throws IOException { + @Nullable Credentials credentials, @Nullable String endpoint) throws IOException { MetricServiceSettings.Builder settingsBuilder = MetricServiceSettings.newBuilder(); CredentialsProvider credentialsProvider = Optional.ofNullable(credentials) @@ -164,7 +163,6 @@ public static BigtableCloudMonitoringExporter create( } return new BigtableCloudMonitoringExporter( - projectId, MetricServiceClient.create(settingsBuilder.build()), applicationResource, BigtableExporterUtils.getDefaultTaskValue()); @@ -172,14 +170,10 @@ public static BigtableCloudMonitoringExporter create( @VisibleForTesting BigtableCloudMonitoringExporter( - String projectId, - MetricServiceClient client, - @Nullable MonitoredResource applicationResource, - String taskId) { + MetricServiceClient client, @Nullable MonitoredResource applicationResource, String taskId) { this.client = client; this.taskId = taskId; this.applicationResource = applicationResource; - this.bigtableProjectId = projectId; } @Override @@ -211,15 +205,7 @@ private CompletableResultCode exportBigtableResourceMetrics(Collection metricData.getData().getPoints().stream()) - .allMatch(pd -> bigtableProjectId.equals(BigtableExporterUtils.getProjectId(pd)))) { - logger.log(Level.WARNING, "Metric data has different a projectId. Skip exporting."); - return CompletableResultCode.ofFailure(); - } - - List bigtableTimeSeries; + Map> bigtableTimeSeries; try { bigtableTimeSeries = BigtableExporterUtils.convertToBigtableTimeSeries(bigtableMetricData, taskId); @@ -231,37 +217,39 @@ private CompletableResultCode exportBigtableResourceMetrics(Collection> future = exportTimeSeries(projectName, bigtableTimeSeries); - CompletableResultCode bigtableExportCode = new CompletableResultCode(); - ApiFutures.addCallback( - future, - new ApiFutureCallback>() { - @Override - public void onFailure(Throwable throwable) { - if (bigtableExportFailureLogged.compareAndSet(false, true)) { - String msg = "createServiceTimeSeries request failed for bigtable metrics."; - if (throwable instanceof PermissionDeniedException) { - msg += - String.format( - " Need monitoring metric writer permission on project=%s. Follow https://cloud.google.com/bigtable/docs/client-side-metrics-setup to set up permissions.", - projectName.getProject()); - } - logger.log(Level.WARNING, msg, throwable); - } - bigtableExportCode.fail(); - } + bigtableTimeSeries.forEach( + (projectId, ts) -> { + ProjectName projectName = ProjectName.of(projectId); + ApiFuture> future = exportTimeSeries(projectName, ts); + ApiFutures.addCallback( + future, + new ApiFutureCallback>() { + @Override + public void onFailure(Throwable throwable) { + if (bigtableExportFailureLogged.compareAndSet(false, true)) { + String msg = "createServiceTimeSeries request failed for bigtable metrics."; + if (throwable instanceof PermissionDeniedException) { + msg += + String.format( + " Need monitoring metric writer permission on project=%s. Follow https://cloud.google.com/bigtable/docs/client-side-metrics-setup to set up permissions.", + projectName.getProject()); + } + logger.log(Level.WARNING, msg, throwable); + } + bigtableExportCode.fail(); + } - @Override - public void onSuccess(List emptyList) { - // When an export succeeded reset the export failure flag to false so if there's a - // transient failure it'll be logged. - bigtableExportFailureLogged.set(false); - bigtableExportCode.succeed(); - } - }, - MoreExecutors.directExecutor()); + @Override + public void onSuccess(List emptyList) { + // When an export succeeded reset the export failure flag to false so if there's a + // transient failure it'll be logged. + bigtableExportFailureLogged.set(false); + bigtableExportCode.succeed(); + } + }, + MoreExecutors.directExecutor()); + }); return bigtableExportCode; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index 5bf6688e17b0..02c8d4c6a922 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -63,6 +63,7 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -110,17 +111,22 @@ static String getProjectId(PointData pointData) { return pointData.getAttributes().get(BIGTABLE_PROJECT_ID_KEY); } - static List convertToBigtableTimeSeries(List collection, String taskId) { - List allTimeSeries = new ArrayList<>(); + static Map> convertToBigtableTimeSeries( + List collection, String taskId) { + Map> allTimeSeries = new HashMap<>(); for (MetricData metricData : collection) { if (!metricData.getInstrumentationScopeInfo().getName().equals(METER_NAME)) { // Filter out metric data for instruments that are not part of the bigtable builtin metrics continue; } - metricData.getData().getPoints().stream() - .map(pointData -> convertPointToBigtableTimeSeries(metricData, pointData, taskId)) - .forEach(allTimeSeries::add); + + for (PointData pd : metricData.getData().getPoints()) { + String projectId = getProjectId(pd); + List current = allTimeSeries.getOrDefault(projectId, new ArrayList<>()); + current.add(convertPointToBigtableTimeSeries(metricData, pd, taskId)); + allTimeSeries.put(projectId, current); + } } return allTimeSeries; diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java index ca52581a92c9..6760dcdef0b0 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java @@ -65,8 +65,7 @@ public static void registerBuiltinMetrics( SdkMeterProviderBuilder builder, @Nullable String endpoint) throws IOException { - MetricExporter metricExporter = - BigtableCloudMonitoringExporter.create(projectId, credentials, endpoint); + MetricExporter metricExporter = BigtableCloudMonitoringExporter.create(credentials, endpoint); for (Map.Entry entry : BuiltinMetricsConstants.getAllViews().entrySet()) { builder.registerView(entry.getKey(), entry.getValue()); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java index 81629e2d9ddc..4609021adcd1 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java @@ -89,7 +89,7 @@ public void setUp() { exporter = new BigtableCloudMonitoringExporter( - projectId, fakeMetricServiceClient, /* applicationResource= */ null, taskId); + fakeMetricServiceClient, /* applicationResource= */ null, taskId); attributes = Attributes.builder() @@ -301,7 +301,6 @@ public void testTimeSeriesForMetricWithGceOrGkeResource() { String gceProjectId = "fake-gce-project"; BigtableCloudMonitoringExporter exporter = new BigtableCloudMonitoringExporter( - projectId, fakeMetricServiceClient, MonitoredResource.newBuilder() .setType("gce-instance") From bb503f710ced6255055daddfea868b6d2cd48806 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 22 Oct 2024 12:01:30 -0400 Subject: [PATCH 2/6] address comments and add a test --- .../data/v2/BigtableDataClientFactory.java | 1 - .../data/v2/stub/EnhancedBigtableStub.java | 24 ++-- .../BigtableCloudMonitoringExporter.java | 1 + .../stub/metrics/BigtableExporterUtils.java | 1 + .../v2/stub/metrics/BuiltinMetricsView.java | 35 +++++- .../CustomOpenTelemetryMetricsProvider.java | 2 +- .../stub/metrics/DefaultMetricsProvider.java | 44 ++++--- .../BigtableCloudMonitoringExporterTest.java | 114 ++++++++++++++++++ .../metrics/DefaultMetricsProviderTest.java | 39 ++++++ 9 files changed, 236 insertions(+), 25 deletions(-) create mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java index 34ec77bdfc62..200ed146d530 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataClientFactory.java @@ -88,7 +88,6 @@ public static BigtableDataClientFactory create(BigtableDataSettings defaultSetti // the OTEL instance and log the exception instead. openTelemetry = EnhancedBigtableStub.getOpenTelemetry( - defaultSettings.getProjectId(), defaultSettings.getMetricsProvider(), sharedClientContext.getCredentials(), defaultSettings.getStubSettings().getMetricsEndpoint()); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java index da0831304c39..36a7c539321a 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStub.java @@ -227,7 +227,6 @@ public static EnhancedBigtableStub create(EnhancedBigtableStubSettings settings) // the OTEL instance and log the exception instead. openTelemetry = getOpenTelemetry( - settings.getProjectId(), settings.getMetricsProvider(), clientContext.getCredentials(), settings.getMetricsEndpoint()); @@ -276,10 +275,7 @@ public static ClientContext createClientContext(EnhancedBigtableStubSettings set // the OTEL instance and log the exception instead. openTelemetry = getOpenTelemetry( - settings.getProjectId(), - settings.getMetricsProvider(), - credentials, - settings.getMetricsEndpoint()); + settings.getMetricsProvider(), credentials, settings.getMetricsEndpoint()); } catch (Throwable t) { logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t); } @@ -387,9 +383,23 @@ public static ApiTracerFactory createBigtableTracerFactory( return new CompositeTracerFactory(tracerFactories.build()); } + /** + * getOpenTelemetry is called from the following places: + * + *
    + *
  • EnhancedBigtableStub.createClientContext: gets the openTelemetry to create meter for + * per_connection_error_count + *
  • EnhancedBigtableStub.create(settings): gets the openTelemetry to pass into + * BigtableTracerFactory to create meters for built in metrics + *
  • BigtableDataClientFactory.create(defaultSettings): gets the openTelemetry to pass around + * when creating BigtableTracerFactory + *
+ * + *

DefaultMetricsProvider will always return the same openTelemetry instance for the same + * credentials and metrics endpoint. + */ @Nullable public static OpenTelemetry getOpenTelemetry( - String projectId, MetricsProvider metricsProvider, @Nullable Credentials defaultCredentials, @Nullable String metricsEndpoint) @@ -404,7 +414,7 @@ public static OpenTelemetry getOpenTelemetry( ? BigtableDataSettings.getMetricsCredentials() : defaultCredentials; DefaultMetricsProvider defaultMetricsProvider = (DefaultMetricsProvider) metricsProvider; - return defaultMetricsProvider.getOpenTelemetry(projectId, metricsEndpoint, credentials); + return defaultMetricsProvider.getOpenTelemetry(metricsEndpoint, credentials); } else if (metricsProvider instanceof NoopMetricsProvider) { return null; } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java index 8aec408e960c..ff5bcd81c150 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporter.java @@ -205,6 +205,7 @@ private CompletableResultCode exportBigtableResourceMetrics(Collection> bigtableTimeSeries; try { bigtableTimeSeries = diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index 02c8d4c6a922..21cab9a19df9 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -113,6 +113,7 @@ static String getProjectId(PointData pointData) { static Map> convertToBigtableTimeSeries( List collection, String taskId) { + // List of timeseries by project id Map> allTimeSeries = new HashMap<>(); for (MetricData metricData : collection) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java index 6760dcdef0b0..07679af8d247 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BuiltinMetricsView.java @@ -38,33 +38,64 @@ private BuiltinMetricsView() {} /** * Register built-in metrics on the {@link SdkMeterProviderBuilder} with application default * credentials and default endpoint. + * + * @deprecated projectId is no longer used. Call {@link + * #registerBuiltinMetrics(SdkMeterProviderBuilder)} instead. */ + @Deprecated public static void registerBuiltinMetrics(String projectId, SdkMeterProviderBuilder builder) throws IOException { BuiltinMetricsView.registerBuiltinMetrics( - projectId, GoogleCredentials.getApplicationDefault(), builder); + GoogleCredentials.getApplicationDefault(), builder, null); + } + + /** + * Register built-in metrics on the {@link SdkMeterProviderBuilder} with application default + * credentials and default endpoint. + */ + public static void registerBuiltinMetrics(SdkMeterProviderBuilder builder) throws IOException { + BuiltinMetricsView.registerBuiltinMetrics( + GoogleCredentials.getApplicationDefault(), builder, null); } /** * Register built-in metrics on the {@link SdkMeterProviderBuilder} with custom credentials and * default endpoint. + * + * @deprecated projectId is no longer used. Call {@link #registerBuiltinMetrics(Credentials, + * SdkMeterProviderBuilder, String)} instead. */ + @Deprecated public static void registerBuiltinMetrics( String projectId, @Nullable Credentials credentials, SdkMeterProviderBuilder builder) throws IOException { - BuiltinMetricsView.registerBuiltinMetrics(projectId, credentials, builder, null); + BuiltinMetricsView.registerBuiltinMetrics(credentials, builder, null); } /** * Register built-in metrics on the {@link SdkMeterProviderBuilder} with custom credentials and * endpoint. + * + * @deprecated projectId is no longer used. Call {@link #registerBuiltinMetrics(Credentials, + * SdkMeterProviderBuilder, String)} instead. */ + @Deprecated public static void registerBuiltinMetrics( String projectId, @Nullable Credentials credentials, SdkMeterProviderBuilder builder, @Nullable String endpoint) throws IOException { + registerBuiltinMetrics(credentials, builder, endpoint); + } + + /** + * Register built-in metrics on the {@link SdkMeterProviderBuilder} with custom credentials and + * endpoint. + */ + public static void registerBuiltinMetrics( + @Nullable Credentials credentials, SdkMeterProviderBuilder builder, @Nullable String endpoint) + throws IOException { MetricExporter metricExporter = BigtableCloudMonitoringExporter.create(credentials, endpoint); for (Map.Entry entry : BuiltinMetricsConstants.getAllViews().entrySet()) { diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java index 8c1c5c1c9014..d728d657ae77 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/CustomOpenTelemetryMetricsProvider.java @@ -27,7 +27,7 @@ * SdkMeterProviderBuilder sdkMeterProvider = SdkMeterProvider.builder(); * * // register Builtin metrics on your meter provider with default credentials - * BuiltinMetricsView.registerBuiltinMetrics("project-id", sdkMeterProvider); + * BuiltinMetricsView.registerBuiltinMetrics(sdkMeterProvider); * * // register other metrics reader and views * sdkMeterProvider.registerMetricReader(..); diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java index c6b0a80c7699..e92f51a27915 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java @@ -17,12 +17,14 @@ import com.google.api.core.InternalApi; import com.google.auth.Credentials; +import com.google.auto.value.AutoValue; import com.google.common.base.MoreObjects; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; import java.io.IOException; +import java.util.concurrent.ConcurrentHashMap; import javax.annotation.Nullable; /** @@ -36,30 +38,44 @@ public final class DefaultMetricsProvider implements MetricsProvider { public static DefaultMetricsProvider INSTANCE = new DefaultMetricsProvider(); - private OpenTelemetry openTelemetry; - private String projectId; + private final ConcurrentHashMap otels = new ConcurrentHashMap<>(); private DefaultMetricsProvider() {} @InternalApi public OpenTelemetry getOpenTelemetry( - String projectId, String metricsEndpoint, @Nullable Credentials credentials) - throws IOException { - this.projectId = projectId; - if (openTelemetry == null) { + @Nullable String metricsEndpoint, @Nullable Credentials credentials) throws IOException { + Key key = Key.create(metricsEndpoint, credentials); + OpenTelemetry otel = otels.get(key); + if (otel == null) { SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder(); - BuiltinMetricsView.registerBuiltinMetrics( - projectId, credentials, meterProvider, metricsEndpoint); - openTelemetry = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); + BuiltinMetricsView.registerBuiltinMetrics(credentials, meterProvider, metricsEndpoint); + otel = OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); + otels.put(key, otel); } - return openTelemetry; + return otel; } @Override public String toString() { - return MoreObjects.toStringHelper(this) - .add("projectId", projectId) - .add("openTelemetry", openTelemetry) - .toString(); + // don't log credentials + MoreObjects.ToStringHelper toStringHelper = MoreObjects.toStringHelper(this); + otels.forEach( + (k, v) -> + toStringHelper.add(k.metricsEndpoint() == null ? "null" : k.metricsEndpoint(), v)); + return toStringHelper.toString(); + } + + @AutoValue + abstract static class Key { + @Nullable + abstract String metricsEndpoint(); + + @Nullable + abstract Credentials credentials(); + + static Key create(@Nullable String metricsEndpoint, @Nullable Credentials credentials) { + return new AutoValue_DefaultMetricsProvider_Key(metricsEndpoint, credentials); + } } } diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java index 4609021adcd1..657db7d8aeb0 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableCloudMonitoringExporterTest.java @@ -24,7 +24,10 @@ import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.TABLE_ID_KEY; import static com.google.cloud.bigtable.data.v2.stub.metrics.BuiltinMetricsConstants.ZONE_ID_KEY; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.google.api.Distribution; @@ -35,6 +38,7 @@ import com.google.cloud.monitoring.v3.MetricServiceClient; import com.google.cloud.monitoring.v3.stub.MetricServiceStub; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.monitoring.v3.CreateTimeSeriesRequest; import com.google.monitoring.v3.TimeSeries; import com.google.protobuf.Empty; @@ -53,6 +57,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.List; +import java.util.Map; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -376,6 +382,114 @@ public void testTimeSeriesForMetricWithGceOrGkeResource() { taskId); } + @Test + public void testExportingToMultipleProjects() { + ArgumentCaptor argumentCaptor = + ArgumentCaptor.forClass(CreateTimeSeriesRequest.class); + + UnaryCallable mockCallable = mock(UnaryCallable.class); + when(mockMetricServiceStub.createServiceTimeSeriesCallable()).thenReturn(mockCallable); + ApiFuture future = ApiFutures.immediateFuture(Empty.getDefaultInstance()); + when(mockCallable.futureCall(any())).thenReturn(future); + + long startEpoch = 10; + long endEpoch = 15; + HistogramPointData histogramPointData1 = + ImmutableHistogramPointData.create( + startEpoch, + endEpoch, + attributes, + 3d, + true, + 1d, // min + true, + 2d, // max + Arrays.asList(1.0), + Arrays.asList(1L, 2L)); + + MetricData histogramData1 = + ImmutableMetricData.createDoubleHistogram( + resource, + scope, + "bigtable.googleapis.com/internal/client/operation_latencies", + "description", + "ms", + ImmutableHistogramData.create( + AggregationTemporality.CUMULATIVE, ImmutableList.of(histogramPointData1))); + + HistogramPointData histogramPointData2 = + ImmutableHistogramPointData.create( + startEpoch, + endEpoch, + attributes.toBuilder().put(BIGTABLE_PROJECT_ID_KEY, "another-project").build(), + 50d, + true, + 5d, // min + true, + 30d, // max + Arrays.asList(1.0), + Arrays.asList(5L, 10L)); + + MetricData histogramData2 = + ImmutableMetricData.createDoubleHistogram( + resource, + scope, + "bigtable.googleapis.com/internal/client/operation_latencies", + "description", + "ms", + ImmutableHistogramData.create( + AggregationTemporality.CUMULATIVE, ImmutableList.of(histogramPointData2))); + + exporter.export(Arrays.asList(histogramData1, histogramData2)); + + verify(mockCallable, times(2)).futureCall(argumentCaptor.capture()); + + List allValues = argumentCaptor.getAllValues(); + + assertThat(allValues).hasSize(2); + + List> labelsMap = new ArrayList<>(); + List counts = new ArrayList<>(); + allValues.forEach( + value -> { + labelsMap.add(value.getTimeSeriesList().get(0).getResource().getLabelsMap()); + counts.add( + value + .getTimeSeriesList() + .get(0) + .getPoints(0) + .getValue() + .getDistributionValue() + .getCount()); + }); + + assertThat(labelsMap) + .containsExactly( + ImmutableMap.of( + BIGTABLE_PROJECT_ID_KEY.getKey(), + projectId, + INSTANCE_ID_KEY.getKey(), + instanceId, + TABLE_ID_KEY.getKey(), + tableId, + CLUSTER_ID_KEY.getKey(), + cluster, + ZONE_ID_KEY.getKey(), + zone), + ImmutableMap.of( + BIGTABLE_PROJECT_ID_KEY.getKey(), + "another-project", + INSTANCE_ID_KEY.getKey(), + instanceId, + TABLE_ID_KEY.getKey(), + tableId, + CLUSTER_ID_KEY.getKey(), + cluster, + ZONE_ID_KEY.getKey(), + zone)); + assertThat(counts).containsExactly(3l, 15l); + } + private static class FakeMetricServiceClient extends MetricServiceClient { protected FakeMetricServiceClient(MetricServiceStub stub) { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java new file mode 100644 index 000000000000..e79a002208c2 --- /dev/null +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java @@ -0,0 +1,39 @@ +package com.google.cloud.bigtable.data.v2.stub.metrics; + +import com.google.auth.oauth2.GoogleCredentials; +import com.google.cloud.NoCredentials; +import io.opentelemetry.api.OpenTelemetry; +import java.io.IOException; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DefaultMetricsProviderTest { + + @Test + public void test() throws IOException { + DefaultMetricsProvider provider = DefaultMetricsProvider.INSTANCE; + + OpenTelemetry toCompare = + provider.getOpenTelemetry("fake-endpoint:123", NoCredentials.getInstance()); + // different endpoint should return a different instance + OpenTelemetry otel2 = provider.getOpenTelemetry(null, NoCredentials.getInstance()); + // different endpoint and credential should return a different instance + OpenTelemetry otel3 = provider.getOpenTelemetry(null, null); + // same endpoint, different credential, should return a different instance + OpenTelemetry otel4 = + provider.getOpenTelemetry("fake-endpoint:123", GoogleCredentials.getApplicationDefault()); + // everything is the same, should return the same instance + OpenTelemetry otel5 = + provider.getOpenTelemetry("fake-endpoint:123", NoCredentials.getInstance()); + + Assert.assertNotEquals(toCompare, otel2); + Assert.assertNotEquals(toCompare, otel3); + Assert.assertNotEquals(toCompare, otel4); + Assert.assertEquals(toCompare, otel5); + + provider.toString(); + } +} From 54987b22068646dbba1602e4c5c846979ec725a2 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Tue, 22 Oct 2024 13:10:23 -0400 Subject: [PATCH 3/6] add license header --- .../stub/metrics/DefaultMetricsProviderTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java index e79a002208c2..8eea85715946 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java @@ -1,3 +1,18 @@ +/* + * 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 + * + * https://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.bigtable.data.v2.stub.metrics; import com.google.auth.oauth2.GoogleCredentials; From 31bf0618728f9698ab2e3db0aeab0c8f4e0ff3e6 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 28 Oct 2024 10:38:07 -0400 Subject: [PATCH 4/6] update --- .../stub/metrics/BigtableExporterUtils.java | 2 +- .../metrics/DefaultMetricsProviderTest.java | 35 +++++++++++++++++-- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index 21cab9a19df9..797f7f1eca9b 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -111,9 +111,9 @@ static String getProjectId(PointData pointData) { return pointData.getAttributes().get(BIGTABLE_PROJECT_ID_KEY); } + // Returns a list of timeseries by project id static Map> convertToBigtableTimeSeries( List collection, String taskId) { - // List of timeseries by project id Map> allTimeSeries = new HashMap<>(); for (MetricData metricData : collection) { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java index 8eea85715946..05bbf6db02c2 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java @@ -15,10 +15,15 @@ */ package com.google.cloud.bigtable.data.v2.stub.metrics; -import com.google.auth.oauth2.GoogleCredentials; +import com.google.auth.Credentials; import com.google.cloud.NoCredentials; +import com.google.common.collect.ImmutableMap; import io.opentelemetry.api.OpenTelemetry; import java.io.IOException; +import java.net.URI; +import java.util.Arrays; +import java.util.List; +import java.util.Map; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; @@ -38,8 +43,7 @@ public void test() throws IOException { // different endpoint and credential should return a different instance OpenTelemetry otel3 = provider.getOpenTelemetry(null, null); // same endpoint, different credential, should return a different instance - OpenTelemetry otel4 = - provider.getOpenTelemetry("fake-endpoint:123", GoogleCredentials.getApplicationDefault()); + OpenTelemetry otel4 = provider.getOpenTelemetry("fake-endpoint:123", new FakeCredentials()); // everything is the same, should return the same instance OpenTelemetry otel5 = provider.getOpenTelemetry("fake-endpoint:123", NoCredentials.getInstance()); @@ -51,4 +55,29 @@ public void test() throws IOException { provider.toString(); } + + private static class FakeCredentials extends Credentials { + @Override + public String getAuthenticationType() { + return "fake"; + } + + @Override + public Map> getRequestMetadata(URI uri) throws IOException { + return ImmutableMap.of("my-header", Arrays.asList("fake-credential")); + } + + @Override + public boolean hasRequestMetadata() { + return true; + } + + @Override + public boolean hasRequestMetadataOnly() { + return true; + } + + @Override + public void refresh() throws IOException {} + } } From 5b525002af34a1b5f61d21c457cc604424df80db Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 27 Nov 2024 15:28:44 -0500 Subject: [PATCH 5/6] update --- .../data/v2/BigtableDataSettings.java | 2 + .../data/v2/stub/BigtableClientContext.java | 4 +- .../stub/metrics/DefaultMetricsProvider.java | 9 +- .../v2/stub/EnhancedBigtableStubTest.java | 19 +++-- .../metrics/DefaultMetricsProviderTest.java | 83 ------------------- 5 files changed, 14 insertions(+), 103 deletions(-) delete mode 100644 google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java index 928159aa6dae..25ff2ff30d83 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/BigtableDataSettings.java @@ -30,6 +30,7 @@ import com.google.cloud.bigtable.data.v2.stub.BigtableBatchingCallSettings; import com.google.cloud.bigtable.data.v2.stub.EnhancedBigtableStubSettings; import com.google.cloud.bigtable.data.v2.stub.metrics.MetricsProvider; +import com.google.cloud.bigtable.data.v2.stub.metrics.NoopMetricsProvider; import com.google.common.base.MoreObjects; import com.google.common.base.Strings; import io.grpc.ManagedChannelBuilder; @@ -127,6 +128,7 @@ public static Builder newBuilderForEmulator(String hostname, int port) { .setEndpoint(hostname + ":" + port) // disable channel refreshing when creating an emulator .setRefreshingChannel(false) + .setMetricsProvider(NoopMetricsProvider.INSTANCE) // disable exporting metrics for emulator .setTransportChannelProvider( InstantiatingGrpcChannelProvider.newBuilder() .setMaxInboundMessageSize(256 * 1024 * 1024) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java index 0bc62fc5f390..a8e18f364e55 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/BigtableClientContext.java @@ -73,9 +73,7 @@ public static BigtableClientContext create(EnhancedBigtableStubSettings settings // the OTEL instance and log the exception instead. openTelemetry = getOpenTelemetryFromMetricsProvider( - settings.getMetricsProvider(), - credentials, - settings.getMetricsEndpoint()); + settings.getMetricsProvider(), credentials, settings.getMetricsEndpoint()); } catch (Throwable t) { logger.log(Level.WARNING, "Failed to get OTEL, will skip exporting client side metrics", t); } diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java index fd6d111e4ad9..ae4df858936d 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProvider.java @@ -17,17 +17,11 @@ import com.google.api.core.InternalApi; import com.google.auth.Credentials; -<<<<<<< HEAD -import com.google.auto.value.AutoValue; -import com.google.common.base.MoreObjects; -======= ->>>>>>> main import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.sdk.OpenTelemetrySdk; import io.opentelemetry.sdk.metrics.SdkMeterProvider; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; import java.io.IOException; -import java.util.concurrent.ConcurrentHashMap; import javax.annotation.Nullable; /** @@ -45,8 +39,7 @@ private DefaultMetricsProvider() {} @InternalApi public OpenTelemetry getOpenTelemetry( - @Nullable String metricsEndpoint, @Nullable Credentials credentials) - throws IOException { + @Nullable String metricsEndpoint, @Nullable Credentials credentials) throws IOException { SdkMeterProviderBuilder meterProvider = SdkMeterProvider.builder(); BuiltinMetricsView.registerBuiltinMetrics(credentials, meterProvider, metricsEndpoint); return OpenTelemetrySdk.builder().setMeterProvider(meterProvider.build()).build(); diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java index 495250fe1354..67ac3f24d958 100644 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java +++ b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/EnhancedBigtableStubTest.java @@ -825,15 +825,16 @@ public void testExecuteQueryWaitTimeoutWorksWithMetadataFuture() settings.setStreamWatchdogProvider( InstantiatingWatchdogProvider.create().withCheckInterval(WATCHDOG_CHECK_DURATION)); - EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build()); - ApiFuture future = - stub.executeQueryCallable().call(Statement.of(WAIT_TIME_QUERY)).metadataFuture(); - - ExecutionException e = assertThrows(ExecutionException.class, future::get); - assertThat(e.getCause()).isInstanceOf(WatchdogTimeoutException.class); - assertThat(e.getCause().getMessage()) - .contains("Canceled due to timeout waiting for next response"); - assertThat(e).hasMessageThat().contains("Canceled due to timeout waiting for next response"); + try (EnhancedBigtableStub stub = EnhancedBigtableStub.create(settings.build())) { + ApiFuture future = + stub.executeQueryCallable().call(Statement.of(WAIT_TIME_QUERY)).metadataFuture(); + + ExecutionException e = assertThrows(ExecutionException.class, future::get); + assertThat(e.getCause()).isInstanceOf(WatchdogTimeoutException.class); + assertThat(e.getCause().getMessage()) + .contains("Canceled due to timeout waiting for next response"); + assertThat(e).hasMessageThat().contains("Canceled due to timeout waiting for next response"); + } } private static class MetadataInterceptor implements ServerInterceptor { diff --git a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java b/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java deleted file mode 100644 index 05bbf6db02c2..000000000000 --- a/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/metrics/DefaultMetricsProviderTest.java +++ /dev/null @@ -1,83 +0,0 @@ -/* - * 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 - * - * https://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.bigtable.data.v2.stub.metrics; - -import com.google.auth.Credentials; -import com.google.cloud.NoCredentials; -import com.google.common.collect.ImmutableMap; -import io.opentelemetry.api.OpenTelemetry; -import java.io.IOException; -import java.net.URI; -import java.util.Arrays; -import java.util.List; -import java.util.Map; -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class DefaultMetricsProviderTest { - - @Test - public void test() throws IOException { - DefaultMetricsProvider provider = DefaultMetricsProvider.INSTANCE; - - OpenTelemetry toCompare = - provider.getOpenTelemetry("fake-endpoint:123", NoCredentials.getInstance()); - // different endpoint should return a different instance - OpenTelemetry otel2 = provider.getOpenTelemetry(null, NoCredentials.getInstance()); - // different endpoint and credential should return a different instance - OpenTelemetry otel3 = provider.getOpenTelemetry(null, null); - // same endpoint, different credential, should return a different instance - OpenTelemetry otel4 = provider.getOpenTelemetry("fake-endpoint:123", new FakeCredentials()); - // everything is the same, should return the same instance - OpenTelemetry otel5 = - provider.getOpenTelemetry("fake-endpoint:123", NoCredentials.getInstance()); - - Assert.assertNotEquals(toCompare, otel2); - Assert.assertNotEquals(toCompare, otel3); - Assert.assertNotEquals(toCompare, otel4); - Assert.assertEquals(toCompare, otel5); - - provider.toString(); - } - - private static class FakeCredentials extends Credentials { - @Override - public String getAuthenticationType() { - return "fake"; - } - - @Override - public Map> getRequestMetadata(URI uri) throws IOException { - return ImmutableMap.of("my-header", Arrays.asList("fake-credential")); - } - - @Override - public boolean hasRequestMetadata() { - return true; - } - - @Override - public boolean hasRequestMetadataOnly() { - return true; - } - - @Override - public void refresh() throws IOException {} - } -} From 6cba2ed39c22ac294e9a1d64cae91f5512cb9e9a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Mon, 2 Dec 2024 15:20:09 -0500 Subject: [PATCH 6/6] address comment --- .../bigtable/data/v2/stub/metrics/BigtableExporterUtils.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java index 797f7f1eca9b..821c2295e0d2 100644 --- a/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java +++ b/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/metrics/BigtableExporterUtils.java @@ -124,7 +124,8 @@ static Map> convertToBigtableTimeSeries( for (PointData pd : metricData.getData().getPoints()) { String projectId = getProjectId(pd); - List current = allTimeSeries.getOrDefault(projectId, new ArrayList<>()); + List current = + allTimeSeries.computeIfAbsent(projectId, ignored -> new ArrayList<>()); current.add(convertPointToBigtableTimeSeries(metricData, pd, taskId)); allTimeSeries.put(projectId, current); }