Support for (auto) instrumentation for Spring Cloud Gateway MVC routes

[holgerstolzenberg]

Is your feature request related to a problem? Please describe.

Currently, it looks like "only" the reactive variant of Spring Cloud Gateway is natively supported by OpenTelemetry starter. In a recent project, we decided to go with the MVC variant, as in the advent of virtual threads, reactive approach was ruled out due to the complexities it introduces.

I was able to work around this by implementing a custom HandlerInterceptor:

package eu.eliagroup.gras.iam.gatekeeper.service.core.opentelemetry;

import static java.lang.String.format;
import static java.util.Optional.ofNullable;
import static org.springframework.core.Ordered.HIGHEST_PRECEDENCE;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapGetter;
import jakarta.annotation.Nonnull;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import lombok.val;
import org.slf4j.MDC;
import org.springframework.core.annotation.Order;
import org.springframework.stereotype.Component;
import org.springframework.web.servlet.HandlerInterceptor;

/// The interceptor sets up the trace context for any given route.
///
/// It relies on the W3C Trace Context Propagator to extract the trace context from the request
/// headers.
///
/// After request completion, the trace- and span IDs get removed from the MDC.
@Component
@Order(HIGHEST_PRECEDENCE)
final class TraceContextInterceptor implements HandlerInterceptor {
  private static final String TRACE_ID = "trace_id";
  private static final String SPAN_ID = "span_id";

  @Override
  public boolean preHandle(
      @Nonnull final HttpServletRequest request,
      @Nonnull final HttpServletResponse response,
      @Nonnull final Object handler) {
    val extractedContext =
        W3CTraceContextPropagator.getInstance()
            .extract(Context.current(), request, new HeaderExtractor());

    try (final var ignored = extractedContext.makeCurrent()) {
      val currentSpan = Span.current();
      currentSpan.updateName(
          format("%s %s", request.getMethod().toUpperCase(), request.getRequestURI()));

      final var spanContext = currentSpan.getSpanContext();

      if (spanContext.isValid()) {
        MDC.put(TRACE_ID, spanContext.getTraceId());
        MDC.put(SPAN_ID, spanContext.getSpanId());
      }
    }

    return true;
  }

  @Override
  public void afterCompletion(
      @Nonnull final HttpServletRequest request,
      @Nonnull final HttpServletResponse response,
      @Nonnull final Object handler,
      final Exception ex) {
    MDC.remove(TRACE_ID);
    MDC.remove(SPAN_ID);
  }

  private static final class HeaderExtractor implements TextMapGetter<HttpServletRequest> {
    @Override
    public Iterable<String> keys(final HttpServletRequest carrier) {
      return () -> carrier.getHeaderNames().asIterator();
    }

    @Override
    public String get(final HttpServletRequest carrier, final String headerName) {
      return ofNullable(carrier).map(c -> c.getHeader(headerName)).orElse(null);
    }
  }
}

TBH - the feature is not of high priority, yet I think it would be a nice addition.

Describe the solution you'd like

It would be nice if the starter instruments Gateway MVC routes automagically.

Describe alternatives you've considered

None.

Additional context

None.

Tip

_{React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it. Learn more here.}

[laurit]

Currently, it looks like "only" the reactive variant of Spring Cloud Gateway is natively supported by OpenTelemetry starter.

could you elaborate what exactly are you looking for, a better span name for the server span or something else? Does it work any better with the agent?

TBH - the feature is not of high priority, yet I think it would be a nice addition.

we are open to contributions

I have a hard time understanding the provided TraceContextInterceptor. W3CTraceContextPropagator.getInstance().extract() is usually used to extract context with remote parent span from context propagation headers. Typically one would extract the context and use makeCurrent to bind that context to current thread so that spans created during the processing of the request would appear as children of the remote parent span. The scope opened by makeCurrent would be closed at the end of the request processing. If there was a remote parent span then updateName wouldn't do anything because the name of the remote span can't be updated, it is in the remote system. Usually one would want to update the name of the local server span instead of the remote parent. Makes me wonder whether instead of W3CTraceContextPropagator.getInstance().extract() you really should use Context.current(). Then calling Context.current() and makeCurrent doesn't make sense as it doesn't do anything, you could have just called Span.current().
Semantic conventions https://opentelemetry.io/docs/specs/semconv/http/http-spans/#name disallow using URI in the span name because it could have high cardinality. Just updating the span name isn't usually enough as it won't affect the metrics. As for adding the trace and span id to mdc I would have thought that the starter already takes care of that.

[holgerstolzenberg]

Okay so I try to sum up:

The provided TraceContextInterceptor is just a best guess implementation that in the end gave me the necessary features I needed. The implementation for sure can be improved, it being my first time working deeper with OTEL. I will give Context.current() I try and see if it fits.

The interceptor was implemented to get the handler filter chain instrumented. Yes, Spring Cloud Gateway MVC uses the Spring MVC filter chain and I think it gets properly instrumented, also in my project. Yet, the actual log output from the handler methods of the gateway lacked the tracecontext (or at least I think so). I did not deep dive on the Gateway MVC architecture, but as I have understood it so far, the gateway handler chain is just executed by one filter, it kinda being a chain in the chain. I currently have no clue if certain async stuff or threading is involved in this or not.

My initial expectation would have been, I just add the opentelemetry starter, and the log output coming from a gateway handler method (like e.g. BeforeFilterFunctions or AfterFilterFunction) would have the log context properly propagated, showing the trace ID in the logs as everything around it also does.