fix: documenting and updating span / metric error cases

docs-website/docs.json

@@ -181,6 +181,7 @@
                 "icon": "projector",
                 "pages": [
                   "router/metrics-and-monitoring",
+                  "router/metrics-and-monitoring/span-error-status",
                   "router/metrics-and-monitoring/prometheus-metric-reference",
                   "router/metrics-and-monitoring/grafana"
                 ]

docs-website/router/metrics-and-monitoring.mdx

@@ -173,12 +173,16 @@ All metrics are tracked along different dimensions. A dimension is an attribute
 
 You can use the `router.http.requests.error` metric to track errors across router and subgraph requests. In addition to that, you can also use the `router.http.requests` metric to identify errors. We attach the following fields:
 
-* `wg.request.error`: Identify if an error occurred. This applies to a request that didn't result in a successful HTTP or GraphQL response. Only set when it is `true`. Be aware that a Status-Code `200` can still be an error in GraphQL.
+* `wg.request.error`: Identify if a server-side error occurred. This applies to a request that didn't result in a successful HTTP or GraphQL response. Only set when it is `true`. Be aware that a Status-Code `200` can still be an error in GraphQL. Client disconnections are **not** counted as errors.
 
 * `http.status_code`: The status code of the response.
 
 You have two ways to query for errors. Both metrics can be quite useful depending on the query scenario. In general, we recommend to use the `router.http.requests.error` metric.
 
+<Info>
+  For a detailed breakdown of which scenarios mark spans as ERROR and increment error metrics, see [Span Error Status & Error Metrics](/router/open-telemetry/span-error-status).
+</Info>
+
 #### Subgraph specific
 
 * `wg.subgraph.name`: The name of the subgraph

docs-website/router/metrics-and-monitoring/span-error-status.mdx

