Skip to content

Commit

Permalink
feat: Refactor OpenTelemetry Tracer and Metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
surbhigarg92 committed Jan 10, 2024
1 parent 588adba commit 863dc63
Show file tree
Hide file tree
Showing 47 changed files with 1,198 additions and 416 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ nosetests.xml
.settings
.DS_Store
.classpath
.tool-versions

# Built documentation
docs/
Expand Down
2 changes: 1 addition & 1 deletion google-cloud-spanner/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,6 @@
<className>com/google/cloud/spanner/connection/Connection</className>
<method>void rollbackToSavepoint(java.lang.String)</method>
</difference>

<!-- Delay start transaction -->
<difference>
<differenceType>7012</differenceType>
Expand Down Expand Up @@ -460,6 +459,7 @@
<differenceType>7013</differenceType>
<className>com/google/cloud/spanner/Dialect</className>
<method>java.lang.String getDefaultSchema()</method>
</difference>
<difference>
<differenceType>7005</differenceType>
<className>com/google/cloud/spanner/PartitionedDmlTransaction</className>
Expand Down
5 changes: 0 additions & 5 deletions google-cloud-spanner/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
<site.installationModule>google-cloud-spanner</site.installationModule>
<opencensus.version>0.31.1</opencensus.version>
<opentelemetry.version>1.32.0</opentelemetry.version>
<graalvm.version>22.3.3</graalvm.version>
<spanner.testenv.config.class>com.google.cloud.spanner.GceTestEnvConfig</spanner.testenv.config.class>
<spanner.testenv.instance>projects/gcloud-devel/instances/spanner-testing-east1</spanner.testenv.instance>
<spanner.gce.config.project_id>gcloud-devel</spanner.gce.config.project_id>
Expand Down Expand Up @@ -396,7 +395,6 @@
<version>2.2</version>
<scope>test</scope>
</dependency>

<!-- Benchmarking dependencies -->
<dependency>
<groupId>org.openjdk.jmh</groupId>
Expand All @@ -410,7 +408,6 @@
<version>1.37</version>
<scope>test</scope>
</dependency>

<!-- OpenTelemetry test dependencies -->
<dependency>
<groupId>io.opentelemetry</groupId>
Expand All @@ -436,9 +433,7 @@
<version>${opentelemetry.version}</version>
<scope>test</scope>
</dependency>

</dependencies>

