Skip to content

Commit 2455496

Browse files
nidhiii-27nidhiii-27
authored
refactor(storage): remove gcp.storage.client.retries counter [Generated-by: AI]
1 parent 74489ff commit 2455496

2 files changed

Lines changed: 9 additions & 25 deletions

File tree

‎java-storage/google-cloud-storage/src/main/java/com/google/cloud/storage/Retrying.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,6 @@ public <Response, Model> Model run(
209209
Attributes baseAttributes = baseBuilder.build();
210210

211211
LongCounter attemptsCounter = metrics.getAttempts();
212-
LongCounter retriesCounter = metrics.getRetries();
213212
LongCounter errorsCounter = metrics.getErrors();
214213

215214
try (Scope ignore = activeContext.getOtelContext().makeCurrent()) {
@@ -221,7 +220,6 @@ public <Response, Model> Model run(
221220
alg,
222221
Jitterer.threadLocalRandom()));
223222
AtomicReference<Exception> failure = new AtomicReference<>();
224-
AtomicReference<String> lastAttemptErrorType = new AtomicReference<>("UNKNOWN");
225223
AtomicBoolean attemptAgain = new AtomicBoolean(false);
226224
Throwable terminalThrowable = null;
227225

@@ -235,32 +233,21 @@ public <Response, Model> Model run(
235233
return result == null ? null : d.decode(result);
236234
} catch (StorageException se) {
237235
String errorType = StorageErrorTypeMapper.mapThrowable(se);
238-
lastAttemptErrorType.set(errorType);
239236
recordAttemptFailure(
240237
attemptsCounter, errorsCounter, baseAttributes, activeContext, se, errorType);
241238
ctx.recordError(se, () -> attemptAgain.set(true), failure::set);
242239
} catch (IllegalArgumentException iae) {
243240
String errorType = StorageErrorTypeMapper.mapThrowable(iae);
244-
lastAttemptErrorType.set(errorType);
245241
recordAttemptFailure(
246242
attemptsCounter, errorsCounter, baseAttributes, activeContext, iae, errorType);
247243
ctx.recordError(iae, () -> attemptAgain.set(true), failure::set);
248244
} catch (Exception e) {
249245
String errorType = StorageErrorTypeMapper.mapThrowable(e);
250-
lastAttemptErrorType.set(errorType);
251246
recordAttemptFailure(
252247
attemptsCounter, errorsCounter, baseAttributes, activeContext, e, errorType);
253248
BaseServiceException coalesced = StorageException.coalesce(e);
254249
ctx.recordError(coalesced, () -> attemptAgain.set(true), failure::set);
255250
}
256-
if (attemptAgain.get() && retriesCounter != null) {
257-
Attributes retryAttributes =
258-
baseAttributes.toBuilder()
259-
.put(StorageClientMetrics.KEY_ERROR_TYPE, lastAttemptErrorType.get())
260-
.put(StorageClientMetrics.KEY_NETWORK_PATH, resolveNetworkPath(activeContext))
261-
.build();
262-
retriesCounter.add(1, retryAttributes);
263-
}
264251
} while (attemptAgain.get());
265252

266253
Exception throwable = failure.get();

‎java-storage/google-cloud-storage/src/test/java/com/google/cloud/storage/RetryingMetricsTest.java‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ATTEMPTS;
2626
import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_ERRORS;
2727
import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_OPERATIONS;
28-
import static com.google.cloud.storage.StorageClientMetrics.METRIC_GCP_STORAGE_CLIENT_RETRIES;
2928
import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_DNS_FAILURE;
3029
import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_NOT_FOUND;
3130
import static com.google.cloud.storage.StorageErrorTypeMapper.STATUS_OK;
@@ -99,7 +98,7 @@ public void successfulCallOnFirstAttempt_recordsExpectedMetrics() {
9998
alwaysRetrier.run(
10099
() -> "success", Decoder.identity(), StorageMetricsContext.create("GetObject"));
101100
assertThat(result).isEqualTo("success");
102-
assertOutcomeMetrics(STATUS_OK, 1L, 0L, 0L);
101+
assertOutcomeMetrics(STATUS_OK, 1L, 0L);
103102
assertThat(assertDuration(STATUS_OK).getAttributes().get(KEY_GCP_CLIENT_SERVICE))
104103
.isEqualTo("storage");
105104
}
@@ -121,7 +120,7 @@ public void retriedCallSuccessOnThirdAttempt_recordsRetriesAndErrors() {
121120

122121
assertThat(result).isEqualTo("recovered");
123122
assertThat(attemptCounter.get()).isEqualTo(3);
124-
assertOutcomeMetrics(STATUS_OK, 3L, 2L, 2L);
123+
assertOutcomeMetrics(STATUS_OK, 3L, 2L);
125124
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_UNAVAILABLE, 2L);
126125
assertDuration(STATUS_OK);
127126
}
@@ -143,7 +142,7 @@ public void terminalFailureAllAttemptsFail_recordsExhaustedRetries() {
143142

144143
assertThat(attemptCounter.get()).isEqualTo(3);
145144
assertThat(se.getCode()).isEqualTo(503);
146-
assertOutcomeMetrics(STATUS_UNAVAILABLE, 3L, 2L, 3L);
145+
assertOutcomeMetrics(STATUS_UNAVAILABLE, 3L, 3L);
147146
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_UNAVAILABLE, 3L);
148147
assertDuration(STATUS_UNAVAILABLE);
149148
}
@@ -258,7 +257,7 @@ public void nonRetryableError_singleAttemptTerminalFailure_recordsErrorAndTermin
258257
StorageMetricsContext.create("GetObject")));
259258