@@ -0,0 +1,111 @@
+---
+title: "Span Error Status & Error Metrics"
+description: "Understand when the router marks spans as ERROR and how errors are reflected in metrics"
+sidebarTitle: "Span Error Status"
+icon: "triangle-exclamation"
+---
+
+The router uses OpenTelemetry spans to trace the full lifecycle of a GraphQL request. This page documents when and why spans are marked with an `ERROR` status, how errors flow into metrics, and how client-side vs server-side failures are distinguished.
+
+## Overview
+
+Not every unsuccessful response marks a span as `ERROR`. The router distinguishes between:
+
+- **Client-side events**: Events like client disconnections that are not server failures (these do NOT mark spans as ERROR)
+- **Server-side errors**: All other failures — caused by the router, its subgraphs, or invalid client input like malformed queries (these mark spans as ERROR)
+
+## When Spans Are NOT Marked as ERROR
+
+### Client Disconnection / Context Cancellation
+
+When a request's context is canceled (`context.Canceled`), the router does **not** mark the span as `ERROR` and does **not** increment error metrics. This typically occurs when:
+
+- A client disconnects during request processing (the most common cause)
+- The router is shutting down gracefully and cancels in-flight requests
+
+In both cases, the work was **interrupted**, not **failed** — marking it as an error would be misleading:
+
+- It would inflate error rates and trigger false alerts
+- It does not indicate a problem with the router or subgraphs
+- The router logs these events at `DEBUG` level, not `ERROR`
+
+The error is still recorded as a span **event** (via `RecordError`) so it remains visible in traces for debugging, but the span status stays `UNSET`.
+
+The response message is set to `"Client disconnected"` with HTTP status `408 Request Timeout` for observability, but this does not affect span status or error metrics.
+
+<Info>
+  This is distinct from **server timeouts** (`context.DeadlineExceeded`), which indicate that a configured deadline was exceeded. Timeouts ARE tracked as errors because they point to a subgraph or configuration issue that should be investigated.
+</Info>
+
+### Successful Requests
+
+Requests that complete successfully (HTTP 2xx, no subgraph errors) leave the span status as `UNSET` (the OpenTelemetry default for successful operations).
+
+## When Spans Are Marked as ERROR
+
+Any error not listed above is treated as a server-side error. The span status is set to `ERROR`, the error is recorded on the span, and error metrics are incremented. This includes but is not limited to:
+
+### Authentication Failure
+
+When a request fails authentication, both the router root span and the authentication span are marked as `ERROR`. The response returns HTTP `401 Unauthorized`.
+
+### Subgraph Fetch Error
+
+When the router fails to fetch a response from a subgraph (network error, timeout, non-2xx status code), the **Engine - Fetch** span for that subgraph is marked as `ERROR`. The error is also recorded in the `router.http.requests.error` metric with subgraph-level dimensions.
+
+Downstream GraphQL errors from the subgraph response are captured as **span events** on the fetch span, with attributes:
+- `wg.subgraph.error.extended_code`: The error extension code from the subgraph response
+- `wg.subgraph.error.message`: The error message from the subgraph response
+
+### Persisted Operation Error
+
+When a persisted operation cannot be loaded (CDN failure, operation not found), the span is marked as `ERROR`.
+
+### Operation Processing Errors (Parse, Normalize, Validate, Plan)
+
+Each stage of GraphQL operation processing has its own span. If any stage fails, that stage's span is marked as `ERROR`, and the error propagates to the router root span:
+
+- **Operation - Parse**: Malformed GraphQL syntax
+- **Operation - Normalize**: Variable normalization or remapping failures
+- **Operation - Validate**: Query depth violations, validation rule failures
+- **Operation - Plan**: Query plan generation failures
+
+### GraphQL Execution Error
+
+When the GraphQL engine encounters errors during resolution (e.g., subgraph returns errors that prevent successful data merging), the root execution span is marked as `ERROR`. The error is propagated to the router root span.
+
+### Batch Request Error
+
+When a batched GraphQL request fails at the batch-level (malformed JSON array, encoding failure), the request span is marked as `ERROR`.
+
+### Subscription Resolution Failure
+
+When a subscription fails to resolve (excluding client disconnections), the span is marked as `ERROR` and an HTTP `500` response is returned.
+
+### Rate Limit Exceeded
+
+When a request exceeds the configured rate limit, the span is marked as `ERROR`.
+
+### Authorization Failure (In-Resolver)
+
+When field-level authorization fails during resolution, the span is marked as `ERROR`.
+
+## Metrics
+
+- `router.http.requests.error`: A dedicated counter for failed requests. Incremented only for server-side errors.
+- `router.http.requests`: The general request counter. When an error occurs, the `wg.request.error=true` attribute is attached, allowing you to filter error vs non-error requests from the same metric.
+
+Both metrics share the same error classification: a request is counted as an error **only** when it is a server-side failure. Client disconnections are excluded.
+
+## Subgraph-Level Error Tracking
+
+Subgraph errors are tracked at a more granular level on the **Engine - Fetch** span:
+
+1. The fetch span status is set to `ERROR` when `responseInfo.Err` is not nil
+2. Individual downstream errors are recorded as **span events** with error codes and messages
+3. Error codes are deduplicated and sorted to reduce metric cardinality
+4. The `router.http.requests.error` metric is recorded with subgraph-specific dimensions (`wg.subgraph.name`, `wg.subgraph.id`)
+
+<Tip>
+  Use the `router.http.requests.error` metric with the `wg.subgraph.name` dimension to identify which subgraphs are contributing the most errors to your federated graph.
+</Tip>

router-tests/telemetry/span_error_status_test.go

