From f90840e6fef1f1a30c1eaac8e5048e86c1f9f4d2 Mon Sep 17 00:00:00 2001 From: Luke Wood Date: Sat, 15 Sep 2018 18:34:19 +0100 Subject: [PATCH] Introduce JAEGER_CONFIG_MANAGER_ENDPOINT property Introduce a new property, JAEGER_CONFIG_MANAGER_ENDPOINT, that should be set to the URL of the Jaeger Config Manager sampling endpoint. If not set, fall back to the now-deprecated JAEGER_SAMPLER_MANAGER_HOST_PORT as the host and port of the URL. If JAEGER_AGENT_HOST has been set as a property, further fall back to that value as the host of the URL (using the default port of 5778). This brings the SamplerConfiguration in-line with the Go client. See also: * https://github.com/jaegertracing/jaeger-client-java/issues/521 * https://github.com/jaegertracing/jaeger-client-java/issues/547 * https://github.com/jaegertracing/jaeger-client-go/issues/326 * https://github.com/jaegertracing/jaeger-client-go/issues/282 Signed-off-by: Luke Wood --- .../java/io/jaegertracing/Configuration.java | 40 +++++++++++++++---- .../samplers/HttpSamplingManager.java | 17 ++++---- .../io/jaegertracing/ConfigurationTest.java | 31 ++++++++++++++ .../samplers/HttpSamplingManagerTest.java | 2 +- 4 files changed, 74 insertions(+), 16 deletions(-) diff --git a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java index c4a6933d9..ce4a864dd 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java +++ b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java @@ -123,6 +123,11 @@ public class Configuration { */ public static final String JAEGER_SAMPLER_MANAGER_HOST_PORT = JAEGER_PREFIX + "SAMPLER_MANAGER_HOST_PORT"; + /** + * The config manager endpoint URL. + */ + public static final String JAEGER_CONFIG_MANAGER_ENDPOINT = JAEGER_PREFIX + "CONFIG_MANAGER_ENDPOINT"; + /** * The service name. */ @@ -332,26 +337,36 @@ public static class SamplerConfiguration { private Number param; /** - * HTTP host:port of the sampling manager that can provide sampling strategy to this service. + * HTTP URL of the sampling manager that can provide sampling strategy to this service. * Optional. */ - private String managerHostPort; + private String serverUrl; public SamplerConfiguration() { } public static SamplerConfiguration fromEnv() { + String configManagerEndpoint = getProperty(JAEGER_CONFIG_MANAGER_ENDPOINT); + if (configManagerEndpoint == null) { + if (getProperty(JAEGER_SAMPLER_MANAGER_HOST_PORT) != null) { + configManagerEndpoint = "http://" + getProperty(JAEGER_SAMPLER_MANAGER_HOST_PORT) + "/sampling"; + } else if (getProperty(JAEGER_AGENT_HOST) != null) { + configManagerEndpoint = "http://" + getProperty(JAEGER_AGENT_HOST) + ":" + + HttpSamplingManager.DEFAULT_SERVER_PORT + "/sampling"; + } + } + return new SamplerConfiguration() .withType(getProperty(JAEGER_SAMPLER_TYPE)) .withParam(getPropertyAsNum(JAEGER_SAMPLER_PARAM)) - .withManagerHostPort(getProperty(JAEGER_SAMPLER_MANAGER_HOST_PORT)); + .withServerUrl(configManagerEndpoint); } // for tests Sampler createSampler(String serviceName, Metrics metrics) { String samplerType = stringOrDefault(this.getType(), RemoteControlledSampler.TYPE); Number samplerParam = numberOrDefault(this.getParam(), ProbabilisticSampler.DEFAULT_SAMPLING_PROBABILITY); - String hostPort = stringOrDefault(this.getManagerHostPort(), HttpSamplingManager.DEFAULT_HOST_PORT); + String serverUrl = stringOrDefault(this.getServerUrl(), HttpSamplingManager.DEFAULT_SERVER_URL); if (samplerType.equals(ConstSampler.TYPE)) { return new ConstSampler(samplerParam.intValue() != 0); @@ -367,7 +382,7 @@ Sampler createSampler(String serviceName, Metrics metrics) { if (samplerType.equals(RemoteControlledSampler.TYPE)) { return new RemoteControlledSampler.Builder(serviceName) - .withSamplingManager(new HttpSamplingManager(hostPort)) + .withSamplingManager(new HttpSamplingManager(serverUrl)) .withInitialSampler(new ProbabilisticSampler(samplerParam.doubleValue())) .withMetrics(metrics) .build(); @@ -384,8 +399,8 @@ public Number getParam() { return param; } - public String getManagerHostPort() { - return managerHostPort; + public String getServerUrl() { + return serverUrl; } public SamplerConfiguration withType(String type) { @@ -398,8 +413,17 @@ public SamplerConfiguration withParam(Number param) { return this; } + /** + * @deprecated Use {@link #withServerUrl(String)} instead + */ + @Deprecated public SamplerConfiguration withManagerHostPort(String managerHostPort) { - this.managerHostPort = managerHostPort; + this.serverUrl = "http://" + managerHostPort + "/sampling"; + return this; + } + + public SamplerConfiguration withServerUrl(String serverUrl) { + this.serverUrl = serverUrl; return this; } } diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/samplers/HttpSamplingManager.java b/jaeger-core/src/main/java/io/jaegertracing/internal/samplers/HttpSamplingManager.java index c4fbc927b..6ce4883f4 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/samplers/HttpSamplingManager.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/samplers/HttpSamplingManager.java @@ -28,20 +28,23 @@ @ToString public class HttpSamplingManager implements SamplingManager { - public static final String DEFAULT_HOST_PORT = "localhost:5778"; - private final String hostPort; + public static final String DEFAULT_SERVER_HOST = "localhost"; + public static final int DEFAULT_SERVER_PORT = 5778; + public static final String DEFAULT_SERVER_URL = + "http://" + DEFAULT_SERVER_HOST + ":" + DEFAULT_SERVER_PORT + "/sampling"; + private final String serverUrl; @ToString.Exclude private final Gson gson = new Gson(); /** - * This constructor expects running sampling manager on {@link #DEFAULT_HOST_PORT}. + * This constructor expects running sampling manager on {@link #DEFAULT_SERVER_URL}. */ public HttpSamplingManager() { - this(DEFAULT_HOST_PORT); + this(DEFAULT_SERVER_URL); } - public HttpSamplingManager(String hostPort) { - this.hostPort = hostPort != null ? hostPort : DEFAULT_HOST_PORT; + public HttpSamplingManager(String serverUrl) { + this.serverUrl = serverUrl != null ? serverUrl : DEFAULT_SERVER_URL; } SamplingStrategyResponse parseJson(String json) { @@ -59,7 +62,7 @@ public SamplingStrategyResponse getSamplingStrategy(String serviceName) try { jsonString = makeGetRequest( - "http://" + hostPort + "/?service=" + URLEncoder.encode(serviceName, "UTF-8")); + serverUrl + "?service=" + URLEncoder.encode(serviceName, "UTF-8")); } catch (IOException e) { throw new SamplingStrategyErrorException( "http call to get sampling strategy from local agent failed.", e); diff --git a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java index c2472de62..079d3d41b 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java @@ -62,6 +62,7 @@ public void clearProperties() throws NoSuchFieldException, IllegalAccessExceptio System.clearProperty(Configuration.JAEGER_SAMPLER_TYPE); System.clearProperty(Configuration.JAEGER_SAMPLER_PARAM); System.clearProperty(Configuration.JAEGER_SAMPLER_MANAGER_HOST_PORT); + System.clearProperty(Configuration.JAEGER_CONFIG_MANAGER_ENDPOINT); System.clearProperty(Configuration.JAEGER_SERVICE_NAME); System.clearProperty(Configuration.JAEGER_TAGS); System.clearProperty(Configuration.JAEGER_ENDPOINT); @@ -110,6 +111,36 @@ public void testSamplerConstInvalidParam() { assertNull(samplerConfig.getParam()); } + @Test + public void testConfigManagerUrl() { + System.setProperty(Configuration.JAEGER_CONFIG_MANAGER_ENDPOINT, "http://example.com/sampling"); + SamplerConfiguration samplerConfig = SamplerConfiguration.fromEnv(); + assertEquals("http://example.com/sampling", samplerConfig.getServerUrl()); + } + + @Test + public void testConfigManagerUrlOverridesSamplerManagerHostPort() { + System.setProperty(Configuration.JAEGER_CONFIG_MANAGER_ENDPOINT, "http://example.com/sampling"); + System.setProperty(Configuration.JAEGER_SAMPLER_MANAGER_HOST_PORT, "example.org:80"); + SamplerConfiguration samplerConfig = SamplerConfiguration.fromEnv(); + assertEquals("http://example.com/sampling", samplerConfig.getServerUrl()); + } + + @Test + public void testSamplerManagerHostPortOverridesAgentHost() { + System.setProperty(Configuration.JAEGER_SAMPLER_MANAGER_HOST_PORT, "example.org:80"); + System.setProperty(Configuration.JAEGER_AGENT_HOST, "example.com"); + SamplerConfiguration samplerConfig = SamplerConfiguration.fromEnv(); + assertEquals("http://example.org:80/sampling", samplerConfig.getServerUrl()); + } + + @Test + public void testAgentHostOverridesDefault() { + System.setProperty(Configuration.JAEGER_AGENT_HOST, "example.com"); + SamplerConfiguration samplerConfig = SamplerConfiguration.fromEnv(); + assertEquals("http://example.com:5778/sampling", samplerConfig.getServerUrl()); + } + @Test public void testReporterConfiguration() { System.setProperty(Configuration.JAEGER_REPORTER_LOG_SPANS, "true"); diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/samplers/HttpSamplingManagerTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/samplers/HttpSamplingManagerTest.java index 210751ca9..9027730bc 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/samplers/HttpSamplingManagerTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/samplers/HttpSamplingManagerTest.java @@ -67,7 +67,7 @@ protected Application configure() { @Test public void testGetSamplingStrategy() throws Exception { URI uri = target().getUri(); - undertest = new HttpSamplingManager(uri.getHost() + ":" + uri.getPort()); + undertest = new HttpSamplingManager("http://" + uri.getHost() + ":" + uri.getPort()); SamplingStrategyResponse response = undertest.getSamplingStrategy("clairvoyant"); assertNotNull(response.getProbabilisticSampling()); }