Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c35835e

Browse filesBrowse files
rahul2393rahul2393
authored
Revert "feat: built in metrics for afe latency and connectivity error (#3724)"
This reverts commit e13a2f9.
1 parent e13a2f9 commit c35835e
Copy full SHA for c35835e
Expand file treeCollapse file tree

11 files changed

+50
-245
lines changed

‎google-cloud-spanner/clirr-ignored-differences.xml

Copy file name to clipboardExpand all lines: google-cloud-spanner/clirr-ignored-differences.xml
+4-11Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -751,13 +751,6 @@
751751
<method>boolean isEnableBuiltInMetrics()</method>
752752
</difference>
753753

754-
<!-- Added AFE Server Timing option -->
755-
<difference>
756-
<differenceType>7012</differenceType>
757-
<className>com/google/cloud/spanner/SpannerOptions$SpannerEnvironment</className>
758-
<method>boolean isEnableAFEServerTiming()</method>
759-
</difference>
760-
761754
<!-- Added Monitoring host option -->
762755
<difference>
763756
<differenceType>7012</differenceType>
@@ -814,7 +807,7 @@
814807
<className>com/google/cloud/spanner/connection/Connection</className>
815808
<method>boolean isKeepTransactionAlive()</method>
816809
</difference>
817-
810+
818811
<!-- Automatic DML batching -->
819812
<difference>
820813
<differenceType>7012</differenceType>
@@ -846,7 +839,7 @@
846839
<className>com/google/cloud/spanner/connection/Connection</className>
847840
<method>boolean isAutoBatchDmlUpdateCountVerification()</method>
848841
</difference>
849-
842+
850843
<!-- Retry DML as Partitioned DML -->
851844
<difference>
852845
<differenceType>7012</differenceType>
@@ -870,7 +863,7 @@
870863
<className>com/google/cloud/spanner/connection/Connection</className>
871864
<method>java.lang.Object runTransaction(com.google.cloud.spanner.connection.Connection$TransactionCallable)</method>
872865
</difference>
873-
866+
874867
<!-- Added experimental host option -->
875868
<difference>
876869
<differenceType>7012</differenceType>
@@ -899,7 +892,7 @@
899892
<className>com/google/cloud/spanner/connection/Connection</className>
900893
<method>java.lang.String getDefaultSequenceKind()</method>
901894
</difference>
902-
895+
903896
<!-- Default isolation level -->
904897
<difference>
905898
<differenceType>7012</differenceType>

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java

Copy file name to clipboardExpand all lines: google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsConstant.java
+16-15Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import io.opentelemetry.sdk.metrics.InstrumentSelector;
2727
import io.opentelemetry.sdk.metrics.InstrumentType;
2828
import io.opentelemetry.sdk.metrics.View;
29-
import java.util.List;
3029
import java.util.Map;
3130
import java.util.Set;
3231
import java.util.stream.Collectors;
@@ -38,9 +37,6 @@ public class BuiltInMetricsConstant {
3837
public static final String GAX_METER_NAME = OpenTelemetryMetricsRecorder.GAX_METER_NAME;
3938
static final String SPANNER_METER_NAME = "spanner-java";
4039
static final String GFE_LATENCIES_NAME = "gfe_latencies";
41-
static final String AFE_LATENCIES_NAME = "afe_latencies";
42-
static final String GFE_CONNECTIVITY_ERROR_NAME = "gfe_connectivity_error_count";
43-
static final String AFE_CONNECTIVITY_ERROR_NAME = "afe_connectivity_error_count";
4440
static final String OPERATION_LATENCIES_NAME = "operation_latencies";
4541
static final String ATTEMPT_LATENCIES_NAME = "attempt_latencies";
4642
static final String OPERATION_LATENCY_NAME = "operation_latency";
@@ -54,10 +50,7 @@ public class BuiltInMetricsConstant {
5450
ATTEMPT_LATENCIES_NAME,
5551
OPERATION_COUNT_NAME,
5652
ATTEMPT_COUNT_NAME,
57-
GFE_LATENCIES_NAME,
58-
AFE_LATENCIES_NAME,
59-
GFE_CONNECTIVITY_ERROR_NAME,
60-
AFE_CONNECTIVITY_ERROR_NAME)
53+
GFE_LATENCIES_NAME)
6154
.stream()
6255
.map(m -> METER_NAME + '/' + m)
6356
.collect(Collectors.toSet());
@@ -109,14 +102,14 @@ public class BuiltInMetricsConstant {
109102
DIRECT_PATH_ENABLED_KEY,
110103
DIRECT_PATH_USED_KEY);
111104

