From 5459f473c7777895d37b77aa1695754241e7b640 Mon Sep 17 00:00:00 2001 From: DanMaas Date: Tue, 5 Mar 2019 23:39:00 -0600 Subject: [PATCH 1/2] ensure the same span is available after parsing requests, closes #21 --- .../internal/DefaultServerTracingHandler.java | 16 ++--- .../zipkin/ServerTracingModuleSpec.groovy | 66 ++++++++++++++++--- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/main/java/ratpack/zipkin/internal/DefaultServerTracingHandler.java b/src/main/java/ratpack/zipkin/internal/DefaultServerTracingHandler.java index d0b351c..a532327 100644 --- a/src/main/java/ratpack/zipkin/internal/DefaultServerTracingHandler.java +++ b/src/main/java/ratpack/zipkin/internal/DefaultServerTracingHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -19,8 +19,6 @@ import brave.http.HttpServerHandler; import brave.http.HttpTracing; import brave.propagation.TraceContext; -import javax.inject.Inject; - import com.google.common.net.HostAndPort; import ratpack.handling.Context; import ratpack.handling.Handler; @@ -35,6 +33,7 @@ import ratpack.zipkin.ServerResponse; import ratpack.zipkin.ServerTracingHandler; +import javax.inject.Inject; import java.util.Optional; /** @@ -58,15 +57,16 @@ public void handle(Context ctx) throws Exception { ServerRequest request = new ServerRequestImpl(ctx.getRequest()); final Span span = handler.handleReceive(extractor, request); + //place the Span in scope so that downstream code (e.g. Ratpack handlers + //further on in the chain) can see the Span. + Tracer.SpanInScope scope = tracing.tracer().withSpanInScope(span); + ctx.getResponse().beforeSend(response -> { + scope.close(); ServerResponse serverResponse = new ServerResponseImpl(response, request, ctx.getPathBinding()); handler.handleSend(serverResponse, null, span); }); - //place the Span in scope so that downstream code (e.g. Ratpack handlers - //further on in the chain) can see the Span. - try (Tracer.SpanInScope scope = tracing.tracer().withSpanInScope(span)) { - ctx.next(); - } + ctx.next(); } private static class ServerRequestImpl implements ServerRequest { diff --git a/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy b/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy index 644ae81..b856462 100644 --- a/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy +++ b/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy @@ -1,5 +1,5 @@ /** - * Copyright 2016-2018 The OpenZipkin Authors + * Copyright 2016-2019 The OpenZipkin Authors * * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except * in compliance with the License. You may obtain a copy of the License at @@ -14,28 +14,34 @@ package ratpack.zipkin import brave.SpanCustomizer +import brave.Tracer import brave.http.HttpSampler import brave.propagation.B3Propagation +import brave.sampler.Sampler +import com.fasterxml.jackson.core.type.TypeReference +import com.fasterxml.jackson.databind.ObjectMapper +import io.netty.handler.codec.http.HttpResponseStatus import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer +import ratpack.groovy.test.embed.GroovyEmbeddedApp +import ratpack.guice.Guice import ratpack.handling.Context import ratpack.handling.Handler +import ratpack.http.HttpMethod +import ratpack.jackson.Jackson import ratpack.path.PathBinding import ratpack.zipkin.internal.ZipkinHttpClientImpl import ratpack.zipkin.support.B3PropagationHeaders +import ratpack.zipkin.support.TestReporter +import spock.lang.Specification import spock.lang.Unroll +import zipkin2.Span import zipkin2.reporter.Reporter -import static org.assertj.core.api.Assertions.* +import java.util.concurrent.CountDownLatch +import java.util.concurrent.atomic.AtomicReference -import brave.sampler.Sampler -import io.netty.handler.codec.http.HttpResponseStatus -import ratpack.groovy.test.embed.GroovyEmbeddedApp -import ratpack.guice.Guice -import ratpack.http.HttpMethod -import ratpack.zipkin.support.TestReporter -import spock.lang.Specification -import zipkin2.Span +import static org.assertj.core.api.Assertions.assertThat class ServerTracingModuleSpec extends Specification { @@ -573,4 +579,44 @@ class ServerTracingModuleSpec extends Specification { then: assertThat(reporter.getSpans()).isEmpty() } + + def 'Should provide the same trace context before and after parsing the request'() { + given: + def latch = new CountDownLatch(1) + def spans = new AtomicReference() + def app = GroovyEmbeddedApp.of { server -> + server.registry(Guice.registry { binding -> + binding.module(ServerTracingModule.class, { config -> + config + .serviceName("embedded") + .sampler(Sampler.ALWAYS_SAMPLE) + .spanReporterV2(reporter) + }) + }).handlers { + chain -> + chain.all { ctx -> + def tracer = ctx.get(Tracer) + def context = tracer.currentSpan().context() + def initialSpan = context.traceId() + "=" + context.spanId() + ctx.request.body.then { + def parsedContext = tracer.currentSpan().context() + def parsedSpan = parsedContext.traceId() + "=" + parsedContext.spanId() + ctx.render(Jackson.json([initialSpan, parsedSpan])) + } + } + } + } + when: + app.test { t -> + def resp = t.request { spec -> + spec.post().body.text("test") + } + spans.set(new ObjectMapper().readValue(resp.body.text, new TypeReference>(){})) + latch.countDown() + } + then: + latch.await() + reporter.getSpans().size() == 1 + spans.get().get(0) == spans.get().get(1) + } } \ No newline at end of file From a56a9b21a690eb5468693d017c6b65ef96e30968 Mon Sep 17 00:00:00 2001 From: Lance Linder Date: Sun, 10 Mar 2019 15:01:53 -0500 Subject: [PATCH 2/2] Adjusting async parsing scope test --- .../zipkin/ServerTracingModuleSpec.groovy | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy b/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy index b856462..934ef26 100644 --- a/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy +++ b/src/test/groovy/ratpack/zipkin/ServerTracingModuleSpec.groovy @@ -23,6 +23,7 @@ import com.fasterxml.jackson.databind.ObjectMapper import io.netty.handler.codec.http.HttpResponseStatus import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer +import ratpack.form.Form import ratpack.groovy.test.embed.GroovyEmbeddedApp import ratpack.guice.Guice import ratpack.handling.Context @@ -582,41 +583,40 @@ class ServerTracingModuleSpec extends Specification { def 'Should provide the same trace context before and after parsing the request'() { given: - def latch = new CountDownLatch(1) - def spans = new AtomicReference() def app = GroovyEmbeddedApp.of { server -> - server.registry(Guice.registry { binding -> - binding.module(ServerTracingModule.class, { config -> - config - .serviceName("embedded") - .sampler(Sampler.ALWAYS_SAMPLE) - .spanReporterV2(reporter) - }) - }).handlers { - chain -> - chain.all { ctx -> - def tracer = ctx.get(Tracer) - def context = tracer.currentSpan().context() - def initialSpan = context.traceId() + "=" + context.spanId() - ctx.request.body.then { - def parsedContext = tracer.currentSpan().context() - def parsedSpan = parsedContext.traceId() + "=" + parsedContext.spanId() - ctx.render(Jackson.json([initialSpan, parsedSpan])) + server.registry(Guice.registry { binding -> + binding.module(ServerTracingModule.class, { config -> + config + .serviceName("embedded") + .sampler(Sampler.ALWAYS_SAMPLE) + .spanReporterV2(reporter) + }) + }).handlers { + chain -> + chain.all { ctx -> + def tracer = ctx.get(Tracer) + tracer.currentSpan().tag("handler", "") + + ctx.parse(Form).then { form -> + tracer.currentSpan().tag("before_render", form.get("param")) + ctx.render("ok") + tracer.currentSpan().tag("after_render", form.get("param")) + } } - } + } } - } when: app.test { t -> - def resp = t.request { spec -> - spec.post().body.text("test") + t.request { spec -> + spec.post().body.text("param=the_param") } - spans.set(new ObjectMapper().readValue(resp.body.text, new TypeReference>(){})) - latch.countDown() } then: - latch.await() reporter.getSpans().size() == 1 - spans.get().get(0) == spans.get().get(1) + def span = reporter.getSpans().first() + assertThat(span.tags()).containsKey("handler") + assertThat(span.tags()).containsKey("before_render") + assertThat(span.tags()).containsKey("after_render") + assertThat(span.tags()).containsValue("the_param") } } \ No newline at end of file