Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Add builder patter and deprecate some APIs #424

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ public class RemoteBaggageRestrictionManager implements BaggageRestrictionManage
private final Restriction invalidRestriction;
private final Restriction validRestriction;

/**
* @deprecated use {@link Builder}
*/
@Deprecated
public RemoteBaggageRestrictionManager(
String serviceName,
BaggageRestrictionManagerProxy proxy,
Expand All @@ -50,6 +54,10 @@ public RemoteBaggageRestrictionManager(
this(serviceName, proxy, metrics, denyBaggageOnInitializationFailure, DEFAULT_REFRESH_INTERVAL_MS);
}

/**
* @deprecated use {@link Builder}
*/
@Deprecated
public RemoteBaggageRestrictionManager(
String serviceName,
BaggageRestrictionManagerProxy proxy,
Expand All @@ -60,29 +68,7 @@ public RemoteBaggageRestrictionManager(
this(serviceName, proxy, metrics, denyBaggageOnInitializationFailure, refreshIntervalMs, DEFAULT_INITIAL_DELAY_MS);
}

/**
* Creates a RemoteBaggageRestrictionManager that fetches {@link BaggageRestrictionResponse} from a remote
* agent and keeps track of {@link Restriction} for a service.
*
* {@param initialDelayMs} is only exposed for testing purposes so users can determine when the first call to
* remote agent is made. Under normal operations, this RemoteBaggageRestrictionManager will start up and
* asynchronously fetch restrictions. If the user wants to know if restrictions are ready, they can check via
* isReady().
*
* @param serviceName restrictions for this service are kept track of.
* @param proxy proxy to remote agent.
* @param metrics metrics for metrics emission.
* @param denyBaggageOnInitializationFailure determines the startup failure mode of RemoteBaggageRestrictionManager.
* If DenyBaggageOnInitializationFailure is true,
* RemoteBaggageRestrictionManager will not allow any baggage to be written
* until baggage restrictions have been retrieved from agent. If
* DenyBaggageOnInitializationFailure is false,
* RemoteBaggageRestrictionManager will allow any baggage to be written
* until baggage restrictions have been retrieved from agent.
* @param refreshIntervalMs how often restriction are fetched from remote agent.
* @param initialDelayMs delay before first fetch of restrictions.
*/
RemoteBaggageRestrictionManager(
private RemoteBaggageRestrictionManager(
String serviceName,
BaggageRestrictionManagerProxy proxy,
Metrics metrics,
Expand Down Expand Up @@ -155,4 +141,79 @@ public Restriction getRestriction(String service, String key) {
}
return invalidRestriction;
}

public static class Builder {
private String serviceName;
private BaggageRestrictionManagerProxy proxy;
private Metrics metrics;
private boolean denyBaggageOnInitializationFailure;
private int refreshIntervalMs = DEFAULT_REFRESH_INTERVAL_MS;
private int initialDelayMs = DEFAULT_INITIAL_DELAY_MS;

/**
* @param serviceName restrictions for this service are kept track of.
*/
public Builder(String serviceName) {
this.serviceName = serviceName;
}

/**
* @param proxy proxy to remote agent.
*/
public Builder withProxy(BaggageRestrictionManagerProxy proxy) {
this.proxy = proxy;
return this;
}

public Builder withMetrics(Metrics metrics) {
this.metrics = metrics;
return this;
}

/**
* If {@code denyBaggageOnInitializationFailure} is true,
* {@link RemoteBaggageRestrictionManager} will not allow any baggage to be written until
* baggage restrictions have been retrieved from agent. If {@code denyBaggageOnInitializationFailure} is
* false, {@link RemoteBaggageRestrictionManager} will allow any baggage to be written until
* baggage restrictions have been retrieved from agent.
*
* @param denyBaggageOnInitializationFailure determines the startup failure mode of
* {@link RemoteBaggageRestrictionManager}
*/
public Builder withDenyBaggageInitializationFailure(boolean denyBaggageOnInitializationFailure) {
this.denyBaggageOnInitializationFailure = denyBaggageOnInitializationFailure;
return this;
}

/**
* @param refreshIntervalMs how often restriction are fetched from remote agent.
*/
public Builder withRefreshIntervalMs(int refreshIntervalMs) {
this.refreshIntervalMs = refreshIntervalMs;
return this;
}

/**
* {@code initialDelayMs} is only exposed for testing purposes so users can determine when
* the first call to remote agent is made. Under normal operations,
* this {@link RemoteBaggageRestrictionManager} will start up and asynchronously fetch restrictions.
* If the user wants to know if restrictions are ready, they can check via
* {@link RemoteBaggageRestrictionManager#isReady()}.
*
* @param initialDelayMs delay before first fetch of restrictions.
*/
public Builder withInitialDelayMs(int initialDelayMs) {
this.initialDelayMs = initialDelayMs;
return this;
}

/**
* Creates a RemoteBaggageRestrictionManager that fetches {@link BaggageRestrictionResponse} from a remote
* agent and keeps track of {@link Restriction} for a service.
*/
public RemoteBaggageRestrictionManager build() {
return new RemoteBaggageRestrictionManager(serviceName, proxy, metrics, denyBaggageOnInitializationFailure,
refreshIntervalMs, initialDelayMs);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public class RemoteReporter implements Reporter {
private final int closeEnqueueTimeout;
private final Metrics metrics;

RemoteReporter(Sender sender, int flushInterval, int maxQueueSize, int closeEnqueueTimeout,
private RemoteReporter(Sender sender, int flushInterval, int maxQueueSize, int closeEnqueueTimeout,
Metrics metrics) {
this.sender = sender;
this.metrics = metrics;
Expand Down Expand Up @@ -187,6 +187,7 @@ public static class Builder {
private Sender sender;
private int flushInterval = DEFAULT_FLUSH_INTERVAL_MS;
private int maxQueueSize = DEFAULT_MAX_QUEUE_SIZE;
private int closeEnqueTimeout = DEFAULT_CLOSE_ENQUEUE_TIMEOUT_MILLIS;
private Metrics metrics;

public Builder withFlushInterval(int flushInterval) {
Expand All @@ -209,14 +210,19 @@ public Builder withSender(Sender sender) {
return this;
}

public Builder withCloseEnqueueTimeout(int closeEnqueueTimeoutMs) {
this.closeEnqueTimeout = closeEnqueueTimeoutMs;
return this;
}

public RemoteReporter build() {
if (sender == null) {
sender = new UdpSender();
}
if (metrics == null) {
metrics = new Metrics(new InMemoryMetricsFactory());
}
return new RemoteReporter(sender, flushInterval, maxQueueSize, DEFAULT_CLOSE_ENQUEUE_TIMEOUT_MILLIS, metrics);
return new RemoteReporter(sender, flushInterval, maxQueueSize, closeEnqueTimeout, metrics);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ public class HttpSender extends ThriftSender {
*
* Uses the default {@link okhttp3.OkHttpClient} which uses {@link okhttp3.ConnectionPool#ConnectionPool()}.
* Use {@link HttpSender.Builder} if you need to add more parameters
* @deprecated use {@link Builder}
*/
@Deprecated
public HttpSender(String endpoint) {
this(new Builder(endpoint));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ public class RemoteBaggageRestrictionManagerTest {
public void setUp() throws Exception {
metricsFactory = new InMemoryMetricsFactory();
metrics = new Metrics(metricsFactory);
undertest = new RemoteBaggageRestrictionManager(SERVICE_NAME, baggageRestrictionProxy, metrics,
false);
undertest = new RemoteBaggageRestrictionManager.Builder(SERVICE_NAME)
.withProxy(baggageRestrictionProxy)
.withMetrics(metrics)
.withDenyBaggageInitializationFailure(false)
.build();
}

@After
Expand All @@ -76,8 +79,13 @@ public void testUpdateBaggageRestrictions() throws Exception {
public void testAllowBaggageOnInitializationFailure() throws Exception {
when(baggageRestrictionProxy.getBaggageRestrictions(SERVICE_NAME))
.thenThrow(new BaggageRestrictionManagerException("error"));
undertest = new RemoteBaggageRestrictionManager(SERVICE_NAME, baggageRestrictionProxy, metrics,
false, 60000, 60000);
undertest = new RemoteBaggageRestrictionManager.Builder(SERVICE_NAME)
.withProxy(baggageRestrictionProxy)
.withMetrics(metrics)
.withDenyBaggageInitializationFailure(false)
.withRefreshIntervalMs(60000)
.withInitialDelayMs(60000)
.build();

assertTrue(undertest.getRestriction(SERVICE_NAME, BAGGAGE_KEY).isKeyAllowed());
undertest.updateBaggageRestrictions();
Expand All @@ -92,8 +100,13 @@ public void testAllowBaggageOnInitializationFailure() throws Exception {
public void testDenyBaggageOnInitializationFailure() throws Exception {
when(baggageRestrictionProxy.getBaggageRestrictions(SERVICE_NAME))
.thenReturn(new ArrayList<BaggageRestrictionResponse>(Arrays.asList(RESTRICTION)));
undertest = new RemoteBaggageRestrictionManager(SERVICE_NAME, baggageRestrictionProxy, metrics,
true, 60000, 60000);
undertest = new RemoteBaggageRestrictionManager.Builder(SERVICE_NAME)
.withProxy(baggageRestrictionProxy)
.withMetrics(metrics)
.withDenyBaggageInitializationFailure(true)
.withRefreshIntervalMs(60000)
.withInitialDelayMs(60000)
.build();

assertFalse(undertest.getRestriction(SERVICE_NAME, BAGGAGE_KEY).isKeyAllowed());
undertest.updateBaggageRestrictions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,13 @@ public void testAppendWhenQueueFull() {
@Ignore("See https://github.com/jaegertracing/jaeger-client-java/issues/340")
public void testCloseWhenQueueFull() {
int closeTimeoutMillis = 5;
reporter = new RemoteReporter(sender, Integer.MAX_VALUE, maxQueueSize, closeTimeoutMillis, metrics);
reporter = new RemoteReporter.Builder()
.withSender(sender)
.withFlushInterval(Integer.MAX_VALUE)
.withMaxQueueSize(maxQueueSize)
.withCloseEnqueueTimeout(closeTimeoutMillis)
.withMetrics(metrics)
.build();
tracer = new Tracer.Builder("test-remote-reporter")
.withReporter(reporter)
.withSampler(new ConstSampler(true))
Expand Down