112-
static List<Double> BUCKET_BOUNDARIES =
113-
ImmutableList.of(
114-
0.0, 0.5, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0, 15.0,
115-
16.0, 17.0, 18.0, 19.0, 20.0, 25.0, 30.0, 40.0, 50.0, 65.0, 80.0, 100.0, 130.0, 160.0,
116-
200.0, 250.0, 300.0, 400.0, 500.0, 650.0, 800.0, 1000.0, 2000.0, 5000.0, 10000.0, 20000.0,
117-
50000.0, 100000.0, 200000.0, 400000.0, 800000.0, 1600000.0, 3200000.0);
118105
static Aggregation AGGREGATION_WITH_MILLIS_HISTOGRAM =
119-
Aggregation.explicitBucketHistogram(BUCKET_BOUNDARIES);
106+
Aggregation.explicitBucketHistogram(
107+
ImmutableList.of(
108+
0.0, 0.5, 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0, 9.0, 10.0, 11.0, 12.0, 13.0, 14.0,
109+
15.0, 16.0, 17.0, 18.0, 19.0, 20.0, 25.0, 30.0, 40.0, 50.0, 65.0, 80.0, 100.0, 130.0,
110+
160.0, 200.0, 250.0, 300.0, 400.0, 500.0, 650.0, 800.0, 1000.0, 2000.0, 5000.0,
111+
10000.0, 20000.0, 50000.0, 100000.0, 200000.0, 400000.0, 800000.0, 1600000.0,
112+
3200000.0));
120113

