clean up

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

@@ -183,7 +183,7 @@ private Response<?> attempt(final HttpRequest httpRequest, final HttpPipelineNex
 
                 return attempt(httpRequest, next, tryCount + 1, suppressedLocal);
             } else {
-                logRetry(logger.atError(), tryCount, null, err, true, instrumentationContext);
+                logRetry(logger.atWarning(), tryCount, null, err, true, instrumentationContext);
 
                 if (suppressed != null) {
                     suppressed.forEach(err::addSuppressed);

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/instrumentation/fallback/FallbackSpan.java

@@ -74,11 +74,7 @@ public void end(Throwable error) {
         }
 
         log.setEventName(SPAN_ENDED_EVENT_NAME);
-        if (error != null) {
-            log.log(null, error);
-        } else {
-            log.log(null);
-        }
+        log.log(null);
     }
 
     /**

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/instrumentation/fallback/FallbackSpanBuilder.java

@@ -26,7 +26,7 @@ private FallbackSpanBuilder() {
     FallbackSpanBuilder(ClientLogger logger, String spanName, SpanKind spanKind,
         InstrumentationContext instrumentationContext) {
         this.parentSpanContext = FallbackSpanContext.fromInstrumentationContext(instrumentationContext);
-        this.log = logger.atInfo();
+        this.log = logger.atVerbose();
         if (log.isEnabled()) {
             log.addKeyValue(SPAN_NAME_KEY, spanName).addKeyValue(SPAN_KIND_KEY, spanKind.name());
             if (parentSpanContext.isValid()) {

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/instrumentation/fallback/FallbackTracer.java

@@ -11,6 +11,9 @@
 import io.clientcore.core.instrumentation.tracing.SpanKind;
 import io.clientcore.core.instrumentation.tracing.Tracer;
 
+import java.util.HashMap;
+import java.util.Map;
+
 final class FallbackTracer implements Tracer {
     private static final ClientLogger LOGGER = new ClientLogger(FallbackTracer.class);
     private final boolean isEnabled;
@@ -28,7 +31,12 @@ private static ClientLogger getLogger(InstrumentationOptions<?> instrumentationO
         if (providedLogger instanceof ClientLogger) {
             return (ClientLogger) providedLogger;
         }
-        return new ClientLogger(libraryOptions.getLibraryName() + ".tracing");
+
+        Map<String, Object> libraryContext = new HashMap<>(2);
+        libraryContext.put("library.name", libraryOptions.getLibraryName());
+        libraryContext.put("library.version", libraryOptions.getLibraryVersion());
+
+        return new ClientLogger(libraryOptions.getLibraryName() + ".tracing", libraryContext);
     }
 
     /**

sdk/clientcore/core/src/main/java/io/clientcore/core/instrumentation/logging/ClientLogger.java

@@ -67,8 +67,20 @@ public ClientLogger(Class<?> clazz) {
      * @throws RuntimeException when logging configuration is invalid depending on SLF4J implementation.
      */
     public ClientLogger(String className) {
+        this(className, null);
+    }
+
+    /**
+     * Retrieves a logger for the passed class name.
+     *
+     * @param className Class name creating the logger.
+     * @param context Context to be populated on every log record written with this logger.
+     * Objects are serialized with {@code toString()} method.
+     * @throws RuntimeException when logging configuration is invalid depending on SLF4J implementation.
+     */
+    public ClientLogger(String className, Map<String, Object> context) {
         logger = new Slf4jLoggerShim(getClassPathFromClassName(className));
-        globalContext = null;
+        globalContext = context == null ? null : Collections.unmodifiableMap(context);
     }
 
     /**
@@ -508,10 +520,6 @@ public <T extends Throwable> T log(String message, T throwable) {
         }
 
         private String getMessageWithContext(String message) {
-            if (message == null) {
-                message = "";
-            }
-
             if (this.context != null && this.context.isValid()) {
                 // TODO (limolkova) we can set context from implicit current span
                 // we should also support OTel as a logging provider and avoid adding redundant
@@ -523,10 +531,16 @@ private String getMessageWithContext(String message) {
 
             int pairsCount
                 = (keyValuePairs == null ? 0 : keyValuePairs.size()) + (globalPairs == null ? 0 : globalPairs.size());
-            int speculatedSize = 20 + pairsCount * 20 + message.length();
+
+            int messageLength = message == null ? 0 : message.length();
+            int speculatedSize = 20 + pairsCount * 20 + messageLength;
             try (AccessibleByteArrayOutputStream outputStream = new AccessibleByteArrayOutputStream(speculatedSize);
                 JsonWriter jsonWriter = DefaultJsonWriter.toStream(outputStream, null)) {
-                jsonWriter.writeStartObject().writeStringField("message", message);
+                jsonWriter.writeStartObject();
+
+                if (message != null) {
+                    jsonWriter.writeStringField("message", message);
+                }
 
                 if (globalPairs != null) {
                     for (Map.Entry<String, Object> kvp : globalPairs.entrySet()) {

sdk/clientcore/core/src/test/java/io/clientcore/core/http/pipeline/HttpInstrumentationLoggingTests.java

@@ -108,10 +108,10 @@ public void testBasicHttpLogging(Set<String> allowedParams, String expectedUri)
         assertEquals(2, logMessages.size());
 
         assertRequestLog(logMessages.get(0), expectedUri, request, null, 0);
-        assertEquals(8, logMessages.get(0).size());
+        assertEquals(7, logMessages.get(0).size());
 
         assertResponseLog(logMessages.get(1), expectedUri, response, 0);
-        assertEquals(12, logMessages.get(1).size());
+        assertEquals(11, logMessages.get(1).size());
     }
 
     @Test
@@ -130,10 +130,10 @@ public void testHttpLoggingTracingDisabled() throws IOException {
         assertEquals(2, logMessages.size());
 
         assertRequestLog(logMessages.get(0), request);
-        assertEquals(6, logMessages.get(0).size());
+        assertEquals(5, logMessages.get(0).size());
 
         assertResponseLog(logMessages.get(1), response);
-        assertEquals(10, logMessages.get(1).size());
+        assertEquals(9, logMessages.get(1).size());
     }
 
     @Test
@@ -155,10 +155,10 @@ public void testHttpLoggingTracingDisabledCustomContext() throws IOException {
         assertEquals(2, logMessages.size());
 
         assertRequestLog(logMessages.get(0), request);
-        assertEquals(8, logMessages.get(0).size());
+        assertEquals(7, logMessages.get(0).size());
 
         assertResponseLog(logMessages.get(1), response);
-        assertEquals(12, logMessages.get(1).size());
+        assertEquals(11, logMessages.get(1).size());
     }
 
     @Test
@@ -306,7 +306,7 @@ public void testBasicHttpLoggingRequestOff(Set<String> allowedParams, String exp
         assertEquals(1, logMessages.size());
 
         assertResponseLog(logMessages.get(0), expectedUri, response, 0);
-        assertEquals(12, logMessages.get(0).size());
+        assertEquals(11, logMessages.get(0).size());
     }
 
     @ParameterizedTest
@@ -624,7 +624,7 @@ public void tracingWithRedirects() throws IOException {
         assertResponseLog(logMessages.get(1), REDACTED_URI, 0, 302, firstRedirectContext.get());
 
         assertRedirectLog(logMessages.get(2), 0, 3, true, "http://redirecthost/1?param=REDACTED&api-version=REDACTED",
-            HttpMethod.GET, "", parentContext);
+            HttpMethod.GET, null, parentContext);
         assertResponseLog(logMessages.get(4), "http://redirecthost/1?param=REDACTED&api-version=42", response, 0);
     }
 
@@ -670,7 +670,7 @@ public void redirectToTheSameUri() throws IOException {
         List<Map<String, Object>> logMessages = parseLogMessages(logCaptureStream);
 
         assertEquals(2, logMessages.size());
-        assertRedirectLog(logMessages.get(0), 0, 3, true, "http://redirecthost/", HttpMethod.GET, "", parentContext);
+        assertRedirectLog(logMessages.get(0), 0, 3, true, "http://redirecthost/", HttpMethod.GET, null, parentContext);
         assertRedirectLog(logMessages.get(1), 1, 3, false, "http://redirecthost/", HttpMethod.GET,
             "Request was redirected more than once to the same URI.", parentContext);
     }
@@ -695,8 +695,8 @@ public void redirectAttemptsExhausted() throws IOException {
         List<Map<String, Object>> logMessages = parseLogMessages(logCaptureStream);
 
         assertEquals(3, logMessages.size());
-        assertRedirectLog(logMessages.get(0), 0, 3, true, "http://redirecthost/1", HttpMethod.GET, "", parentContext);
-        assertRedirectLog(logMessages.get(1), 1, 3, true, "http://redirecthost/2", HttpMethod.GET, "", parentContext);
+        assertRedirectLog(logMessages.get(0), 0, 3, true, "http://redirecthost/1", HttpMethod.GET, null, parentContext);
+        assertRedirectLog(logMessages.get(1), 1, 3, true, "http://redirecthost/2", HttpMethod.GET, null, parentContext);
         assertRedirectLog(logMessages.get(2), 2, 3, false, "http://redirecthost/3", HttpMethod.GET,
             "Redirect attempts have been exhausted.", parentContext);
     }
@@ -788,7 +788,7 @@ private void assertRequestLog(Map<String, Object> log, String expectedUri, HttpR
 
         assertEquals(getLength(request.getBody(), request.getHeaders()), (int) log.get("http.request.body.size"));
         assertEquals(request.getHttpMethod().toString(), log.get("http.request.method"));
-        assertEquals("", log.get("message"));
+        assertNull(log.get("message"));
 
         if (context == null) {
             context = request.getRequestOptions().getInstrumentationContext();
@@ -809,7 +809,7 @@ private void assertRetryLog(Map<String, Object> log, int tryCount, int maxAttemp
             assertTrue((boolean) log.get("retry.was_last_attempt"));
         }
         assertEquals(maxAttempts, log.get("retry.max_attempt_count"));
-        assertEquals("", log.get("message"));
+        assertNull(log.get("message"));
         assertTraceContext(log, context);
     }
 
@@ -882,7 +882,7 @@ private void assertResponseLog(Map<String, Object> log, String expectedUri, int
 
         assertInstanceOf(Double.class, log.get("http.request.time_to_response"));
         assertInstanceOf(Double.class, log.get("http.request.duration"));
-        assertEquals("", log.get("message"));
+        assertNull(log.get("message"));
         assertTraceContext(log, context);
     }
 
@@ -903,7 +903,7 @@ private void assertResponseAndExceptionLog(Map<String, Object> log, String expec
         assertInstanceOf(Double.class, log.get("http.request.duration"));
         assertEquals(error.getMessage(), log.get("exception.message"));
         assertEquals(error.getClass().getCanonicalName(), log.get("exception.type"));
-        assertEquals("", log.get("message"));
+        assertNull(log.get("message"));
         assertTraceContext(log, response.getRequest().getRequestOptions().getInstrumentationContext());
     }
 
@@ -927,8 +927,7 @@ private void assertExceptionLog(Map<String, Object> log, String expectedUri, Htt
         assertInstanceOf(Double.class, log.get("http.request.duration"));
         assertEquals(error.getMessage(), log.get("exception.message"));
         assertEquals(error.getClass().getCanonicalName(), log.get("exception.type"));
-
-        assertEquals("", log.get("message"));
+        assertNull(log.get("message"));
 
         if (context == null) {
             context = request.getRequestOptions().getInstrumentationContext();

sdk/clientcore/core/src/test/java/io/clientcore/core/implementation/instrumentation/fallback/FallbackInstrumentationTests.java

@@ -45,6 +45,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -385,13 +386,14 @@ public void basicTracingLogsLevel(ClientLogger.LogLevel logLevel, boolean expect
 
     public static Stream<Arguments> logLevels() {
         return Stream.of(Arguments.of(ClientLogger.LogLevel.ERROR, false),
-            Arguments.of(ClientLogger.LogLevel.WARNING, false), Arguments.of(ClientLogger.LogLevel.INFORMATIONAL, true),
+            Arguments.of(ClientLogger.LogLevel.WARNING, false),
+            Arguments.of(ClientLogger.LogLevel.INFORMATIONAL, false),
             Arguments.of(ClientLogger.LogLevel.VERBOSE, true));
     }
 
     @Test
     public void basicTracingLogsEnabled() {
-        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.INFORMATIONAL, logCaptureStream);
+        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.VERBOSE, logCaptureStream);
         InstrumentationOptions<?> options = new InstrumentationOptions<>().setProvider(logger);
         Instrumentation instrumentation = Instrumentation.create(options, DEFAULT_LIB_OPTIONS);
         Tracer tracer = instrumentation.getTracer();
@@ -410,11 +412,16 @@ public void basicTracingLogsEnabled() {
         Map<String, Object> loggedSpan = logMessages.get(0);
         assertSpanLog(loggedSpan, "test-span", "INTERNAL", span.getInstrumentationContext(), null);
         assertTrue((Double) loggedSpan.get("span.duration") <= duration.toNanos() / 1_000_000.0);
+
+        // lib info is null since custom logger is provided, we can't add global context.
+        // we'll add it in user app in common case
+        assertNull(loggedSpan.get("library.name"));
+        assertNull(loggedSpan.get("library.version"));
     }
 
     @Test
     public void tracingWithAttributesLogsEnabled() {
-        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.INFORMATIONAL, logCaptureStream);
+        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.VERBOSE, logCaptureStream);
         InstrumentationOptions<?> options = new InstrumentationOptions<>().setProvider(logger);
         Tracer tracer = Instrumentation.create(options, DEFAULT_LIB_OPTIONS).getTracer();
 
@@ -452,7 +459,7 @@ public void tracingWithAttributesLogsEnabled() {
 
     @Test
     public void tracingWithExceptionLogsEnabled() {
-        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.INFORMATIONAL, logCaptureStream);
+        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.VERBOSE, logCaptureStream);
         InstrumentationOptions<?> options = new InstrumentationOptions<>().setProvider(logger);
         Tracer tracer = Instrumentation.create(options, DEFAULT_LIB_OPTIONS).getTracer();
 
@@ -466,13 +473,11 @@ public void tracingWithExceptionLogsEnabled() {
         Map<String, Object> loggedSpan = logMessages.get(0);
         assertSpanLog(loggedSpan, "test-span", "SERVER", span.getInstrumentationContext(),
             exception.getClass().getCanonicalName());
-        assertEquals(exception.getMessage(), loggedSpan.get("exception.message"));
-        assertEquals("java.io.IOException", loggedSpan.get("exception.type"));
     }
 
     @Test
     public void tracingLogsEnabledParent() {
-        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.INFORMATIONAL, logCaptureStream);
+        ClientLogger logger = setupLogLevelAndGetLogger(ClientLogger.LogLevel.VERBOSE, logCaptureStream);
         InstrumentationOptions<?> options = new InstrumentationOptions<>().setProvider(logger);
         Tracer tracer = Instrumentation.create(options, DEFAULT_LIB_OPTIONS).getTracer();
 

sdk/clientcore/core/src/test/java/io/clientcore/core/instrumentation/logging/ClientLoggerTests.java

@@ -262,7 +262,7 @@ public void logWithNullSupplier(LogLevel logLevel) {
         }
 
         String logValues = byteArraySteamToString(logCaptureStream);
-        assertMessage(Collections.singletonMap("message", ""), logValues, logLevel, logLevel);
+        assertMessage(Collections.emptyMap(), logValues, logLevel, logLevel);
     }
 
     @ParameterizedTest
@@ -443,7 +443,6 @@ public void logWithContextNullMessage() {
         logger.atVerbose().addKeyValue("connectionId", "foo").addKeyValue("linkName", true).log(null);
 
         Map<String, Object> expectedMessage = new HashMap<>();
-        expectedMessage.put("message", "");
         expectedMessage.put("connectionId", "foo");
         expectedMessage.put("linkName", true);
 
@@ -507,7 +506,6 @@ public void logWithContextNullSupplier() {
         logger.atError().addKeyValue("connectionId", "foo").addKeyValue("linkName", (String) null).log(null);
 
         Map<String, Object> expectedMessage = new HashMap<>();
-        expectedMessage.put("message", "");
         expectedMessage.put("connectionId", "foo");
         expectedMessage.put("linkName", null);
 
@@ -679,7 +677,6 @@ public void logWithContextRuntimeException(LogLevel logLevelToConfigure) {
                 .log(null, runtimeException));
 
         Map<String, Object> expectedMessage = new HashMap<>();
-        expectedMessage.put("message", "");
         expectedMessage.put("connectionId", "foo");
         expectedMessage.put("linkName", "bar");
         expectedMessage.put("exception.type", runtimeException.getClass().getCanonicalName());
@@ -710,7 +707,6 @@ public void logWithContextThrowable(LogLevel logLevelToConfigure) {
                 .log(null, ioException));
 
         Map<String, Object> expectedMessage = new HashMap<>();
-        expectedMessage.put("message", "");
         expectedMessage.put("connectionId", "foo");
         expectedMessage.put("linkName", "bar");
         expectedMessage.put("exception.type", ioException.getClass().getCanonicalName());