From f6486a83328c401c1c7f63f69df25015768de6f6 Mon Sep 17 00:00:00 2001 From: surbhigarg92 Date: Tue, 16 Jan 2024 11:19:09 +0530 Subject: [PATCH] test --- .../com/google/cloud/spanner/SessionPool.java | 9 ---- .../google/cloud/spanner/SpannerOptions.java | 54 ++++++++++++------- .../google/cloud/spanner/TraceWrapper.java | 19 ++----- .../FailOnOverkillTraceComponentImpl.java | 10 ++++ .../cloud/spanner/OpenTelemetrySpanTest.java | 11 ++-- .../spanner/ResumableStreamIteratorTest.java | 5 +- .../RetryOnInvalidatedSessionTest.java | 2 - .../google/cloud/spanner/SessionPoolTest.java | 2 - .../com/google/cloud/spanner/SpanTest.java | 2 + .../spanner/spi/v1/SpannerRpcMetricsTest.java | 1 - 10 files changed, 61 insertions(+), 54 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java index da91658456b..cc24dd2ba0f 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java @@ -66,7 +66,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.ForwardingListenableFuture; import com.google.common.util.concurrent.ForwardingListenableFuture.SimpleForwardingListenableFuture; import com.google.common.util.concurrent.ListenableFuture; @@ -81,8 +80,6 @@ import io.opencensus.metrics.MetricOptions; import io.opencensus.metrics.MetricRegistry; import io.opencensus.metrics.Metrics; -import io.opencensus.trace.Annotation; -import io.opencensus.trace.AttributeValue; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; @@ -2450,12 +2447,6 @@ PooledSessionFuture replaceSession(SessionNotFoundException e, PooledSessionFutu } } - private Annotation sessionAnnotation(Session session) { - AttributeValue sessionId = AttributeValue.stringAttributeValue(session.getName()); - return Annotation.fromDescriptionAndAttributes( - "Using Session", ImmutableMap.of("sessionId", sessionId)); - } - private void incrementNumSessionsInUse() { synchronized (lock) { if (maxSessionsInUse < ++numSessionsInUse) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 072c86931ec..bb949efd36d 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -78,13 +78,13 @@ import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; import org.threeten.bp.Duration; /** Options for the Cloud Spanner service. */ public class SpannerOptions extends ServiceOptions { private static final long serialVersionUID = 2789571558532701170L; private static SpannerEnvironment environment = SpannerEnvironmentImpl.INSTANCE; - private static boolean enableOpenTelemetryTraces = false; private static boolean enableOpenCensusMetrics = true; private static boolean enableOpenTelemetryMetrics = false; @@ -153,6 +153,9 @@ enum TracingFramework { OPEN_TELEMETRY } + private static final Object lock = new Object(); + + @GuardedBy("lock") private static TracingFramework activeTracingFramework; /** Interface that can be used to provide {@link CallCredentials} to {@link SpannerOptions}. */ @@ -1255,7 +1258,10 @@ public Builder setEmulatorHost(String emulatorHost) { return this; } - /** Sets opentelemetry object. */ + /** + * Sets OpenTelemetry object to be used for Spanner Metrics and Traces. GlobalOpenTelemetry will + * be used as fallback if this options is not set. + */ public Builder setOpenTelemetry(OpenTelemetry openTelemetry) { this.openTelemetry = openTelemetry; return this; @@ -1304,8 +1310,11 @@ public SpannerOptions build() { this.numChannels = this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS; } - if (activeTracingFramework == null) { - activeTracingFramework = TracingFramework.OPEN_CENSUS; + + synchronized (lock) { + if (activeTracingFramework == null) { + activeTracingFramework = TracingFramework.OPEN_CENSUS; + } } return new SpannerOptions(this); } @@ -1341,23 +1350,28 @@ public static void useDefaultEnvironment() { * default, OpenCensus traces are enabled. */ public static void enableOpenTelemetryTraces() { - if (activeTracingFramework != null - && activeTracingFramework != TracingFramework.OPEN_TELEMETRY) { - throw new IllegalStateException( - "ActiveTracingFramework is set to OpenCensus. Cannot be reset."); + synchronized (lock) { + if (activeTracingFramework != null + && activeTracingFramework != TracingFramework.OPEN_TELEMETRY) { + throw new IllegalStateException( + "ActiveTracingFramework is set to OpenCensus and cannot be reset after SpannerOptions object is created."); + } + activeTracingFramework = TracingFramework.OPEN_TELEMETRY; } - activeTracingFramework = TracingFramework.OPEN_TELEMETRY; } /** Enables OpenCensus traces. Enabling OpenCensus traces will disable OpenTelemetry traces. */ @ObsoleteApi( "The OpenCensus project is deprecated. Use enableOpenTelemetryTraces to switch to OpenTelemetry traces") public static void enableOpenCensusTraces() { - if (activeTracingFramework != null && activeTracingFramework != TracingFramework.OPEN_CENSUS) { - throw new IllegalStateException( - "ActiveTracingFramework is set to OpenTelemetry. Cannot be reset."); + synchronized (lock) { + if (activeTracingFramework != null + && activeTracingFramework != TracingFramework.OPEN_CENSUS) { + throw new IllegalStateException( + "ActiveTracingFramework is set to OpenTelemetry and cannot be reset after SpannerOptions object is created."); + } + activeTracingFramework = TracingFramework.OPEN_CENSUS; } - activeTracingFramework = TracingFramework.OPEN_CENSUS; } @ObsoleteApi( @@ -1371,9 +1385,15 @@ static void resetActiveTracingFramework() { } public static TracingFramework getActiveTracingFramework() { - return activeTracingFramework; + synchronized (lock) { + if (activeTracingFramework == null) { + return TracingFramework.OPEN_CENSUS; + } + return activeTracingFramework; + } } + /** Disables OpenCensus metrics. Disable OpenCensus metrics before creating Spanner client. */ public static void disableOpenCensusMetrics() { SpannerOptions.enableOpenCensusMetrics = false; } @@ -1387,6 +1407,7 @@ public static boolean isEnabledOpenCensusMetrics() { return SpannerOptions.enableOpenCensusMetrics; } + /** Enables OpenTelemetry metrics. Enable OpenTelemetry metrics before creating Spanner client. */ public static void enableOpenTelemetryMetrics() { SpannerOptions.enableOpenTelemetryMetrics = true; } @@ -1395,11 +1416,6 @@ public static boolean isEnabledOpenTelemetryMetrics() { return SpannerOptions.enableOpenTelemetryMetrics; } - @VisibleForTesting - public static void disableOpenTelemetryMetrics() { - SpannerOptions.enableOpenTelemetryMetrics = false; - } - @Override protected String getDefaultProject() { String projectId = getDefaultProjectId(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java index 740eb0c45ee..25796968e9e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TraceWrapper.java @@ -42,16 +42,11 @@ ISpan spanBuilder(String spanName) { ISpan spanBuilderWithExplicitParent(String spanName, ISpan parentSpan) { if (SpannerOptions.getActiveTracingFramework().equals(TracingFramework.OPEN_TELEMETRY)) { - OpenTelemetrySpan otParentSpan; - if (!(parentSpan instanceof OpenTelemetrySpan)) { - otParentSpan = new OpenTelemetrySpan(null); - } else { - otParentSpan = (OpenTelemetrySpan) parentSpan; - } + OpenTelemetrySpan otParentSpan = (OpenTelemetrySpan) parentSpan; io.opentelemetry.api.trace.Span otSpan; - if (otParentSpan.getOpenTelemetrySpan() != null) { + if (otParentSpan != null && otParentSpan.getOpenTelemetrySpan() != null) { otSpan = openTelemetryTracer .spanBuilder(spanName) @@ -64,15 +59,11 @@ ISpan spanBuilderWithExplicitParent(String spanName, ISpan parentSpan) { return new OpenTelemetrySpan(otSpan); } else { - OpenCensusSpan parentOcSpan; - if (!(parentSpan instanceof OpenCensusSpan)) { - parentOcSpan = new OpenCensusSpan(null); - } else { - parentOcSpan = (OpenCensusSpan) parentSpan; - } + OpenCensusSpan parentOcSpan = (OpenCensusSpan) parentSpan; Span ocSpan = openCensusTracer - .spanBuilderWithExplicitParent(spanName, parentOcSpan.getOpenCensusSpan()) + .spanBuilderWithExplicitParent( + spanName, parentOcSpan != null ? parentOcSpan.getOpenCensusSpan() : null) .startSpan(); return new OpenCensusSpan(ocSpan); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/FailOnOverkillTraceComponentImpl.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/FailOnOverkillTraceComponentImpl.java index 7a421ac1b49..4d9b4ddd805 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/FailOnOverkillTraceComponentImpl.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/FailOnOverkillTraceComponentImpl.java @@ -29,6 +29,7 @@ import io.opencensus.trace.SpanBuilder; import io.opencensus.trace.SpanContext; import io.opencensus.trace.SpanId; +import io.opencensus.trace.Status; import io.opencensus.trace.TraceComponent; import io.opencensus.trace.TraceId; import io.opencensus.trace.TraceOptions; @@ -87,9 +88,18 @@ public void addAnnotation(Annotation annotation) { annotations.add(annotation.getDescription()); } + @Override + public void putAttributes(Map attributes) {} + + @Override + public void addAttributes(Map attributes) {} + @Override public void addLink(Link link) {} + @Override + public void setStatus(Status status) {} + @Override public void end(EndSpanOptions options) { synchronized (this) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetrySpanTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetrySpanTest.java index b2889d7c6f0..5a3796e3473 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetrySpanTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/OpenTelemetrySpanTest.java @@ -32,7 +32,6 @@ import io.grpc.Status; import io.grpc.inprocess.InProcessServerBuilder; import io.opencensus.trace.Tracing; -import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.OpenTelemetry; import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator; import io.opentelemetry.context.propagation.ContextPropagators; @@ -52,9 +51,11 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.experimental.categories.Category; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +@Category(TracerTest.class) @RunWith(JUnit4.class) public class OpenTelemetrySpanTest { @@ -212,7 +213,6 @@ public class OpenTelemetrySpanTest { @BeforeClass public static void startStaticServer() throws Exception { - // Incorporating OpenCensus tracer to ensure that OpenTraces traces are utilized if enabled, // regardless of the presence of OpenCensus tracer. java.lang.reflect.Field field = Tracing.class.getDeclaredField("traceComponent"); @@ -233,8 +233,6 @@ public static void startStaticServer() throws Exception { // OpenTelemetry Configuration - SpannerOptions.resetActiveTracingFramework(); - SpannerOptions.enableOpenTelemetryTraces(); mockSpanner = new MockSpannerServiceImpl(); mockSpanner.setAbortProbability(0.0D); // We don't want any unpredictable aborted transactions. mockSpanner.putStatementResult(StatementResult.query(SELECT1, SELECT1_RESULTSET)); @@ -247,6 +245,8 @@ public static void startStaticServer() throws Exception { server = InProcessServerBuilder.forName(uniqueName).addService(mockSpanner).build().start(); channelProvider = LocalChannelProvider.create(uniqueName); + failOnOverkillTraceComponent.clearSpans(); + failOnOverkillTraceComponent.clearAnnotations(); } @AfterClass @@ -259,6 +259,8 @@ public static void stopServer() throws InterruptedException { @Before public void setUp() throws Exception { + SpannerOptions.resetActiveTracingFramework(); + SpannerOptions.enableOpenTelemetryTraces(); spanExporter = InMemorySpanExporter.create(); SdkTracerProvider tracerProvider = @@ -266,7 +268,6 @@ public void setUp() throws Exception { .addSpanProcessor(SimpleSpanProcessor.create(spanExporter)) .build(); - GlobalOpenTelemetry.resetForTest(); OpenTelemetry openTelemetry = OpenTelemetrySdk.builder() .setPropagators(ContextPropagators.create(W3CTraceContextPropagator.getInstance())) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ResumableStreamIteratorTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ResumableStreamIteratorTest.java index 4260adadb95..217e818d42c 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ResumableStreamIteratorTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ResumableStreamIteratorTest.java @@ -16,7 +16,6 @@ package com.google.cloud.spanner; -import static com.google.cloud.spanner.OpenCensusSpan.END_SPAN_OPTIONS; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; @@ -136,6 +135,8 @@ public boolean isWithBeginTransaction() { @Before public void setUp() { + SpannerOptions.resetActiveTracingFramework(); + SpannerOptions.enableOpenTelemetryTraces(); initWithLimit(Integer.MAX_VALUE); } @@ -213,7 +214,7 @@ public void closedOCSpan() { assertThat(consume(resumableStreamIterator)).containsExactly("a", "b").inOrder(); resumableStreamIterator.close("closed"); - verify(mockSpan).end(END_SPAN_OPTIONS); + verify(mockSpan).end(OpenCensusSpan.END_SPAN_OPTIONS); } @Test diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RetryOnInvalidatedSessionTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RetryOnInvalidatedSessionTest.java index 6efdfb046db..85b91ed3981 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RetryOnInvalidatedSessionTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/RetryOnInvalidatedSessionTest.java @@ -162,8 +162,6 @@ public static Collection data() { @BeforeClass public static void startStaticServer() throws IOException { - SpannerOptions.resetActiveTracingFramework(); - SpannerOptions.enableOpenTelemetryTraces(); mockSpanner = new MockSpannerServiceImpl(); mockSpanner.setAbortProbability(0.0D); // We don't want any unpredictable aborted transactions. mockSpanner.putStatementResult( diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java index d2c926eb557..d9b5853926c 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java @@ -1961,8 +1961,6 @@ public void testOpenTelemetrySessionMetrics() throws Exception { verifyMetricData(metricDataCollection, NUM_SESSIONS_IN_POOL, 1, NUM_SESSIONS_IN_USE, 2); verifyMetricData(metricDataCollection, NUM_SESSIONS_IN_POOL, 1, NUM_SESSIONS_AVAILABLE, 1); } - - SpannerOptions.disableOpenTelemetryMetrics(); } private static void verifyMetricData( diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java index 9ff12160447..a678240bcd7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpanTest.java @@ -168,6 +168,8 @@ public static void stopServer() throws InterruptedException { @Before public void setUp() throws Exception { + SpannerOptions.resetActiveTracingFramework(); + SpannerOptions.enableOpenCensusTraces(); // Incorporating OpenTelemetry configuration to ensure that OpenCensus traces are utilized by // default, // regardless of the presence of OpenTelemetry configuration. diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java index 9f2c3a7e21d..049ce0d1960 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java @@ -187,7 +187,6 @@ public void sendHeaders(Metadata headers) { @AfterClass public static void stopServer() throws InterruptedException { - SpannerOptions.disableOpenTelemetryMetrics(); if (spannerWithOpenTelemetry != null) { spannerWithOpenTelemetry.close(); server.shutdown();