Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.api.gax.rpc.ApiCallContext;
import com.google.api.gax.rpc.ServerStream;
import com.google.protobuf.ByteString;
import com.google.protobuf.UnsafeByteOperations;
import com.google.spanner.adapter.v1.AdaptMessageRequest;
import com.google.spanner.adapter.v1.AdaptMessageResponse;
import com.google.spanner.adapter.v1.AdapterClient;
Expand Down Expand Up @@ -71,7 +72,10 @@ ByteString sendGrpcRequest(
.setName(sessionManager.getSession().getName())
.setProtocol("cassandra")
.putAllAttachments(attachments)
.setPayload(ByteString.copyFrom(payload))
// It is safe to use UnsafeByteOperations to wrap the payload without copying, as the
// underlying `payload` byte array is not modified after this point. This avoids an
// unnecessary memory copy for every request, which is a performance optimization.
.setPayload(UnsafeByteOperations.unsafeWrap(payload))
Comment thread
mayurkale22 marked this conversation as resolved.
.build();

List<ByteString> collectedPayloads = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ final class DriverConnectionHandler implements Runnable {
private static final char WRITE_ACTION_QUERY_ID_PREFIX = 'W';
private static final String ROUTE_TO_LEADER_HEADER_KEY = "x-goog-spanner-route-to-leader";
private static final String MAX_COMMIT_DELAY_ATTACHMENT_KEY = "max_commit_delay";
private static final String ADAPT_MESSAGE_METHOD = "Adapter.AdaptMessage";
private static final ByteBufAllocator byteBufAllocator = ByteBufAllocator.DEFAULT;
private static final FrameCodec<ByteBuf> serverFrameCodec =
FrameCodec.defaultServer(new ByteBufPrimitiveCodec(byteBufAllocator), Compressor.none());
Expand All @@ -77,8 +76,6 @@ final class DriverConnectionHandler implements Runnable {
private static final GrpcCallContext DEFAULT_CONTEXT = GrpcCallContext.createDefault();
private static final Map<String, List<String>> ROUTE_TO_LEADER_HEADER_MAP =
ImmutableMap.of(ROUTE_TO_LEADER_HEADER_KEY, Collections.singletonList("true"));
private static final Map<String, String> SUCCESS_METRIC_ATTRIBUTES =
ImmutableMap.of("method", ADAPT_MESSAGE_METHOD, "status", "OK");
private static final GrpcCallContext DEFAULT_CONTEXT_WITH_LAR =
GrpcCallContext.createDefault().withExtraHeaders(ROUTE_TO_LEADER_HEADER_MAP);
private static final byte[] EMPTY_BYTES = new byte[0];
Expand Down Expand Up @@ -188,8 +185,8 @@ private void recordMetrics(Instant startTime) {
return;
}
final long latency = Duration.between(startTime, Instant.now()).toMillis();
metricsRecorder.recordOperationCount(1, SUCCESS_METRIC_ATTRIBUTES);
metricsRecorder.recordOperationLatency(latency, SUCCESS_METRIC_ATTRIBUTES);
metricsRecorder.recordOperationCount(1);
metricsRecorder.recordOperationLatency(latency);
}

private static int readNBytesJava8(InputStream in, byte[] b, int off, int len)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
import static com.google.cloud.spanner.adapter.metrics.BuiltInMetricsConstant.INSTANCE_CONFIG_ID_KEY;
import static com.google.cloud.spanner.adapter.metrics.BuiltInMetricsConstant.INSTANCE_ID_KEY;
import static com.google.cloud.spanner.adapter.metrics.BuiltInMetricsConstant.LOCATION_ID_KEY;
import static com.google.cloud.spanner.adapter.metrics.BuiltInMetricsConstant.METHOD_KEY;
import static com.google.cloud.spanner.adapter.metrics.BuiltInMetricsConstant.PROJECT_ID_KEY;
import static com.google.cloud.spanner.adapter.metrics.BuiltInMetricsConstant.STATUS_KEY;

import com.google.api.gax.core.GaxProperties;
import com.google.cloud.opentelemetry.detection.AttributeKeys;
Expand All @@ -48,8 +50,6 @@
import java.lang.reflect.Method;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;
Expand Down Expand Up @@ -117,18 +117,20 @@ Attributes createResourceAttributes(String projectId, String instanceId) {
.put(INSTANCE_ID_KEY.getKey(), instanceId)
.put(LOCATION_ID_KEY.getKey(), detectClientLocation())
.put("gcp.resource_type", BuiltInMetricsConstant.SPANNER_RESOURCE_TYPE);

return attributesBuilder.build();
}

public Map<String, String> createDefaultAttributes(String databaseId) {
Map<String, String> defaultAttributes = new HashMap<>();
defaultAttributes.put(DATABASE_KEY.getKey(), databaseId);
defaultAttributes.put(
CLIENT_NAME_KEY.getKey(),
"spanner-cassandra-java/" + GaxProperties.getLibraryVersion(getClass()));
defaultAttributes.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue());
return defaultAttributes;
public Attributes createDefaultAttributes(String databaseId) {
AttributesBuilder defaultAttributesBuilder =
Attributes.builder()
.put(DATABASE_KEY.getKey(), databaseId)
.put(
CLIENT_NAME_KEY.getKey(),
"spanner-cassandra-java/" + GaxProperties.getLibraryVersion(getClass()))
.put(CLIENT_UID_KEY.getKey(), getDefaultTaskValue())
.put(METHOD_KEY.getKey(), "Adapter.AdaptMessage")
.put(STATUS_KEY.getKey(), "OK");
return defaultAttributesBuilder.build();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,18 @@
package com.google.cloud.spanner.adapter.metrics;

import com.google.api.gax.core.GaxProperties;
import com.google.common.base.Preconditions;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import io.opentelemetry.api.metrics.DoubleHistogram;
import io.opentelemetry.api.metrics.LongCounter;
import io.opentelemetry.api.metrics.Meter;
import java.util.Map;

/** Implementation for recording built in metrics. */
public final class BuiltInMetricsRecorder {

private final LongCounter operationCountRecorder;
private final DoubleHistogram operationLatencyRecorder;
private final Map<String, String> defaultAttr;
private final Attributes defaultAttr;

/**
* Creates the following instruments for the following metrics:
Expand All @@ -43,7 +40,7 @@ public final class BuiltInMetricsRecorder {
* @param openTelemetry OpenTelemetry instance
* @param defaultAttr Default attibutes
*/
public BuiltInMetricsRecorder(OpenTelemetry openTelemetry, Map<String, String> defaultAttr) {
public BuiltInMetricsRecorder(OpenTelemetry openTelemetry, Attributes defaultAttr) {
Meter meter =
openTelemetry
.meterBuilder(BuiltInMetricsConstant.SPANNER_METER_NAME)
Expand Down Expand Up @@ -72,19 +69,11 @@ public BuiltInMetricsRecorder(OpenTelemetry openTelemetry, Map<String, String> d
this.defaultAttr = defaultAttr;
}

public void recordOperationLatency(double operationLatency, Map<String, String> attributes) {
operationLatencyRecorder.record(operationLatency, toOtelAttributes(attributes));
public void recordOperationLatency(double operationLatency) {
operationLatencyRecorder.record(operationLatency, defaultAttr);
}

public void recordOperationCount(long count, Map<String, String> attributes) {
operationCountRecorder.add(count, toOtelAttributes(attributes));
}

Attributes toOtelAttributes(Map<String, String> attributes) {
Preconditions.checkNotNull(attributes, "Attributes map cannot be null");
AttributesBuilder attributesBuilder = Attributes.builder();
attributes.forEach(attributesBuilder::put);
this.defaultAttr.forEach(attributesBuilder::put);
return attributesBuilder.build();
public void recordOperationCount(long count) {
operationCountRecorder.add(count, defaultAttr);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ public final class DriverConnectionHandlerTest {
private AdapterClientWrapper mockAdapterClient;
private Socket mockSocket;
private ByteArrayOutputStream outputStream;
private static final Map<String, String> expectedAttributes =
ImmutableMap.of("method", "Adapter.AdaptMessage", "status", "OK");

public DriverConnectionHandlerTest() {}

Expand Down Expand Up @@ -206,7 +204,7 @@ public void successfulDmlExecuteMessage() throws IOException {
.containsExactly("x-goog-spanner-route-to-leader", ImmutableList.of("true"));
assertThat(attachmentsCaptor.getValue())
.containsExactly(preparedQueryKey, "query", "max_commit_delay", "100");
verify(mockMetricsRecorder).recordOperationCount(1L, expectedAttributes);
verify(mockMetricsRecorder).recordOperationCount(1L);
}

@Test
Expand Down Expand Up @@ -249,7 +247,7 @@ public void successfulBatchMessage() throws IOException {
.sendGrpcRequest(any(), any(), contextCaptor.capture(), any(int.class));
assertThat(contextCaptor.getValue().getExtraHeaders())
.containsExactly("x-goog-spanner-route-to-leader", ImmutableList.of("true"));
verify(mockMetricsRecorder).recordOperationCount(1L, expectedAttributes);
verify(mockMetricsRecorder).recordOperationCount(1L);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
*/
package com.google.cloud.spanner.adapter.metrics;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand All @@ -30,9 +28,6 @@
import io.opentelemetry.api.metrics.LongCounterBuilder;
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.api.metrics.MeterBuilder;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -47,7 +42,7 @@ public class BuiltInMetricsRecorderTest {
private LongCounter mockOperationCounter;

private BuiltInMetricsRecorder metricsRecorder;
private Map<String, String> defaultAttributes;
private Attributes defaultAttributes;

@Before
public void setUp() {
Expand Down Expand Up @@ -78,74 +73,43 @@ public void setUp() {
when(mockCounterBuilder.build()).thenReturn(mockOperationCounter);

// 5. Setup default attributes and create the recorder instance
defaultAttributes = new HashMap<>();
defaultAttributes.put("db", "test-db");
defaultAttributes.put("client_id", "test-client");
metricsRecorder = new BuiltInMetricsRecorder(mockOpenTelemetry, defaultAttributes);
}

@Test
public void toOtelAttributes_withNull_throwsException() {
assertThrows(NullPointerException.class, () -> metricsRecorder.toOtelAttributes(null));
}

@Test
public void toOtelAttributes_mergesAttributesCorrectly() {
Map<String, String> specificAttributes = new HashMap<>();
specificAttributes.put("method", "testMethod");
specificAttributes.put("status", "OK");

Attributes result = metricsRecorder.toOtelAttributes(specificAttributes);

Attributes expected =
defaultAttributes =
Attributes.builder()
.put("db", "test-db")
.put("client_id", "test-client")
.put("method", "testMethod")
.put("method", "myMethod")
.put("status", "OK")
.build();

assertThat(result).isEqualTo(expected);
}

@Test
public void toOtelAttributes_withEmptySpecificAttributes_returnsDefaults() {
Attributes result = metricsRecorder.toOtelAttributes(Collections.emptyMap());

Attributes expected =
Attributes.builder().put("db", "test-db").put("client_id", "test-client").build();

assertThat(result).isEqualTo(expected);
metricsRecorder = new BuiltInMetricsRecorder(mockOpenTelemetry, defaultAttributes);
}

@Test
public void recordOperationLatency_callsRecordWithCorrectValues() {
double latency = 123.45;
Map<String, String> attributes = Collections.singletonMap("status", "ERROR");

metricsRecorder.recordOperationLatency(latency, attributes);
metricsRecorder.recordOperationLatency(latency);

Attributes expectedAttributes =
Attributes.builder()
.put("db", "test-db")
.put("client_id", "test-client")
.put("status", "ERROR")
.put("method", "myMethod")
.put("status", "OK")
.build();
verify(mockLatencyRecorder).record(latency, expectedAttributes);
}

@Test
public void recordOperationCount_callsAddWithCorrectValues() {
long count = 5L;
Map<String, String> attributes = Collections.singletonMap("method", "myMethod");

metricsRecorder.recordOperationCount(count, attributes);
metricsRecorder.recordOperationCount(count);

Attributes expectedAttributes =
Attributes.builder()
.put("db", "test-db")
.put("client_id", "test-client")
.put("method", "myMethod")
.put("status", "OK")
.build();
verify(mockOperationCounter).add(count, expectedAttributes);
}
Expand Down
Loading