Skip to content

Commit 98281a1

Browse files
author
Liudmila Molkova
authored
Disambiguate LoggingEvent.log(Supplier<String>) and log(Throwable) - remove log(Throwable) (Azure#44858)
Disambiguate LoggingEvent.log(Supplier<String>) and log(Throwable): remove logThrowable
1 parent 72aed10 commit 98281a1

File tree

27 files changed

+175
-185
lines changed

27 files changed

+175
-185
lines changed

sdk/clientcore/core/src/main/java/io/clientcore/core/http/models/ETag.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ public static ETag fromString(String eTag) {
6666
boolean startsWithQuote = eTag.charAt(0) == '"';
6767
boolean startsWithWeakETagPrefix = eTag.startsWith(WEAK_ETAG_PREFIX_QUOTE);
6868
if (!endsWithQuote || (!startsWithQuote && !startsWithWeakETagPrefix)) {
69-
throw LOGGER.atError()
70-
.addKeyValue("ETag", eTag)
71-
.log(new IllegalArgumentException(
72-
"The ETag should be null, '*', be wrapped in quotes, or be wrapped " + "in quotes prefixed by W/"));
69+
String exceptionMessage = String.format(
70+
"The ETag '%s' is invalid, it should be null, '*', be wrapped in quotes, or be wrapped in quotes prefixed by W/",
71+
eTag);
72+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(exceptionMessage));
7373
}
7474

7575
return new ETag(eTag);

sdk/clientcore/core/src/main/java/io/clientcore/core/http/models/ProxyOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ private static ProxyOptions attemptToLoadSystemProxy(Configuration configuration
320320

321321
return proxyOptions;
322322
} catch (URISyntaxException ex) {
323-
LOGGER.atWarning().addKeyValue(URL_FULL_KEY, proxyProperty).log(INVALID_PROXY_URI, ex);
323+
LOGGER.atWarning().addKeyValue(URL_FULL_KEY, proxyProperty).setThrowable(ex).log(INVALID_PROXY_URI);
324324
return null;
325325
}
326326
}

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpInstrumentationPolicy.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,8 @@ private static Map<String, String> getProperties(String propertiesFileName) {
393393
} catch (IOException ex) {
394394
LOGGER.atWarning()
395395
.addKeyValue("propertiesFileName", propertiesFileName)
396-
.log("Failed to read properties.", ex);
396+
.setThrowable(ex)
397+
.log("Failed to read properties.");
397398
}
398399

399400
return Collections.emptyMap();
@@ -482,7 +483,8 @@ private <T extends Throwable> T logException(ClientLogger logger, HttpRequest re
482483
return throwable;
483484
}
484485

485-
log.setEventName(HTTP_RESPONSE_EVENT_NAME)
486+
log.setThrowable(unwrap(throwable))
487+
.setEventName(HTTP_RESPONSE_EVENT_NAME)
486488
.setInstrumentationContext(context)
487489
.addKeyValue(HTTP_REQUEST_METHOD_KEY, request.getHttpMethod())
488490
.addKeyValue(HTTP_REQUEST_RESEND_COUNT_KEY, tryCount)
@@ -501,7 +503,7 @@ private <T extends Throwable> T logException(ClientLogger logger, HttpRequest re
501503
}
502504
}
503505

504-
log.log(null, unwrap(throwable));
506+
log.log();
505507
return throwable;
506508
}
507509

@@ -576,7 +578,8 @@ private static long getContentLength(ClientLogger logger, BinaryData body, HttpH
576578
.addKeyValue(
577579
isRequest ? HTTP_REQUEST_HEADER_CONTENT_LENGTH_KEY : HTTP_RESPONSE_HEADER_CONTENT_LENGTH_KEY,
578580
contentLengthString)
579-
.log("Could not parse the HTTP header content-length", e);
581+
.setThrowable(e)
582+
.log("Could not parse the HTTP header content-length");
580583
}
581584

582585
return contentLength;

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpPipelineBuilder.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,9 +152,10 @@ public HttpPipelineBuilder addPolicy(HttpPipelinePolicy policy) {
152152

153153
HttpPipelinePosition order = policy.getPipelinePosition();
154154
if (order == null) {
155-
throw LOGGER.atError()
156-
.addKeyValue("policyType", policy.getClass())
157-
.log("Policy order cannot be null.", new IllegalArgumentException("Policy order cannot be null."));
155+
String exceptionMessage
156+
= String.format("%s policy has invalid pipeline position - position cannot be null.",
157+
policy.getClass().getCanonicalName());
158+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(exceptionMessage));
158159
}
159160

