Skip to content

Commit

Permalink
Bump OpenTracing API to 0.32.0 (#102)
Browse files Browse the repository at this point in the history 
* Working with stack

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Remove some unused code

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* small fixes

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Fix

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
  • Loading branch information
@pavolloffay
pavolloffay committed Apr 25, 2019
1 parent 61b70a9 commit 6966272
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 115 deletions.
8 changes: 1 addition & 7 deletions ...t/java/io/opentracing/contrib/spring/web/interceptor/itest/common/app/TestController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package io.opentracing.contrib.spring.web.interceptor.itest.common.app;

import java.util.concurrent.Callable;
import java.util.concurrent.Future;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
Expand All @@ -30,7 +29,6 @@
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.context.request.async.AsyncWebRequest;
import org.springframework.web.context.request.async.DeferredResult;
import org.springframework.web.servlet.ModelAndView;

Expand Down Expand Up @@ -79,11 +77,7 @@ public ModelAndView view() {
@RequestMapping("/localSpan")
public String localSpan(HttpServletRequest request) {
verifyActiveSpan();
io.opentracing.Tracer.SpanBuilder spanBuilder = tracer.buildSpan("localSpan");
Span localSpan = spanBuilder
.startManual();
localSpan.finish();

tracer.buildSpan("localSpan").start().finish();
return "sync";
}

Expand Down
43 changes: 19 additions & 24 deletions ...in/java/io/opentracing/contrib/spring/web/client/TracingAsyncRestTemplateInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,52 +64,47 @@ public ListenableFuture<ClientHttpResponse> intercept(final HttpRequest httpRequ
byte[] body,
AsyncClientHttpRequestExecution execution) throws IOException {

final Scope scope = tracer.buildSpan(httpRequest.getMethod().toString())
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT).startActive(false);
tracer.inject(scope.span().context(), Format.Builtin.HTTP_HEADERS, new HttpHeadersCarrier(httpRequest.getHeaders()));
final Span span = tracer.buildSpan(httpRequest.getMethod().toString())
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
.start();
tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS, new HttpHeadersCarrier(httpRequest.getHeaders()));

for (RestTemplateSpanDecorator spanDecorator : spanDecorators) {
try {
spanDecorator.onRequest(httpRequest, scope.span());
spanDecorator.onRequest(httpRequest, span);
} catch (RuntimeException exDecorator) {
log.error("Exception during decorating span", exDecorator);
}
}

ListenableFuture<ClientHttpResponse> future = execution.executeAsync(httpRequest, body);

final Span span = scope.span();

future.addCallback(new ListenableFutureCallback<ClientHttpResponse>() {
@Override
public void onSuccess(ClientHttpResponse httpResponse) {
try (Scope asyncScope = tracer.scopeManager().activate(span, true)) {
try (Scope scope = tracer.activateSpan(span)) {
ListenableFuture<ClientHttpResponse> future = execution.executeAsync(httpRequest, body);
future.addCallback(new ListenableFutureCallback<ClientHttpResponse>() {
@Override
public void onSuccess(ClientHttpResponse httpResponse) {
for (RestTemplateSpanDecorator spanDecorator: spanDecorators) {
try {
spanDecorator.onResponse(httpRequest, httpResponse, scope.span());
spanDecorator.onResponse(httpRequest, httpResponse, span);
} catch (RuntimeException exDecorator) {
log.error("Exception during decorating span", exDecorator);
}
}
span.finish();
}
}

@Override
public void onFailure(Throwable ex) {
try (Scope asyncScope = tracer.scopeManager().activate(span, true)) {
@Override
public void onFailure(Throwable ex) {
for (RestTemplateSpanDecorator spanDecorator: spanDecorators) {
try {
spanDecorator.onError(httpRequest, ex, scope.span());
spanDecorator.onError(httpRequest, ex, span);
} catch (RuntimeException exDecorator) {
log.error("Exception during decorating span", exDecorator);
}
}
span.finish();
}
}
});

scope.close();

return future;
});
return future;
}
}
}
42 changes: 22 additions & 20 deletions ...rc/main/java/io/opentracing/contrib/spring/web/client/TracingRestTemplateInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.opentracing.contrib.spring.web.client;

import io.opentracing.Span;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -70,39 +71,40 @@ public ClientHttpResponse intercept(HttpRequest httpRequest, byte[] body,
ClientHttpRequestExecution execution) throws IOException {
ClientHttpResponse httpResponse;

try (Scope scope = tracer.buildSpan(httpRequest.getMethod().toString())
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT).startActive(true)) {
tracer.inject(scope.span().context(), Format.Builtin.HTTP_HEADERS,
new HttpHeadersCarrier(httpRequest.getHeaders()));
Span span = tracer.buildSpan(httpRequest.getMethod().toString())
.withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
.start();
tracer.inject(span.context(), Format.Builtin.HTTP_HEADERS,
new HttpHeadersCarrier(httpRequest.getHeaders()));

for (RestTemplateSpanDecorator spanDecorator : spanDecorators) {
try {
spanDecorator.onRequest(httpRequest, span);
} catch (RuntimeException exDecorator) {
log.error("Exception during decorating span", exDecorator);
}
}

try (Scope scope = tracer.activateSpan(span)) {
httpResponse = execution.execute(httpRequest, body);
for (RestTemplateSpanDecorator spanDecorator : spanDecorators) {
try {
spanDecorator.onRequest(httpRequest, scope.span());
spanDecorator.onResponse(httpRequest, httpResponse, span);
} catch (RuntimeException exDecorator) {
log.error("Exception during decorating span", exDecorator);
}
}

try {
httpResponse = execution.execute(httpRequest, body);
} catch (Exception ex) {
for (RestTemplateSpanDecorator spanDecorator : spanDecorators) {
try {
spanDecorator.onError(httpRequest, ex, scope.span());
} catch (RuntimeException exDecorator) {
log.error("Exception during decorating span", exDecorator);
}
}
throw ex;
}

} catch (Exception ex) {
for (RestTemplateSpanDecorator spanDecorator : spanDecorators) {
try {
spanDecorator.onResponse(httpRequest, httpResponse, scope.span());
spanDecorator.onError(httpRequest, ex, span);
} catch (RuntimeException exDecorator) {
log.error("Exception during decorating span", exDecorator);
}
}
throw ex;
} finally {
span.finish();
}

return httpResponse;
Expand Down
60 changes: 22 additions & 38 deletions ...rc/main/java/io/opentracing/contrib/spring/web/interceptor/TracingHandlerInterceptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public class TracingHandlerInterceptor extends HandlerInterceptorAdapter {

private static final String SCOPE_STACK = TracingHandlerInterceptor.class.getName() + ".scopeStack";
private static final String CONTINUATION_FROM_ASYNC_STARTED = TracingHandlerInterceptor.class.getName() + ".continuation";
private static final String IS_ERROR_HANDLING_SPAN = TracingHandlerInterceptor.class.getName() + ".error_handling_span";

private Tracer tracer;
private List<HandlerInterceptorSpanDecorator> decorators;
Expand Down Expand Up @@ -78,8 +79,8 @@ static boolean isTraced(HttpServletRequest httpServletRequest) {
}

@Override
public boolean preHandle(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Object handler)
throws Exception {
public boolean preHandle(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Object handler) {
Deque<Scope> activeSpanStack = getScopeStack(httpServletRequest);

if (!isTraced(httpServletRequest)) {
return true;
Expand All @@ -89,82 +90,65 @@ public boolean preHandle(HttpServletRequest httpServletRequest, HttpServletRespo
* 1. check if there is an active span, it has been activated in servlet filter or in this interceptor (forward)
* 2. if there is no active span then it can be handling of an async request or spring boot default error handling
*/
Scope serverSpan = tracer.scopeManager().active();
Span serverSpan = tracer.activeSpan();
if (serverSpan == null) {
if (httpServletRequest.getAttribute(CONTINUATION_FROM_ASYNC_STARTED) != null) {
Span contd = (Span) httpServletRequest.getAttribute(CONTINUATION_FROM_ASYNC_STARTED);
serverSpan = tracer.scopeManager().activate(contd, false);
serverSpan = (Span) httpServletRequest.getAttribute(CONTINUATION_FROM_ASYNC_STARTED);
httpServletRequest.removeAttribute(CONTINUATION_FROM_ASYNC_STARTED);
activeSpanStack.push(tracer.activateSpan(serverSpan));
} else {
// spring boot default error handling, executes interceptor after processing in the filter (ugly huh?)
serverSpan = tracer.buildSpan(httpServletRequest.getMethod())
.addReference(References.FOLLOWS_FROM, TracingFilter.serverSpanContext(httpServletRequest))
.startActive(true);
.start();
httpServletRequest.setAttribute(IS_ERROR_HANDLING_SPAN, true);
activeSpanStack.push(tracer.activateSpan(serverSpan));
}
Deque<Scope> activeSpanStack = getScopeStack(httpServletRequest);
activeSpanStack.push(serverSpan);
}

for (HandlerInterceptorSpanDecorator decorator : decorators) {
decorator.onPreHandle(httpServletRequest, handler, serverSpan.span());
decorator.onPreHandle(httpServletRequest, handler, serverSpan);
}

return true;
}

@Override
public void afterConcurrentHandlingStarted (
HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Object handler)
throws Exception {
HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Object handler) {

if (!isTraced(httpServletRequest)) {
return;
}

Span span;
Deque<Scope> activeSpanStack = getScopeStack(httpServletRequest);
if(activeSpanStack.size() > 0) {
Scope scope = activeSpanStack.pop();
span = scope.span();
onAfterConcurrentHandlingStarted(httpServletRequest, httpServletResponse, handler, span);
scope.close();
} else {
span = tracer.activeSpan();
onAfterConcurrentHandlingStarted(httpServletRequest, httpServletResponse, handler, span);
}

httpServletRequest.setAttribute(CONTINUATION_FROM_ASYNC_STARTED, span);

}

private void onAfterConcurrentHandlingStarted(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse, Object handler, Span span) {
Span span = tracer.activeSpan();
for (HandlerInterceptorSpanDecorator decorator : decorators) {
decorator.onAfterConcurrentHandlingStarted(httpServletRequest, httpServletResponse, handler, span);
}
httpServletRequest.setAttribute(CONTINUATION_FROM_ASYNC_STARTED, span);
}


@Override
public void afterCompletion(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
Object handler, Exception ex) throws Exception {
Object handler, Exception ex) {

if (!isTraced(httpServletRequest)) {
return;
}

Span span = tracer.activeSpan();
for (HandlerInterceptorSpanDecorator decorator : decorators) {
decorator.onAfterCompletion(httpServletRequest, httpServletResponse, handler, ex, span);
}
Deque<Scope> scopeStack = getScopeStack(httpServletRequest);
if(scopeStack.size() > 0) {
Scope scope = scopeStack.pop();
onAfterCompletion(httpServletRequest, httpServletResponse, handler, ex, scope.span());
scope.close();
} else {
onAfterCompletion(httpServletRequest, httpServletResponse, handler, ex, tracer.activeSpan());
}
}

private void onAfterCompletion(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse,
Object handler, Exception ex, Span span) {
for (HandlerInterceptorSpanDecorator decorator : decorators) {
decorator.onAfterCompletion(httpServletRequest, httpServletResponse, handler, ex, span);
if (httpServletRequest.getAttribute(IS_ERROR_HANDLING_SPAN) != null) {
httpServletRequest.removeAttribute(IS_ERROR_HANDLING_SPAN);
span.finish();
}
}

Expand Down
11 changes: 7 additions & 4 deletions ...web/src/test/java/io/opentracing/contrib/spring/web/client/AbstractTracingClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.github.tomakehurst.wiremock.extension.responsetemplating.ResponseTemplateTransformer;
import com.github.tomakehurst.wiremock.junit.WireMockRule;
import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.Tracer;
import io.opentracing.mock.MockSpan;
import io.opentracing.mock.MockTracer;
Expand Down Expand Up @@ -139,15 +140,17 @@ public void testInject() {
@Test
public void testParentSpan() {
{
Scope parent = mockTracer.buildSpan("parent").startActive(true);
Span parent = mockTracer.buildSpan("parent").start();

final String path = "/foo";
final String url = wireMockRule.url(path);
stubFor(get(urlPathEqualTo(path))
.willReturn(ok()));
client.getForEntity(url, String.class);

parent.close();
try (Scope scope = mockTracer.activateSpan(parent)) {
client.getForEntity(url, String.class);
} finally {
parent.finish();
}
}

List<MockSpan> mockSpans = mockTracer.finishedSpans();
Expand Down
19 changes: 10 additions & 9 deletions .../src/test/java/io/opentracing/contrib/spring/web/client/TracingAsyncRestTemplateTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,16 @@ public void testMultipleRequests() throws InterruptedException, ExecutionExcepti
for (int i = 0; i < numberOfCalls; i++) {
final String requestUrl = url + i;

try (final Scope parentSpan = mockTracer.buildSpan("foo")
.withTag("request-url", requestUrl)
.startActive(false)) {
futures.add(executorService.submit(() -> {
try (final Scope span = mockTracer.scopeManager().activate(parentSpan.span(), true)) {
client.getForEntity(requestUrl, String.class);
}
}));
}
Span parentSpan = mockTracer.buildSpan("foo")
.withTag("request-url", requestUrl)
.start() ;
futures.add(executorService.submit(() -> {
try (final Scope span = mockTracer.activateSpan(parentSpan)) {
client.getForEntity(requestUrl, String.class);
} finally {
parentSpan.finish();
}
}));
}

// wait to finish all calls
Expand Down
20 changes: 11 additions & 9 deletions ...ring-web/src/test/java/io/opentracing/contrib/spring/web/client/TracingWebClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package io.opentracing.contrib.spring.web.client;

import io.opentracing.Scope;
import io.opentracing.Span;
import io.opentracing.mock.MockSpan;
import io.opentracing.tag.Tags;
import org.junit.Assert;
Expand Down Expand Up @@ -76,15 +77,16 @@ public void testMultipleRequests() throws InterruptedException, ExecutionExcepti
for (int i = 0; i < numberOfCalls; i++) {
final String requestUrl = url + i;

try (final Scope parentSpan = mockTracer.buildSpan("foo")
.withTag("request-url", requestUrl)
.startActive(false)) {
futures.add(executorService.submit(() -> {
try (final Scope span = mockTracer.scopeManager().activate(parentSpan.span(), true)) {
client.getForEntity(requestUrl, String.class);
}
}));
}
Span parentSpan = mockTracer.buildSpan("foo")
.withTag("request-url", requestUrl)
.start();
futures.add(executorService.submit(() -> {
try (final Scope span = mockTracer.scopeManager().activate(parentSpan)) {
client.getForEntity(requestUrl, String.class);
} finally {
parentSpan.finish();
}
}));
}

// wait to finish all calls
Expand Down
8 changes: 4 additions & 4 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@
<main.basedir>${project.basedir}</main.basedir>

<version.org.awaitility-awaitility>3.0.0</version.org.awaitility-awaitility>
<version.io.opentracing>0.31.0</version.io.opentracing>
<version.io.opentracing.contrib-opentracing-web-servlet-filter>0.2.0</version.io.opentracing.contrib-opentracing-web-servlet-filter>
<version.io.opentracing.contrib-opentracing-tracerresolver>0.1.5</version.io.opentracing.contrib-opentracing-tracerresolver>
<version.io.opentracing.contrib-opentracing-spring-tracer-configuration-starter>0.1.0</version.io.opentracing.contrib-opentracing-spring-tracer-configuration-starter>
<version.io.opentracing>0.32.0</version.io.opentracing>
<version.io.opentracing.contrib-opentracing-web-servlet-filter>0.3.0</version.io.opentracing.contrib-opentracing-web-servlet-filter>
<version.io.opentracing.contrib-opentracing-tracerresolver>0.1.6</version.io.opentracing.contrib-opentracing-tracerresolver>
<version.io.opentracing.contrib-opentracing-spring-tracer-configuration-starter>0.2.0</version.io.opentracing.contrib-opentracing-spring-tracer-configuration-starter>
<version.junit>4.12</version.junit>
<version.org.mockito-mockito-core>2.23.4</version.org.mockito-mockito-core>
<version.org.springframework.boot>2.1.1.RELEASE</version.org.springframework.boot>
Expand Down

0 comments on commit 6966272

Please sign in to comment.