260259
assertThat(se.getCode()).isEqualTo(404);
261-
assertOutcomeMetrics(STATUS_NOT_FOUND, 1L, 0L, 1L);
260+
assertOutcomeMetrics(STATUS_NOT_FOUND, 1L, 1L);
262261
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_NOT_FOUND, 1L);
263262
assertDuration(STATUS_NOT_FOUND);
264263
}
@@ -286,7 +285,7 @@ public void recordAttemptError_specificExceptions_mappedToCanonicalTypes() {
286285

287286
assertThat(attemptCounter.get()).isEqualTo(3);
288287
assertThat(se).isNotNull();
289-
assertOutcomeMetrics(STATUS_DNS_FAILURE, 3L, 2L, 3L);
288+
assertOutcomeMetrics(STATUS_DNS_FAILURE, 3L, 3L);
290289
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_TIMEOUT, 2L);
291290
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_DNS_FAILURE, 1L);
292291
}
@@ -306,7 +305,7 @@ public void illegalArgumentException_parsingFailure_recordedAndRethrown() {
306305

307306
assertThat(se.getCause()).isInstanceOf(IllegalArgumentException.class);
308307
assertThat(se.getCause()).hasMessageThat().contains("malformed json response");
309-
assertOutcomeMetrics(STATUS_UNKNOWN, 1L, 0L, 1L);
308+
assertOutcomeMetrics(STATUS_UNKNOWN, 1L, 1L);
310309
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_ERROR_TYPE, STATUS_UNKNOWN, 1L);
311310
}
312311

@@ -367,8 +366,8 @@ public void run_withCustomMetricsContext_recordsOperationOnRetriesAndErrors() {
367366
customContext);
368367
assertThat(result).isEqualTo("recovered");
369368

370-
assertAttr(METRIC_GCP_STORAGE_CLIENT_RETRIES, KEY_GCP_STORAGE_OPERATION, "GetObject", 1L);
371-
assertAttr(METRIC_GCP_STORAGE_CLIENT_RETRIES, KEY_RPC_METHOD, "GetObject", 0L);
369+
assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_GCP_STORAGE_OPERATION, "GetObject", 2L);
370+
assertAttr(METRIC_GCP_STORAGE_CLIENT_ATTEMPTS, KEY_RPC_METHOD, "GetObject", 0L);
372371
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_GCP_STORAGE_OPERATION, "GetObject", 1L);
373372
assertAttr(METRIC_GCP_STORAGE_CLIENT_ERRORS, KEY_RPC_METHOD, "GetObject", 0L);
374373
}
@@ -398,12 +397,10 @@ private Collection<MetricData> collect() {
398397
}
399398

400399
private void assertOutcomeMetrics(
401-
String terminalStatus, long expectedAttempts, long expectedRetries, long expectedErrors) {
400+
String terminalStatus, long expectedAttempts, long expectedErrors) {
402401
assertAttr(METRIC_GCP_STORAGE_CLIENT_OPERATIONS, KEY_ERROR_TYPE, terminalStatus, 1L);
403402
assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ATTEMPTS))
404403
.isEqualTo(expectedAttempts);
405-
assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_RETRIES))
406-
.isEqualTo(expectedRetries);
407404
assertThat(getCounterValue(collect(), METRIC_GCP_STORAGE_CLIENT_ERRORS))
408405
.isEqualTo(expectedErrors);
409406
}

0 commit comments

Comments
 (0)