160161
if (order == HttpPipelinePosition.BEFORE_REDIRECT) {
@@ -168,10 +169,9 @@ public HttpPipelineBuilder addPolicy(HttpPipelinePolicy policy) {
168169
} else if (order == HttpPipelinePosition.AFTER_INSTRUMENTATION) {
169170
afterInstrumentation.add(policy);
170171
} else {
171-
throw LOGGER.atError()
172-
.addKeyValue("policyType", policy.getClass())
173-
.addKeyValue("order", order)
174-
.log("Unknown policy order.", new IllegalArgumentException("Unknown policy order."));
172+
String exceptionMessage
173+
= String.format("%s policy has unexpected position '%s'.", policy.getClass().getCanonicalName(), order);
174+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(exceptionMessage));
175175
}
176176

177177
return this;

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpRedirectOptions.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public final class HttpRedirectOptions {
3535
public HttpRedirectOptions(int maxAttempts, HttpHeaderName locationHeader,
3636
EnumSet<HttpMethod> allowedRedirectHttpMethods) {
3737
if (maxAttempts < 0) {
38-
throw LOGGER.atError().log(null, new IllegalArgumentException("Max attempts cannot be less than 0."));
38+
throw LOGGER.logThrowableAsError(new IllegalArgumentException("Max attempts cannot be less than 0."));
3939
}
4040
this.maxAttempts = maxAttempts;
4141
this.allowedRedirectHttpMethods

sdk/clientcore/core/src/main/java/io/clientcore/core/http/pipeline/HttpRetryPolicy.java

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,17 +303,14 @@ private void logRetry(LoggingEvent log, int tryCount, Duration delayDuration, Th
303303
.addKeyValue(RETRY_MAX_ATTEMPT_COUNT_KEY, maxRetries)
304304
.addKeyValue(RETRY_WAS_LAST_ATTEMPT_KEY, lastTry)
305305
.setEventName(HTTP_RETRY_EVENT_NAME)
306-
.setInstrumentationContext(context);
306+
.setInstrumentationContext(context)
307+
.setThrowable(throwable);
307308

308309
if (delayDuration != null) {
309310
log.addKeyValue(RETRY_DELAY_KEY, delayDuration.toMillis());
310311
}
311312

312-
if (throwable != null) {
313-
log.log(null, throwable);
314-
} else {
315-
log.log();
316-
}
313+
log.log();
317314
}
318315
}
319316

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/ReflectionUtilsMethodHandle.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ final class ReflectionUtilsMethodHandle implements ReflectionUtilsApi {
6262
throw (Error) throwable;
6363
} else {
6464
LOGGER.atInfo()
65+
.setThrowable(throwable)
6566
.log("Unable to create MethodHandles to use Java 9+ MethodHandles.privateLookupIn. Will "
66-
+ "attempt to fallback to using the package-private constructor.", throwable);
67+
+ "attempt to fallback to using the package-private constructor.");
6768
}
6869
}
6970

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/instrumentation/otel/OTelInitializer.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ public final class OTelInitializer {
151151

152152
instance = new OTelInitializer(true);
153153
} catch (Throwable t) {
154-
LOGGER.atVerbose().log("OpenTelemetry was not initialized.", t);
154+
LOGGER.atVerbose().setThrowable(t).log("OpenTelemetry was not initialized.");
155155
instance = new OTelInitializer(false);
156156
}
157157

