Skip to content

Commit a9a1b1c

Browse files
committed
review comments
1 parent d50610f commit a9a1b1c

File tree

4 files changed

+32
-45
lines changed

4 files changed

+32
-45
lines changed

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ static final class ClosedException extends RuntimeException {
128128
this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), gapicRpc);
129129
this.instanceClient =
130130
new InstanceAdminClientImpl(options.getProjectId(), gapicRpc, dbAdminClient);
131+
SpannerRpcMetrics.initializeRPCMetrics(SpannerOptions.getOpenTelemetry());
131132
}
132133

133134
SpannerImpl(SpannerOptions options) {

google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,8 +1212,6 @@ public SpannerOptions build() {
12121212
this.numChannels =
12131213
this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS;
12141214
}
1215-
1216-
SpannerRpcMetrics.initializeRPCMetrics(openTelemetry);
12171215
return new SpannerOptions(this);
12181216
}
12191217
}

google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java

Lines changed: 31 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,27 @@ class HeaderInterceptor implements ClientInterceptor {
6969

7070
HeaderInterceptor() {}
7171

72+
private class SpannerProperties {
73+
String projectId;
74+
String instanceId;
75+
String databaseId;
76+
77+
SpannerProperties(String projectId, String instanceId, String databaseId) {
78+
this.databaseId = databaseId;
79+
this.instanceId = instanceId;
80+
this.projectId = projectId;
81+
}
82+
}
83+
7284
@Override
7385
public <ReqT, RespT> ClientCall<ReqT, RespT> interceptCall(
7486
MethodDescriptor<ReqT, RespT> method, CallOptions callOptions, Channel next) {
7587
return new SimpleForwardingClientCall<ReqT, RespT>(next.newCall(method, callOptions)) {
7688
@Override
7789
public void start(Listener<RespT> responseListener, Metadata headers) {
78-
TagContext tagContext = getTagContext(headers, method.getFullMethodName());
79-
Attributes attributes = getMetricAttributes(headers, method.getFullMethodName());
90+
SpannerProperties spannerProperties = setProjectPropertes(headers);
91+
TagContext tagContext = getTagContext(method.getFullMethodName(), spannerProperties);
92+
Attributes attributes = getMetricAttributes(method.getFullMethodName(), spannerProperties);
8093
super.start(
8194
new SimpleForwardingClientCallListener<RespT>(responseListener) {
8295
@Override
@@ -114,18 +127,7 @@ private void processHeader(Metadata metadata, TagContext tagContext, Attributes
114127
}
115128
}
116129

117-
private TagContext getTagContext(
118-
String method, String projectId, String instanceId, String databaseId) {
119-
return TAGGER
120-
.currentBuilder()
121-
.putLocal(PROJECT_ID, TagValue.create(projectId))
122-
.putLocal(INSTANCE_ID, TagValue.create(instanceId))
123-
.putLocal(DATABASE_ID, TagValue.create(databaseId))
124-
.putLocal(METHOD, TagValue.create(method))
125-
.build();
126-
}
127-
128-
private TagContext getTagContext(Metadata headers, String method) {
130+
private SpannerProperties setProjectPropertes(Metadata headers) {
129131
String projectId = "undefined-project";
130132
String instanceId = "undefined-database";
131133
String databaseId = "undefined-database";
@@ -144,32 +146,24 @@ private TagContext getTagContext(Metadata headers, String method) {
144146
LOGGER.log(LEVEL, "Error parsing google cloud resource header: " + googleResourcePrefix);
145147
}
146148
}
147-
return getTagContext(method, projectId, instanceId, databaseId);
149+
return new SpannerProperties(projectId, instanceId, databaseId);
148150
}
149151

150-
private Attributes getMetricAttributes(Metadata headers, String method) {
151-
String projectId = "undefined-project";
152-
String instanceId = "undefined-database";
153-
String databaseId = "undefined-database";
154-
if (headers.get(GOOGLE_CLOUD_RESOURCE_PREFIX_KEY) != null) {
155-
String googleResourcePrefix = headers.get(GOOGLE_CLOUD_RESOURCE_PREFIX_KEY);
156-
Matcher matcher = GOOGLE_CLOUD_RESOURCE_PREFIX_PATTERN.matcher(googleResourcePrefix);
157-
if (matcher.find()) {
158-
projectId = matcher.group("project");
159-
if (matcher.group("instance") != null) {
160-
instanceId = matcher.group("instance");
161-
}
162-
if (matcher.group("database") != null) {
163-
databaseId = matcher.group("database");
164-
}
165-
} else {
166-
LOGGER.log(LEVEL, "Error parsing google cloud resource header: " + googleResourcePrefix);
167-
}
168-
}
152+
private TagContext getTagContext(String method, SpannerProperties spannerProperties) {
153+
return TAGGER
154+
.currentBuilder()
155+
.putLocal(PROJECT_ID, TagValue.create(spannerProperties.projectId))
156+
.putLocal(INSTANCE_ID, TagValue.create(spannerProperties.instanceId))
157+
.putLocal(DATABASE_ID, TagValue.create(spannerProperties.databaseId))
158+
.putLocal(METHOD, TagValue.create(method))
159+
.build();
160+
}
161+
162+
private Attributes getMetricAttributes(String method, SpannerProperties spannerProperties) {
169163
AttributesBuilder attributesBuilder = Attributes.builder();
170-
attributesBuilder.put("database", databaseId);
171-
attributesBuilder.put("instance_id", instanceId);
172-
attributesBuilder.put("project_id", projectId);
164+
attributesBuilder.put("database", spannerProperties.databaseId);
165+
attributesBuilder.put("instance_id", spannerProperties.instanceId);
166+
attributesBuilder.put("project_id", spannerProperties.projectId);
173167
attributesBuilder.put("method", method);
174168

175169
return attributesBuilder.build();

google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/SpannerRpcMetricsTest.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,24 +66,18 @@ public class SpannerRpcMetricsTest {
6666
private static InetSocketAddress address;
6767
private static Spanner spanner;
6868
private static DatabaseClient databaseClient;
69-
7069
private static final Map<SpannerRpc.Option, Object> optionsMap = new HashMap<>();
71-
7270
private static MockSpannerServiceImpl mockSpannerNoHeader;
7371
private static Server serverNoHeader;
7472
private static InetSocketAddress addressNoHeader;
7573
private static Spanner spannerNoHeader;
7674
private static DatabaseClient databaseClientNoHeader;
77-
7875
private static String instanceId = "fake-instance";
7976
private static String databaseId = "fake-database";
8077
private static String projectId = "fake-project";
81-
8278
private static AtomicInteger fakeServerTiming = new AtomicInteger(new Random().nextInt(1000) + 1);
83-
8479
private static final Statement SELECT1AND2 =
8580
Statement.of("SELECT 1 AS COL1 UNION ALL SELECT 2 AS COL1");
86-
8781
private static final ResultSetMetadata SELECT1AND2_METADATA =
8882
ResultSetMetadata.newBuilder()
8983
.setRowType(

0 commit comments

Comments
 (0)