@@ -0,0 +1,178 @@
+package telemetry
+
+import (
+	"context"
+	"net/http"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"github.com/wundergraph/cosmo/router-tests/testenv"
+	"github.com/wundergraph/cosmo/router/pkg/trace/tracetest"
+	"go.opentelemetry.io/otel/codes"
+	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
+	"go.opentelemetry.io/otel/sdk/metric/metricdata"
+	sdktrace "go.opentelemetry.io/otel/sdk/trace"
+	"go.uber.org/zap/zapcore"
+)
+
+// rootSpan returns the last span in the list, which is the root server span.
+// OTEL exports child spans before parents, so the root is always last.
+func rootSpan(spans []sdktrace.ReadOnlySpan) sdktrace.ReadOnlySpan {
+	if len(spans) == 0 {
+		return nil
+	}
+	return spans[len(spans)-1]
+}
+
+func TestClientDisconnectionBehavior(t *testing.T) {
+	t.Parallel()
+
+	t.Run("span status is not error but exception event is recorded", func(t *testing.T) {
+		t.Parallel()
+
+		exporter := tracetest.NewInMemoryExporter(t)
+
+		testenv.Run(t, &testenv.Config{
+			TraceExporter: exporter,
+			Subgraphs: testenv.SubgraphsConfig{
+				Employees: testenv.SubgraphConfig{
+					Delay: 2 * time.Second,
+				},
+			},
+		}, func(t *testing.T, xEnv *testenv.Environment) {
+			ctx, cancel := context.WithTimeout(t.Context(), 200*time.Millisecond)
+			defer cancel()
+
+			req, err := http.NewRequestWithContext(ctx, http.MethodPost, xEnv.GraphQLRequestURL(),
+				strings.NewReader(`{"query":"{ employees { id } }"}`))
+			require.NoError(t, err)
+			req.Header.Set("Content-Type", "application/json")
+
+			client := &http.Client{}
+			_, err = client.Do(req)
+			require.Error(t, err)
+
+			time.Sleep(500 * time.Millisecond)
+
+			rootSpan := rootSpan(exporter.GetSpans().Snapshots())
+			require.NotNil(t, rootSpan, "expected root span to be exported for client disconnections")
+
+			require.NotEqual(t, codes.Error, rootSpan.Status().Code,
+				"root span should not be marked as error for client disconnections")
+
+			hasExceptionEvent := false
+			for _, event := range rootSpan.Events() {
+				if event.Name == "exception" {
+					hasExceptionEvent = true
+					break
+				}
+			}
+			require.True(t, hasExceptionEvent,
+				"root span should have an exception event recorded for client disconnections")
+		})
+	})
+
+	t.Run("error metrics are not inflated but request count is recorded", func(t *testing.T) {
+		t.Parallel()
+
+		metricReader := sdkmetric.NewManualReader()
+
+		testenv.Run(t, &testenv.Config{
+			MetricReader: metricReader,
+			Subgraphs: testenv.SubgraphsConfig{
+				Employees: testenv.SubgraphConfig{
+					Delay: 2 * time.Second,
+				},
+			},
+		}, func(t *testing.T, xEnv *testenv.Environment) {
+			ctx, cancel := context.WithTimeout(t.Context(), 200*time.Millisecond)
+			defer cancel()
+
+			req, err := http.NewRequestWithContext(ctx, http.MethodPost, xEnv.GraphQLRequestURL(),
+				strings.NewReader(`{"query":"{ employees { id } }"}`))
+			require.NoError(t, err)
+			req.Header.Set("Content-Type", "application/json")
+
+			client := &http.Client{}
+			_, _ = client.Do(req)
+
+			time.Sleep(500 * time.Millisecond)
+
+			var rm metricdata.ResourceMetrics
+			err = metricReader.Collect(t.Context(), &rm)
+			require.NoError(t, err)
+
+			var requestCountFound bool
+			for _, scopeMetric := range rm.ScopeMetrics {
+				for _, m := range scopeMetric.Metrics {
+					if m.Name == "router.http.requests" {
+						requestCountFound = true
+					}
+				}
+			}
+			require.True(t, requestCountFound, "request count metric should be recorded even for client disconnections")
+		})
+	})
+
+	t.Run("log level is info not error", func(t *testing.T) {
+		t.Parallel()
+
+		testenv.Run(t, &testenv.Config{
+			LogObservation: testenv.LogObservationConfig{
+				Enabled:  true,
+				LogLevel: zapcore.DebugLevel,
+			},
+			Subgraphs: testenv.SubgraphsConfig{
+				Employees: testenv.SubgraphConfig{
+					Delay: 2 * time.Second,
+				},
+			},
+		}, func(t *testing.T, xEnv *testenv.Environment) {
+			ctx, cancel := context.WithTimeout(t.Context(), 200*time.Millisecond)
+			defer cancel()
+
+			req, err := http.NewRequestWithContext(ctx, http.MethodPost, xEnv.GraphQLRequestURL(),
+				strings.NewReader(`{"query":"{ employees { id } }"}`))
+			require.NoError(t, err)
+			req.Header.Set("Content-Type", "application/json")
+
+			client := &http.Client{}
+			_, _ = client.Do(req)
+
+			time.Sleep(500 * time.Millisecond)
+
+			errorLogs := xEnv.Observer().FilterLevelExact(zapcore.ErrorLevel).All()
+			for _, entry := range errorLogs {
+				require.NotContains(t, entry.Message, "context canceled",
+					"context canceled should not be logged at error level")
+			}
+		})
+	})
+
+	t.Run("other errors still mark span as error", func(t *testing.T) {
+		t.Parallel()
+
+		exporter := tracetest.NewInMemoryExporter(t)
+
+		testenv.Run(t, &testenv.Config{
+			TraceExporter: exporter,
+			Subgraphs: testenv.SubgraphsConfig{
+				Employees: testenv.SubgraphConfig{
+					CloseOnStart: true,
+				},
+			},
+		}, func(t *testing.T, xEnv *testenv.Environment) {
+			res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
+				Query: `{ employees { id } }`,
+			})
+			require.Contains(t, res.Body, "errors")
+
+			rootSpan := rootSpan(exporter.GetSpans().Snapshots())
+			require.NotNil(t, rootSpan, "root span should exist")
+			require.Equal(t, codes.Error, rootSpan.Status().Code,
+				"root span should be marked as error for real failures")
+		})
+	})
+}