@@ -211,7 +211,7 @@ private OTelInitializer(boolean initialized) {
211211
* @param t the error
212212
*/
213213
public static void initError(ClientLogger logger, Throwable t) {
214-
logger.atVerbose().log("OpenTelemetry version is incompatible.", t);
214+
logger.atVerbose().setThrowable(t).log("OpenTelemetry version is incompatible.");
215215
INSTANCE.initialized = false;
216216
}
217217

@@ -223,7 +223,9 @@ public static void initError(ClientLogger logger, Throwable t) {
223223
*/
224224
public static void runtimeError(ClientLogger logger, Throwable t) {
225225
if (INSTANCE.initialized) {
226-
logger.atWarning().log("Unexpected error when invoking OpenTelemetry, turning tracing off.", t);
226+
logger.atWarning()
227+
.setThrowable(t)
228+
.log("Unexpected error when invoking OpenTelemetry, turning tracing off.");
227229
}
228230

229231
INSTANCE.initialized = false;

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/instrumentation/otel/OTelInstrumentation.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,9 @@ public OTelInstrumentation(InstrumentationOptions applicationOptions, SdkInstrum
120120
String host, int port) {
121121
Object explicitOTel = applicationOptions == null ? null : applicationOptions.getTelemetryProvider();
122122
if (explicitOTel != null && !OTEL_CLASS.isInstance(explicitOTel)) {
123-
throw LOGGER.atError()
124-
.addKeyValue("expectedProvider", OTEL_CLASS.getName())
125-
.addKeyValue("actualProvider", explicitOTel.getClass().getName())
126-
.log("Unexpected telemetry provider type.",
127-
new IllegalArgumentException("Telemetry provider is not an instance of " + OTEL_CLASS.getName()));
123+
String message = String.format("Telemetry provider has invalid type '%s'. Instance of '%s' was expected.",
124+
explicitOTel.getClass().getCanonicalName(), OTEL_CLASS.getCanonicalName());
125+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
128126
}
129127

130128
Object otelInstance = explicitOTel != null ? explicitOTel : GET_GLOBAL_OTEL_INVOKER.invoke();

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/utils/AuthenticateChallengeParser.java

Lines changed: 39 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -173,31 +173,27 @@ public List<AuthenticateChallenge> parse() {
173173
scheme = token.scheme;
174174
} else if (token.token68 != null) {
175175
if (scheme == null) {
176-
throw LOGGER.atError()
177-
.addKeyValue("challenge", challenge)
178-
.log(new IllegalArgumentException("Challenge had token68 before scheme."));
176+
String message = String.format("Challenge '%s' had token68 before scheme.", challenge);
177+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
179178
} else if (token68 != null) {
180-
throw LOGGER.atError()
181-
.addKeyValue("challenge", challenge)
182-
.log(new IllegalArgumentException("Challenge had multiple token68s."));
179+
String message = String.format("Challenge '%s' had multiple token68s.", challenge);
180+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
183181
}
184182

185183
token68 = token.token68;
186184
} else if (token.authParam != null) {
187185
if (scheme == null) {
188-
throw LOGGER.atError()
189-
.addKeyValue("challenge", challenge)
190-
.log(new IllegalArgumentException("Challenge had auth-param before scheme."));
186+
String message = String.format("Challenge '%s' had auth-param before scheme.", challenge);
187+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
191188
}
192189

193190
if (parameters == null) {
194191
parameters = new LinkedHashMap<>();
195192
}
196193

197194
if (parameters.put(token.authParam.getKey(), token.authParam.getValue()) != null) {
198-
throw LOGGER.atError()
199-
.addKeyValue("challenge", challenge)
200-
.log(new IllegalArgumentException("Challenge had duplicate auth-param."));
195+
String message = String.format("Challenge '%s' had duplicate auth-param.", challenge);
196+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
201197
}
202198
}
203199
}
@@ -218,9 +214,8 @@ private AuthenticateChallenge createChallenge(String scheme, String token68, Map
218214
return new AuthenticateChallenge(scheme, token68);
219215
}
220216

221-
throw LOGGER.atError()
222-
.addKeyValue("challenge", challenge)
223-
.log(new IllegalArgumentException("Challenge had both token68 and auth-params."));
217+
String message = String.format("Challenge '%s' had both token68 and auth-params.", challenge);
218+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
224219
}
225220

226221
boolean next() {
@@ -275,10 +270,9 @@ private AuthenticateChallengeToken handleSchemeToken(int schemeStartInclusive, i
275270
char currentChar, boolean alreadyInNextState) {
276271
String scheme = challenge.substring(schemeStartInclusive, schemeEndExclusive);
277272
if (!isValidToken(scheme)) {
278-
throw LOGGER.atError()
279-
.addKeyValue("challenge", challenge)
280-
.addKeyValue("scheme", scheme)
281-
.log(new IllegalArgumentException("Scheme contained an invalid character."));
273+
String message
274+
= String.format("Scheme '%s' in challenge '%s' contained an invalid character.", scheme, challenge);
275+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
282276
}
283277

284278
// Iterate until the next non-space character, unless the scheme terminated with a comma.
@@ -317,10 +311,9 @@ private void handleScheme() {
317311
c = iterateUntilNextNonSpace();
318312
if (c != '=' && c != ',' && currentIndex < challengeLength) {
319313
// The next character is neither a comma nor an equal sign, throw an exception.
320-
throw LOGGER.atError()
321-
.addKeyValue("challenge", challenge)
322-
.log(new IllegalArgumentException(
323-
"Challenge had more than one token68 or auth-param in the same comma separator."));
314+
String message = String.format(
315+
"Challenge '%s' had more than one token68 or auth-param in the same comma separator.", challenge);
316+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
324317
}
325318

326319
if (c == ',' || currentIndex == challengeLength) {
@@ -355,10 +348,9 @@ private void handleScheme() {
355348
// If the character following the last equal sign isn't a comma or end of challenge, there is an error.
356349
c = (c == ',' || currentIndex == challengeLength) ? c : iterateUntilNextNonSpace();
357350
if (currentIndex < challengeLength && c != ',') {
358-
throw LOGGER.atError()
359-
.addKeyValue("challenge", challenge)
360-
.log(new IllegalArgumentException(
361-
"Challenge had more than one token68 or auth-param in the same comma separator."));
351+
String message = String.format(
352+
"Challenge '%s' had more than one token68 or auth-param in the same comma separator.", challenge);
353+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
362354
}
363355
}
364356

@@ -370,10 +362,9 @@ private void handleScheme() {
370362
private char createAuthParamToken(int keyStartInclusive, int keyEndExclusive, char currentChar) {
371363
String authParamKey = challenge.substring(keyStartInclusive, keyEndExclusive);
372364
if (!isValidToken(authParamKey)) {
373-
throw LOGGER.atError()
374-
.addKeyValue("challenge", challenge)
375-
.addKeyValue("authParamKey", authParamKey)
376-
.log(new IllegalArgumentException("Auth-param key contained an invalid character."));
365+
String message = String.format("Auth-param key '%s' in challenge '%s' contained an invalid character.",
366+
authParamKey, challenge);
367+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
377368
}
378369

379370
int start = currentIndex;
@@ -388,9 +379,9 @@ private char createAuthParamToken(int keyStartInclusive, int keyEndExclusive, ch
388379
currentChar = iterateUntil(c1 -> c1 == '"' && challenge.charAt(currentIndex - 1) != '\\');
389380
if (currentChar != '"') {
390381
// Only time this should happen is reaching the end of the challenge.
391-
throw LOGGER.atError()
392-
.addKeyValue("challenge", challenge)
393-
.log(new IllegalArgumentException("Quoted-string was not terminated with a double quote."));
382+
String message = String
383+
.format("Quoted-string in challenge '%s' was not terminated with a double quote.", challenge);
384+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
394385
}
395386

396387
authParamValue = challenge.substring(start, currentIndex).replace("\\\\", "");
@@ -399,10 +390,10 @@ private char createAuthParamToken(int keyStartInclusive, int keyEndExclusive, ch
399390
currentChar = iterateUntil(c1 -> c1 == ' ' || c1 == '\t' || c1 == ',');
400391
authParamValue = challenge.substring(start, currentIndex);
401392
if (!isValidToken(authParamValue)) {
402-
throw LOGGER.atError()
403-
.addKeyValue("challenge", challenge)
404-
.addKeyValue("authParamValue", authParamValue)
405-
.log(new IllegalArgumentException("Auth-param value contained an invalid character."));
393+
String message
394+
= String.format("Auth-param value '%s' in challenge '%s' contained an invalid character.",
395+
authParamValue, challenge);
396+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
406397
}
407398
}
408399

@@ -412,10 +403,9 @@ private char createAuthParamToken(int keyStartInclusive, int keyEndExclusive, ch
412403
// After the scheme only a single token68 or auth-param is allowed. If after any trailing spaces the next
413404
// character isn't a comma throw an exception.
414405
if (currentIndex < challengeLength && currentChar != ',') {
415-
throw LOGGER.atError()
416-
.addKeyValue("challenge", challenge)
417-
.log(new IllegalArgumentException(
418-
"Challenge had more than one token68 or auth-param in the same comma separator."));
406+
String message = String.format(
407+
"Challenge '%s' had more than one token68 or auth-param in the same comma separator.", challenge);
408+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
419409
}
420410

421411
token = new AuthenticateChallengeToken(null, null, new AbstractMap.SimpleEntry<>(authParamKey, authParamValue));
@@ -490,21 +480,18 @@ private static String validateToken68(String challenge, int start, int end) {
490480
while (i < end) {
491481
c = challenge.charAt(i);
492482
if (c != '=') {
493-
throw LOGGER.atError()
494-
.addKeyValue("challenge", challenge)
495-
.addKeyValue("token68", challenge.substring(start, end))
496-
.addKeyValue("character", c)
497-
.log(new IllegalArgumentException("Token68 contained invalid character."));
483+
String message
484+
= String.format("Token68 '%s' in challenge '%s' contained an invalid character '%s'.",
485+
challenge.substring(start, end), challenge, c);
486+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
498487
}
499488

500489
i++;
501490
}
502491
} else if (!isValidToken68Character(c)) {
503-
throw LOGGER.atError()
504-
.addKeyValue("challenge", challenge)
505-
.addKeyValue("token68", challenge.substring(start, end))
506-
.addKeyValue("character", c)
507-
.log(new IllegalArgumentException("Token68 contained invalid character."));
492+
String message = String.format("Token68 '%s' in challenge '%s' contained an invalid character '%s'.",
493+
challenge.substring(start, end), challenge, c);
494+
throw LOGGER.logThrowableAsError(new IllegalArgumentException(message));
508495
}
509496
}
510497

0 commit comments

Comments
 (0)