Skip to content

Commit df887c3

Browse files
buffer51bulldozer-bot[bot]
authored andcommitted
Failing streaming endpoints do return trace IDs (#45)
## Before this PR A failing streaming endpoint would not return the trace ID as a response header. ## After this PR Currently this PR doesn't fix the issue, it just adds unit tests that show the problem.
1 parent 55198c9 commit df887c3

File tree

2 files changed

+72
-1
lines changed

2 files changed

+72
-1
lines changed

tracing-jersey/src/main/java/com/palantir/tracing/jersey/TraceEnrichingFilter.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ public void filter(ContainerRequestContext requestContext, ContainerResponseCont
8787
if (maybeSpan.isPresent()) {
8888
Span span = maybeSpan.get();
8989
headers.putSingle(TraceHttpHeaders.TRACE_ID, span.getTraceId());
90+
} else {
91+
// When the filter is called twice (e.g. an exception is thrown in a streaming call),
92+
// the current trace will be empty. To allow clients to still get the trace ID corresponding to
93+
// the failure, we retrieve it from the requestContext.
94+
Optional.ofNullable(requestContext.getProperty(TRACE_ID_PROPERTY_NAME))
95+
.ifPresent(s -> headers.putSingle(TraceHttpHeaders.TRACE_ID, s));
9096
}
9197
}
9298

tracing-jersey/src/test/java/com/palantir/tracing/jersey/TraceEnrichingFilterTest.java

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import javax.ws.rs.container.ContainerRequestContext;
5050
import javax.ws.rs.core.MediaType;
5151
import javax.ws.rs.core.Response;
52+
import javax.ws.rs.core.StreamingOutput;
5253
import javax.ws.rs.core.UriInfo;
5354
import org.glassfish.jersey.client.JerseyClientBuilder;
5455
import org.junit.After;
@@ -156,6 +157,36 @@ public void testTraceState_withoutRequestHeadersGeneratesValidTraceResponseHeade
156157
assertThat(spanCaptor.getValue().getOperation(), is("GET /trace"));
157158
}
158159

160+
@Test
161+
public void testTraceState_withoutRequestHeadersGeneratesValidTraceResponseHeadersWhenFailing() {
162+
Response response = target.path("/failing-trace").request().get();
163+
assertThat(response.getHeaderString(TraceHttpHeaders.TRACE_ID), not(nullValue()));
164+
assertThat(response.getHeaderString(TraceHttpHeaders.PARENT_SPAN_ID), is(nullValue()));
165+
assertThat(response.getHeaderString(TraceHttpHeaders.SPAN_ID), is(nullValue()));
166+
verify(observer).consume(spanCaptor.capture());
167+
assertThat(spanCaptor.getValue().getOperation(), is("GET /failing-trace"));
168+
}
169+
170+
@Test
171+
public void testTraceState_withoutRequestHeadersGeneratesValidTraceResponseHeadersWhenStreaming() {
172+
Response response = target.path("/streaming-trace").request().get();
173+
assertThat(response.getHeaderString(TraceHttpHeaders.TRACE_ID), not(nullValue()));
174+
assertThat(response.getHeaderString(TraceHttpHeaders.PARENT_SPAN_ID), is(nullValue()));
175+
assertThat(response.getHeaderString(TraceHttpHeaders.SPAN_ID), is(nullValue()));
176+
verify(observer).consume(spanCaptor.capture());
177+
assertThat(spanCaptor.getValue().getOperation(), is("GET /streaming-trace"));
178+
}
179+
180+
@Test
181+
public void testTraceState_withoutRequestHeadersGeneratesValidTraceResponseHeadersWhenFailingToStream() {
182+
Response response = target.path("/failing-streaming-trace").request().get();
183+
assertThat(response.getHeaderString(TraceHttpHeaders.TRACE_ID), not(nullValue()));
184+
assertThat(response.getHeaderString(TraceHttpHeaders.PARENT_SPAN_ID), is(nullValue()));
185+
assertThat(response.getHeaderString(TraceHttpHeaders.SPAN_ID), is(nullValue()));
186+
verify(observer).consume(spanCaptor.capture());
187+
assertThat(spanCaptor.getValue().getOperation(), is("GET /failing-streaming-trace"));
188+
}
189+
159190
@Test
160191
public void testTraceState_withSamplingHeaderWithoutTraceIdDoesNotUseTraceSampler() {
161192
target.path("/trace").request()
@@ -216,13 +247,33 @@ public final void run(Configuration config, final Environment env) throws Except
216247

217248
public static final class TracingTestResource implements TracingTestService {
218249
@Override
219-
public void getTraceOperation() {}
250+
public void getTraceOperation() {
251+
throw new RuntimeException("FAIL");
252+
}
220253

221254
@Override
222255
public void postTraceOperation() {}
223256

224257
@Override
225258
public void getTraceWithPathParam() {}
259+
260+
@Override
261+
public void getFailingTraceOperation() {
262+
throw new RuntimeException();
263+
}
264+
265+
@Override
266+
public StreamingOutput getFailingStreamingTraceOperation() {
267+
return os -> {
268+
throw new RuntimeException();
269+
};
270+
}
271+
272+
@Override
273+
public StreamingOutput getStreamingTraceOperation() {
274+
return os -> {
275+
};
276+
}
226277
}
227278

228279
@Path("/")
@@ -240,5 +291,19 @@ public interface TracingTestService {
240291
@GET
241292
@Path("/trace/{param}")
242293
void getTraceWithPathParam();
294+
295+
@GET
296+
@Path("/failing-trace")
297+
void getFailingTraceOperation();
298+
299+
@GET
300+
@Path("/failing-streaming-trace")
301+
@Produces(MediaType.APPLICATION_OCTET_STREAM)
302+
StreamingOutput getFailingStreamingTraceOperation();
303+
304+
@GET
305+
@Path("/streaming-trace")
306+
@Produces(MediaType.APPLICATION_OCTET_STREAM)
307+
StreamingOutput getStreamingTraceOperation();
243308
}
244309
}

0 commit comments

Comments
 (0)