router/core/errors.go

@@ -111,21 +111,28 @@ func logInternalErrorsFromReport(report *operationreport.Report, requestLogger *
 // trackFinalResponseError sets the final response error on the request context and
 // attaches it to the span. This is used to process the error in the outer middleware
 // and therefore only intended to be used in the GraphQL handler.
+// Client disconnections (context.Canceled) are tracked separately and do not mark
+// the span as ERROR or count toward error metrics, since they are not server-side failures.
 func trackFinalResponseError(ctx context.Context, err error) {
 	if err == nil {
 		return
 	}
 
-	span := trace.SpanFromContext(ctx)
 	requestContext := getRequestContext(ctx)
 	if requestContext == nil {
 		return
 	}
 
 	requestContext.SetError(err)
+
+	if errors.Is(err, context.Canceled) {
+		return
+	}
+
 	requestContext.graphQLErrorServices = getAggregatedSubgraphServiceNames(requestContext.error)
 	requestContext.graphQLErrorCodes = getAggregatedSubgraphErrorCodes(requestContext.error)
 
+	span := trace.SpanFromContext(ctx)
 	rtrace.AttachErrToSpan(span, err)
 }
 

router/core/graphql_prehandler.go

@@ -457,9 +457,17 @@ func (h *PreHandler) Handler(next http.Handler) http.Handler {
 		// and enrich the context to make it available in the request context as well for metrics etc.
 		next.ServeHTTP(ww, r)
 
-		// Mark the root span of the router as failed, so we can easily identify failed requests
+		// Mark the root span of the router as failed, so we can easily identify failed requests.
+		// Client disconnections are not server-side errors and should not mark the root span as ERROR.
 		if requestContext.error != nil {
-			rtrace.AttachErrToSpan(trace.SpanFromContext(r.Context()), requestContext.error)
+			contextSpan := trace.SpanFromContext(r.Context())
+
+			if errors.Is(requestContext.error, context.Canceled) {
+				// For disconnects just record the error but don't set the status
+				contextSpan.RecordError(requestContext.error)
+			} else {
+				rtrace.AttachErrToSpan(contextSpan, requestContext.error)
+			}
 		}
 	})
 }