From 12620a47256f68e7807597f1e633b13d2dbcc8db Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Sat, 30 Jan 2016 16:47:23 +0000 Subject: [PATCH] Fix NPE in TracePreZuulFilter Fixes gh-135 --- .../instrument/zuul/TracePreZuulFilter.java | 18 +++-- .../zuul/TracePostZuulFilterTests.java | 62 +++++++++++++++ .../zuul/TracePreZuulFilterTests.java | 76 +++++++++++++++++++ 3 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java index 45f5e4118..f70e2d0e6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java @@ -33,8 +33,8 @@ import com.netflix.zuul.context.RequestContext; * @author Dave Syer * */ -public class TracePreZuulFilter extends ZuulFilter implements -ApplicationEventPublisherAware { +public class TracePreZuulFilter extends ZuulFilter + implements ApplicationEventPublisherAware { private ApplicationEventPublisher publisher; @@ -58,7 +58,8 @@ ApplicationEventPublisherAware { public Object run() { RequestContext ctx = RequestContext.getCurrentContext(); Map response = ctx.getZuulRequestHeaders(); - // N.B. this will only work with the simple host filter (not ribbon) unless you set hystrix.execution.isolation.strategy=SEMAPHORE + // N.B. this will only work with the simple host filter (not ribbon) unless you + // set hystrix.execution.isolation.strategy=SEMAPHORE Span span = getCurrentSpan(); if (span == null) { setHeader(response, Span.NOT_SAMPLED_NAME, "true"); @@ -68,6 +69,9 @@ ApplicationEventPublisherAware { setHeader(response, Span.SPAN_ID_NAME, span.getSpanId()); setHeader(response, Span.TRACE_ID_NAME, span.getTraceId()); setHeader(response, Span.SPAN_NAME_NAME, span.getName()); + if (!span.isExportable()) { + setHeader(response, Span.NOT_SAMPLED_NAME, "true"); + } setHeader(response, Span.PARENT_ID_NAME, getParentId(span)); setHeader(response, Span.PROCESS_ID_NAME, span.getProcessId()); // TODO: the client sent event should come from the client not the filter! @@ -84,8 +88,7 @@ ApplicationEventPublisherAware { } private Long getParentId(Span span) { - return !span.getParents().isEmpty() ? span - .getParents().get(0) : null; + return !span.getParents().isEmpty() ? span.getParents().get(0) : null; } public void setHeader(Map request, String name, String value) { @@ -93,8 +96,11 @@ ApplicationEventPublisherAware { request.put(name, value); } } + public void setHeader(Map request, String name, Long value) { - setHeader(request, name, Span.toHex(value)); + if (value != null) { + setHeader(request, name, Span.toHex(value)); + } } @Override diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java new file mode 100644 index 000000000..7d5f3438c --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java @@ -0,0 +1,62 @@ +/* + * Copyright 2015 the original author or 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.zuul; + +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.verify; + +import java.util.Random; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.cloud.sleuth.event.ClientReceivedEvent; +import org.springframework.cloud.sleuth.sampler.AlwaysSampler; +import org.springframework.cloud.sleuth.trace.DefaultTracer; +import org.springframework.cloud.sleuth.trace.SpanContextHolder; +import org.springframework.context.ApplicationEventPublisher; + +import com.netflix.zuul.context.RequestContext; + +/** + * @author Dave Syer + * + */ +public class TracePostZuulFilterTests { + + private ApplicationEventPublisher publisher = Mockito.mock(ApplicationEventPublisher.class); + + private DefaultTracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(), Mockito.mock(ApplicationEventPublisher.class)); + + private TracePostZuulFilter filter = new TracePostZuulFilter(this.tracer); + + @After + @Before + public void clean() { + RequestContext.getCurrentContext().unset(); + SpanContextHolder.removeCurrentSpan(); + } + + @Test + public void filterPublishesEvent() throws Exception { + this.filter.setApplicationEventPublisher(this.publisher); + this.tracer.startTrace("start"); + this.filter.run(); + verify(this.publisher).publishEvent(isA(ClientReceivedEvent.class)); + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java new file mode 100644 index 000000000..6b1a2f576 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java @@ -0,0 +1,76 @@ +/* + * Copyright 2015 the original author or 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.zuul; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertThat; + +import java.util.Random; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.sampler.AlwaysSampler; +import org.springframework.cloud.sleuth.sampler.IsTracingSampler; +import org.springframework.cloud.sleuth.trace.DefaultTracer; +import org.springframework.cloud.sleuth.trace.SpanContextHolder; +import org.springframework.context.ApplicationEventPublisher; + +import com.netflix.zuul.context.RequestContext; + +/** + * @author Dave Syer + * + */ +public class TracePreZuulFilterTests { + + private ApplicationEventPublisher publisher = Mockito.mock(ApplicationEventPublisher.class); + + private DefaultTracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(), this.publisher); + + private TracePreZuulFilter filter = new TracePreZuulFilter(this.tracer); + + @After + @Before + public void clean() { + RequestContext.getCurrentContext().unset(); + SpanContextHolder.removeCurrentSpan(); + } + + @Test + public void filterAddsHeaders() throws Exception { + this.tracer.startTrace("start"); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertThat(ctx.getZuulRequestHeaders().get(Span.TRACE_ID_NAME), is(notNullValue())); + assertThat(ctx.getZuulRequestHeaders().get(Span.NOT_SAMPLED_NAME), is(nullValue())); + } + + @Test + public void notSampledIfNotExportable() throws Exception { + this.tracer.startTrace("start", new IsTracingSampler()); + this.filter.run(); + RequestContext ctx = RequestContext.getCurrentContext(); + assertThat(ctx.getZuulRequestHeaders().get(Span.TRACE_ID_NAME), is(notNullValue())); + assertThat(ctx.getZuulRequestHeaders().get(Span.NOT_SAMPLED_NAME), is(notNullValue())); + } + +}