Merge remote-tracking branch 'origin/main' into observability/tracing/attribute-coverage

Author: diegomarquezp

.github/workflows/ci.yaml

@@ -20,8 +20,33 @@ on:
   pull_request:
 name: ci
 jobs:
+  # detect whether or note we should run "bulk" (non-handwritten) unit tests
+  bulk-filter:
+    runs-on: ubuntu-latest
+    permissions:
+      pull-requests: read
+    # Set job outputs to values from filter step
+    outputs:
+      src: ${{ steps.filter.outputs.src }}
+      ci: ${{ steps.filter.outputs.ci }}
+    steps:
+    - uses: actions/checkout@v4
+    - uses: dorny/paths-filter@v3
+      id: filter
+      with:
+        # we want to run tests if source code is changed or the scripts
+        # used to run the unit tests
+        filters: |
+          src:
+            - '**/*.java'
+            - '**/pom.xml'
+          ci:
+            - '.github/workflows/ci.yaml'
+            - '.kokoro/**'
+  # these unit tests are "bulk" (non-handwritten) libraries
   units:
     runs-on: ubuntu-latest
+    needs: bulk-filter
     strategy:
       fail-fast: false
       matrix:
@@ -42,11 +67,13 @@ jobs:
         path: ~/.m2/repository
         key: ${{ runner.os }}-maven-unified-${{ steps.date.outputs.week_of_year }}
     - run: .kokoro/build.sh
+      if: ${{ needs.bulk-filter.outputs.src == 'true' || needs.bulk-filter.outputs.ci == 'true' }}
       env:
         JOB_TYPE: test
         JOB_NAME: units-${{matrix.java}}
   units-8-runtime:
     runs-on: ubuntu-latest
+    needs: bulk-filter
     name: "units (8)"
     steps:
     - name: Get current week within the year
@@ -73,6 +100,7 @@ jobs:
         path: ~/.m2/repository
         key: ${{ runner.os }}-maven-unified-${{ steps.date.outputs.week_of_year }}
     - run: .kokoro/build.sh
+      if: ${{ needs.bulk-filter.outputs.src == 'true' || needs.bulk-filter.outputs.ci == 'true' }}
       shell: bash      
       env:
         JOB_TYPE: test

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/telemetry/BigQueryTelemetryTracer.java

