Skip to content

Commit

Permalink
Pulls up ConstantHttpEndpointSupplier (#251)
Browse files Browse the repository at this point in the history
Signed-off-by: Adrian Cole <adrian@tetrate.io>
  • Loading branch information
codefromthecrypt authored Feb 12, 2024
1 parent 0f03cf6 commit b5fbad8
Show file tree
Hide file tree
Showing 15 changed files with 81 additions and 66 deletions.
2 changes: 1 addition & 1 deletion RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ internal detail.

Someone who wants to employ client-side loadbalancer logic can do that inside the
`HttpEndpointSupplier`, and return a chosen endpoint. This will work because the supplier is
documented to not be cached, unless it implements `HttpEndpointSupplier.Constant`.
documented to not be cached, unless it is a `ConstantHttpEndpointSupplier`.

### Why is `HttpEndpointSupplier` closeable?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import zipkin2.TestObjects;
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.InMemoryReporterMetrics;
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.SpanBytesEncoder;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import brave.propagation.TraceContext;
import java.io.Closeable;
import zipkin2.Span;
import zipkin2.reporter.SpanBytesEncoder;
import zipkin2.reporter.Reporter;
import zipkin2.reporter.SpanBytesEncoder;

/**
* This allows you to send spans recorded by Brave to a pre-configured {@linkplain Reporter Zipkin
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2016-2024 The OpenZipkin 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 zipkin2.reporter;

/**
* HTTP {@link BytesMessageSender senders} check for this type, and will cache its first value.
*
* @since 3.3
*/
public final class ConstantHttpEndpointSupplier implements HttpEndpointSupplier {
/**
* HTTP {@link BytesMessageSender sender} builders check for this symbol, and return the input as
* a {@linkplain ConstantHttpEndpointSupplier} result rather than perform dynamic lookups.
*
* @since 3.3
*/
public static final HttpEndpointSupplier.Factory FACTORY = new Factory() {
@Override public ConstantHttpEndpointSupplier create(String endpoint) {
return new ConstantHttpEndpointSupplier(endpoint);
}
};

public static ConstantHttpEndpointSupplier create(String endpoint) {
return new ConstantHttpEndpointSupplier(endpoint);
}

private final String endpoint;

ConstantHttpEndpointSupplier(String endpoint) {
if (endpoint == null) throw new NullPointerException("endpoint == null");
this.endpoint = endpoint;
}

@Override public String get() {
return endpoint;
}

@Override public void close() {
}

@Override public String toString() {
return endpoint;
}
}
45 changes: 7 additions & 38 deletions core/src/main/java/zipkin2/reporter/HttpEndpointSupplier.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
* <p>{@link BytesMessageSender senders} should implement the following logic:
* <ul>
* <li>During build, the sender should invoke the {@linkplain Factory}.</li>
* <li>If the result is {@link Constant}, build the sender to use a static value.</li>
* <li>If the result is {@link ConstantHttpEndpointSupplier}, build the sender to use a static
* value.</li>
* <li>Otherwise, call {@link HttpEndpointSupplier#get()} each time
* {@linkplain BytesMessageSender#send(List)} is invoked.</li>
* <li>Call {@link #close()} once during {@link BytesMessageSender#close()}.</li>
Expand All @@ -48,23 +49,11 @@
* @since 3.3
*/
public interface HttpEndpointSupplier extends Closeable {
/**
* HTTP {@link BytesMessageSender sender} builders check for this symbol, and return the input as
* a {@linkplain Constant} result rather than perform dynamic lookups.
*
* @since 3.3
*/
Factory CONSTANT_FACTORY = new Factory() {
@Override public Constant create(String endpoint) {
return new Constant(endpoint);
}
};

/**
* Returns a possibly cached endpoint to an HTTP {@link BytesMessageSender sender}.
*
* <p>This will be called inside {@linkplain BytesMessageSender#send(List)}, unless this is an
* instance of {@linkplain Constant}.
* instance of {@linkplain ConstantHttpEndpointSupplier}.
*
* @since 3.3
*/
Expand All @@ -81,35 +70,15 @@ public interface HttpEndpointSupplier extends Closeable {
interface Factory {

/**
* Returns a possibly {@linkplain Constant} endpoint supplier, given a static endpoint from
* configuration.
* Returns a possibly {@linkplain ConstantHttpEndpointSupplier} endpoint supplier, given a
* static endpoint from configuration.
*
* <p>Note: Some factories may perform I/O to lazy-create a {@linkplain Constant} endpoint.
* <p>Note: Some factories may perform I/O to lazy-create a
* {@linkplain ConstantHttpEndpointSupplier} endpoint.
*
* @param endpoint a static HTTP endpoint from configuration. For example,
* http://localhost:9411/api/v2/spans
*/
HttpEndpointSupplier create(String endpoint);
}

/**
* HTTP {@link BytesMessageSender senders} check for this type, and will cache its first value.
*
* @since 3.3
*/
final class Constant implements HttpEndpointSupplier {
private final String endpoint;

public Constant(String endpoint) {
if (endpoint == null) throw new NullPointerException("endpoint == null");
this.endpoint = endpoint;
}

@Override public String get() {
return endpoint;
}

@Override public void close() {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import java.io.Closeable;
import java.io.Flushable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.concurrent.CountDownLatch;
Expand All @@ -33,7 +32,6 @@
import zipkin2.reporter.Component;
import zipkin2.reporter.Reporter;
import zipkin2.reporter.ReporterMetrics;
import zipkin2.reporter.Sender;

import static java.lang.String.format;
import static java.util.logging.Level.FINE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package zipkin2.reporter;

import java.io.IOException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.CheckResult;
import zipkin2.reporter.ClosedSenderException;
import zipkin2.reporter.ConstantHttpEndpointSupplier;
import zipkin2.reporter.Encoding;
import zipkin2.reporter.HttpEndpointSupplier;
import zipkin2.reporter.Sender;
Expand Down Expand Up @@ -98,7 +99,7 @@ public static Builder newBuilder() {

public static final class Builder {
final OkHttpClient.Builder clientBuilder;
HttpEndpointSupplier.Factory endpointSupplierFactory = HttpEndpointSupplier.CONSTANT_FACTORY;
HttpEndpointSupplier.Factory endpointSupplierFactory = ConstantHttpEndpointSupplier.FACTORY;
String endpoint;
Encoding encoding = Encoding.JSON;
boolean compressionEnabled = true;
Expand Down Expand Up @@ -207,7 +208,7 @@ public OkHttpSender build() {
if (endpointSupplier == null) {
throw new NullPointerException("endpointSupplierFactory.create() returned null");
}
if (endpointSupplier instanceof HttpEndpointSupplier.Constant) {
if (endpointSupplier instanceof ConstantHttpEndpointSupplier) {
endpoint = endpointSupplier.get(); // eagerly resolve the endpoint
return new OkHttpSender(this, new ConstantHttpUrlSupplier(endpoint));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import zipkin2.reporter.AsyncReporter;
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.ClosedSenderException;
import zipkin2.reporter.ConstantHttpEndpointSupplier;
import zipkin2.reporter.HttpEndpointSupplier;
import zipkin2.reporter.HttpEndpointSupplier.Constant;

import static java.util.stream.Collectors.toList;
import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -42,14 +42,14 @@ class OkHttpSenderTest {

// Change the supplier, but not the endpoint.
sender = sender.toBuilder()
.endpointSupplierFactory(e -> new Constant("http://localhost:29092"))
.endpointSupplierFactory(e -> ConstantHttpEndpointSupplier.create("http://localhost:29092"))
.build();
assertThat(sender)
.hasToString("OkHttpSender{http://localhost:29092/}");

// Change the supplier, and see the prior endpoint.
sender = sender.toBuilder()
.endpointSupplierFactory(HttpEndpointSupplier.CONSTANT_FACTORY)
.endpointSupplierFactory(ConstantHttpEndpointSupplier.FACTORY)
.build();
assertThat(sender)
.hasToString("OkHttpSender{http://localhost:19092/}");
Expand Down Expand Up @@ -86,7 +86,7 @@ class OkHttpSenderTest {
@Test void endpointSupplierFactory_constant() {
sender.close();
sender = sender.toBuilder()
.endpointSupplierFactory(e -> new Constant("http://localhost:29092"))
.endpointSupplierFactory(e -> ConstantHttpEndpointSupplier.create("http://localhost:29092"))
.build();

// The connection supplier has a constant URL
Expand All @@ -97,7 +97,7 @@ class OkHttpSenderTest {

@Test void endpointSupplierFactory_constantBad() {
OkHttpSender.Builder builder = sender.toBuilder()
.endpointSupplierFactory(e -> new Constant("htp://localhost:9411/api/v1/spans"));
.endpointSupplierFactory(e -> ConstantHttpEndpointSupplier.create("htp://localhost:9411/api/v1/spans"));

assertThatThrownBy(builder::build)
.isInstanceOf(IllegalArgumentException.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import org.springframework.beans.factory.config.AbstractFactoryBean;
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.ReporterMetrics;
import zipkin2.reporter.Sender;

abstract class BaseAsyncFactoryBean extends AbstractFactoryBean {
BytesMessageSender sender;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.Encoding;
import zipkin2.reporter.ReporterMetrics;
import zipkin2.reporter.Sender;
import zipkin2.reporter.SpanBytesEncoder;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
import org.junit.jupiter.api.Test;
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.ReporterMetrics;
import zipkin2.reporter.Sender;
import zipkin2.reporter.brave.AsyncZipkinSpanHandler;
import zipkin2.reporter.brave.ZipkinSpanHandler;

import static org.assertj.core.api.Assertions.assertThat;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2023 The OpenZipkin Authors
* Copyright 2016-2024 The OpenZipkin 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
Expand All @@ -18,9 +18,7 @@
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import zipkin2.reporter.Reporter;
import zipkin2.reporter.Sender;
import zipkin2.reporter.brave.ZipkinSpanHandler;
import zipkin2.reporter.urlconnection.URLConnectionSender;

import static org.assertj.core.api.Assertions.assertThat;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import zipkin2.reporter.Callback;
import zipkin2.reporter.CheckResult;
import zipkin2.reporter.ClosedSenderException;
import zipkin2.reporter.ConstantHttpEndpointSupplier;
import zipkin2.reporter.Encoding;
import zipkin2.reporter.HttpEndpointSupplier;
import zipkin2.reporter.Sender;
Expand All @@ -52,7 +53,7 @@ public static Builder newBuilder() {
}

public static final class Builder {
HttpEndpointSupplier.Factory endpointSupplierFactory = HttpEndpointSupplier.CONSTANT_FACTORY;
HttpEndpointSupplier.Factory endpointSupplierFactory = ConstantHttpEndpointSupplier.FACTORY;
String endpoint;
Encoding encoding = Encoding.JSON;
int messageMaxBytes = 500000;
Expand Down Expand Up @@ -141,7 +142,7 @@ public URLConnectionSender build() {
if (endpointSupplier == null) {
throw new NullPointerException("endpointSupplierFactory.create() returned null");
}
if (endpointSupplier instanceof HttpEndpointSupplier.Constant) {
if (endpointSupplier instanceof ConstantHttpEndpointSupplier) {
endpoint = endpointSupplier.get(); // eagerly resolve the endpoint
return new URLConnectionSender(this, new ConstantHttpURLConnectionSupplier(endpoint));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import java.net.ConnectException;
import java.net.MalformedURLException;
import java.net.URI;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
Expand All @@ -26,8 +25,8 @@
import zipkin2.reporter.AsyncReporter;
import zipkin2.reporter.BytesMessageSender;
import zipkin2.reporter.ClosedSenderException;
import zipkin2.reporter.ConstantHttpEndpointSupplier;
import zipkin2.reporter.HttpEndpointSupplier;
import zipkin2.reporter.HttpEndpointSupplier.Constant;

import static java.util.stream.Collectors.toList;
import static org.assertj.core.api.Assertions.assertThat;
Expand All @@ -44,14 +43,14 @@ class URLConnectionSenderTest {

// Change the supplier, but not the endpoint
sender = sender.toBuilder()
.endpointSupplierFactory(e -> new Constant("http://localhost:29092"))
.endpointSupplierFactory(e -> ConstantHttpEndpointSupplier.create("http://localhost:29092"))
.build();
assertThat(sender)
.hasToString("URLConnectionSender{http://localhost:29092}");

// Change the supplier, and see the prior endpoint.
sender = sender.toBuilder()
.endpointSupplierFactory(HttpEndpointSupplier.CONSTANT_FACTORY)
.endpointSupplierFactory(ConstantHttpEndpointSupplier.FACTORY)
.build();
assertThat(sender)
.hasToString("URLConnectionSender{http://localhost:19092}");
Expand Down Expand Up @@ -86,7 +85,7 @@ class URLConnectionSenderTest {
@Test void endpointSupplierFactory_constant() throws MalformedURLException {
sender.close();
sender = sender.toBuilder()
.endpointSupplierFactory(e -> new Constant("http://localhost:29092"))
.endpointSupplierFactory(e -> ConstantHttpEndpointSupplier.create("http://localhost:29092"))
.build();

// The connection supplier has a constant URL
Expand All @@ -97,7 +96,7 @@ class URLConnectionSenderTest {

@Test void endpointSupplierFactory_constantBad() {
URLConnectionSender.Builder builder = sender.toBuilder()
.endpointSupplierFactory(e -> new Constant("htp://localhost:9411/api/v1/spans"));
.endpointSupplierFactory(e -> ConstantHttpEndpointSupplier.create("htp://localhost:9411/api/v1/spans"));

assertThatThrownBy(builder::build)
.isInstanceOf(IllegalArgumentException.class)
Expand Down

0 comments on commit b5fbad8

Please sign in to comment.