naming/comments/extra white space clean up

Author: ldetmer

java-bigquery/google-cloud-bigquery/src/main/java/com/google/cloud/bigquery/spi/v2/HttpBigQueryRpc.java

@@ -1788,7 +1788,6 @@ private static Attributes otelAttributesFromOptions(Map<Option, ?> options) {
     return builder.build();
   }
 
-  @InternalApi("Visible for testing")
   /* Temporary development gate for HttpTracingRequestInitializer rollout:
   must be explicitly enabled with the system property
    */

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

@@ -21,10 +21,7 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.trace.Span;
 
-/**
- * General BigQuery Telemetry class that stores generic telemetry attributes and any associated
- * logic to calculate.
- */
+/** BigQuery Telemetry class that stores generic telemetry attributes and values */
 @BetaApi
 @InternalApi
 public final class BigQueryTelemetryTracer {

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

@@ -63,22 +63,17 @@ public void initialize(HttpRequest request) throws IOException {
     if (delegate != null) {
       delegate.initialize(request);
     }
-
     if (tracer == null) {
       return;
     }
-
     // Get the current active span (created by HttpBigQueryRpc) and add HTTP attributes to it
     Span span = Span.current();
     if (!span.getSpanContext().isValid()) {
       // No active span to exists, skip instrumentation
       return;
     }
-
     String host = request.getUrl().getHost();
     Integer port = request.getUrl().getPort();
-
-    // Add initial HTTP attributes to the existing span
     addInitialHttpAttributesToSpan(span, host, port);
   }
 

java-bigquery/google-cloud-bigquery/src/test/java/com/google/cloud/bigquery/spi/v2/HttpBigQueryRpcTest.java

@@ -908,9 +908,7 @@ public void testOtelAttributesFromOptionsGetAddedtoSpan() throws Exception {
     }
 
     @Test
-    public void testHttpTracingEnabled() throws Exception {
-      String originalProperty =
-          System.getProperty("com.google.cloud.bigquery.http.tracing.dev.enabled");
+    public void testHttpTracingEnabledAddsAdditionalAttributes() throws Exception {
       try {
         System.setProperty("com.google.cloud.bigquery.http.tracing.dev.enabled", "true");
         HttpBigQueryRpc customRpc = createRpc(true);
@@ -949,19 +947,12 @@ public void testHttpTracingEnabled() throws Exception {
             "http", rpcSpan.getAttributes().get(AttributeKey.stringKey("rpc.system.name")));
         assertNotNull(rpcSpan.getAttributes().get(AttributeKey.stringKey("server.address")));
       } finally {
-        if (originalProperty != null) {
-          System.setProperty(
-              "com.google.cloud.bigquery.http.tracing.dev.enabled", originalProperty);
-        } else {
-          System.clearProperty("com.google.cloud.bigquery.http.tracing.dev.enabled");
-        }
+        System.clearProperty("com.google.cloud.bigquery.http.tracing.dev.enabled");
       }
     }
 
     @Test
-    public void testHttpTracingDisabled() throws Exception {
-      String originalProperty =
-          System.getProperty("com.google.cloud.bigquery.http.tracing.dev.enabled");
+    public void testHttpTracingDisabledDoesNotAddAdditionalAttributes() throws Exception {
       try {
         System.setProperty("com.google.cloud.bigquery.http.tracing.dev.enabled", "false");
         HttpBigQueryRpc customRpc = createRpc(true);
@@ -999,12 +990,7 @@ public void testHttpTracingDisabled() throws Exception {
         assertThat(rpcSpan.getAttributes().get(AttributeKey.stringKey("rpc.system.name"))).isNull();
         assertThat(rpcSpan.getAttributes().get(AttributeKey.stringKey("server.address"))).isNull();
       } finally {
-        if (originalProperty != null) {
-          System.setProperty(
-              "com.google.cloud.bigquery.http.tracing.dev.enabled", originalProperty);
-        } else {
-          System.clearProperty("com.google.cloud.bigquery.http.tracing.dev.enabled");
-        }
+        System.clearProperty("com.google.cloud.bigquery.http.tracing.dev.enabled");
       }
     }
   }

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

@@ -17,6 +17,7 @@
 package com.google.cloud.bigquery.telemetry;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -47,7 +48,6 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-/** Unit tests for TracingHttpRequestInitializer */
 public class HttpTracingRequestInitializerTest {
 
   private static final String BASE_URL =
@@ -72,12 +72,12 @@ public void setUp() {
         OpenTelemetrySdk.builder().setTracerProvider(tracerProvider).build();
     tracer = openTelemetry.getTracer("test-tracer");
     initializer = new HttpTracingRequestInitializer(null, tracer);
+    parentSpan = tracer.spanBuilder(SPAN_NAME).startSpan();
+    spanScope = parentSpan.makeCurrent();
   }
 
   @Test
   public void testRequestAttributesAreSetIfSpanExists() throws IOException {
-    parentSpan = tracer.spanBuilder(SPAN_NAME).startSpan();
-    spanScope = parentSpan.makeCurrent();
     HttpTransport transport = createTransport();
     HttpRequest request = buildGetRequest(transport, initializer, BASE_URL);
 
@@ -97,9 +97,7 @@ public void testRequestAttributesAreSetIfSpanExists() throws IOException {
 
   @Test
   public void testExistingParentAttributesAreNotAffectedByRequestAttributes() throws IOException {
-    parentSpan =
-        tracer.spanBuilder(SPAN_NAME).setAttribute("parent_attribute", "value").startSpan();
-    spanScope = parentSpan.makeCurrent();
+    parentSpan.setAttribute("parent_attribute", "value");
     HttpTransport transport = createTransport();
     HttpRequest request = buildGetRequest(transport, initializer, BASE_URL);
 
@@ -124,13 +122,18 @@ public void testNoSpanIsCreatedIfNoActiveSpan() throws IOException {
         new HttpTracingRequestInitializer(delegateInitializer, tracer);
 
     HttpTransport transport = createTransport();
+    // close span before building the request so there is no current span during initialization
+    spanScope.close();
+    parentSpan.end();
+
     HttpRequest request = buildGetRequest(transport, tracingInitializer, BASE_URL);
 
     HttpResponse response = request.execute();
     response.disconnect();
 
     List<SpanData> spans = spanExporter.getFinishedSpanItems();
-    assertEquals(0, spans.size());
+    assertEquals(1, spans.size());
+    assertNull(spans.get(0).getAttributes().get(BigQueryTelemetryTracer.GCP_CLIENT_ARTIFACT));
     verify(delegateInitializer, times(1)).initialize(any(HttpRequest.class));
   }