Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebClient Observations are missing error #32389

Closed
Xiphoseer opened this issue Mar 7, 2024 · 3 comments
Closed

WebClient Observations are missing error #32389

Xiphoseer opened this issue Mar 7, 2024 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches theme: observability An issue related to observability and tracing type: bug A general bug
Milestone

Comments

@Xiphoseer
Copy link

I'm using WebClient in a setup with

  • org.springframework:spring-webflux:6.1.4
  • io.micrometer:micrometer-tracing-bridge-otel:1.2.3
  • io.opentelemetry:opentelemetry-exporter-zipkin:1.31.0

and am missing status/error metadata in the spans that show up in Zipkin.

When a HTTP GET request fails, the outcome in zipkin is a http get span with (among others) the tags:

  • exception: WebClientRequestException
  • outcome: UNKNOWN
  • status: CLIENT_ERROR
  • otel.library.name, otel.library.version, otel.scope.name, and otel.scope.version

What is missing is both:

  • error (Convention for Zipkin to mark the span in red)
  • otel.status_code (from io.opentelemetry.exporter.zipkin.OtelToZipkinSpanTransformer)

Working backwards, the OtelToZipkinSpanTransformer would set both of these, if there was a status
set on the OpenTelemetry Span / SpanData, which was not the case in debugging.

These would be set in io.micrometer.tracing.otel.bridge.OtelSpan::error, which is an implementation of
io.micrometer.tracing.Span. Going further back, that method is called by io.micrometer.tracing.handler.TracingObservationHandler,
either when the ERROR tag is set, or when onError is called.

That should be called by io.micrometer.observation.SimpleObservation#notifyOnError, which is an implementation detail of io.micrometer.observation.SimpleObservation#error.

But it turns out WebClient doesn't call that function! Instead, at


it only calls org.springframework.web.reactive.function.client.ClientRequestObservationContext#setError, whereas
SimpleObservation both sets the error in the contex and dispatches the notification:

@Override
public Observation error(Throwable error) {
    this.context.setError(error);
    notifyOnError();
    return this;
}

To me this last part seems like the best place to fix this issue, i.e. to replace .doOnError(observationContext::setError) with something like .doOnError(e -> observation.error(e)).

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 7, 2024
@Xiphoseer
Copy link
Author

Note: technically it's io.micrometer.tracing.handler.PropagatingSenderTracingObservationHandler not TracingObservationHandler, which is provided by Spring Boot in MicrometerTracingAutoConfiguration, but the mechanism stays the same.

@Xiphoseer Xiphoseer changed the title [webflux] Reactive ClientHttp Observations have unknown outcome on error [webflux] Reactive ClientHttp Observations are missing error Mar 7, 2024
@Xiphoseer
Copy link
Author

Xiphoseer commented Mar 7, 2024

I protoyped a fix locally with the following bean, which works to make zipkin recognize the span as failed, but outcome is still UNKNOWN, which is probably fine.

@Bean
@ConditionalOnMissingBean
@ConditionalOnClass(ClientRequestObservationContext.class)
@Order(SENDER_TRACING_OBSERVATION_HANDLER_ORDER)
public PropagatingSenderTracingObservationHandler<?> propagatingSenderTracingObservationHandler(Tracer tracer, Propagator propagator) {
    return new PropagatingSenderTracingObservationHandler<>(tracer, propagator) {
        @Override
        public void customizeSenderSpan(SenderContext context, Span span) {
            // Fix for: https://github.com/spring-projects/spring-framework/issues/32389
            if (ClientRequestObservationContext.class.isAssignableFrom(context.getClass())) {
                Throwable error = context.getError();
                if (error != null) {
                    span.error(error);
                }
            }
            super.customizeSenderSpan(context, span);
        }
    };
}

@bclozel bclozel self-assigned this Mar 8, 2024
@bclozel bclozel changed the title [webflux] Reactive ClientHttp Observations are missing error WebClient Observations are missing error Mar 8, 2024
@bclozel bclozel added type: bug A general bug theme: observability An issue related to observability and tracing and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 8, 2024
@bclozel bclozel added this to the 6.1.5 milestone Mar 8, 2024
@bclozel bclozel added the for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x label Mar 8, 2024
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels Mar 8, 2024
@bclozel bclozel closed this as completed in f6bc828 Mar 8, 2024
@bclozel
Copy link
Member

bclozel commented Mar 8, 2024

Thanks @Xiphoseer for the report and analysis, this has been fixed in 6.1.x and backported to 6.0.x as well. It will be shipped with the next set of releases on March 14th.

@jhoeller jhoeller added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches theme: observability An issue related to observability and tracing type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants