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

Removes guava dependency #72

Merged
merged 1 commit into from Jun 7, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -7,7 +7,6 @@

import com.github.kristofa.brave.BraveHttpHeaders;
import com.github.kristofa.brave.client.ClientRequestAdapter;
import com.google.common.base.Optional;

class ApacheRequestAdapter implements ClientRequestAdapter {

Expand All @@ -28,13 +27,9 @@ public String getMethod() {
}

@Override
public Optional<String> getSpanName() {
Optional<String> spanName = Optional.absent();
public String getSpanName() {
final Header spanNameHeader = request.getFirstHeader(BraveHttpHeaders.SpanName.getName());
if (spanNameHeader != null) {
spanName = Optional.fromNullable(spanNameHeader.getValue());
}
return spanName;
return spanNameHeader != null ? spanNameHeader.getValue() : null;
}

@Override
Expand Down
Expand Up @@ -8,7 +8,6 @@
import com.github.kristofa.brave.ClientTracer;
import com.github.kristofa.brave.client.ClientRequestInterceptor;
import com.github.kristofa.brave.client.spanfilter.SpanNameFilter;
import com.google.common.base.Optional;

/**
* Apache HttpClient {@link HttpRequestInterceptor} that adds brave/zipkin annotations to outgoing client request.
Expand All @@ -25,46 +24,33 @@
public class BraveHttpRequestInterceptor implements HttpRequestInterceptor {

private final ClientRequestInterceptor clientRequestInterceptor;
private final Optional<String> serviceName;
private final String serviceName;

/**
* Creates a new instance.
*
* @param clientTracer ClientTracer should not be <code>null</code>.
* @param serviceName Optional Service Name override
* @param spanNameFilter
* @param serviceName Nullable Service Name override
* @param spanNameFilter Nullable {@link SpanNameFilter}
*/
public BraveHttpRequestInterceptor(final ClientTracer clientTracer, final Optional<String> serviceName,
public BraveHttpRequestInterceptor(final ClientTracer clientTracer, final String serviceName,
final SpanNameFilter spanNameFilter) {
this(clientTracer, serviceName, Optional.of(spanNameFilter));
Validate.notNull(clientTracer);
clientRequestInterceptor = new ClientRequestInterceptor(clientTracer, spanNameFilter);
this.serviceName = serviceName;
}

/**
* Creates a new instance.
*
* @param clientTracer ClientTracer should not be <code>null</code>.
* @param serviceName Optional Service Name override
* @param serviceName Nullable Service Name override
*/
public BraveHttpRequestInterceptor(final ClientTracer clientTracer, final Optional<String> serviceName) {
this(clientTracer, serviceName, Optional.<SpanNameFilter>absent());
public BraveHttpRequestInterceptor(final ClientTracer clientTracer, final String serviceName) {
this(clientTracer, serviceName, null);

}

/**
* Private constructor, creates a new instance.
*
* @param clientTracer ClientTracer should not be <code>null</code>.
* @param serviceName Optional Service Name override
* @param spanNameFilter optional {@link SpanNameFilter}
*/
private BraveHttpRequestInterceptor(final ClientTracer clientTracer, final Optional<String> serviceName,
final Optional<SpanNameFilter> spanNameFilter) {
Validate.notNull(clientTracer);
Validate.notNull(serviceName);
clientRequestInterceptor = new ClientRequestInterceptor(clientTracer, spanNameFilter);
this.serviceName = serviceName;
}

/**
* {@inheritDoc}
*/
Expand Down
Expand Up @@ -9,7 +9,6 @@
import com.github.kristofa.test.http.Method;
import com.github.kristofa.test.http.MockHttpServer;
import com.github.kristofa.test.http.UnsatisfiedExpectationException;
import com.google.common.base.Optional;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpGet;
Expand Down Expand Up @@ -77,7 +76,7 @@ public void testTracingTrue() throws ClientProtocolException, IOException, Unsat
responseProvider.set(request, response);

final CloseableHttpClient httpclient =
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, Optional.<String>absent()))
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, null))
.addInterceptorFirst(new BraveHttpResponseInterceptor(clientTracer)).build();
try {
final HttpGet httpGet = new HttpGet(REQUEST);
Expand Down Expand Up @@ -112,7 +111,7 @@ public void testTracingFalse() throws ClientProtocolException, IOException, Unsa
responseProvider.set(request, response);

final CloseableHttpClient httpclient =
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, Optional.<String>absent()))
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, null))
.addInterceptorFirst(new BraveHttpResponseInterceptor(clientTracer)).build();
try {
final HttpGet httpGet = new HttpGet(REQUEST);
Expand Down Expand Up @@ -150,7 +149,7 @@ public void testQueryParams() throws ClientProtocolException, IOException, Unsat
responseProvider.set(request, response);

final CloseableHttpClient httpclient =
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, Optional.<String>absent()))
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, null))
.addInterceptorFirst(new BraveHttpResponseInterceptor(clientTracer)).build();
try {
final HttpGet httpGet = new HttpGet(REQUEST_WITH_QUERY_PARAMS);
Expand Down Expand Up @@ -188,7 +187,7 @@ public void testTracingTrueWithServiceNameOverride() throws ClientProtocolExcept
responseProvider.set(request, response);

final CloseableHttpClient httpclient =
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, Optional.of(SERVICE)))
HttpClients.custom().addInterceptorFirst(new BraveHttpRequestInterceptor(clientTracer, SERVICE))
.addInterceptorFirst(new BraveHttpResponseInterceptor(clientTracer)).build();
try {
final HttpGet httpGet = new HttpGet(REQUEST);
Expand Down
@@ -1,7 +1,5 @@
package com.github.kristofa.brave.client;

import com.google.common.base.Optional;

import java.net.URI;

/**
Expand All @@ -28,10 +26,10 @@ public interface ClientRequestAdapter {
/**
* Get optional span name.
*
* @return Optional span name. In case span name is not provided ClientRequestInterceptor will
* @return nullable span name. In case span name is not provided ClientRequestInterceptor will
* use a default way to build span name.
*/
Optional<String> getSpanName();
String getSpanName();

/**
* ClientRequestInterceptor will submit http headers using this method
Expand Down
Expand Up @@ -5,7 +5,6 @@
import com.github.kristofa.brave.ClientTracer;
import com.github.kristofa.brave.SpanId;
import com.github.kristofa.brave.client.spanfilter.SpanNameFilter;
import com.google.common.base.Optional;

/**
* Intercepts a client request and takes care of tracing the request. It will decide if we need to trace current request and
Expand All @@ -24,9 +23,9 @@ public class ClientRequestInterceptor {

private final ClientTracer clientTracer;
private static final String REQUEST_ANNOTATION = "request";
private final Optional<SpanNameFilter> spanNameFilter;
private final SpanNameFilter spanNameFilter;

public ClientRequestInterceptor(final ClientTracer clientTracer, final Optional<SpanNameFilter> spanNameFilter) {
public ClientRequestInterceptor(final ClientTracer clientTracer, final SpanNameFilter spanNameFilter) {
Validate.notNull(clientTracer);
this.clientTracer = clientTracer;
this.spanNameFilter = spanNameFilter;
Expand All @@ -36,49 +35,46 @@ public ClientRequestInterceptor(final ClientTracer clientTracer, final Optional<
* Handles a client request.
*
* @param clientRequestAdapter Provides context about the request.
* @param serviceNameOverride Optional name of the service the client request calls. In case it is not specified the name
* @param serviceNameOverride Nullable name of the service the client request calls. In case it is not specified the name
* will be derived from the URI of the request. It is important the used service name should be same on client
* as on server side.
*/
public void handle(final ClientRequestAdapter clientRequestAdapter, final Optional<String> serviceNameOverride) {
public void handle(final ClientRequestAdapter clientRequestAdapter, final String serviceNameOverride) {
final String spanName = getSpanName(clientRequestAdapter, serviceNameOverride);
final SpanId newSpanId = clientTracer.startNewSpan(spanName);
ClientRequestHeaders.addTracingHeaders(clientRequestAdapter, newSpanId, spanName);
final Optional<String> serviceName = getServiceName(clientRequestAdapter, serviceNameOverride);
if (serviceName.isPresent()) {
clientTracer.setCurrentClientServiceName(serviceName.get());
final String serviceName = getServiceName(clientRequestAdapter, serviceNameOverride);
if (serviceName != null) {
clientTracer.setCurrentClientServiceName(serviceName);
}
clientTracer.submitBinaryAnnotation(REQUEST_ANNOTATION, clientRequestAdapter.getMethod() + " "
+ clientRequestAdapter.getUri());
clientTracer.setClientSent();
}

private Optional<String> getServiceName(final ClientRequestAdapter clientRequestAdapter,
final Optional<String> serviceNameOverride) {
Optional<String> serviceName;
if (serviceNameOverride.isPresent()) {
private String getServiceName(final ClientRequestAdapter clientRequestAdapter, final String serviceNameOverride) {
String serviceName = null;
if (serviceNameOverride != null) {
serviceName = serviceNameOverride;
} else {
final String path = clientRequestAdapter.getUri().getPath();
final String[] split = path.split("/");
if (split.length > 2 && path.startsWith("/")) {
// If path starts with '/', then context is between first two '/'. Left over is path for service.
final int contextPathSeparatorIndex = path.indexOf("/", 1);
serviceName = Optional.of(path.substring(1, contextPathSeparatorIndex));
} else {
serviceName = Optional.absent();
serviceName = path.substring(1, contextPathSeparatorIndex);
}
}
return serviceName;
}

private String getSpanName(final ClientRequestAdapter clientRequestAdapter, final Optional<String> serviceNameOverride) {
private String getSpanName(final ClientRequestAdapter clientRequestAdapter, final String serviceNameOverride) {
String spanName;
final Optional<String> spanNameFromRequest = clientRequestAdapter.getSpanName();
final String spanNameFromRequest = clientRequestAdapter.getSpanName();
final String path = clientRequestAdapter.getUri().getPath();
if (spanNameFromRequest.isPresent()) {
return spanNameFromRequest.get();
} else if (serviceNameOverride.isPresent()) {
if (spanNameFromRequest != null) {
return spanNameFromRequest;
} else if (serviceNameOverride != null) {
spanName = path;
} else {
final String[] split = path.split("/");
Expand All @@ -90,8 +86,8 @@ private String getSpanName(final ClientRequestAdapter clientRequestAdapter, fina
spanName = path;
}
}
if (spanNameFilter.isPresent()) {
spanName = spanNameFilter.get().filterSpanName(spanName);
if (spanNameFilter != null) {
spanName = spanNameFilter.filterSpanName(spanName);
}
return spanName;
}
Expand Down
@@ -1,8 +1,6 @@
package com.github.kristofa.brave.client.spanfilter;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;

import java.util.ArrayList;
import java.util.List;
import java.util.regex.Pattern;

Expand All @@ -11,7 +9,6 @@
* this filter will only allow these span names to be set as the client span name (else "[span name not defined]").
*/
public class PatternBasedSpanNameFilterImpl implements SpanNameFilter {
@VisibleForTesting
static final String DEFAULT_SPAN_NAME = "[span name not defined]";

private final Iterable<SpanNamePattern> spanNamePatterns;
Expand All @@ -22,7 +19,7 @@ public PatternBasedSpanNameFilterImpl(Iterable<String> spanNamePatterns) {
}

public PatternBasedSpanNameFilterImpl(Iterable<String> spanNamePatterns, String defaultSpanName) {
final List<SpanNamePattern> patternNamePairs = Lists.newArrayList();
final List<SpanNamePattern> patternNamePairs = new ArrayList<>();
if (spanNamePatterns != null) {
for (String spanNamePattern : spanNamePatterns) {
if (spanNamePattern != null) {
Expand Down
Expand Up @@ -19,7 +19,6 @@
import com.github.kristofa.brave.ClientTracer;
import com.github.kristofa.brave.SpanId;
import com.github.kristofa.brave.client.spanfilter.SpanNameFilter;
import com.google.common.base.Optional;

public class ClientRequestInterceptorTest {

Expand All @@ -42,24 +41,23 @@ public class ClientRequestInterceptorTest {
public void setUp() throws Exception {
mockClientTracer = mock(ClientTracer.class);
mockSpanNameFilter = mock(SpanNameFilter.class);
final Optional<SpanNameFilter> optionalSpanNameFilter = Optional.absent();
interceptor = new ClientRequestInterceptor(mockClientTracer, optionalSpanNameFilter);
interceptor = new ClientRequestInterceptor(mockClientTracer, null);
clientRequestAdapter = mock(ClientRequestAdapter.class);
when(clientRequestAdapter.getUri()).thenReturn(URI.create(FULL_PATH));
when(clientRequestAdapter.getMethod()).thenReturn(METHOD);
when(clientRequestAdapter.getSpanName()).thenReturn(Optional.<String>absent());
when(clientRequestAdapter.getSpanName()).thenReturn(null);
}

@Test(expected = IllegalArgumentException.class)
public void testBraveHttpRequestInterceptor() {
new ClientRequestInterceptor(null, Optional.of(mockSpanNameFilter));
new ClientRequestInterceptor(null, mockSpanNameFilter);
}

@Test
public void testProcessNoTracing() throws HttpException, IOException {
when(mockClientTracer.startNewSpan(PATH)).thenReturn(null);

interceptor.handle(clientRequestAdapter, Optional.<String>absent());
interceptor.handle(clientRequestAdapter, null);

final InOrder inOrder = inOrder(mockClientTracer, clientRequestAdapter);

Expand All @@ -79,7 +77,7 @@ public void testProcessTracing() throws HttpException, IOException {
when(spanId.getTraceId()).thenReturn(TRACE_ID);
when(mockClientTracer.startNewSpan(PATH)).thenReturn(spanId);

interceptor.handle(clientRequestAdapter, Optional.<String>absent());
interceptor.handle(clientRequestAdapter, null);

final InOrder inOrder = inOrder(mockClientTracer, clientRequestAdapter);

Expand All @@ -104,7 +102,7 @@ public void testProcessTracingNoParentId() throws HttpException, IOException {
when(spanId.getTraceId()).thenReturn(TRACE_ID);
when(mockClientTracer.startNewSpan(PATH)).thenReturn(spanId);

interceptor.handle(clientRequestAdapter, Optional.<String>absent());
interceptor.handle(clientRequestAdapter, null);

final InOrder inOrder = inOrder(mockClientTracer, clientRequestAdapter);

Expand All @@ -128,7 +126,7 @@ public void testHandleTracingWithServiceNameOverride() throws HTTPException, IOE
when(spanId.getTraceId()).thenReturn(TRACE_ID);
when(mockClientTracer.startNewSpan(FULL_PATH)).thenReturn(spanId);

interceptor.handle(clientRequestAdapter, Optional.of(SERVICE_NAME));
interceptor.handle(clientRequestAdapter, SERVICE_NAME);

final InOrder inOrder = inOrder(mockClientTracer, clientRequestAdapter);

Expand All @@ -154,8 +152,8 @@ public void testHandleServiceWithSpanNameFilter() {
when(mockClientTracer.startNewSpan(FILTERED_PATH)).thenReturn(spanId);
when(mockSpanNameFilter.filterSpanName(PATH)).thenReturn(FILTERED_PATH);

interceptor = new ClientRequestInterceptor(mockClientTracer, Optional.of(mockSpanNameFilter));
interceptor.handle(clientRequestAdapter, Optional.<String>absent());
interceptor = new ClientRequestInterceptor(mockClientTracer, mockSpanNameFilter);
interceptor.handle(clientRequestAdapter, null);

final InOrder inOrder = inOrder(mockClientTracer, clientRequestAdapter, mockSpanNameFilter);
inOrder.verify(mockSpanNameFilter).filterSpanName(PATH);
Expand Down
@@ -1,8 +1,8 @@
package com.github.kristofa.brave.client.spanfilter;

import com.google.common.collect.Lists;
import org.junit.Test;

import java.util.Arrays;
import java.util.Collections;

import static org.junit.Assert.*;
Expand All @@ -15,7 +15,7 @@ public void testMultiplePatterns() throws Exception {
final String remove = "/api/{something}/{else}/remove";
final String anythingElse = "/api/{anythingelse}";
final PatternBasedSpanNameFilterImpl filter
= new PatternBasedSpanNameFilterImpl(Lists.newArrayList(add, remove, anythingElse));
= new PatternBasedSpanNameFilterImpl(Arrays.asList(add, remove, anythingElse));

assertEquals(add, filter.filterSpanName("/api/clients/relationships/add"));
assertEquals(remove, filter.filterSpanName("/api/science/math/remove"));
Expand All @@ -26,7 +26,7 @@ public void testMultiplePatterns() throws Exception {
public void testNoMatch() throws Exception {
final String add = "/api/{something}/{else}/add";
final PatternBasedSpanNameFilterImpl filter
= new PatternBasedSpanNameFilterImpl(Lists.newArrayList(add));
= new PatternBasedSpanNameFilterImpl(Arrays.asList(add));

assertEquals(PatternBasedSpanNameFilterImpl.DEFAULT_SPAN_NAME, filter.filterSpanName("/api/nomatch"));
}
Expand All @@ -48,15 +48,17 @@ public void testHandlesEmpty() throws Exception {
@Test
public void testHandlesNullPatterns() throws Exception {
final String undef = "not-defined";
final PatternBasedSpanNameFilterImpl filter = new PatternBasedSpanNameFilterImpl(Lists.<String>newArrayList(null, null), undef);
final PatternBasedSpanNameFilterImpl filter = new PatternBasedSpanNameFilterImpl(Arrays.asList(
null, null), undef);

assertEquals(undef, filter.filterSpanName("/api/not-a-match"));
}

@Test
public void testCaseInsensitive() throws Exception {
final String updatePattern = "/api/{userid}/update";
final PatternBasedSpanNameFilterImpl filter = new PatternBasedSpanNameFilterImpl(Lists.newArrayList(updatePattern));
final PatternBasedSpanNameFilterImpl filter = new PatternBasedSpanNameFilterImpl(Arrays.asList(
updatePattern));

assertEquals(updatePattern, filter.filterSpanName("/api/12/update"));
assertEquals(updatePattern, filter.filterSpanName("/api/12/UPDATE"));
Expand Down