Add unit tests and refactor

DataDog · Oct 30, 2024 · 454d17a · 454d17a
1 parent d383d31
commit 454d17a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 13 deletions.
diff --git a/tracer/src/Datadog.Trace/Telemetry/Collectors/MetricsTelemetryCollectorBase.cs b/tracer/src/Datadog.Trace/Telemetry/Collectors/MetricsTelemetryCollectorBase.cs
@@ -15,13 +15,11 @@ namespace Datadog.Trace.Telemetry;
 internal abstract partial class MetricsTelemetryCollectorBase
 {
     private static readonly string[] _unknownWafAndRulesVersionTags = { "waf_version:unknown", "event_rules_version:unknown" };
-    private static readonly string[] _unknownWafVersionTags = { "waf_version:unknown" };
     private readonly TimeSpan _aggregationInterval;
     private readonly Action? _aggregationNotification;
     private readonly Task _aggregateTask;
     private readonly TaskCompletionSource<bool> _processExit = new();
     private string[]? _wafAndRulesVersionTags;
-    private string[]? _wafVersionTags;
 
     protected MetricsTelemetryCollectorBase()
         : this(TimeSpan.FromSeconds(10))
@@ -59,7 +57,6 @@ public void SetWafAndRulesVersion(string wafVersion, string? eventRulesVersion)
     {
         // Setting this an array so we can reuse it for multiple metrics
         _wafAndRulesVersionTags = new[] { $"waf_version:{wafVersion}", $"event_rules_version:{eventRulesVersion ?? "unknown"}" };
-        _wafVersionTags = new[] { $"waf_version:{wafVersion}" };
     }
 
     protected static AggregatedMetric[] GetPublicApiCountBuffer()
@@ -152,7 +149,7 @@ protected void AddMetricData(
                             type: metricType)
                         {
                             Namespace = metric.NameSpace,
-                            Tags = GetTags(metric.NameSpace, metricValues.Tags, metric.Name)
+                            Tags = GetTags(metric.NameSpace, metricValues.Tags)
                         });
                 }
 
@@ -185,7 +182,7 @@ protected void AddDistributionData(
                             common: metric.IsCommon)
                         {
                             Namespace = metric.NameSpace,
-                            Tags = GetTags(metric.NameSpace, metricValues.Tags, metric.Name),
+                            Tags = GetTags(metric.NameSpace, metricValues.Tags),
                         });
                 }
 
@@ -214,25 +211,24 @@ private async Task AggregateMetricsLoopAsync()
         }
     }
 
-    private string[]? GetTags(string? ns, string[]? metricKeyTags, string name)
+    private string[]? GetTags(string? ns, string[]? metricKeyTags)
     {
         if (ns != MetricNamespaceConstants.ASM)
         {
             return metricKeyTags;
         }
 
-        bool isRasp = name.StartsWith("rasp", StringComparison.OrdinalIgnoreCase);
-
         if (metricKeyTags is null)
         {
-            return (isRasp ?
-                _wafVersionTags ?? _unknownWafVersionTags :
-                _wafAndRulesVersionTags ?? _unknownWafAndRulesVersionTags);
+            return _wafAndRulesVersionTags ?? _unknownWafAndRulesVersionTags;
         }
 
-        metricKeyTags[0] = (_wafAndRulesVersionTags ?? _unknownWafAndRulesVersionTags)[0];
+        if (string.Equals(metricKeyTags[0], "waf_version"))
+        {
+            metricKeyTags[0] = (_wafAndRulesVersionTags ?? _unknownWafAndRulesVersionTags)[0];
+        }
 
-        if (!isRasp)
+        if (metricKeyTags.Length > 1 && string.Equals(metricKeyTags[1], "event_rules_version"))
         {
             metricKeyTags[1] = (_wafAndRulesVersionTags ?? _unknownWafAndRulesVersionTags)[1];
         }

diff --git a/tracer/test/Datadog.Trace.Tests/Telemetry/Metrics/MetricTests.cs b/tracer/test/Datadog.Trace.Tests/Telemetry/Metrics/MetricTests.cs
@@ -141,6 +141,38 @@ public void OnlyAllowedMetricsAreSubmitted()
         }
     }
 
+    /*
+        if a metric uses waf_version then it's always the first element
+        if a metric uses event_rules_version then it's always the second element
+        if a metric uses event_rules_version then we always have a waf_version
+    */
+
+    [Fact]
+    public void CheckASMTags()
+    {
+        var actual = GetImplementedMetricsAndTags();
+
+        foreach (var metric in actual)
+        {
+            foreach (var permutation in metric.TagPermutations)
+            {
+                for (int i = 0; i < permutation.Length; i++)
+                {
+                    if (permutation[i].StartsWith("waf_version"))
+                    {
+                        i.Should().Be(0, $"waf_version should always be the first tag for {metric.Metric}");
+                    }
+
+                    if (permutation[i].StartsWith("event_rules_version"))
+                    {
+                        i.Should().Be(1, $"event_rules_version should always be the second tag for {metric.Metric}");
+                        permutation[0].Should().StartWith("waf_version", $"event_rules_version should always be accompanied by waf_version for {metric.Metric}");
+                    }
+                }
+            }
+        }
+    }
+
     private static List<ImplementedMetricAndTags> GetImplementedMetricsAndTags()
     {
         var results = new List<ImplementedMetricAndTags>();