121114
static Map<InstrumentSelector, View> getAllViews() {
122115
ImmutableMap.Builder<InstrumentSelector, View> views = ImmutableMap.builder();
@@ -136,6 +129,14 @@ static Map<InstrumentSelector, View> getAllViews() {
136129
BuiltInMetricsConstant.AGGREGATION_WITH_MILLIS_HISTOGRAM,
137130
InstrumentType.HISTOGRAM,
138131
"ms");
132+
defineView(
133+
views,
134+
BuiltInMetricsConstant.SPANNER_METER_NAME,
135+
BuiltInMetricsConstant.GFE_LATENCIES_NAME,
136+
BuiltInMetricsConstant.GFE_LATENCIES_NAME,
137+
BuiltInMetricsConstant.AGGREGATION_WITH_MILLIS_HISTOGRAM,
138+
InstrumentType.HISTOGRAM,
139+
"ms");
139140
defineView(
140141
views,
141142
BuiltInMetricsConstant.GAX_METER_NAME,

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java

Copy file name to clipboardExpand all lines: google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsRecorder.java
+2-44Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import io.opentelemetry.api.common.Attributes;
2424
import io.opentelemetry.api.common.AttributesBuilder;
2525
import io.opentelemetry.api.metrics.DoubleHistogram;
26-
import io.opentelemetry.api.metrics.LongCounter;
2726
import io.opentelemetry.api.metrics.Meter;
2827
import java.util.Map;
2928

@@ -36,9 +35,6 @@
3635
class BuiltInMetricsRecorder extends OpenTelemetryMetricsRecorder {
3736

3837
private final DoubleHistogram gfeLatencyRecorder;
39-
private final DoubleHistogram afeLatencyRecorder;
40-
private final LongCounter gfeHeaderMissingCountRecorder;
41-
private final LongCounter afeHeaderMissingCountRecorder;
4238

4339
/**
4440
* Creates the following instruments for the following metrics:
@@ -63,27 +59,6 @@ class BuiltInMetricsRecorder extends OpenTelemetryMetricsRecorder {
6359
.setDescription(
6460
"Latency between Google's network receiving an RPC and reading back the first byte of the response")
6561
.setUnit("ms")
66-
.setExplicitBucketBoundariesAdvice(BuiltInMetricsConstant.BUCKET_BOUNDARIES)
67-
.build();
68-
this.afeLatencyRecorder =
69-
meter
70-
.histogramBuilder(serviceName + '/' + BuiltInMetricsConstant.AFE_LATENCIES_NAME)
71-
.setDescription(
72-
"Latency between Spanner API Frontend receiving an RPC and starting to write back the response.")
73-
.setExplicitBucketBoundariesAdvice(BuiltInMetricsConstant.BUCKET_BOUNDARIES)
74-
.setUnit("ms")
75-
.build();
76-
this.gfeHeaderMissingCountRecorder =
77-
meter
78-
.counterBuilder(serviceName + '/' + BuiltInMetricsConstant.GFE_CONNECTIVITY_ERROR_NAME)
79-
.setDescription("Number of requests that failed to reach the Google network.")
80-
.setUnit("1")
81-
.build();
82-
this.afeHeaderMissingCountRecorder =
83-
meter
84-
.counterBuilder(serviceName + '/' + BuiltInMetricsConstant.AFE_CONNECTIVITY_ERROR_NAME)
85-
.setDescription("Number of requests that failed to reach the Spanner API Frontend.")
86-
.setUnit("1")
8762
.build();
8863
}
8964

@@ -94,25 +69,8 @@ class BuiltInMetricsRecorder extends OpenTelemetryMetricsRecorder {
9469
* @param gfeLatency Attempt Latency in ms
9570
* @param attributes Map of the attributes to store
9671
*/
97-
void recordServerTimingHeaderMetrics(
98-
Long gfeLatency,
99-
Long afeLatency,
100-
Long gfeHeaderMissingCount,
101-
Long afeHeaderMissingCount,
102-
Map<String, String> attributes) {
103-
io.opentelemetry.api.common.Attributes otelAttributes = toOtelAttributes(attributes);
104-
if (gfeLatency != null) {
105-
gfeLatencyRecorder.record(gfeLatency, otelAttributes);
106-
}
107-
if (gfeHeaderMissingCount > 0) {
108-
gfeHeaderMissingCountRecorder.add(gfeHeaderMissingCount, otelAttributes);
109-
}
110-
if (afeLatency != null) {
111-
afeLatencyRecorder.record(afeLatency, otelAttributes);
112-
}
113-
if (afeHeaderMissingCount > 0) {
114-
afeHeaderMissingCountRecorder.add(afeHeaderMissingCount, otelAttributes);
115-
}
72+
void recordGFELatency(double gfeLatency, Map<String, String> attributes) {
73+
gfeLatencyRecorder.record(gfeLatency, toOtelAttributes(attributes));
11674
}
11775

11876
Attributes toOtelAttributes(Map<String, String> attributes) {

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java

Copy file name to clipboardExpand all lines: google-cloud-spanner/src/main/java/com/google/cloud/spanner/BuiltInMetricsTracer.java
+21-30Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,8 @@ class BuiltInMetricsTracer extends MetricsTracer implements ApiTracer {
3737
private final BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder;
3838
// These are RPC specific attributes and pertain to a specific API Trace
3939
private final Map<String, String> attributes = new HashMap<>();
40+
4041
private Long gfeLatency = null;
41-
private Long afeLatency = null;
42-
private long gfeHeaderMissingCount = 0;
43-
private long afeHeaderMissingCount = 0;
4442

4543
BuiltInMetricsTracer(
4644
MethodName methodName, BuiltInMetricsRecorder builtInOpenTelemetryMetricsRecorder) {
@@ -56,9 +54,10 @@ class BuiltInMetricsTracer extends MetricsTracer implements ApiTracer {
5654
@Override
5755
public void attemptSucceeded() {
5856
super.attemptSucceeded();
59-
attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString());
60-
builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics(
61-
gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes);
57+
if (gfeLatency != null) {
58+
attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.OK.toString());
59+
builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes);
60+
}
6261
}
6362

6463
/**
@@ -68,9 +67,10 @@ public void attemptSucceeded() {
6867
@Override
6968
public void attemptCancelled() {
7069
super.attemptCancelled();
71-
attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.CANCELLED.toString());
72-
builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics(
73-
gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes);
70+
if (gfeLatency != null) {
71+
attributes.put(STATUS_ATTRIBUTE, StatusCode.Code.CANCELLED.toString());
72+
builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes);
73+
}
7474
}
7575

7676
/**
@@ -84,9 +84,10 @@ public void attemptCancelled() {
8484
@Override
8585
public void attemptFailedDuration(Throwable error, java.time.Duration delay) {
8686
super.attemptFailedDuration(error, delay);
87-
attributes.put(STATUS_ATTRIBUTE, extractStatus(error));
88-
builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics(
89-
gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes);
87+
if (gfeLatency != null) {
88+
attributes.put(STATUS_ATTRIBUTE, extractStatus(error));
89+
builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes);
90+
}
9091
}
9192

9293
/**
@@ -99,9 +100,10 @@ public void attemptFailedDuration(Throwable error, java.time.Duration delay) {
99100
@Override
100101
public void attemptFailedRetriesExhausted(Throwable error) {
101102
super.attemptFailedRetriesExhausted(error);
102-
attributes.put(STATUS_ATTRIBUTE, extractStatus(error));
103-
builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics(
104-
gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes);
103+
if (gfeLatency != null) {
104+
attributes.put(STATUS_ATTRIBUTE, extractStatus(error));
105+
builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes);
106+
}
105107
}
106108

107109
/**
@@ -114,27 +116,16 @@ public void attemptFailedRetriesExhausted(Throwable error) {
114116
@Override
115117
public void attemptPermanentFailure(Throwable error) {
116118
super.attemptPermanentFailure(error);
117-
attributes.put(STATUS_ATTRIBUTE, extractStatus(error));
118-
builtInOpenTelemetryMetricsRecorder.recordServerTimingHeaderMetrics(
119-
gfeLatency, afeLatency, gfeHeaderMissingCount, afeHeaderMissingCount, attributes);
119+
if (gfeLatency != null) {
120+
attributes.put(STATUS_ATTRIBUTE, extractStatus(error));
121+
builtInOpenTelemetryMetricsRecorder.recordGFELatency(gfeLatency, attributes);
122+
}
120123
}
121124

122125
void recordGFELatency(Long gfeLatency) {
123126
this.gfeLatency = gfeLatency;
124127
}
125128

126-
void recordAFELatency(Long afeLatency) {
127-
this.afeLatency = afeLatency;
128-
}
129-
130-
void recordGfeHeaderMissingCount(Long value) {
131-
this.gfeHeaderMissingCount = value;
132-
}
133-
134-
void recordAfeHeaderMissingCount(Long value) {
135-
this.afeHeaderMissingCount = value;
136-
}
137-
138129
@Override
139130
public void addAttributes(Map<String, String> attributes) {
140131
super.addAttributes(attributes);

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java

Copy file name to clipboardExpand all lines: google-cloud-spanner/src/main/java/com/google/cloud/spanner/CompositeTracer.java
-24Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -198,28 +198,4 @@ public void recordGFELatency(Long gfeLatency) {
198198
}
199199
}
200200
}
201-
202-
public void recordGfeHeaderMissingCount(Long value) {
203-
for (ApiTracer child : children) {
204-
if (child instanceof BuiltInMetricsTracer) {
205-
((BuiltInMetricsTracer) child).recordGfeHeaderMissingCount(value);
206-
}
207-
}
208-
}
209-
210-
public void recordAFELatency(Long afeLatency) {
211-
for (ApiTracer child : children) {
212-
if (child instanceof BuiltInMetricsTracer) {
213-
((BuiltInMetricsTracer) child).recordAFELatency(afeLatency);
214-
}
215-
}
216-
}
217-
218-
public void recordAfeHeaderMissingCount(Long value) {
219-
for (ApiTracer child : children) {
220-
if (child instanceof BuiltInMetricsTracer) {
221-
((BuiltInMetricsTracer) child).recordAfeHeaderMissingCount(value);
222-
}
223-
}
224-
}
225201
}

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

Copy file name to clipboardExpand all lines: google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java
-7Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -683,10 +683,6 @@ private static boolean isEmulatorEnabled(SpannerOptions options, String emulator
683683
&& options.getHost().endsWith(emulatorHost);
684684
}
685685

686-
public static boolean isEnableAFEServerTiming() {
687-
return !Boolean.parseBoolean(System.getenv("SPANNER_DISABLE_AFE_SERVER_TIMING"));
688-
}
689-
690686
private static final RetrySettings ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS =
691687
RetrySettings.newBuilder()
692688
.setInitialRetryDelayDuration(Duration.ofSeconds(5L))
@@ -2034,9 +2030,6 @@ <ReqT, RespT> GrpcCallContext newCallContext(
20342030
if (endToEndTracingEnabled) {
20352031
context = context.withExtraHeaders(metadataProvider.newEndToEndTracingHeader());
20362032
}
2037-
if (isEnableAFEServerTiming()) {
2038-
context = context.withExtraHeaders(metadataProvider.newAfeServerTimingHeader());
2039-
}
20402033
if (callCredentialsProvider != null) {
20412034
CallCredentials callCredentials = callCredentialsProvider.getCallCredentials();
20422035
if (callCredentials != null) {

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

Copy file name to clipboardExpand all lines: google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/HeaderInterceptor.java
+1-14Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ class HeaderInterceptor implements ClientInterceptor {
7272
private static final Metadata.Key<String> SERVER_TIMING_HEADER_KEY =
7373
Metadata.Key.of("server-timing", Metadata.ASCII_STRING_MARSHALLER);
7474
private static final String GFE_TIMING_HEADER = "gfet4t7";
75-
private static final String AFE_TIMING_HEADER = "afet4t7";
7675
private static final Metadata.Key<String> GOOGLE_CLOUD_RESOURCE_PREFIX_KEY =
7776
Metadata.Key.of("google-cloud-resource-prefix", Metadata.ASCII_STRING_MARSHALLER);
7877
private static final Pattern SERVER_TIMING_PATTERN =
@@ -175,25 +174,13 @@ private void processHeader(
175174
if (compositeTracer != null) {
176175
compositeTracer.recordGFELatency(gfeLatency);
177176
}
177+
178178
if (span != null) {
179179
span.setAttribute("gfe_latency", String.valueOf(gfeLatency));
180180
}
181181
} else {
182182
measureMap.put(SPANNER_GFE_HEADER_MISSING_COUNT, 1L).record(tagContext);
183183
spannerRpcMetrics.recordGfeHeaderMissingCount(1L, attributes);
184-
if (compositeTracer != null) {
185-
compositeTracer.recordGfeHeaderMissingCount(1L);
186-
}
187-
}
188-
189-
// Record AFE metrics
190-
if (compositeTracer != null && GapicSpannerRpc.isEnableAFEServerTiming()) {
191-
if (serverTimingMetrics.containsKey(AFE_TIMING_HEADER)) {
192-
long afeLatency = serverTimingMetrics.get(AFE_TIMING_HEADER);
193-
compositeTracer.recordAFELatency(afeLatency);
194-
} else {
195-
compositeTracer.recordAfeHeaderMissingCount(1L);
196-
}
197184
}
198185
} catch (NumberFormatException e) {
199186
LOGGER.log(LEVEL, "Invalid server-timing object in header: {}", serverTiming);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.