@@ -32,9 +32,8 @@ public final class BigQueryTelemetryTracer {
   private BigQueryTelemetryTracer() {}
 
   public static final String BQ_GCP_CLIENT_SERVICE = "bigquery";
-  public static final String BQ_GCP_CLIENT_REPO = "googleapis/java-bigquery";
+  public static final String BQ_GCP_CLIENT_REPO = "googleapis/google-cloud-java";
   public static final String BQ_GCP_CLIENT_ARTIFACT = "google-cloud-bigquery";
-  public static final String BQ_GCP_CLIENT_LANGUAGE = "java";
 
   // TODO: migrate to use gax attributes keys
   // https://github.com/googleapis/google-cloud-java/issues/12099
@@ -47,8 +46,6 @@ private BigQueryTelemetryTracer() {}
       AttributeKey.stringKey("gcp.client.repo");
   public static final AttributeKey<String> GCP_CLIENT_ARTIFACT =
       AttributeKey.stringKey("gcp.client.artifact");
-  public static final AttributeKey<String> GCP_CLIENT_LANGUAGE =
-      AttributeKey.stringKey("gcp.client.language");
   public static final AttributeKey<String> GCP_RESOURCE_DESTINATION_ID =
       AttributeKey.stringKey("gcp.resource.destination.id");
   public static final AttributeKey<String> RPC_SYSTEM_NAME =
@@ -61,18 +58,13 @@ private BigQueryTelemetryTracer() {}
   public static final AttributeKey<String> STATUS_MESSAGE =
       AttributeKey.stringKey("status.message");
 
-  // Common Server Attributes
-  public static final AttributeKey<String> SERVER_ADDRESS =
-      AttributeKey.stringKey("server.address");
-  public static final AttributeKey<Long> SERVER_PORT = AttributeKey.longKey("server.port");
   public static final AttributeKey<String> URL_TEMPLATE = AttributeKey.stringKey("url.template");
   public static final AttributeKey<String> URL_DOMAIN = AttributeKey.stringKey("url.domain");
 
   public static void addCommonAttributeToSpan(Span span) {
     span.setAttribute(GCP_CLIENT_SERVICE, BQ_GCP_CLIENT_SERVICE)
         .setAttribute(GCP_CLIENT_REPO, BQ_GCP_CLIENT_REPO)
         .setAttribute(GCP_CLIENT_ARTIFACT, BQ_GCP_CLIENT_ARTIFACT)
-        .setAttribute(GCP_CLIENT_LANGUAGE, BQ_GCP_CLIENT_LANGUAGE)
         .setAttribute(GCP_CLIENT_VERSION, Version.VERSION);
   }
 

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/telemetry/HttpTracingRequestInitializer.java

@@ -51,6 +51,9 @@ public class HttpTracingRequestInitializer implements HttpRequestInitializer {
       AttributeKey.longKey("http.request.body.size");
   public static final AttributeKey<Long> HTTP_RESPONSE_BODY_SIZE =
       AttributeKey.longKey("http.response.body.size");
+  public static final AttributeKey<String> SERVER_ADDRESS =
+      AttributeKey.stringKey("server.address");
+  public static final AttributeKey<Long> SERVER_PORT = AttributeKey.longKey("server.port");
 
   @VisibleForTesting public static final String HTTP_RPC_SYSTEM_NAME = "http";
 
@@ -119,10 +122,10 @@ private void addInitialHttpAttributesToSpan(Span span, HttpRequest request) {
     BigQueryTelemetryTracer.addCommonAttributeToSpan(span);
     span.setAttribute(BigQueryTelemetryTracer.RPC_SYSTEM_NAME, HTTP_RPC_SYSTEM_NAME);
     String host = request.getUrl().getHost();
-    span.setAttribute(BigQueryTelemetryTracer.SERVER_ADDRESS, host);
+    span.setAttribute(SERVER_ADDRESS, host);
     int port = request.getUrl().getPort();
     if (port > 0) {
-      span.setAttribute(BigQueryTelemetryTracer.SERVER_PORT, (long) port);
+      span.setAttribute(SERVER_PORT, (long) port);
     }
     span.setAttribute(URL_FULL, getSanitizedUrl(request));
   }

java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/it/ITOpenTelemetryTest.java

@@ -19,6 +19,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -30,11 +31,13 @@
 import com.google.cloud.bigquery.testing.RemoteBigQueryHelper;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.common.AttributeKey;
+import io.opentelemetry.api.trace.SpanKind;
 import io.opentelemetry.api.trace.Tracer;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
 import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.sdk.trace.data.StatusData;
 import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
 import java.io.IOException;
 import java.util.List;
@@ -86,12 +89,15 @@ public void testListDatasetsTraced() {
     for (SpanData span : spans) {
       if (span.getName().equals("com.google.cloud.bigquery.BigQueryRpc.listDatasets")) {
         foundRpcSpan = true;
+        assertEquals(SpanKind.CLIENT, span.getKind());
+        assertEquals(StatusData.unset(), span.getStatus());
         Map<AttributeKey<?>, Object> attrs = span.getAttributes().asMap();
         checkGeneralAttributes(attrs);
         assertEquals("GET", attrs.get(HttpTracingRequestInitializer.HTTP_REQUEST_METHOD));
         assertEquals("DatasetService", attrs.get(AttributeKey.stringKey("bq.rpc.service")));
         assertEquals("ListDatasets", attrs.get(AttributeKey.stringKey("bq.rpc.method")));
-        assertEquals("bigquery.googleapis.com", attrs.get(BigQueryTelemetryTracer.SERVER_ADDRESS));
+        assertEquals(
+            "bigquery.googleapis.com", attrs.get(HttpTracingRequestInitializer.SERVER_ADDRESS));
         assertEquals(200L, attrs.get(HttpTracingRequestInitializer.HTTP_RESPONSE_STATUS_CODE));
         assertEquals("bigquery.googleapis.com", attrs.get(BigQueryTelemetryTracer.URL_DOMAIN));
         assertEquals(
@@ -102,6 +108,10 @@ public void testListDatasetsTraced() {
             attrs.get(BigQueryTelemetryTracer.GCP_RESOURCE_DESTINATION_ID));
         assertEquals(
             "projects/{+projectId}/datasets", attrs.get(BigQueryTelemetryTracer.URL_TEMPLATE));
+        assertNull(attrs.get(BigQueryTelemetryTracer.STATUS_MESSAGE));
+        assertNull(attrs.get(BigQueryTelemetryTracer.ERROR_TYPE));
+        assertNull(attrs.get(BigQueryTelemetryTracer.EXCEPTION_TYPE));
+        assertNull(attrs.get(HttpTracingRequestInitializer.HTTP_REQUEST_RESEND_COUNT));
       }
     }
     assertTrue(foundRpcSpan, "Expected to find BigQueryRpc.listDatasets span");
@@ -126,6 +136,8 @@ public void testGetDatasetNotFoundTraced() {
     for (SpanData span : spans) {
       if (span.getName().equals("com.google.cloud.bigquery.BigQueryRpc.getDataset")) {
         foundRpcSpan = true;
+        assertEquals(SpanKind.CLIENT, span.getKind());
+        assertEquals(StatusData.error(), span.getStatus());
         Map<AttributeKey<?>, Object> attrs = span.getAttributes().asMap();
         checkGeneralAttributes(attrs);
         assertEquals("GET", attrs.get(HttpTracingRequestInitializer.HTTP_REQUEST_METHOD));
@@ -138,7 +150,8 @@ public void testGetDatasetNotFoundTraced() {
         assertEquals(
             "https://bigquery.googleapis.com/bigquery/v2/projects/gcloud-devel/datasets/non_existent_dataset?prettyPrint=false",
             attrs.get(HttpTracingRequestInitializer.URL_FULL));
-        assertEquals("bigquery.googleapis.com", attrs.get(BigQueryTelemetryTracer.SERVER_ADDRESS));
+        assertEquals(
+            "bigquery.googleapis.com", attrs.get(HttpTracingRequestInitializer.SERVER_ADDRESS));
         assertEquals("bigquery.googleapis.com", attrs.get(BigQueryTelemetryTracer.URL_DOMAIN));
         assertEquals(
             "//bigquery.googleapis.com/projects/gcloud-devel/datasets/non_existent_dataset",
@@ -155,7 +168,7 @@ public void testGetDatasetNotFoundTraced() {
   }
 
   @Test
-  public void testConnectionErrorRetriesTraced() {
+  public void testClientErrorAndRetriesTraced() {
     // Pass invalid host to force connection error and retries
     BigQuery bq =
         bigqueryHelper.getOptions().toBuilder()
@@ -181,14 +194,16 @@ public void testConnectionErrorRetriesTraced() {
     for (SpanData span : spans) {
       if (span.getName().equals("com.google.cloud.bigquery.BigQueryRpc.listDatasets")) {
         rpcSpanCount++;
+        assertEquals(SpanKind.CLIENT, span.getKind());
+        assertEquals(StatusData.error().getStatusCode(), span.getStatus().getStatusCode());
         Map<AttributeKey<?>, Object> attrs = span.getAttributes().asMap();
         checkGeneralAttributes(attrs);
         assertEquals(
             "https://invalid-host-name-12345.com:8080/bigquery/v2/projects/gcloud-devel/datasets?prettyPrint=false",
             (String) attrs.get(HttpTracingRequestInitializer.URL_FULL));
         assertEquals(
-            "invalid-host-name-12345.com", attrs.get(BigQueryTelemetryTracer.SERVER_ADDRESS));
-        assertEquals(8080L, attrs.get(BigQueryTelemetryTracer.SERVER_PORT));
+            "invalid-host-name-12345.com", attrs.get(HttpTracingRequestInitializer.SERVER_ADDRESS));
+        assertEquals(8080L, attrs.get(HttpTracingRequestInitializer.SERVER_PORT));
         assertEquals("invalid-host-name-12345.com", attrs.get(BigQueryTelemetryTracer.URL_DOMAIN));
         assertEquals(
             "projects/{+projectId}/datasets", attrs.get(BigQueryTelemetryTracer.URL_TEMPLATE));
@@ -254,11 +269,26 @@ public void testSimultaneousCallsDoNotAffectResendCountForEachother() {
     assertEquals(5, cancelJobSpanCount, "Expected 5 attempts total for cancelJob call");
   }
 
+  @Test
+  public void testTracingDisabledNoSpansCollected() {
+    BigQuery bq =
+        bigqueryHelper.getOptions().toBuilder()
+            .setEnableOpenTelemetryTracing(false)
+            .setOpenTelemetryTracer(tracer)
+            .build()
+            .getService();
+
+    bq.listDatasets();
+
+    List<SpanData> spans = memoryExporter.getFinishedSpanItems();
+    assertTrue(spans.isEmpty(), "Expected no spans to be collected when tracing is disabled");
+  }
+
   private static void checkRetryAttribute(SpanData span, int listDataSpanCount) {
     Map<AttributeKey<?>, Object> attrs = span.getAttributes().asMap();
     Long resendCount = (Long) attrs.get(HttpTracingRequestInitializer.HTTP_REQUEST_RESEND_COUNT);
     if (listDataSpanCount == 1) {
-      assertTrue(resendCount == null || resendCount == 0);
+      assertNull(resendCount, "Expected no resend count for first attempt");
     } else {
       assertNotNull(resendCount, "Expected resend count for retry attempt " + listDataSpanCount);
       assertEquals((long) (listDataSpanCount - 1), resendCount.longValue());
@@ -275,9 +305,6 @@ private void checkGeneralAttributes(Map<AttributeKey<?>, Object> attrs) {
     assertEquals(
         BigQueryTelemetryTracer.BQ_GCP_CLIENT_REPO,
         attrs.get(BigQueryTelemetryTracer.GCP_CLIENT_REPO));
-    assertEquals(
-        BigQueryTelemetryTracer.BQ_GCP_CLIENT_LANGUAGE,
-        attrs.get(BigQueryTelemetryTracer.GCP_CLIENT_LANGUAGE));
     assertEquals(
         BigQueryTelemetryTracer.BQ_GCP_CLIENT_ARTIFACT,
         attrs.get(BigQueryTelemetryTracer.GCP_CLIENT_ARTIFACT));

java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/telemetry/BigQueryTelemetryTracerTest.java

@@ -181,9 +181,6 @@ public void testAddCommonAttributeToSpan() {
     assertEquals(
         BigQueryTelemetryTracer.BQ_GCP_CLIENT_ARTIFACT,
         spanData.getAttributes().get(BigQueryTelemetryTracer.GCP_CLIENT_ARTIFACT));
-    assertEquals(
-        BigQueryTelemetryTracer.BQ_GCP_CLIENT_LANGUAGE,
-        spanData.getAttributes().get(BigQueryTelemetryTracer.GCP_CLIENT_LANGUAGE));
     assertEquals(
         Version.VERSION, spanData.getAttributes().get(BigQueryTelemetryTracer.GCP_CLIENT_VERSION));
   }

java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/telemetry/HttpTracingRequestInitializerTest.java

@@ -435,8 +435,9 @@ private void closeAndVerifySpanData(
     assertEquals(1, spans.size());
     SpanData span = spans.get(0);
     assertEquals(SPAN_NAME, span.getName());
-    assertEquals(BIGQUERY_DOMAIN, span.getAttributes().get(BigQueryTelemetryTracer.SERVER_ADDRESS));
-    assertEquals(443, span.getAttributes().get(BigQueryTelemetryTracer.SERVER_PORT));
+    assertEquals(
+        BIGQUERY_DOMAIN, span.getAttributes().get(HttpTracingRequestInitializer.SERVER_ADDRESS));
+    assertEquals(443, span.getAttributes().get(HttpTracingRequestInitializer.SERVER_PORT));
     assertEquals(
         BigQueryTelemetryTracer.BQ_GCP_CLIENT_SERVICE,
         span.getAttributes().get(BigQueryTelemetryTracer.GCP_CLIENT_SERVICE));
@@ -446,9 +447,6 @@ private void closeAndVerifySpanData(
     assertEquals(
         BigQueryTelemetryTracer.BQ_GCP_CLIENT_ARTIFACT,
         span.getAttributes().get(BigQueryTelemetryTracer.GCP_CLIENT_ARTIFACT));
-    assertEquals(
-        BigQueryTelemetryTracer.BQ_GCP_CLIENT_LANGUAGE,
-        span.getAttributes().get(BigQueryTelemetryTracer.GCP_CLIENT_LANGUAGE));
     assertEquals(
         HttpTracingRequestInitializer.HTTP_RPC_SYSTEM_NAME,
         span.getAttributes().get(BigQueryTelemetryTracer.RPC_SYSTEM_NAME));

release-please-config.json

@@ -8,6 +8,16 @@
   "packages": {
     ".": {
       "extra-files": [
+        {
+					"type": "generic",
+					"path": "java-*/**/Version.java",
+					"glob": true
+				},
+        {
+					"type": "generic",
+					"path": "sdk-platform-java/java-showcase*/Version.java",
+					"glob": true
+				},
         ".github/workflows/generated_files_sync.yaml",
         ".github/workflows/hermetic_library_generation.yaml",
         "sdk-platform-java/WORKSPACE",

sdk-platform-java/DEVELOPMENT.md

@@ -0,0 +1,66 @@
+# Development Setup
+
+You need Java 11 or higher to run the build. The build produces Java 8-compatible
+bytecode.
+
+Install [`bazelisk`](https://github.com/bazelbuild/bazelisk) in your `PATH`
+for gapic-generator-java's Bazel build.
+
+## Set Up
+
+1. Clone this repo.
+
+2. (OPTIONAL) Copy the Git pre-commit hooks. This will automatically check the build, run
+   tests, and perform linting before each commit. (Symlinks don't seem to work,
+   but if you find a way, please add it here!)
+
+    ```sh
+    cp .githooks/pre-commit .git/hooks/pre-commit
+    ```
+
+## Run Tests for All Modules
+
+- Run all unit tests in all modules.
+
+    ```sh
+    mvn install
+    ```
+
+## Code Formatting
+
+- Run linter checks without actually doing the formatting.
+
+    ```sh
+    mvn fmt:check
+    ```
+
+- Format files.
+
+    ```sh
+    mvn fmt:format
+    ```
+
+## Testing guide
+There are 4 layers of testing in this repo:
+1. Traditional unit tests. These tests usually focus on testing one very specific test case at a time. For example, [MethodTest](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/src/test/java/com/google/api/generator/gapic/model/MethodTest.java) 
+2. Golden unit tests. These tests start with a test proto, we create different test cases in the proto, load the proto in unit tests and save the generated file as golden files. For example, [routing header proto](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/src/test/proto/explicit_dynamic_routing_header_testing.proto) -> Load in the [unit test](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposerTest.java#L56-L64) -> Saved as [golden](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/goldens/GrpcRoutingHeadersStub.golden)    
+3. Showcase integration tests. They test the generated library's behavior against a mock server, see [README.md](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/java-showcase/README.md) of showcase module for details.
+4. Golden integration tests. These tests generate a full client library with real protos from googleapis, for example, golden integration tests for [compute](https://github.com/googleapis/sdk-platform-java/tree/main/test/integration/goldens/compute). However, we _should_ stop adding new ones, and rely on golden unit tests and showcase tests.
+
+Based on where the code changes are, we should add different tests, in general
+
+- If the changes are in `gax` or `api-common` only, you _must_ add traditional unit tests, you _may_ add showcase integration tests if you are modifying a public setting.
+- If the changes are in `gapic-generator-java` only, showcase integration tests are most likely not needed
+  - If they are in the `composer` module, you _must_ add golden unit tests, you _may_ add traditional unit tests for better coverage and easier testing.  
+  - If they are in the `parser` module, you _should_ add traditional unit tests with a test proto if possible, you _may_ add golden unit tests to easily see the changes. For example, [routing_rule_parser_testing](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/src/test/proto/routing_rule_parser_testing.proto) that is only used for testing [RoutingRuleParser](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/RoutingRuleParser.java). 
+  - If they are in `other modules(ast, model, writer etc.)`, you _must_ add traditional unit tests, you _may_ add golden unit tests to easily see the changes.
+- If the changes are in both `gax` and `gapic-generator-java`, you _must_ add all test layers, including traditional unit tests, golden unit tests and showcase integration tests. 
+
+### How to run tests?
+
+See the [Unit Tests](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/DEVELOPMENT.md#unit-tests) section in gapic-generator-java/DEVELOPMENT.md for how to run traditional and golden unit tests.
+
+See [Running the Integration Tests](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/java-showcase/README.md#running-the-integration-tests) and [Update the Golden Showcase Files](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/java-showcase/README.md#update-the-golden-showcase-files) sections in java-showcase/README.md for how to run and update showcase integration tests.
+
+See the [Integration Tests](https://github.com/googleapis/google-cloud-java/blob/main/sdk-platform-java/gapic-generator-java/DEVELOPMENT.md#integration-tests) section in gapic-generator-java/DEVELOPMENT.md for how to run golden integration tests.
+