refactor(zio-opentelemetry): improve logging layer and update error handling in tracing

Author: cheleb

observability/zio-opentelemetry/src/main/scala/sttp/tapir/server/ziopentelemetry/ZIOpenTelemetryApp.scala

@@ -96,23 +96,22 @@ trait ZIOpenTelemetryApp extends ZIOApp {
     tracer <- tracerProvider
   } yield OtelProviders(tracer, meter, logger))
 
-  /** The console log layer, provide a console logger to the ZIO application.
+  /** The base log layer.
     *
     * Default implementation does nothing, hence uses the default ZIO console logger, which logs to stdout.
     *
-    * You should override this to use a different logger:
+    * You should override this to use a different logger.
+    * Exmples include:
+    *.  - To bridge SLF4J to ZIO logging, you can use the following layer:
+    * {{{
+    *   override def consoleLogLayer: ZLayer[Any, Nothing, Unit] = Slf4jBridge.init(...)
+    * }}}
     *   - No logs at all:
-    *
     * {{{
     *   override def consoleLogLayer: ZLayer[Any, Nothing, Unit] = Runtime.removeDefaultLoggers
     * }}}
-    *
-    *   - SLF4J, Logback, etc. To use SLF4J, you can use the following layer:
-    *     {{{
-    *   override def consoleLogLayer: ZLayer[Any, Nothing, Unit] = Runtime.removeDefaultLoggers >>> SLF4J.slf4j
-    *     }}}
     */
-  def consoleLogLayer: ZLayer[Any, Nothing, Unit] = ZLayer.unit
+  def baseLogLayer: ZLayer[Any, Nothing, Unit] = ZLayer.unit
 
   /** The OpenTelemetry metrics layer for the ZIOpenTelemetry trait.
     *
@@ -154,7 +153,7 @@ trait ZIOpenTelemetryApp extends ZIOApp {
   )
 
   def observabilityLayer: ULayer[ContextStorage & OpenTelemetrySdk & Meter & Tracing] =
-    consoleLogLayer >>>
+    baseLogLayer >>>
       OpenTelemetry.contextZIO >+> (otelProviders >>>
         openTelemetryLive) >+> (otel4zLogging ++ zioMetrics ++ otel4zMetrics(resourceName) ++ otel4zTracing(resourceName))
 }

observability/zio-opentelemetry/src/main/scala/sttp/tapir/server/ziopentelemetry/ZIOpenTelemetryTracing.scala

@@ -82,6 +82,9 @@ class ZIOpenTelemetryTracing(
         )
         .flatMap { case (span, finalize) =>
           handleRequest(span, finalize, request, endpoints)
+            .tapError { e =>
+                spanError(span, finalize)(Right(e))
+             }
         }
 
       /** Handle the request, setting span attributes and status based on the result.
@@ -136,7 +139,7 @@ class ZIOpenTelemetryTracing(
             }
             ZIO.succeed(response.copy(body = Some(Right(ZioStreamHttpResponseBody(wrapped, contentLength)).asInstanceOf[B])))
           case _ =>
-            ZIO.when(response.isServerError)(
+            ZIO.when(response.isServerError || response.isClientError)(
               spanError(span, finalize)(Left(response.code))
             ) *> finalize *> ZIO.succeed(response)
         })

observability/zio-opentelemetry/src/test/scala/sttp/tapir/server/ziopentelemetry/ZIOpenTelemetryTracingSpec.scala

@@ -357,7 +357,7 @@ object ZIOpenTelemetryTracingSpec extends ZIOSpecDefault {
         assert(finishedSpans.get(0).getAttributes.get(ErrorAttributes.ERROR_TYPE))(equalTo("RuntimeException"))
       }
     },
-    test("4xx client error does NOT set span error status") {
+    test("4xx client error does  set span error status") {
       val ep = endpoint
         .in("client-error")
         .out(stringBody)
@@ -369,7 +369,7 @@ object ZIOpenTelemetryTracingSpec extends ZIOSpecDefault {
         span <- runRequestSingleSpan(List(ep), request)
       } yield {
         // 4xx is not a server error, so span status should NOT be ERROR
-        assert(span.getStatus.getStatusCode)(not(equalTo(OtelStatusCode.ERROR))) &&
+        assert(span.getStatus.getStatusCode)(equalTo(OtelStatusCode.ERROR)) &&
         assert(span.getAttributes.get(HttpAttributes.HTTP_RESPONSE_STATUS_CODE))(equalTo(java.lang.Long.valueOf(400L)))
       }
     }
@@ -437,7 +437,7 @@ object ZIOpenTelemetryTracingSpec extends ZIOSpecDefault {
   // ─── Endpoint Matching Behavior ────────────────────────────────────────────
 
   private val endpointMatchingSuite = suite("Endpoint Matching Behavior")(
-    test("unmatched request does not set error status") {
+    test("unmatched request does set error status") {
       val ep = endpoint
         .in("person")
         .out(stringBody)
@@ -450,7 +450,7 @@ object ZIOpenTelemetryTracingSpec extends ZIOSpecDefault {
         spans <- runRequest(List(ep), request)
       } yield {
         assertTrue(spans.size() == 1) &&
-        assert(spans.get(0).getStatus.getStatusCode)(not(equalTo(OtelStatusCode.ERROR)))
+        assert(spans.get(0).getStatus.getStatusCode)(equalTo(OtelStatusCode.ERROR))
       }
     },
     test("decode failure on matched endpoint - span is created with initial name") {
@@ -471,7 +471,7 @@ object ZIOpenTelemetryTracingSpec extends ZIOSpecDefault {
         // the endpoint is not confirmed, so span retains the initial request-based name.
         assertTrue(spans.size() >= 1) &&
         assertTrue(spans.get(0).getKind == SpanKind.SERVER) &&
-        assert(spans.get(0).getStatus.getStatusCode)(not(equalTo(OtelStatusCode.ERROR)))
+        assert(spans.get(0).getStatus.getStatusCode)(equalTo(OtelStatusCode.ERROR))
       }
     },
     test("security failure on matched endpoint updates span name") {