diff --git a/README.md b/README.md index e3edf6677..3a1f1f2d2 100644 --- a/README.md +++ b/README.md @@ -79,8 +79,9 @@ JAEGER_REPORTER_LOG_SPANS | no | Whether the reporter should also log the spans JAEGER_REPORTER_MAX_QUEUE_SIZE | no | The reporter's maximum queue size JAEGER_REPORTER_FLUSH_INTERVAL | no | The reporter's flush interval (ms) JAEGER_SAMPLER_TYPE | no | The sampler type -JAEGER_SAMPLER_PARAM | no | The sampler parameter (number) -JAEGER_SAMPLER_MANAGER_HOST_PORT | no | The host name and port when using the remote controlled sampler +JAEGER_SAMPLER_PARAM | no | The sampler parameter (double) +JAEGER_SAMPLER_MANAGER_HOST_PORT | no | (DEPRECATED) The host name and port when using the remote controlled sampler +JAEGER_SAMPLING_ENDPOINT | no | The url for the remote sampling conf when using sampler type remote. Default is http://127.0.0.1:5778/sampling JAEGER_TAGS | no | A comma separated list of `name = value` tracer level tags, which get added to all reported spans. The value can also refer to an environment variable using the format `${envVarName:default}`, where the `:default` is optional, and identifies a value to be used if the environment variable cannot be found JAEGER_TRACEID_128BIT | no | Whether to use 128bit TraceID instead of 64bit diff --git a/src/Jaeger/Configuration.cs b/src/Jaeger/Configuration.cs index 8c535c3ed..56942de41 100644 --- a/src/Jaeger/Configuration.cs +++ b/src/Jaeger/Configuration.cs @@ -87,6 +87,11 @@ public class Configuration /// public const string JaegerSamplerManagerHostPort = JaegerPrefix + "SAMPLER_MANAGER_HOST_PORT"; + /// + /// The url for the remote sampling conf when using sampler type remote. + /// + public const string JaegerSamplingEndpoint = JaegerPrefix + "SAMPLING_ENDPOINT"; + /// /// The service name. /// @@ -161,7 +166,7 @@ public static Configuration FromIConfiguration(ILoggerFactory loggerFactory, ICo { ILogger logger = loggerFactory.CreateLogger(); - return new Configuration(GetProperty(JaegerServiceName, configuration), loggerFactory) + return new Configuration(GetProperty(JaegerServiceName, logger, configuration), loggerFactory) .WithTracerTags(TracerTagsFromIConfiguration(logger, configuration)) .WithTraceId128Bit(GetPropertyAsBool(JaegerTraceId128Bit, logger, configuration).GetValueOrDefault(false)) .WithReporter(ReporterConfiguration.FromIConfiguration(loggerFactory, configuration)) @@ -312,10 +317,16 @@ public class SamplerConfiguration /// /// HTTP host:port of the sampling manager that can provide sampling strategy to this service. - /// Optional. /// + [Obsolete("Please use SamplingEndpoint instead!")] public string ManagerHostPort { get; private set; } + /// + /// The URL of the sampling manager that can provide sampling strategy to this service. + /// Optional. + /// + public string SamplingEndpoint { get; private set; } + public SamplerConfiguration(ILoggerFactory loggerFactory) { _loggerFactory = loggerFactory; @@ -327,11 +338,14 @@ public SamplerConfiguration(ILoggerFactory loggerFactory) public static SamplerConfiguration FromIConfiguration(ILoggerFactory loggerFactory, IConfiguration configuration) { ILogger logger = loggerFactory.CreateLogger(); - + +#pragma warning disable CS0618 // Supress warning on obsolete method: WithManagerHostPort return new SamplerConfiguration(loggerFactory) - .WithType(GetProperty(JaegerSamplerType, configuration)) + .WithType(GetProperty(JaegerSamplerType, logger, configuration)) .WithParam(GetPropertyAsDouble(JaegerSamplerParam, logger, configuration)) - .WithManagerHostPort(GetProperty(JaegerSamplerManagerHostPort, configuration)); + .WithManagerHostPort(GetProperty(JaegerSamplerManagerHostPort, logger, configuration, JaegerSamplingEndpoint)) + .WithSamplingEndpoint(GetProperty(JaegerSamplingEndpoint, logger, configuration)); +#pragma warning restore CS0618 // Supress warning on obsolete method: WithManagerHostPort } /// @@ -347,9 +361,12 @@ public static SamplerConfiguration FromEnv(ILoggerFactory loggerFactory) public virtual ISampler GetSampler(string serviceName, IMetrics metrics) { +#pragma warning disable CS0618 // Supress warning on obsolete property: ManagerHostPort string samplerType = StringOrDefault(Type, RemoteControlledSampler.Type); double samplerParam = Param.GetValueOrDefault(ProbabilisticSampler.DefaultSamplingProbability); string hostPort = StringOrDefault(ManagerHostPort, HttpSamplingManager.DefaultHostPort); + string samplingEndpoint = StringOrDefault(SamplingEndpoint, "http://" + hostPort); +#pragma warning disable CS0618 // Supress warning on obsolete property: ManagerHostPort switch (samplerType) { @@ -362,7 +379,7 @@ public virtual ISampler GetSampler(string serviceName, IMetrics metrics) case RemoteControlledSampler.Type: return new RemoteControlledSampler.Builder(serviceName) .WithLoggerFactory(_loggerFactory) - .WithSamplingManager(new HttpSamplingManager(hostPort)) + .WithSamplingManager(new HttpSamplingManager(samplingEndpoint)) .WithInitialSampler(new ProbabilisticSampler(samplerParam)) .WithMetrics(metrics) .Build(); @@ -383,11 +400,18 @@ public SamplerConfiguration WithParam(double? param) return this; } + [Obsolete("Use WithSamplingEndpoint instead!")] public SamplerConfiguration WithManagerHostPort(string managerHostPort) { ManagerHostPort = managerHostPort; return this; } + + public SamplerConfiguration WithSamplingEndpoint(string samplingEndpoint) + { + SamplingEndpoint = samplingEndpoint; + return this; + } } /// @@ -412,7 +436,7 @@ public static CodecConfiguration FromIConfiguration(ILoggerFactory loggerFactory ILogger logger = loggerFactory.CreateLogger(); CodecConfiguration codecConfiguration = new CodecConfiguration(loggerFactory); - string propagation = GetProperty(JaegerPropagation, configuration); + string propagation = GetProperty(JaegerPropagation, logger, configuration); if (propagation != null) { foreach (string format in propagation.Split(',')) @@ -706,13 +730,13 @@ public static SenderConfiguration FromIConfiguration(ILoggerFactory loggerFactor { ILogger logger = loggerFactory.CreateLogger(); - string agentHost = GetProperty(JaegerAgentHost, configuration); + string agentHost = GetProperty(JaegerAgentHost, logger, configuration); int? agentPort = GetPropertyAsInt(JaegerAgentPort, logger, configuration); - string collectorEndpoint = GetProperty(JaegerEndpoint, configuration); - string authToken = GetProperty(JaegerAuthToken, configuration); - string authUsername = GetProperty(JaegerUser, configuration); - string authPassword = GetProperty(JaegerPassword, configuration); + string collectorEndpoint = GetProperty(JaegerEndpoint, logger, configuration); + string authToken = GetProperty(JaegerAuthToken, logger, configuration); + string authUsername = GetProperty(JaegerUser, logger, configuration); + string authPassword = GetProperty(JaegerPassword, logger, configuration); return new SenderConfiguration(loggerFactory) .WithAgentHost(agentHost) @@ -740,14 +764,20 @@ private static string StringOrDefault(string value, string defaultValue) return value != null && value.Length > 0 ? value : defaultValue; } - private static string GetProperty(string name, IConfiguration configuration) + private static string GetProperty(string name, ILogger logger, IConfiguration configuration, string replacedBy = null) { - return configuration[name]; + var value = configuration[name]; + if (replacedBy != null && value != null) + { + logger.LogWarning($"The entry {name} is obsolete. Use {replacedBy} instead!"); + } + + return value; } private static int? GetPropertyAsInt(string name, ILogger logger, IConfiguration configuration) { - string value = GetProperty(name, configuration); + string value = GetProperty(name, logger, configuration); if (!string.IsNullOrEmpty(value)) { if (int.TryParse(value, NumberStyles.Integer, CultureInfo.InvariantCulture, out int intValue)) @@ -764,7 +794,7 @@ private static string GetProperty(string name, IConfiguration configuration) private static double? GetPropertyAsDouble(string name, ILogger logger, IConfiguration configuration) { - string value = GetProperty(name, configuration); + string value = GetProperty(name, logger, configuration); if (!string.IsNullOrEmpty(value)) { if (double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out double doubleValue)) @@ -795,7 +825,7 @@ private static string GetProperty(string name, IConfiguration configuration) /// private static bool? GetPropertyAsBool(string name, ILogger logger, IConfiguration configuration) { - string value = GetProperty(name, configuration); + string value = GetProperty(name, logger, configuration); if (!string.IsNullOrEmpty(value)) { if (string.Equals(value, "1", StringComparison.Ordinal)) @@ -822,7 +852,7 @@ private static string GetProperty(string name, IConfiguration configuration) private static Dictionary TracerTagsFromIConfiguration(ILogger logger, IConfiguration configuration) { Dictionary tracerTagMaps = null; - string tracerTags = GetProperty(JaegerTags, configuration); + string tracerTags = GetProperty(JaegerTags, logger, configuration); if (!string.IsNullOrEmpty(tracerTags)) { string[] tags = tracerTags.Split(','); @@ -835,7 +865,7 @@ private static string GetProperty(string name, IConfiguration configuration) { tracerTagMaps = new Dictionary(); } - tracerTagMaps[tagValue[0].Trim()] = ResolveValue(tagValue[1].Trim(), configuration); + tracerTagMaps[tagValue[0].Trim()] = ResolveValue(tagValue[1].Trim(), logger, configuration); } else { @@ -846,14 +876,14 @@ private static string GetProperty(string name, IConfiguration configuration) return tracerTagMaps; } - private static string ResolveValue(string value, IConfiguration configuration) + private static string ResolveValue(string value, ILogger logger, IConfiguration configuration) { if (value.StartsWith("${") && value.EndsWith("}")) { string[] kvp = value.Substring(2, value.Length - 3).Split(':'); if (kvp.Length > 0) { - string propertyValue = GetProperty(kvp[0].Trim(), configuration); + string propertyValue = GetProperty(kvp[0].Trim(), logger, configuration); if (propertyValue == null && kvp.Length > 1) { propertyValue = kvp[1].Trim(); diff --git a/src/Jaeger/Samplers/HttpSamplingManager.cs b/src/Jaeger/Samplers/HttpSamplingManager.cs index 8f237675f..6a988c158 100644 --- a/src/Jaeger/Samplers/HttpSamplingManager.cs +++ b/src/Jaeger/Samplers/HttpSamplingManager.cs @@ -8,20 +8,28 @@ namespace Jaeger.Samplers { public class HttpSamplingManager : ISamplingManager { - public const string DefaultHostPort = "localhost:5778"; + public const string DefaultHostPort = "127.0.0.1:5778"; + public const string DefaultEndpoint = "http://" + DefaultHostPort + "/sampling"; private readonly IHttpClient _httpClient; - private readonly string _hostPort; + private readonly string _endpoint; - public HttpSamplingManager(string hostPort = DefaultHostPort) - : this(new DefaultHttpClient(), hostPort) + public HttpSamplingManager(string endpoint = DefaultEndpoint) + : this(new DefaultHttpClient(), endpoint) { } - public HttpSamplingManager(IHttpClient httpClient, string hostPort = DefaultHostPort) + public HttpSamplingManager(IHttpClient httpClient, string endpoint = DefaultEndpoint) { _httpClient = httpClient ?? throw new ArgumentNullException(nameof(httpClient)); - _hostPort = hostPort ?? DefaultHostPort; + _endpoint = endpoint ?? DefaultEndpoint; + + // Workaround for obsolete HostPort notation. + // UriBuilder needs the schema if host is not an IP and port is given: + if (!_endpoint.StartsWith("http://") && !_endpoint.StartsWith("https://")) + { + _endpoint = "http://" + _endpoint; + } } internal SamplingStrategyResponse ParseJson(string json) @@ -31,15 +39,15 @@ internal SamplingStrategyResponse ParseJson(string json) public async Task GetSamplingStrategyAsync(string serviceName) { - string url = "http://" + _hostPort + "/?service=" + Uri.EscapeDataString(serviceName); - string jsonString = await _httpClient.MakeGetRequestAsync(url).ConfigureAwait(false); + Uri uri = new UriBuilder(_endpoint) {Query = "service=" + Uri.EscapeDataString(serviceName)}.Uri; + string jsonString = await _httpClient.MakeGetRequestAsync(uri.AbsoluteUri).ConfigureAwait(false); return ParseJson(jsonString); } public override string ToString() { - return $"{nameof(HttpSamplingManager)}(HostPort={_hostPort})"; + return $"{nameof(HttpSamplingManager)}(Endpoint={_endpoint})"; } } } diff --git a/test/Jaeger.Tests/ConfigurationTests.cs b/test/Jaeger.Tests/ConfigurationTests.cs index 69c5f710d..5d7f34431 100644 --- a/test/Jaeger.Tests/ConfigurationTests.cs +++ b/test/Jaeger.Tests/ConfigurationTests.cs @@ -8,9 +8,12 @@ using Jaeger.Metrics; using Jaeger.Samplers; using Jaeger.Senders; +using Jaeger.Util; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Configuration; +using NSubstitute; +using NSubstitute.ReceivedExtensions; using OpenTracing; using OpenTracing.Noop; using OpenTracing.Propagation; @@ -49,6 +52,7 @@ private void ClearProperties() ClearProperty(Configuration.JaegerSamplerType); ClearProperty(Configuration.JaegerSamplerParam); ClearProperty(Configuration.JaegerSamplerManagerHostPort); + ClearProperty(Configuration.JaegerSamplingEndpoint); ClearProperty(Configuration.JaegerServiceName); ClearProperty(Configuration.JaegerTags); ClearProperty(Configuration.JaegerTraceId128Bit); @@ -527,6 +531,22 @@ public void TestRateLimitingSampler() Assert.True(sampler is RateLimitingSampler); } + [Fact] + public void TestDeprecatedSamplerManagerHostPort() + { + ILoggerFactory loggerFactory = Substitute.For(); + ILogger logger = Substitute.For(); + loggerFactory.CreateLogger().Returns(logger); + + SetProperty(Configuration.JaegerSamplerManagerHostPort, HttpSamplingManager.DefaultHostPort); + SamplerConfiguration samplerConfiguration = SamplerConfiguration.FromEnv(loggerFactory); + ISampler sampler = samplerConfiguration.GetSampler("name", + new MetricsImpl(NoopMetricsFactory.Instance)); + Assert.True(sampler is RemoteControlledSampler); + loggerFactory.Received(1).CreateLogger(); + logger.Received(1).Log(LogLevel.Warning, Arg.Any(), Arg.Any(), null, Arg.Any>()); + } + internal class TestTextMap : ITextMap { private Dictionary _values = new Dictionary(); diff --git a/test/Jaeger.Tests/Samplers/HttpSamplingManagerTests.cs b/test/Jaeger.Tests/Samplers/HttpSamplingManagerTests.cs index fdc2970e3..564efc621 100644 --- a/test/Jaeger.Tests/Samplers/HttpSamplingManagerTests.cs +++ b/test/Jaeger.Tests/Samplers/HttpSamplingManagerTests.cs @@ -22,14 +22,25 @@ public class HttpSamplingManagerTests public HttpSamplingManagerTests() { _httpClient = Substitute.For(); - _undertest = new HttpSamplingManager(_httpClient, "www.example.com"); + _undertest = new HttpSamplingManager(_httpClient, "http://www.example.com/sampling"); + } + + [Fact] + public async Task TestAllowHostPortSyntax() + { + var instance = new HttpSamplingManager(_httpClient, "example.com:80"); + _httpClient.MakeGetRequestAsync("http://example.com/?service=clairvoyant") + .Returns("{\"strategyType\":\"PROBABILISTIC\",\"probabilisticSampling\":{\"samplingRate\":0.001},\"rateLimitingSampling\":null}"); + + SamplingStrategyResponse response = await instance.GetSamplingStrategyAsync("clairvoyant"); + Assert.NotNull(response.ProbabilisticSampling); } [Fact] public async Task TestGetSamplingStrategy() { - _httpClient.MakeGetRequestAsync("http://www.example.com/?service=clairvoyant") - .Returns("{\"strategyType\":0,\"probabilisticSampling\":{\"samplingRate\":0.001},\"rateLimitingSampling\":null}"); + _httpClient.MakeGetRequestAsync("http://www.example.com/sampling?service=clairvoyant") + .Returns("{\"strategyType\":\"PROBABILISTIC\",\"probabilisticSampling\":{\"samplingRate\":0.001},\"rateLimitingSampling\":null}"); SamplingStrategyResponse response = await _undertest.GetSamplingStrategyAsync("clairvoyant"); Assert.NotNull(response.ProbabilisticSampling); @@ -38,7 +49,7 @@ public async Task TestGetSamplingStrategy() [Fact] public async Task TestGetSamplingStrategyError() { - _httpClient.MakeGetRequestAsync("http://www.example.com/?service=") + _httpClient.MakeGetRequestAsync("http://www.example.com/sampling?service=") .Returns(new Func(_ => { throw new InvalidOperationException(); })); await Assert.ThrowsAsync(() => _undertest.GetSamplingStrategyAsync("")); @@ -87,7 +98,7 @@ public void TestParsePerOperationSampling() [Fact] public async Task TestDefaultConstructor() { - _httpClient.MakeGetRequestAsync("http://localhost:5778/?service=name") + _httpClient.MakeGetRequestAsync("http://127.0.0.1:5778/sampling?service=name") .Returns(new Func(_ => { throw new InvalidOperationException(); })); HttpSamplingManager httpSamplingManager = new HttpSamplingManager(_httpClient); diff --git a/test/Jaeger.Tests/Samplers/Resources/per_operation_sampling.json b/test/Jaeger.Tests/Samplers/Resources/per_operation_sampling.json index 7486b0fb4..40fd23417 100644 --- a/test/Jaeger.Tests/Samplers/Resources/per_operation_sampling.json +++ b/test/Jaeger.Tests/Samplers/Resources/per_operation_sampling.json @@ -21,5 +21,5 @@ "samplingRate": 0.001 }, "rateLimitingSampling": null, - "strategyType": 0 + "strategyType": "PROBABILISTIC" } diff --git a/test/Jaeger.Tests/Samplers/Resources/probabilistic_sampling.json b/test/Jaeger.Tests/Samplers/Resources/probabilistic_sampling.json index 19f0f400a..6e6ccc911 100644 --- a/test/Jaeger.Tests/Samplers/Resources/probabilistic_sampling.json +++ b/test/Jaeger.Tests/Samplers/Resources/probabilistic_sampling.json @@ -1,7 +1,7 @@ { - "strategyType": 0, - "probabilisticSampling": { - "samplingRate": 0.01 - }, - "rateLimitingSampling": null + "strategyType": "PROBABILISTIC", + "probabilisticSampling": { + "samplingRate": 0.01 + }, + "rateLimitingSampling": null } diff --git a/test/Jaeger.Tests/Samplers/Resources/ratelimiting_sampling.json b/test/Jaeger.Tests/Samplers/Resources/ratelimiting_sampling.json index 9cbbe883b..86b03aaf2 100644 --- a/test/Jaeger.Tests/Samplers/Resources/ratelimiting_sampling.json +++ b/test/Jaeger.Tests/Samplers/Resources/ratelimiting_sampling.json @@ -1,7 +1,7 @@ { - "strategyType": 1, - "probabilisticSampling": null, - "rateLimitingSampling": { - "maxTracesPerSecond": 2.1 - } + "strategyType": "RATE_LIMITING", + "probabilisticSampling": null, + "rateLimitingSampling": { + "maxTracesPerSecond": 2.1 + } }