<profiles>
<profile>
<id>java9</id>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@
import com.google.spanner.v1.Transaction;
import com.google.spanner.v1.TransactionOptions;
import com.google.spanner.v1.TransactionSelector;
import io.opencensus.trace.Tracing;
import io.opentelemetry.context.Context;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;
import javax.annotation.Nullable;
Expand All @@ -71,10 +69,8 @@ abstract class AbstractReadContext
abstract static class Builder<B extends Builder<?, T>, T extends AbstractReadContext> {
private SessionImpl session;
private SpannerRpc rpc;
private ISpan span =
new DualSpan(
Tracing.getTracer().getCurrentSpan(),
io.opentelemetry.api.trace.Span.fromContext(Context.current()));
private ISpan span;
private TraceWrapper tracer;
private int defaultPrefetchChunks = SpannerOptions.Builder.DEFAULT_PREFETCH_CHUNKS;
private QueryOptions defaultQueryOptions = SpannerOptions.Builder.DEFAULT_QUERY_OPTIONS;
private DirectedReadOptions defaultDirectedReadOption;
Expand Down Expand Up @@ -103,6 +99,11 @@ B setSpan(ISpan span) {
return self();
}

B setTracer(TraceWrapper tracer) {
this.tracer = tracer;
return self();
}

B setDefaultPrefetchChunks(int defaultPrefetchChunks) {
this.defaultPrefetchChunks = defaultPrefetchChunks;
return self();
Expand Down Expand Up @@ -395,10 +396,7 @@ void initTransaction() {
span.addAnnotation(
"Transaction Creation Done",
ImmutableMap.of(
"Id",
transaction.getId().toStringUtf8(),
"Timestamp",
Timestamp.fromProto(transaction.getReadTimestamp()).toString()));
"Id", transaction.getId().toStringUtf8(), "Timestamp", timestamp.toString()));

} catch (SpannerException e) {
span.addAnnotation("Transaction Creation Failed", e);
Expand All @@ -413,6 +411,7 @@ void initTransaction() {
final SpannerRpc rpc;
final ExecutorProvider executorProvider;
ISpan span;
TraceWrapper tracer;
private final int defaultPrefetchChunks;
private final QueryOptions defaultQueryOptions;

Expand Down Expand Up @@ -445,6 +444,7 @@ void initTransaction() {
this.span = builder.span;
this.executorProvider = builder.executorProvider;
this.clock = builder.clock;
this.tracer = builder.tracer;
}

@Override
Expand Down Expand Up @@ -702,6 +702,7 @@ ResultSet executeQueryInternalWithOptions(
MAX_BUFFERED_CHUNKS,
SpannerImpl.QUERY,
span,
tracer,
rpc.getExecuteQueryRetrySettings(),
rpc.getExecuteQueryRetryableCodes()) {
@Override
Expand Down Expand Up @@ -847,6 +848,7 @@ ResultSet readInternalWithOptions(
MAX_BUFFERED_CHUNKS,
SpannerImpl.READ,
span,
tracer,
rpc.getReadRetrySettings(),
rpc.getReadRetryableCodes()) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import com.google.spanner.v1.Transaction;
import com.google.spanner.v1.TypeCode;
import io.grpc.Context;
import io.opencensus.trace.Tracing;
import java.io.IOException;
import java.io.Serializable;
import java.math.BigDecimal;
Expand Down Expand Up @@ -77,7 +76,6 @@

/** Implementation of {@link ResultSet}. */
abstract class AbstractResultSet<R> extends AbstractStructReader implements ResultSet {
private static final TraceWrapper tracer = new TraceWrapper(Tracing.getTracer());
private static final com.google.protobuf.Value NULL_VALUE =
com.google.protobuf.Value.newBuilder().setNullValue(NullValue.NULL_VALUE).build();

Expand Down Expand Up @@ -1089,6 +1087,7 @@ abstract static class ResumableStreamIterator extends AbstractIterator<PartialRe
private final LinkedList<PartialResultSet> buffer = new LinkedList<>();
private final int maxBufferSize;
private final ISpan span;
private final TraceWrapper tracer;
private CloseableIterator<PartialResultSet> stream;
private ByteString resumeToken;
private boolean finished;
Expand All @@ -1103,10 +1102,12 @@ protected ResumableStreamIterator(
int maxBufferSize,
String streamName,
ISpan parent,
TraceWrapper tracer,
RetrySettings streamingRetrySettings,
Set<Code> retryableCodes) {
checkArgument(maxBufferSize >= 0);
this.maxBufferSize = maxBufferSize;
this.tracer = tracer;
this.span = tracer.spanBuilderWithExplicitParent(streamName, parent);
this.streamingRetrySettings = Preconditions.checkNotNull(streamingRetrySettings);
this.retryableCodes = Preconditions.checkNotNull(retryableCodes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.google.spanner.v1.PartitionReadRequest;
import com.google.spanner.v1.PartitionResponse;
import com.google.spanner.v1.TransactionSelector;
import io.opencensus.trace.Tracing;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -62,7 +61,9 @@ public BatchReadOnlyTransaction batchReadOnlyTransaction(TimestampBound bound) {
.setExecutorProvider(sessionClient.getSpanner().getAsyncExecutorProvider())
.setDefaultPrefetchChunks(sessionClient.getSpanner().getDefaultPrefetchChunks())
.setDefaultDirectedReadOptions(
sessionClient.getSpanner().getOptions().getDirectedReadOptions()),
sessionClient.getSpanner().getOptions().getDirectedReadOptions())
.setSpan(sessionClient.getSpanner().getTracer().getCurrentSpan())
.setTracer(sessionClient.getSpanner().getTracer()),
checkNotNull(bound));
}

Expand All @@ -81,7 +82,9 @@ public BatchReadOnlyTransaction batchReadOnlyTransaction(BatchTransactionId batc
.setExecutorProvider(sessionClient.getSpanner().getAsyncExecutorProvider())
.setDefaultPrefetchChunks(sessionClient.getSpanner().getDefaultPrefetchChunks())
.setDefaultDirectedReadOptions(
sessionClient.getSpanner().getOptions().getDirectedReadOptions()),
sessionClient.getSpanner().getOptions().getDirectedReadOptions())
.setSpan(sessionClient.getSpanner().getTracer().getCurrentSpan())
.setTracer(sessionClient.getSpanner().getTracer()),
batchTransactionId);
}

Expand All @@ -95,11 +98,6 @@ private static class BatchReadOnlyTransactionImpl extends MultiUseReadOnlyTransa
super(builder.setTimestampBound(bound));
this.sessionName = session.getName();
this.options = session.getOptions();
setSpan(
new DualSpan(
Tracing.getTracer().getCurrentSpan(),
io.opentelemetry.api.trace.Span.fromContext(
io.opentelemetry.context.Context.current())));
initTransaction();
}

Expand All @@ -108,11 +106,6 @@ private static class BatchReadOnlyTransactionImpl extends MultiUseReadOnlyTransa
super(builder.setTransactionId(batchTransactionId.getTransactionId()));
this.sessionName = session.getName();
this.options = session.getOptions();
setSpan(
new DualSpan(
Tracing.getTracer().getCurrentSpan(),
io.opentelemetry.api.trace.Span.fromContext(
io.opentelemetry.context.Context.current())));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,25 @@
import com.google.common.base.Function;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.spanner.v1.BatchWriteResponse;
import io.opencensus.trace.Tracing;
import javax.annotation.Nullable;

class DatabaseClientImpl implements DatabaseClient {
private static final String READ_WRITE_TRANSACTION = "CloudSpanner.ReadWriteTransaction";
private static final String READ_ONLY_TRANSACTION = "CloudSpanner.ReadOnlyTransaction";
private static final String PARTITION_DML_TRANSACTION = "CloudSpanner.PartitionDMLTransaction";
private static final TraceWrapper tracer = new TraceWrapper(Tracing.getTracer());
private final TraceWrapper tracer;
@VisibleForTesting final String clientId;
@VisibleForTesting final SessionPool pool;

@VisibleForTesting
DatabaseClientImpl(SessionPool pool) {
this("", pool);
DatabaseClientImpl(SessionPool pool, TraceWrapper tracer) {
this("", pool, tracer);
}

DatabaseClientImpl(String clientId, SessionPool pool) {
DatabaseClientImpl(String clientId, SessionPool pool, TraceWrapper tracer) {
this.clientId = clientId;
this.pool = pool;
this.tracer = tracer;
}

@VisibleForTesting
Expand Down Expand Up @@ -198,9 +198,8 @@ public TransactionRunner readWriteTransaction(TransactionOption... options) {
return getSession().readWriteTransaction(options);
} catch (RuntimeException e) {
span.setStatus(e);
throw e;
} finally {
span.end();
throw e;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.cloud.spanner;

import io.opencensus.trace.Status;
import java.util.Map;

interface ISpan {
Expand All @@ -40,7 +39,7 @@ interface ISpan {

void setStatus(Throwable e);

void setStatus(Status status);
void setStatus(ErrorCode errorCode);

void end();
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,19 @@ class MetricRegistryConstants {
/** Unit to represent counts. */
static final String COUNT = "1";

static final String Instrumentation_Scope = "cloud.google.com/java";

static final String METRIC_PREFIX = "cloud.google.com/java/";

// The Metric name and description
static final String MAX_IN_USE_SESSIONS = "cloud.google.com/java/spanner/max_in_use_sessions";
static final String MAX_ALLOWED_SESSIONS = "cloud.google.com/java/spanner/max_allowed_sessions";
static final String GET_SESSION_TIMEOUTS = "cloud.google.com/java/spanner/get_session_timeouts";
static final String NUM_ACQUIRED_SESSIONS = "cloud.google.com/java/spanner/num_acquired_sessions";
static final String NUM_RELEASED_SESSIONS = "cloud.google.com/java/spanner/num_released_sessions";
static final String NUM_SESSIONS_IN_POOL = "cloud.google.com/java/spanner/num_sessions_in_pool";
static final String NUM_SESSIONS_IN_USE = "cloud.google.com/java/spanner/num_in_use_sessions";
static final String NUM_SESSIONS_AVAILABLE =
"cloud.google.com/java/spanner/num_available_sessions";
static final String MAX_IN_USE_SESSIONS = "spanner/max_in_use_sessions";
static final String MAX_ALLOWED_SESSIONS = "spanner/max_allowed_sessions";
static final String GET_SESSION_TIMEOUTS = "spanner/get_session_timeouts";
static final String NUM_ACQUIRED_SESSIONS = "spanner/num_acquired_sessions";
static final String NUM_RELEASED_SESSIONS = "spanner/num_released_sessions";
static final String NUM_SESSIONS_IN_POOL = "spanner/num_sessions_in_pool";
static final String NUM_SESSIONS_IN_USE = "spanner/num_in_use_sessions";
static final String NUM_SESSIONS_AVAILABLE = "spanner/num_available_sessions";
static final String SESSIONS_TYPE = "session_type";

static final String MAX_IN_USE_SESSIONS_DESCRIPTION =
Expand All @@ -89,12 +92,10 @@ class MetricRegistryConstants {
"The number of sessions released by the user and pool maintainer.";
static final String NUM_SESSIONS_IN_POOL_DESCRIPTION = "The number of sessions in the pool.";

static final String Scope = "cloud.google.com/java/spanner";
static final String SPANNER_GFE_LATENCY = "cloud.google.com/java/spanner/gfe_latency";
static final String SPANNER_GFE_LATENCY = "spanner/gfe_latency";
static final String SPANNER_GFE_LATENCY_DESCRIPTION =
"Latency between Google's network receiving an RPC and reading back the first byte of the response";
static final String SPANNER_GFE_HEADER_MISSING_COUNT =
"cloud.google.com/java/spanner/gfe_header_missing_count";
static final String SPANNER_GFE_HEADER_MISSING_COUNT = "spanner/gfe_header_missing_count";
static final String SPANNER_GFE_HEADER_MISSING_COUNT_DESCRIPTION =
"Number of RPC responses received without the server-timing header, most likely means that the RPC never reached Google's network";
static final String MILLISECOND = "ms";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,18 @@

import io.opencensus.common.Scope;

class DualScope implements IScope {
class OpenCensusScope implements IScope {

private final Scope openCensusScope;
private final io.opentelemetry.context.Scope openTelemetryScope;

public DualScope(Scope openCensusScope, io.opentelemetry.context.Scope openTelemetryScope) {
OpenCensusScope(Scope openCensusScope) {
this.openCensusScope = openCensusScope;
this.openTelemetryScope = openTelemetryScope;
}

@Override
public void close() {
if (openCensusScope != null) {
openCensusScope.close();
}
if (openTelemetryScope != null) {
openTelemetryScope.close();
}
}
}
Loading

0 comments on commit 863dc63

Please sign in to comment.