From d5126d75e147450dc44a0488361984c54901068e Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Fri, 18 Oct 2019 15:07:31 -0700 Subject: [PATCH 01/13] [WIP] Add DD_TRACE_GLOBAL_TAGS env variable and APIs for adding global tags --- .../Configuration/ConfigurationKeys.cs | 5 ++++ .../Configuration/JsonConfigurationSource.cs | 18 ++++++++++++ .../Configuration/TracerSettings.cs | 23 +++++++++++++++ src/Datadog.Trace/Datadog.Trace.csproj | 1 + src/Datadog.Trace/Span.cs | 10 +++++++ src/Datadog.Trace/Tracer.cs | 28 +++++++++++++++++++ .../Configuration/ConfigurationSourceTests.cs | 3 ++ .../Datadog.Trace.Tests.csproj | 4 +++ test/Datadog.Trace.Tests/TracerTests.cs | 13 +++++++++ 9 files changed, 105 insertions(+) diff --git a/src/Datadog.Trace/Configuration/ConfigurationKeys.cs b/src/Datadog.Trace/Configuration/ConfigurationKeys.cs index 3c7688d7bff2..57e1e63d1e9e 100644 --- a/src/Datadog.Trace/Configuration/ConfigurationKeys.cs +++ b/src/Datadog.Trace/Configuration/ConfigurationKeys.cs @@ -77,6 +77,11 @@ public static class ConfigurationKeys /// public const string GlobalAnalyticsEnabled = "DD_TRACE_ANALYTICS_ENABLED"; + /// + /// Configuration key for a list of tags to be applied globally to spans. + /// + public const string GlobalTags = "DD_TRACE_GLOBAL_TAGS"; + /// /// Configuration key for enabling or disabling the automatic injection /// of correlation identifiers into the logging context. diff --git a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs index d545e076bc41..306b532f0a79 100644 --- a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using System.IO; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -97,5 +98,22 @@ public T GetValue(string key) ? default : token.Value(); } + + /// + /// Gets a Dictionary containing all of the configuration values where keys and values are strings. + /// + /// Dictionary of key value strings + public Dictionary GetAllEntries() + { + var allEntries = new Dictionary(); + var enumerator = _configuration.GetEnumerator(); + + while (!enumerator.MoveNext()) + { + allEntries.Add(enumerator.Current.Key, enumerator.Current.Value.ToString()); + } + + return allEntries; + } } } diff --git a/src/Datadog.Trace/Configuration/TracerSettings.cs b/src/Datadog.Trace/Configuration/TracerSettings.cs index 3d23593f9d76..c5e08c000193 100644 --- a/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -76,6 +76,24 @@ public TracerSettings(IConfigurationSource source) false; Integrations = new IntegrationSettingsCollection(source); + + if (source == null) + { + GlobalTags = new Dictionary(); + } + else + { + var gTags = source.GetString(ConfigurationKeys.GlobalTags); + if (gTags != null) + { + var gTagsConfig = new JsonConfigurationSource(gTags); + GlobalTags = gTagsConfig.GetAllEntries(); + } + else + { + GlobalTags = new Dictionary(); + } + } } /// @@ -141,6 +159,11 @@ public TracerSettings(IConfigurationSource source) /// public IntegrationSettingsCollection Integrations { get; } + /// + /// Gets or sets the global tags, which are applied to all s. + /// + public Dictionary GlobalTags { get; set; } + /// /// Create a populated from the default sources /// returned by . diff --git a/src/Datadog.Trace/Datadog.Trace.csproj b/src/Datadog.Trace/Datadog.Trace.csproj index 31bdec0add55..09ae389215da 100644 --- a/src/Datadog.Trace/Datadog.Trace.csproj +++ b/src/Datadog.Trace/Datadog.Trace.csproj @@ -6,6 +6,7 @@ Datadog APM Manual instrumentation library for Datadog APM lucas.pimentel.datadog;colinhigginsdatadog;zachmontoyadd + true diff --git a/src/Datadog.Trace/Span.cs b/src/Datadog.Trace/Span.cs index 2a8416379846..f667bb2b2501 100644 --- a/src/Datadog.Trace/Span.cs +++ b/src/Datadog.Trace/Span.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Concurrent; using System.Globalization; +using System.Linq; using System.Text; using Datadog.Trace.ExtensionMethods; using Datadog.Trace.Interfaces; @@ -27,6 +28,15 @@ internal Span(SpanContext context, DateTimeOffset? start) ServiceName = context.ServiceName; StartTime = start ?? Context.TraceContext.UtcNow; + // Apply any global tags + if (Tracer.Instance.Settings.GlobalTags.Count > 0) + { + foreach (var entry in Tracer.Instance.Settings.GlobalTags) + { + SetTag(entry.Key, entry.Value); + } + } + Log.Debug( "Span started: [s_id: {0}, p_id: {1}, t_id: {2}]", SpanId, diff --git a/src/Datadog.Trace/Tracer.cs b/src/Datadog.Trace/Tracer.cs index 073f01eafb3f..2f061cf647f0 100644 --- a/src/Datadog.Trace/Tracer.cs +++ b/src/Datadog.Trace/Tracer.cs @@ -221,6 +221,34 @@ public Span StartSpan(string operationName, ISpanContext parent = null, string s return span; } + /// + /// Adds the , pair as a global tag to be applied to all s. + /// + /// The global tag key + /// The global tag value + public void AddGlobalTag(string key, string value) + { + if (string.IsNullOrWhiteSpace(key)) + { + Log.Warning("Cannot create a global tag with an empty key."); + return; + } + + Tracer.Instance.Settings.GlobalTags.Add(key, value); + } + + /// + /// Adds each in as global tags. + /// + /// s to be added as global tags. + public void AddGlobalTags(IEnumerable> tags) + { + foreach (var entry in tags) + { + Tracer.Instance.Settings.GlobalTags.Add(entry.Key, entry.Value); + } + } + /// /// Writes the specified collection to the agent writer. /// diff --git a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs index 1fe2e0ee620e..b1280b8c5e7b 100644 --- a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs +++ b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs @@ -18,6 +18,7 @@ public static IEnumerable GetDefaultTestData() yield return new object[] { CreateFunc(s => s.ServiceName), null }; yield return new object[] { CreateFunc(s => s.DisabledIntegrationNames.Count), 0 }; yield return new object[] { CreateFunc(s => s.LogsInjectionEnabled), false }; + yield return new object[] { CreateFunc(s => s.GlobalTags.Count), 0 }; } public static IEnumerable GetTestData() @@ -36,6 +37,8 @@ public static IEnumerable GetTestData() yield return new object[] { ConfigurationKeys.ServiceName, "web-service", CreateFunc(s => s.ServiceName), "web-service" }; yield return new object[] { ConfigurationKeys.DisabledIntegrations, "integration1;integration2", CreateFunc(s => s.DisabledIntegrationNames.Count), 2 }; + + yield return new object[] { ConfigurationKeys.GlobalTags, string.Empty, CreateFunc(s => s.GlobalTags.Count), 0 }; } public static Func CreateFunc(Func settingGetter) diff --git a/test/Datadog.Trace.Tests/Datadog.Trace.Tests.csproj b/test/Datadog.Trace.Tests/Datadog.Trace.Tests.csproj index f8f2c539373a..a1d558c4f48b 100644 --- a/test/Datadog.Trace.Tests/Datadog.Trace.Tests.csproj +++ b/test/Datadog.Trace.Tests/Datadog.Trace.Tests.csproj @@ -1,5 +1,9 @@  + + true + + diff --git a/test/Datadog.Trace.Tests/TracerTests.cs b/test/Datadog.Trace.Tests/TracerTests.cs index 96ee8d5d2faa..ee79bc97a92c 100644 --- a/test/Datadog.Trace.Tests/TracerTests.cs +++ b/test/Datadog.Trace.Tests/TracerTests.cs @@ -337,5 +337,18 @@ public void SetServiceName(string envServiceName, string tracerServiceName, stri // reset the environment variable to its original values (if any) when done Environment.SetEnvironmentVariable(name, originalEnv); } + + [Theory] + [InlineData(null, null)] + [InlineData(null, "")] + [InlineData("", null)] + [InlineData("", "avalue")] + public void AddGlobalTag(string key, string value) + { + _tracer.AddGlobalTag(key, value); + + Assert.Single(_tracer.Settings.GlobalTags); + Assert.Equal(value, _tracer.Settings.GlobalTags[key]); + } } } From 6eadd942f177f8a125ada4bbd13c70e04a58ed52 Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Mon, 21 Oct 2019 10:39:15 -0700 Subject: [PATCH 02/13] AddGlobalTag and AddGlobalTags API and unit tests --- .../Configuration/JsonConfigurationSource.cs | 17 ----- .../Configuration/TracerSettings.cs | 4 +- src/Datadog.Trace/Tracer.cs | 5 +- test/Datadog.Trace.Tests/TracerTests.cs | 65 +++++++++++++++++-- 4 files changed, 63 insertions(+), 28 deletions(-) diff --git a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs index 306b532f0a79..e729bc7812ff 100644 --- a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs @@ -98,22 +98,5 @@ public T GetValue(string key) ? default : token.Value(); } - - /// - /// Gets a Dictionary containing all of the configuration values where keys and values are strings. - /// - /// Dictionary of key value strings - public Dictionary GetAllEntries() - { - var allEntries = new Dictionary(); - var enumerator = _configuration.GetEnumerator(); - - while (!enumerator.MoveNext()) - { - allEntries.Add(enumerator.Current.Key, enumerator.Current.Value.ToString()); - } - - return allEntries; - } } } diff --git a/src/Datadog.Trace/Configuration/TracerSettings.cs b/src/Datadog.Trace/Configuration/TracerSettings.cs index c5e08c000193..9f62cdf8c2d3 100644 --- a/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using Newtonsoft.Json; namespace Datadog.Trace.Configuration { @@ -86,8 +87,7 @@ public TracerSettings(IConfigurationSource source) var gTags = source.GetString(ConfigurationKeys.GlobalTags); if (gTags != null) { - var gTagsConfig = new JsonConfigurationSource(gTags); - GlobalTags = gTagsConfig.GetAllEntries(); + GlobalTags = JsonConvert.DeserializeObject>(gTags); } else { diff --git a/src/Datadog.Trace/Tracer.cs b/src/Datadog.Trace/Tracer.cs index 2f061cf647f0..c7c607a6c30b 100644 --- a/src/Datadog.Trace/Tracer.cs +++ b/src/Datadog.Trace/Tracer.cs @@ -224,6 +224,7 @@ public Span StartSpan(string operationName, ISpanContext parent = null, string s /// /// Adds the , pair as a global tag to be applied to all s. /// + /// is whitespace-trimmed before being added. /// The global tag key /// The global tag value public void AddGlobalTag(string key, string value) @@ -234,7 +235,7 @@ public void AddGlobalTag(string key, string value) return; } - Tracer.Instance.Settings.GlobalTags.Add(key, value); + Settings.GlobalTags[key.Trim()] = value; } /// @@ -245,7 +246,7 @@ public void AddGlobalTags(IEnumerable> tags) { foreach (var entry in tags) { - Tracer.Instance.Settings.GlobalTags.Add(entry.Key, entry.Value); + AddGlobalTag(entry.Key, entry.Value); } } diff --git a/test/Datadog.Trace.Tests/TracerTests.cs b/test/Datadog.Trace.Tests/TracerTests.cs index ee79bc97a92c..b117cf0336c2 100644 --- a/test/Datadog.Trace.Tests/TracerTests.cs +++ b/test/Datadog.Trace.Tests/TracerTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Datadog.Trace.Agent; @@ -339,16 +340,66 @@ public void SetServiceName(string envServiceName, string tracerServiceName, stri } [Theory] - [InlineData(null, null)] - [InlineData(null, "")] - [InlineData("", null)] - [InlineData("", "avalue")] - public void AddGlobalTag(string key, string value) + [InlineData(null, null, 0)] + [InlineData(null, "", 0)] + [InlineData("", null, 0)] + [InlineData("", "avalue", 0)] + [InlineData(" ", "123", 0)] + [InlineData("key1", "value1", 1)] + [InlineData("key1", null, 1)] + [InlineData("key1", "", 1)] + public void AddGlobalTag(string key, string value, int expectedCount) { _tracer.AddGlobalTag(key, value); - Assert.Single(_tracer.Settings.GlobalTags); - Assert.Equal(value, _tracer.Settings.GlobalTags[key]); + Assert.Equal(expectedCount, _tracer.Settings.GlobalTags.Count); + } + + [Fact] + public void AddGlobalTag_OverwriteExpected() + { + string key = "k1"; + string firstvalue = "v1"; + string lastValue = "asdf"; + + _tracer.AddGlobalTag(key, firstvalue); + _tracer.AddGlobalTag(key, lastValue); + + Assert.True(_tracer.Settings.GlobalTags.Count == 1); + Assert.True(_tracer.Settings.GlobalTags[key] == lastValue); + } + + [Fact] + public void AddGlobalTags_AllValid() + { + var tags = new Dictionary() + { + { "k1", "v1" }, + { "k2", string.Empty }, + { "k3", null } + }; + + _tracer.AddGlobalTags(tags); + + Assert.Equal(tags.Count, _tracer.Settings.GlobalTags.Count); + } + + [Fact] + public void AddGlobalTags_IncludesTwoInValidKeys() + { + var tags = new Dictionary() + { + { "k1", "v1" }, + { "k2", string.Empty }, + { "k3", null }, + { string.Empty, "yo" }, + { "abc", "def" }, + { " ", "xyz" } + }; + + _tracer.AddGlobalTags(tags); + + Assert.Equal(tags.Count - 2, _tracer.Settings.GlobalTags.Count); } } } From 74841fd1b8da0ddb93e4703bbfe359038930809c Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Mon, 21 Oct 2019 12:14:08 -0700 Subject: [PATCH 03/13] ConfigurationSource tests passing; Tests validating spans are tagged --- src/Datadog.Trace/IDatadogTracer.cs | 3 ++ src/Datadog.Trace/Span.cs | 10 ---- src/Datadog.Trace/Tracer.cs | 9 ++++ .../Configuration/ConfigurationSourceTests.cs | 2 +- test/Datadog.Trace.Tests/TracerTests.cs | 51 +++++++++++++++++++ 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/Datadog.Trace/IDatadogTracer.cs b/src/Datadog.Trace/IDatadogTracer.cs index 28ea43cd6112..227842e9fb21 100644 --- a/src/Datadog.Trace/IDatadogTracer.cs +++ b/src/Datadog.Trace/IDatadogTracer.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using Datadog.Trace.Configuration; using Datadog.Trace.Sampling; namespace Datadog.Trace @@ -14,6 +15,8 @@ internal interface IDatadogTracer ISampler Sampler { get; } + TracerSettings Settings { get; } + Span StartSpan(string operationName, ISpanContext parent, string serviceName, DateTimeOffset? startTime, bool ignoreActiveScope); void Write(List span); diff --git a/src/Datadog.Trace/Span.cs b/src/Datadog.Trace/Span.cs index f667bb2b2501..2a8416379846 100644 --- a/src/Datadog.Trace/Span.cs +++ b/src/Datadog.Trace/Span.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Concurrent; using System.Globalization; -using System.Linq; using System.Text; using Datadog.Trace.ExtensionMethods; using Datadog.Trace.Interfaces; @@ -28,15 +27,6 @@ internal Span(SpanContext context, DateTimeOffset? start) ServiceName = context.ServiceName; StartTime = start ?? Context.TraceContext.UtcNow; - // Apply any global tags - if (Tracer.Instance.Settings.GlobalTags.Count > 0) - { - foreach (var entry in Tracer.Instance.Settings.GlobalTags) - { - SetTag(entry.Key, entry.Value); - } - } - Log.Debug( "Span started: [s_id: {0}, p_id: {1}, t_id: {2}]", SpanId, diff --git a/src/Datadog.Trace/Tracer.cs b/src/Datadog.Trace/Tracer.cs index c7c607a6c30b..c76fda6ca227 100644 --- a/src/Datadog.Trace/Tracer.cs +++ b/src/Datadog.Trace/Tracer.cs @@ -217,6 +217,15 @@ public Span StartSpan(string operationName, ISpanContext parent = null, string s span.SetTag(Tags.Env, env); } + // Apply any global tags + if (((TraceContext)traceContext).Tracer.Settings.GlobalTags.Count > 0) + { + foreach (var entry in ((TraceContext)traceContext).Tracer.Settings.GlobalTags) + { + span.SetTag(entry.Key, entry.Value); + } + } + traceContext.AddSpan(span); return span; } diff --git a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs index b1280b8c5e7b..71cbc134eee9 100644 --- a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs +++ b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs @@ -38,7 +38,7 @@ public static IEnumerable GetTestData() yield return new object[] { ConfigurationKeys.DisabledIntegrations, "integration1;integration2", CreateFunc(s => s.DisabledIntegrationNames.Count), 2 }; - yield return new object[] { ConfigurationKeys.GlobalTags, string.Empty, CreateFunc(s => s.GlobalTags.Count), 0 }; + yield return new object[] { ConfigurationKeys.GlobalTags, "{ 'k1':'v1', 'k2':'v2' }", CreateFunc(s => s.GlobalTags.Count), 2 }; } public static Func CreateFunc(Func settingGetter) diff --git a/test/Datadog.Trace.Tests/TracerTests.cs b/test/Datadog.Trace.Tests/TracerTests.cs index b117cf0336c2..7768affa28bf 100644 --- a/test/Datadog.Trace.Tests/TracerTests.cs +++ b/test/Datadog.Trace.Tests/TracerTests.cs @@ -167,6 +167,32 @@ public void StartActive_SetStartTime_StartTimeIsProperlySet() Assert.Equal(startTime, scope.Span.StartTime); } + [Fact] + public void StartActive_GlobalTag_IsSet() + { + var key = "myTag"; + var value = "myValue"; + + _tracer.AddGlobalTag(key, value); + + var scope = _tracer.StartActive("Operation"); + + Assert.Equal(value, scope.Span.GetTag(key)); + } + + [Fact] + public void StartManual_GlobalTag_IsSet() + { + var key = "myTag"; + var value = "myValue"; + + _tracer.AddGlobalTag(key, value); + + var span = _tracer.StartSpan("Operation", null); + + Assert.Equal(value, span.GetTag(key)); + } + [Fact] public void StartManual_SetOperationName_OperationNameIsSet() { @@ -238,6 +264,31 @@ public void StartActive_2LevelChildren_ChildrenParentProperlySet() Assert.Equal(child1.Span.Context.SpanId, child2.Span.Context.ParentId); } + [Fact] + public void StartActive_2ChildrenOfRoot_GlobalTagsProperlySet() + { + var key1 = "t1"; + var value1 = "terminator"; + var key2 = "t2"; + var value2 = "terminatrix"; + + _tracer.AddGlobalTag(key1, value1); + _tracer.AddGlobalTag(key2, value2); + + var root = _tracer.StartActive("Root"); + var child1 = _tracer.StartActive("Child1"); + child1.Dispose(); + var child2 = _tracer.StartActive("Child2"); + + Assert.Equal(value1, root.Span.GetTag(key1)); + Assert.Equal(value1, child1.Span.GetTag(key1)); + Assert.Equal(value1, child2.Span.GetTag(key1)); + + Assert.Equal(value2, root.Span.GetTag(key2)); + Assert.Equal(value2, child1.Span.GetTag(key2)); + Assert.Equal(value2, child2.Span.GetTag(key2)); + } + [Fact] public async Task StartActive_AsyncChildrenCreation_ChildrenParentProperlySet() { From e28ad1bd87d35d9167c4c01fbbdfdf6e7e93464e Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Mon, 21 Oct 2019 15:40:04 -0700 Subject: [PATCH 04/13] Remove GlobalTag API methods and tests per decision to have just config settings --- .../Configuration/TracerSettings.cs | 14 +-- src/Datadog.Trace/Datadog.Trace.csproj | 1 - src/Datadog.Trace/Tracer.cs | 33 +---- test/Datadog.Trace.Tests/TracerTests.cs | 114 ------------------ 4 files changed, 6 insertions(+), 156 deletions(-) diff --git a/src/Datadog.Trace/Configuration/TracerSettings.cs b/src/Datadog.Trace/Configuration/TracerSettings.cs index 9f62cdf8c2d3..9c767da6848f 100644 --- a/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -78,21 +78,15 @@ public TracerSettings(IConfigurationSource source) Integrations = new IntegrationSettingsCollection(source); - if (source == null) + var gTags = source?.GetString(ConfigurationKeys.GlobalTags); + + if (gTags == null) { GlobalTags = new Dictionary(); } else { - var gTags = source.GetString(ConfigurationKeys.GlobalTags); - if (gTags != null) - { - GlobalTags = JsonConvert.DeserializeObject>(gTags); - } - else - { - GlobalTags = new Dictionary(); - } + GlobalTags = JsonConvert.DeserializeObject>(gTags); } } diff --git a/src/Datadog.Trace/Datadog.Trace.csproj b/src/Datadog.Trace/Datadog.Trace.csproj index 09ae389215da..31bdec0add55 100644 --- a/src/Datadog.Trace/Datadog.Trace.csproj +++ b/src/Datadog.Trace/Datadog.Trace.csproj @@ -6,7 +6,6 @@ Datadog APM Manual instrumentation library for Datadog APM lucas.pimentel.datadog;colinhigginsdatadog;zachmontoyadd - true diff --git a/src/Datadog.Trace/Tracer.cs b/src/Datadog.Trace/Tracer.cs index c76fda6ca227..609716248dd1 100644 --- a/src/Datadog.Trace/Tracer.cs +++ b/src/Datadog.Trace/Tracer.cs @@ -218,9 +218,9 @@ public Span StartSpan(string operationName, ISpanContext parent = null, string s } // Apply any global tags - if (((TraceContext)traceContext).Tracer.Settings.GlobalTags.Count > 0) + if (Settings.GlobalTags.Count > 0) { - foreach (var entry in ((TraceContext)traceContext).Tracer.Settings.GlobalTags) + foreach (var entry in Settings.GlobalTags) { span.SetTag(entry.Key, entry.Value); } @@ -230,35 +230,6 @@ public Span StartSpan(string operationName, ISpanContext parent = null, string s return span; } - /// - /// Adds the , pair as a global tag to be applied to all s. - /// - /// is whitespace-trimmed before being added. - /// The global tag key - /// The global tag value - public void AddGlobalTag(string key, string value) - { - if (string.IsNullOrWhiteSpace(key)) - { - Log.Warning("Cannot create a global tag with an empty key."); - return; - } - - Settings.GlobalTags[key.Trim()] = value; - } - - /// - /// Adds each in as global tags. - /// - /// s to be added as global tags. - public void AddGlobalTags(IEnumerable> tags) - { - foreach (var entry in tags) - { - AddGlobalTag(entry.Key, entry.Value); - } - } - /// /// Writes the specified collection to the agent writer. /// diff --git a/test/Datadog.Trace.Tests/TracerTests.cs b/test/Datadog.Trace.Tests/TracerTests.cs index 7768affa28bf..0b70014d973d 100644 --- a/test/Datadog.Trace.Tests/TracerTests.cs +++ b/test/Datadog.Trace.Tests/TracerTests.cs @@ -167,32 +167,6 @@ public void StartActive_SetStartTime_StartTimeIsProperlySet() Assert.Equal(startTime, scope.Span.StartTime); } - [Fact] - public void StartActive_GlobalTag_IsSet() - { - var key = "myTag"; - var value = "myValue"; - - _tracer.AddGlobalTag(key, value); - - var scope = _tracer.StartActive("Operation"); - - Assert.Equal(value, scope.Span.GetTag(key)); - } - - [Fact] - public void StartManual_GlobalTag_IsSet() - { - var key = "myTag"; - var value = "myValue"; - - _tracer.AddGlobalTag(key, value); - - var span = _tracer.StartSpan("Operation", null); - - Assert.Equal(value, span.GetTag(key)); - } - [Fact] public void StartManual_SetOperationName_OperationNameIsSet() { @@ -264,31 +238,6 @@ public void StartActive_2LevelChildren_ChildrenParentProperlySet() Assert.Equal(child1.Span.Context.SpanId, child2.Span.Context.ParentId); } - [Fact] - public void StartActive_2ChildrenOfRoot_GlobalTagsProperlySet() - { - var key1 = "t1"; - var value1 = "terminator"; - var key2 = "t2"; - var value2 = "terminatrix"; - - _tracer.AddGlobalTag(key1, value1); - _tracer.AddGlobalTag(key2, value2); - - var root = _tracer.StartActive("Root"); - var child1 = _tracer.StartActive("Child1"); - child1.Dispose(); - var child2 = _tracer.StartActive("Child2"); - - Assert.Equal(value1, root.Span.GetTag(key1)); - Assert.Equal(value1, child1.Span.GetTag(key1)); - Assert.Equal(value1, child2.Span.GetTag(key1)); - - Assert.Equal(value2, root.Span.GetTag(key2)); - Assert.Equal(value2, child1.Span.GetTag(key2)); - Assert.Equal(value2, child2.Span.GetTag(key2)); - } - [Fact] public async Task StartActive_AsyncChildrenCreation_ChildrenParentProperlySet() { @@ -389,68 +338,5 @@ public void SetServiceName(string envServiceName, string tracerServiceName, stri // reset the environment variable to its original values (if any) when done Environment.SetEnvironmentVariable(name, originalEnv); } - - [Theory] - [InlineData(null, null, 0)] - [InlineData(null, "", 0)] - [InlineData("", null, 0)] - [InlineData("", "avalue", 0)] - [InlineData(" ", "123", 0)] - [InlineData("key1", "value1", 1)] - [InlineData("key1", null, 1)] - [InlineData("key1", "", 1)] - public void AddGlobalTag(string key, string value, int expectedCount) - { - _tracer.AddGlobalTag(key, value); - - Assert.Equal(expectedCount, _tracer.Settings.GlobalTags.Count); - } - - [Fact] - public void AddGlobalTag_OverwriteExpected() - { - string key = "k1"; - string firstvalue = "v1"; - string lastValue = "asdf"; - - _tracer.AddGlobalTag(key, firstvalue); - _tracer.AddGlobalTag(key, lastValue); - - Assert.True(_tracer.Settings.GlobalTags.Count == 1); - Assert.True(_tracer.Settings.GlobalTags[key] == lastValue); - } - - [Fact] - public void AddGlobalTags_AllValid() - { - var tags = new Dictionary() - { - { "k1", "v1" }, - { "k2", string.Empty }, - { "k3", null } - }; - - _tracer.AddGlobalTags(tags); - - Assert.Equal(tags.Count, _tracer.Settings.GlobalTags.Count); - } - - [Fact] - public void AddGlobalTags_IncludesTwoInValidKeys() - { - var tags = new Dictionary() - { - { "k1", "v1" }, - { "k2", string.Empty }, - { "k3", null }, - { string.Empty, "yo" }, - { "abc", "def" }, - { " ", "xyz" } - }; - - _tracer.AddGlobalTags(tags); - - Assert.Equal(tags.Count - 2, _tracer.Settings.GlobalTags.Count); - } } } From 0cf842865c7d0dbb0630a116df941596522406d5 Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Mon, 21 Oct 2019 18:57:31 -0700 Subject: [PATCH 05/13] Change global tag configuration data format to csv of key-value pairs --- .../CsvMapConfigurationSource.cs | 60 +++++++++++++++++++ .../Configuration/TracerSettings.cs | 8 ++- .../Configuration/ConfigurationSourceTests.cs | 2 +- 3 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs diff --git a/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs b/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs new file mode 100644 index 000000000000..faa6df838005 --- /dev/null +++ b/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs @@ -0,0 +1,60 @@ +using System; +using System.Collections.Concurrent; + +namespace Datadog.Trace.Configuration +{ + /// + /// Represents a configuration source that retrieves values from + /// a comma-separate list of key-value pairs. + /// + public class CsvMapConfigurationSource : StringConfigurationSource + { + private ConcurrentDictionary _data; + + /// + /// Initializes a new instance of the class + /// using a custom configuration string format. + /// + /// Configuration string that is a comma-separated list of key, value pairs where key and value are colon-separated. + /// An example configuration string would be "'globalTag1':'someValue','globalTag2':'anotherValue'" + public CsvMapConfigurationSource(string config) + { + _data = ParseConfig(config); + } + + /// + /// Gets the configuration data as a + /// + public ConcurrentDictionary Data + { + get { return _data; } + } + + /// + public override string GetString(string key) + { + throw new NotImplementedException(); + } + + private ConcurrentDictionary ParseConfig(string config) + { + ConcurrentDictionary data = new ConcurrentDictionary(); + + var entries = config.Split(','); + foreach (var e in entries) + { + var kv = e.Split(':'); + if (kv.Length != 2) + { + continue; + } + + var key = kv[0]; + var value = kv[1]; + data[key] = value; + } + + return data; + } + } +} diff --git a/src/Datadog.Trace/Configuration/TracerSettings.cs b/src/Datadog.Trace/Configuration/TracerSettings.cs index 9c767da6848f..28b857aaee72 100644 --- a/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using System.Linq; @@ -82,11 +83,12 @@ public TracerSettings(IConfigurationSource source) if (gTags == null) { - GlobalTags = new Dictionary(); + GlobalTags = new ConcurrentDictionary(); } else { - GlobalTags = JsonConvert.DeserializeObject>(gTags); + var csvMapConfig = new CsvMapConfigurationSource(gTags); + GlobalTags = csvMapConfig.Data; } } @@ -156,7 +158,7 @@ public TracerSettings(IConfigurationSource source) /// /// Gets or sets the global tags, which are applied to all s. /// - public Dictionary GlobalTags { get; set; } + public ConcurrentDictionary GlobalTags { get; set; } /// /// Create a populated from the default sources diff --git a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs index 71cbc134eee9..97771196e6f0 100644 --- a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs +++ b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs @@ -38,7 +38,7 @@ public static IEnumerable GetTestData() yield return new object[] { ConfigurationKeys.DisabledIntegrations, "integration1;integration2", CreateFunc(s => s.DisabledIntegrationNames.Count), 2 }; - yield return new object[] { ConfigurationKeys.GlobalTags, "{ 'k1':'v1', 'k2':'v2' }", CreateFunc(s => s.GlobalTags.Count), 2 }; + yield return new object[] { ConfigurationKeys.GlobalTags, "'k1':'v1', 'k2':'v2'", CreateFunc(s => s.GlobalTags.Count), 2 }; } public static Func CreateFunc(Func settingGetter) From cdeecff14077e68d19539f148c48a20718c5b496 Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Wed, 23 Oct 2019 10:31:50 -0700 Subject: [PATCH 06/13] var instead of actual type --- src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs b/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs index faa6df838005..d2b548b8ecdc 100644 --- a/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs @@ -38,7 +38,7 @@ public override string GetString(string key) private ConcurrentDictionary ParseConfig(string config) { - ConcurrentDictionary data = new ConcurrentDictionary(); + var data = new ConcurrentDictionary(); var entries = config.Split(','); foreach (var e in entries) From 92d20a17074d1c2b32c8b88237686a0086843b4e Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Thu, 24 Oct 2019 17:22:07 -0700 Subject: [PATCH 07/13] Support Dictionary as a configuration value, initially just used for Global Tags --- .../CompositeConfigurationSource.cs | 8 ++++ .../EnvironmentConfigurationSource.cs | 2 +- .../Configuration/IConfigurationSource.cs | 12 ++++- .../Configuration/JsonConfigurationSource.cs | 36 +++++++++++++++ .../StringConfigurationSource.cs | 46 +++++++++++++++++++ .../Configuration/TracerSettings.cs | 13 +----- ....Trace.ClrProfiler.IntegrationTests.csproj | 12 ++--- .../Configuration/ConfigurationSourceTests.cs | 30 +++++++++++- 8 files changed, 136 insertions(+), 23 deletions(-) diff --git a/src/Datadog.Trace/Configuration/CompositeConfigurationSource.cs b/src/Datadog.Trace/Configuration/CompositeConfigurationSource.cs index 6e5037b91434..9baf8be4df69 100644 --- a/src/Datadog.Trace/Configuration/CompositeConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/CompositeConfigurationSource.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; @@ -98,5 +99,12 @@ IEnumerator IEnumerable.GetEnumerator() { return _sources.GetEnumerator(); } + + /// + public IDictionary GetDictionary(string key) + { + return _sources.Select(source => source.GetDictionary(key)) + .FirstOrDefault(value => value != null); + } } } diff --git a/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs b/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs index 016bc22af7a7..21e5d193c807 100644 --- a/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs @@ -13,5 +13,5 @@ public override string GetString(string key) { return Environment.GetEnvironmentVariable(key); } - } + } } diff --git a/src/Datadog.Trace/Configuration/IConfigurationSource.cs b/src/Datadog.Trace/Configuration/IConfigurationSource.cs index 1a62c505b770..d3e46e871289 100644 --- a/src/Datadog.Trace/Configuration/IConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/IConfigurationSource.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; + namespace Datadog.Trace.Configuration { /// @@ -36,5 +38,13 @@ public interface IConfigurationSource /// The key that identifies the setting. /// The value of the setting, or null if not found. bool? GetBool(string key); - } + + /// + /// Gets the value of + /// the setting with the specified key. + /// + /// The key that identifies the setting. + /// The value of the setting, or null if not found. + IDictionary GetDictionary(string key); + } } diff --git a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs index e729bc7812ff..5ec3e79f0401 100644 --- a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Collections.Generic; using System.IO; using Newtonsoft.Json; @@ -98,5 +99,40 @@ public T GetValue(string key) ? default : token.Value(); } + + /// + /// Gets a containing all of the values. + /// + /// + /// Example JSON where `globalTags` is the configuration key. + /// { + /// "globalTags": { + /// "name1": "value1", + /// "name2": "value2" + /// } + /// } + /// + /// The key that identifies the setting. + /// containing all of the key-value pairs. + /// Thrown if the configuration value is not a valid JSON string." + public IDictionary GetDictionary(string key) + { + var token = _configuration.SelectToken(key, errorWhenNoMatch: false); + if (token == null) + { + return null; + } + + // Presence of a curly brace indicates that this is likely a JSON string. An exception will be thrown + // if it fails to parse or convert to a dictionary. + if (token.ToString().Contains("{")) + { + var dictionary = JToken.Parse(token.ToString()) + ?.ToObject(typeof(ConcurrentDictionary)) as ConcurrentDictionary; + return dictionary; + } + + return StringConfigurationSource.ParseCustomKeyValues(token.ToString()); + } } } diff --git a/src/Datadog.Trace/Configuration/StringConfigurationSource.cs b/src/Datadog.Trace/Configuration/StringConfigurationSource.cs index 4b0aa3748dd4..e4d3075d9e19 100644 --- a/src/Datadog.Trace/Configuration/StringConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/StringConfigurationSource.cs @@ -1,3 +1,6 @@ +using System.Collections.Concurrent; +using System.Collections.Generic; + namespace Datadog.Trace.Configuration { /// @@ -6,6 +9,39 @@ namespace Datadog.Trace.Configuration /// public abstract class StringConfigurationSource : IConfigurationSource { + /// + /// Returns a from parsing + /// . + /// + /// A string containing key-value pairs which are comma-separated, and for which the key and value are colon-separated. + /// of key value pairs. + public static IDictionary ParseCustomKeyValues(string data) + { + var dictionary = new ConcurrentDictionary(); + + if (data == null) + { + return dictionary; + } + + var entries = data.Split(','); + + foreach (var e in entries) + { + var kv = e.Split(':'); + if (kv.Length != 2) + { + continue; + } + + var key = kv[0]; + var value = kv[1]; + dictionary[key] = value; + } + + return dictionary; + } + /// public abstract string GetString(string key); @@ -38,5 +74,15 @@ public abstract class StringConfigurationSource : IConfigurationSource ? result : (bool?)null; } + + /// + /// Gets a from parsing + /// + /// The key + /// containing all of the key-value pairs. + public IDictionary GetDictionary(string key) + { + return ParseCustomKeyValues(GetString(key)); + } } } diff --git a/src/Datadog.Trace/Configuration/TracerSettings.cs b/src/Datadog.Trace/Configuration/TracerSettings.cs index 28b857aaee72..04c749c0352b 100644 --- a/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -79,17 +79,8 @@ public TracerSettings(IConfigurationSource source) Integrations = new IntegrationSettingsCollection(source); - var gTags = source?.GetString(ConfigurationKeys.GlobalTags); - - if (gTags == null) - { - GlobalTags = new ConcurrentDictionary(); - } - else - { - var csvMapConfig = new CsvMapConfigurationSource(gTags); - GlobalTags = csvMapConfig.Data; - } + GlobalTags = source?.GetDictionary(ConfigurationKeys.GlobalTags) as ConcurrentDictionary ?? + new ConcurrentDictionary(); } /// diff --git a/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj b/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj index 27fbd33f27c6..a30c20310a1b 100644 --- a/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj +++ b/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj @@ -6,14 +6,8 @@ - - + + @@ -24,4 +18,6 @@ + + diff --git a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs index 97771196e6f0..de3c4367080e 100644 --- a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs +++ b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs @@ -38,7 +38,19 @@ public static IEnumerable GetTestData() yield return new object[] { ConfigurationKeys.DisabledIntegrations, "integration1;integration2", CreateFunc(s => s.DisabledIntegrationNames.Count), 2 }; - yield return new object[] { ConfigurationKeys.GlobalTags, "'k1':'v1', 'k2':'v2'", CreateFunc(s => s.GlobalTags.Count), 2 }; + yield return new object[] { ConfigurationKeys.GlobalTags, "k1:v1, k2:v2", CreateFunc(s => s.GlobalTags.Count), 2 }; + } + + // JsonConfigurationSource needs to be tested with JSON data, which cannot be used with the other IConfigurationSource implementations. + public static IEnumerable GetJsonTestData() + { + yield return new object[] { ConfigurationKeys.GlobalTags, @"{ ""name1"":""value1"", ""name2"": ""value2""}", CreateFunc(s => s.GlobalTags.Count), 2 }; + } + + public static IEnumerable GetBadJsonTestData() + { + // Extra opening brace + yield return new object[] { ConfigurationKeys.GlobalTags, @"{ {""name1"":""value1"", ""name2"": ""value2""}" }; } public static Func CreateFunc(Func settingGetter) @@ -93,7 +105,8 @@ public void EnvironmentConfigurationSource( [Theory] [MemberData(nameof(GetTestData))] - public void NameValueConfigurationSource1( + [MemberData(nameof(GetJsonTestData))] + public void JsonConfigurationSource( string key, string value, Func settingGetter, @@ -107,5 +120,18 @@ public void NameValueConfigurationSource1( object actualValue = settingGetter(settings); Assert.Equal(expectedValue, actualValue); } + + [Theory] + [MemberData(nameof(GetBadJsonTestData))] + public void JsonConfigurationSource_BadData( + string key, + string value) + { + var config = new Dictionary { [key] = value }; + string json = JsonConvert.SerializeObject(config); + IConfigurationSource source = new JsonConfigurationSource(json); + + Assert.Throws(() => { new TracerSettings(source); }); + } } } From 0879026f587d4f62a8b071f32ec6a053f316ca6d Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Fri, 25 Oct 2019 08:22:48 -0700 Subject: [PATCH 08/13] Remove unused file --- .../CsvMapConfigurationSource.cs | 60 ------------------- 1 file changed, 60 deletions(-) delete mode 100644 src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs diff --git a/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs b/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs deleted file mode 100644 index d2b548b8ecdc..000000000000 --- a/src/Datadog.Trace/Configuration/CsvMapConfigurationSource.cs +++ /dev/null @@ -1,60 +0,0 @@ -using System; -using System.Collections.Concurrent; - -namespace Datadog.Trace.Configuration -{ - /// - /// Represents a configuration source that retrieves values from - /// a comma-separate list of key-value pairs. - /// - public class CsvMapConfigurationSource : StringConfigurationSource - { - private ConcurrentDictionary _data; - - /// - /// Initializes a new instance of the class - /// using a custom configuration string format. - /// - /// Configuration string that is a comma-separated list of key, value pairs where key and value are colon-separated. - /// An example configuration string would be "'globalTag1':'someValue','globalTag2':'anotherValue'" - public CsvMapConfigurationSource(string config) - { - _data = ParseConfig(config); - } - - /// - /// Gets the configuration data as a - /// - public ConcurrentDictionary Data - { - get { return _data; } - } - - /// - public override string GetString(string key) - { - throw new NotImplementedException(); - } - - private ConcurrentDictionary ParseConfig(string config) - { - var data = new ConcurrentDictionary(); - - var entries = config.Split(','); - foreach (var e in entries) - { - var kv = e.Split(':'); - if (kv.Length != 2) - { - continue; - } - - var key = kv[0]; - var value = kv[1]; - data[key] = value; - } - - return data; - } - } -} From 2f4048f97b7819da874da6f00de217b53e85e82f Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Fri, 25 Oct 2019 08:32:29 -0700 Subject: [PATCH 09/13] Spacing fix --- .../Configuration/EnvironmentConfigurationSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs b/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs index 21e5d193c807..016bc22af7a7 100644 --- a/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/EnvironmentConfigurationSource.cs @@ -13,5 +13,5 @@ public override string GetString(string key) { return Environment.GetEnvironmentVariable(key); } - } + } } From d8de2f2d9b0b9fd409500f5af44c0ec547e64a78 Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Fri, 25 Oct 2019 11:11:10 -0700 Subject: [PATCH 10/13] Improved JsonConfigurationSource tests --- .../Configuration/JsonConfigurationSource.cs | 2 +- .../StringConfigurationSource.cs | 2 +- .../Configuration/TracerSettings.cs | 4 +- .../Configuration/ConfigurationSourceTests.cs | 37 +++++++++++-------- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs index 5ec3e79f0401..e7f4ee783a0a 100644 --- a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs @@ -125,7 +125,7 @@ public IDictionary GetDictionary(string key) // Presence of a curly brace indicates that this is likely a JSON string. An exception will be thrown // if it fails to parse or convert to a dictionary. - if (token.ToString().Contains("{")) + if (token.Type == JTokenType.Object) { var dictionary = JToken.Parse(token.ToString()) ?.ToObject(typeof(ConcurrentDictionary)) as ConcurrentDictionary; diff --git a/src/Datadog.Trace/Configuration/StringConfigurationSource.cs b/src/Datadog.Trace/Configuration/StringConfigurationSource.cs index e4d3075d9e19..6ffed4034d2e 100644 --- a/src/Datadog.Trace/Configuration/StringConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/StringConfigurationSource.cs @@ -19,7 +19,7 @@ public static IDictionary ParseCustomKeyValues(string data) { var dictionary = new ConcurrentDictionary(); - if (data == null) + if (string.IsNullOrWhiteSpace(data)) { return dictionary; } diff --git a/src/Datadog.Trace/Configuration/TracerSettings.cs b/src/Datadog.Trace/Configuration/TracerSettings.cs index 04c749c0352b..f9dced94ecc0 100644 --- a/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -79,7 +79,7 @@ public TracerSettings(IConfigurationSource source) Integrations = new IntegrationSettingsCollection(source); - GlobalTags = source?.GetDictionary(ConfigurationKeys.GlobalTags) as ConcurrentDictionary ?? + GlobalTags = source?.GetDictionary(ConfigurationKeys.GlobalTags) as IDictionary ?? new ConcurrentDictionary(); } @@ -149,7 +149,7 @@ public TracerSettings(IConfigurationSource source) /// /// Gets or sets the global tags, which are applied to all s. /// - public ConcurrentDictionary GlobalTags { get; set; } + public IDictionary GlobalTags { get; set; } /// /// Create a populated from the default sources diff --git a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs index de3c4367080e..79aa93f55b34 100644 --- a/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs +++ b/test/Datadog.Trace.Tests/Configuration/ConfigurationSourceTests.cs @@ -44,13 +44,19 @@ public static IEnumerable GetTestData() // JsonConfigurationSource needs to be tested with JSON data, which cannot be used with the other IConfigurationSource implementations. public static IEnumerable GetJsonTestData() { - yield return new object[] { ConfigurationKeys.GlobalTags, @"{ ""name1"":""value1"", ""name2"": ""value2""}", CreateFunc(s => s.GlobalTags.Count), 2 }; + yield return new object[] { @"{ ""DD_TRACE_GLOBAL_TAGS"": { ""name1"":""value1"", ""name2"": ""value2""} }", CreateFunc(s => s.GlobalTags.Count), 2 }; } - public static IEnumerable GetBadJsonTestData() + public static IEnumerable GetBadJsonTestData1() { // Extra opening brace - yield return new object[] { ConfigurationKeys.GlobalTags, @"{ {""name1"":""value1"", ""name2"": ""value2""}" }; + yield return new object[] { @"{ ""DD_TRACE_GLOBAL_TAGS"": { { ""name1"":""value1"", ""name2"": ""value2""} }" }; + } + + public static IEnumerable GetBadJsonTestData2() + { + // Missing closing brace + yield return new object[] { @"{ ""DD_TRACE_GLOBAL_TAGS"": { ""name1"":""value1"", ""name2"": ""value2"" }" }; } public static Func CreateFunc(Func settingGetter) @@ -104,34 +110,33 @@ public void EnvironmentConfigurationSource( } [Theory] - [MemberData(nameof(GetTestData))] [MemberData(nameof(GetJsonTestData))] public void JsonConfigurationSource( - string key, string value, Func settingGetter, object expectedValue) { - var config = new Dictionary { [key] = value }; - string json = JsonConvert.SerializeObject(config); - IConfigurationSource source = new JsonConfigurationSource(json); + IConfigurationSource source = new JsonConfigurationSource(value); var settings = new TracerSettings(source); - object actualValue = settingGetter(settings); + var actualValue = settingGetter(settings); Assert.Equal(expectedValue, actualValue); } [Theory] - [MemberData(nameof(GetBadJsonTestData))] - public void JsonConfigurationSource_BadData( - string key, + [MemberData(nameof(GetBadJsonTestData1))] + public void JsonConfigurationSource_BadData1( string value) { - var config = new Dictionary { [key] = value }; - string json = JsonConvert.SerializeObject(config); - IConfigurationSource source = new JsonConfigurationSource(json); + Assert.Throws(() => { new JsonConfigurationSource(value); }); + } - Assert.Throws(() => { new TracerSettings(source); }); + [Theory] + [MemberData(nameof(GetBadJsonTestData2))] + public void JsonConfigurationSource_BadData2( + string value) + { + Assert.Throws(() => { new JsonConfigurationSource(value); }); } } } From bdb7126dce6996656e60963db88591ce6f699f5d Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Fri, 25 Oct 2019 12:55:43 -0700 Subject: [PATCH 11/13] Use generic ToObject --- src/Datadog.Trace/Configuration/JsonConfigurationSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs index e7f4ee783a0a..bc68d5538dd6 100644 --- a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs @@ -128,7 +128,7 @@ public IDictionary GetDictionary(string key) if (token.Type == JTokenType.Object) { var dictionary = JToken.Parse(token.ToString()) - ?.ToObject(typeof(ConcurrentDictionary)) as ConcurrentDictionary; + ?.ToObject>() as ConcurrentDictionary; return dictionary; } From b08499e550e26ad8ada6dd0407a8530ca67c9576 Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Fri, 25 Oct 2019 13:49:02 -0700 Subject: [PATCH 12/13] Revert Datadog.Trace.ClrProfiler.IntegrationTests.csproj change --- ...Datadog.Trace.ClrProfiler.IntegrationTests.csproj | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj b/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj index a30c20310a1b..27fbd33f27c6 100644 --- a/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj +++ b/test/Datadog.Trace.ClrProfiler.IntegrationTests/Datadog.Trace.ClrProfiler.IntegrationTests.csproj @@ -6,8 +6,14 @@ - - + + @@ -18,6 +24,4 @@ - - From 6812e7b578a1b482072a741910c742f5efbe07be Mon Sep 17 00:00:00 2001 From: Bob Uva Date: Fri, 25 Oct 2019 16:05:09 -0700 Subject: [PATCH 13/13] More changes per code review --- .../Configuration/JsonConfigurationSource.cs | 2 +- .../Configuration/StringConfigurationSource.cs | 8 ++++++++ src/Datadog.Trace/Configuration/TracerSettings.cs | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs index bc68d5538dd6..64c3d2adb6c7 100644 --- a/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/JsonConfigurationSource.cs @@ -127,7 +127,7 @@ public IDictionary GetDictionary(string key) // if it fails to parse or convert to a dictionary. if (token.Type == JTokenType.Object) { - var dictionary = JToken.Parse(token.ToString()) + var dictionary = token ?.ToObject>() as ConcurrentDictionary; return dictionary; } diff --git a/src/Datadog.Trace/Configuration/StringConfigurationSource.cs b/src/Datadog.Trace/Configuration/StringConfigurationSource.cs index 6ffed4034d2e..7c9dca3ec608 100644 --- a/src/Datadog.Trace/Configuration/StringConfigurationSource.cs +++ b/src/Datadog.Trace/Configuration/StringConfigurationSource.cs @@ -19,6 +19,14 @@ public static IDictionary ParseCustomKeyValues(string data) { var dictionary = new ConcurrentDictionary(); + // A null return value means the key was not present, + // and CompositeConfigurationSource depends on this behavior + // (it returns the first non-null value it finds). + if (data == null) + { + return null; + } + if (string.IsNullOrWhiteSpace(data)) { return dictionary; diff --git a/src/Datadog.Trace/Configuration/TracerSettings.cs b/src/Datadog.Trace/Configuration/TracerSettings.cs index f9dced94ecc0..a7e8849e04ae 100644 --- a/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -79,7 +79,7 @@ public TracerSettings(IConfigurationSource source) Integrations = new IntegrationSettingsCollection(source); - GlobalTags = source?.GetDictionary(ConfigurationKeys.GlobalTags) as IDictionary ?? + GlobalTags = source?.GetDictionary(ConfigurationKeys.GlobalTags) ?? new ConcurrentDictionary(); }