From 8740ec412e4e0bf969dcf4fc334c24c2c787e366 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 15 Jul 2022 11:38:21 -0400 Subject: [PATCH 1/3] fix: enable integration test for graal --- .../cloud/bigtable/stats/StatsWrapper.java | 16 +++++++++++++- ...t.java => ITBuiltinViewConstantsTest.java} | 22 +++++++++---------- .../stats/StatsRecorderWrapperTest.java | 6 ++--- 3 files changed, 28 insertions(+), 16 deletions(-) rename google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/{BuiltinViewConstantsTest.java => ITBuiltinViewConstantsTest.java} (59%) diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java index c30dba6e6c..5aab1c7fed 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java @@ -20,7 +20,10 @@ import com.google.api.core.InternalApi; import com.google.api.gax.tracing.SpanName; import io.opencensus.stats.Stats; +import io.opencensus.stats.View; +import io.opencensus.tags.TagKey; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -31,7 +34,6 @@ */ @InternalApi("For internal use only") public class StatsWrapper { - public static StatsRecorderWrapper createRecorder( OperationType operationType, SpanName spanName, Map statsAttributes) { return new StatsRecorderWrapper( @@ -49,4 +51,16 @@ public static List getOperationLatencyViewTagValueStrings() { .map(x -> x.asString()) .collect(Collectors.toCollection(ArrayList::new)); } + + // A workaround to run BuiltinViewConstantsTest as integration test + static Map> getViewToTagMap() { + Map> map = new HashMap<>(); + for (View view : BuiltinViews.BIGTABLE_BUILTIN_VIEWS) { + List tagKeys = view.getColumns(); + map.put( + view.getName().asString(), + tagKeys.stream().map(tagKey -> tagKey.getName()).collect(Collectors.toList())); + } + return map; + } } diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinViewConstantsTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java similarity index 59% rename from google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinViewConstantsTest.java rename to google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java index a7d20f6da1..d82597ad93 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/BuiltinViewConstantsTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java @@ -17,23 +17,21 @@ import static com.google.common.truth.Truth.assertWithMessage; -import io.opencensus.stats.View; +import java.util.List; +import java.util.Map; import org.junit.Test; -public class BuiltinViewConstantsTest { +public class ITBuiltinViewConstantsTest { @Test public void testBasicTagsExistForAllViews() { - for (View v : BuiltinViews.BIGTABLE_BUILTIN_VIEWS) { - assertWithMessage(v.getName() + " should have all basic tags") - .that(v.getColumns()) + Map> viewToTagMap = StatsWrapper.getViewToTagMap(); + for (String view : viewToTagMap.keySet()) { + System.out.println(view); + System.out.println(viewToTagMap.get(view)); + assertWithMessage(view + " should have all basic tags") + .that(viewToTagMap.get(view)) .containsAtLeast( - BuiltinMeasureConstants.PROJECT_ID, - BuiltinMeasureConstants.INSTANCE_ID, - BuiltinMeasureConstants.APP_PROFILE, - BuiltinMeasureConstants.METHOD, - BuiltinMeasureConstants.ZONE, - BuiltinMeasureConstants.CLUSTER, - BuiltinMeasureConstants.TABLE); + "project_id", "instance_id", "app_profile", "method", "zone", "cluster", "table"); } } } diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java index ed67472623..b78ac95855 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java @@ -60,11 +60,11 @@ public void testStreamingOperation() throws InterruptedException { ApiTracerFactory.OperationType.ServerStreaming, SpanName.of("Bigtable", "ReadRows"), ImmutableMap.of( - BuiltinMeasureConstants.PROJECT_ID.getName(), + "project_id", PROJECT_ID, - BuiltinMeasureConstants.INSTANCE_ID.getName(), + "instance_id", INSTANCE_ID, - BuiltinMeasureConstants.APP_PROFILE.getName(), + "app_profile", APP_PROFILE_ID), statsComponent.getStatsRecorder()); From eee9cd67917b45aa5fb3c2c866f0cf0ae41eedf6 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 15 Jul 2022 11:47:23 -0400 Subject: [PATCH 2/3] update --- .../bigtable/stats/ITBuiltinViewConstantsTest.java | 3 +++ .../cloud/bigtable/stats/StatsRecorderWrapperTest.java | 10 +++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java index d82597ad93..8f7faa4949 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java @@ -20,7 +20,10 @@ import java.util.List; import java.util.Map; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +@RunWith(JUnit4.class) public class ITBuiltinViewConstantsTest { @Test public void testBasicTagsExistForAllViews() { diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java index b78ac95855..11cbaf360c 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java @@ -34,7 +34,11 @@ import java.util.Objects; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +// Can only be run as unit test +@RunWith(JUnit4.class) public class StatsRecorderWrapperTest { private final String PROJECT_ID = "fake-project"; @@ -60,11 +64,11 @@ public void testStreamingOperation() throws InterruptedException { ApiTracerFactory.OperationType.ServerStreaming, SpanName.of("Bigtable", "ReadRows"), ImmutableMap.of( - "project_id", + BuiltinMeasureConstants.PROJECT_ID.getName(), PROJECT_ID, - "instance_id", + BuiltinMeasureConstants.INSTANCE_ID.getName(), INSTANCE_ID, - "app_profile", + BuiltinMeasureConstants.APP_PROFILE.getName(), APP_PROFILE_ID), statsComponent.getStatsRecorder()); From 5ffc68c214df47e7cd33bbf8c98d9a2e7f722b01 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 15 Jul 2022 14:33:48 -0400 Subject: [PATCH 3/3] add more comments --- .../java/com/google/cloud/bigtable/stats/StatsWrapper.java | 5 ++++- .../cloud/bigtable/stats/ITBuiltinViewConstantsTest.java | 2 -- .../cloud/bigtable/stats/StatsRecorderWrapperTest.java | 5 ++++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java index 5aab1c7fed..401a1cf975 100644 --- a/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java +++ b/google-cloud-bigtable-stats/src/main/java/com/google/cloud/bigtable/stats/StatsWrapper.java @@ -52,7 +52,10 @@ public static List getOperationLatencyViewTagValueStrings() { .collect(Collectors.toCollection(ArrayList::new)); } - // A workaround to run BuiltinViewConstantsTest as integration test + // A workaround to run ITBuiltinViewConstantsTest as integration test. Integration test runs after + // the packaging step. Opencensus classes will be relocated when they are packaged but the + // integration test files will not be. So the integration tests can't reference any transitive + // dependencies that have been relocated. static Map> getViewToTagMap() { Map> map = new HashMap<>(); for (View view : BuiltinViews.BIGTABLE_BUILTIN_VIEWS) { diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java index 8f7faa4949..9b486f919f 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/ITBuiltinViewConstantsTest.java @@ -29,8 +29,6 @@ public class ITBuiltinViewConstantsTest { public void testBasicTagsExistForAllViews() { Map> viewToTagMap = StatsWrapper.getViewToTagMap(); for (String view : viewToTagMap.keySet()) { - System.out.println(view); - System.out.println(viewToTagMap.get(view)); assertWithMessage(view + " should have all basic tags") .that(viewToTagMap.get(view)) .containsAtLeast( diff --git a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java index 11cbaf360c..abf00e71b3 100644 --- a/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java +++ b/google-cloud-bigtable-stats/src/test/java/com/google/cloud/bigtable/stats/StatsRecorderWrapperTest.java @@ -37,7 +37,10 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -// Can only be run as unit test +// Can only be run as a unit test. Opencensus classes will be relocated when they are packaged but +// the integration test files will not be. So the integration tests can't reference any transitive +// dependencies that have been relocated. To work around this, we'll have to move all the reference +// to opencensus to StatsWrapper. @RunWith(JUnit4.class) public class StatsRecorderWrapperTest {