From bacaa3d24f23e49cfc22e6f1a3623f01741e315b Mon Sep 17 00:00:00 2001 From: Mohamed Asaker Date: Thu, 13 Jun 2024 10:35:20 -0700 Subject: [PATCH 1/4] Tested and Updated X-Ray Sampler --- src/OpenTelemetry.Sampler.AWS/RulesCache.cs | 9 +++++---- src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs | 8 ++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/OpenTelemetry.Sampler.AWS/RulesCache.cs b/src/OpenTelemetry.Sampler.AWS/RulesCache.cs index 2f215b0ce3..2553459005 100644 --- a/src/OpenTelemetry.Sampler.AWS/RulesCache.cs +++ b/src/OpenTelemetry.Sampler.AWS/RulesCache.cs @@ -60,11 +60,12 @@ public void UpdateRules(List newRules) List newRuleAppliers = new List(); foreach (var rule in newRules) { - var currentStatistics = this.RuleAppliers - .FirstOrDefault(currentApplier => currentApplier.RuleName == rule.RuleName) - ?.Statistics ?? new Statistics(); + // If the rule already exists in the current list of appliers, then we reuse it. + // If there are any changes to the rule, UpdateTargets would update the rule. + var ruleApplier = this.RuleAppliers + .FirstOrDefault(currentApplier => currentApplier.RuleName == rule.RuleName) ?? + new SamplingRuleApplier(this.ClientId, this.Clock, rule, new Statistics()); - var ruleApplier = new SamplingRuleApplier(this.ClientId, this.Clock, rule, currentStatistics); newRuleAppliers.Add(ruleApplier); } diff --git a/src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs b/src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs index 32bbae4ed1..ee88ed2c98 100644 --- a/src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs +++ b/src/OpenTelemetry.Sampler.AWS/SamplingRuleApplier.cs @@ -36,7 +36,7 @@ public SamplingRuleApplier(string clientId, Clock clock, SamplingRule rule, Stat this.FixedRateSampler = new ParentBasedSampler(new TraceIdRatioBasedSampler(rule.FixedRate)); // We either have no reservoir sampling or borrow until we get a quota so have no end time. - this.ReservoirEndTime = DateTime.MaxValue; + this.ReservoirEndTime = DateTimeOffset.MaxValue; // We don't have a SamplingTarget so are ready to report a snapshot right away. this.NextSnapshotTime = this.Clock.Now(); @@ -97,15 +97,15 @@ public bool Matches(SamplingParameters samplingParameters, Resource resource) { foreach (var tag in samplingParameters.Tags) { - if (tag.Key.Equals(SemanticConventions.AttributeHttpTarget, StringComparison.Ordinal)) + if (tag.Key.Equals(SemanticConventions.AttributeUrlPath, StringComparison.Ordinal)) { httpTarget = (string?)tag.Value; } - else if (tag.Key.Equals(SemanticConventions.AttributeHttpUrl, StringComparison.Ordinal)) + else if (tag.Key.Equals(SemanticConventions.AttributeUrlFull, StringComparison.Ordinal)) { httpUrl = (string?)tag.Value; } - else if (tag.Key.Equals(SemanticConventions.AttributeHttpMethod, StringComparison.Ordinal)) + else if (tag.Key.Equals(SemanticConventions.AttributeHttpRequestMethod, StringComparison.Ordinal)) { httpMethod = (string?)tag.Value; } From 32c713252031bed42611add9c0c5cb429326b329 Mon Sep 17 00:00:00 2001 From: Mohamed Asaker Date: Thu, 13 Jun 2024 11:35:07 -0700 Subject: [PATCH 2/4] Updated sampler unit tests --- .../TestSamplingRuleApplier.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/OpenTelemetry.Sampler.AWS.Tests/TestSamplingRuleApplier.cs b/test/OpenTelemetry.Sampler.AWS.Tests/TestSamplingRuleApplier.cs index 725eabb8f9..a28c8faa9d 100644 --- a/test/OpenTelemetry.Sampler.AWS.Tests/TestSamplingRuleApplier.cs +++ b/test/OpenTelemetry.Sampler.AWS.Tests/TestSamplingRuleApplier.cs @@ -30,8 +30,8 @@ public void TestRuleMatchesWithAllAttributes() var activityTags = new Dictionary { { "http.host", "localhost" }, - { "http.method", "GET" }, - { "http.url", @"http://127.0.0.1:5000/helloworld" }, + { "http.request.method", "GET" }, + { "url.full", @"http://127.0.0.1:5000/helloworld" }, { "faas.id", "arn:aws:lambda:us-west-2:123456789012:function:my-function" }, }; @@ -59,8 +59,8 @@ public void TestRuleMatchesWithWildcardAttributes() var activityTags = new Dictionary { { "http.host", "localhost" }, - { "http.method", "GET" }, - { "http.url", @"http://127.0.0.1:5000/helloworld" }, + { "http.request.method", "GET" }, + { "url.full", @"http://127.0.0.1:5000/helloworld" }, }; var applier = new SamplingRuleApplier("clientId", new TestClock(), rule, new Statistics()); @@ -132,7 +132,7 @@ public void TestRuleMatchesWithHttpTarget() var activityTags = new Dictionary { - { "http.target", "/helloworld" }, + { "url.path", "/helloworld" }, }; var applier = new SamplingRuleApplier("clientId", new TestClock(), rule, new Statistics()); @@ -164,7 +164,7 @@ public void TestAttributeMatching() var activityTags = new Dictionary { - { "http.target", "/helloworld" }, + { "url.path", "/helloworld" }, { "dog", "bark" }, { "cat", "meow" }, }; @@ -198,7 +198,7 @@ public void TestAttributeMatchingWithLessActivityTags() var activityTags = new Dictionary { - { "http.target", "/helloworld" }, + { "url.path", "/helloworld" }, { "dog", "bark" }, }; From aef32193e81d0cefdf35b42df452195185520b2d Mon Sep 17 00:00:00 2001 From: Mohamed Asaker Date: Thu, 13 Jun 2024 12:10:17 -0700 Subject: [PATCH 3/4] Updated the ruleCache to update the rule during refresh. --- src/OpenTelemetry.Sampler.AWS/RulesCache.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Sampler.AWS/RulesCache.cs b/src/OpenTelemetry.Sampler.AWS/RulesCache.cs index 2553459005..bb55ef54a1 100644 --- a/src/OpenTelemetry.Sampler.AWS/RulesCache.cs +++ b/src/OpenTelemetry.Sampler.AWS/RulesCache.cs @@ -60,12 +60,14 @@ public void UpdateRules(List newRules) List newRuleAppliers = new List(); foreach (var rule in newRules) { - // If the rule already exists in the current list of appliers, then we reuse it. - // If there are any changes to the rule, UpdateTargets would update the rule. + // If the ruleApplier already exists in the current list of appliers, then we reuse it. var ruleApplier = this.RuleAppliers .FirstOrDefault(currentApplier => currentApplier.RuleName == rule.RuleName) ?? new SamplingRuleApplier(this.ClientId, this.Clock, rule, new Statistics()); + // update the rule in the applier in case rule attributes have changed + ruleApplier.Rule = rule; + newRuleAppliers.Add(ruleApplier); } From ae6da76f85d271234e1c78baef810aae712874a9 Mon Sep 17 00:00:00 2001 From: Mohamed Asaker Date: Thu, 13 Jun 2024 12:10:17 -0700 Subject: [PATCH 4/4] Updated the ruleCache to update the rule during refresh. --- src/OpenTelemetry.Sampler.AWS/RulesCache.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/OpenTelemetry.Sampler.AWS/RulesCache.cs b/src/OpenTelemetry.Sampler.AWS/RulesCache.cs index 2553459005..bb55ef54a1 100644 --- a/src/OpenTelemetry.Sampler.AWS/RulesCache.cs +++ b/src/OpenTelemetry.Sampler.AWS/RulesCache.cs @@ -60,12 +60,14 @@ public void UpdateRules(List newRules) List newRuleAppliers = new List(); foreach (var rule in newRules) { - // If the rule already exists in the current list of appliers, then we reuse it. - // If there are any changes to the rule, UpdateTargets would update the rule. + // If the ruleApplier already exists in the current list of appliers, then we reuse it. var ruleApplier = this.RuleAppliers .FirstOrDefault(currentApplier => currentApplier.RuleName == rule.RuleName) ?? new SamplingRuleApplier(this.ClientId, this.Clock, rule, new Statistics()); + // update the rule in the applier in case rule attributes have changed + ruleApplier.Rule = rule; + newRuleAppliers.Add(ruleApplier); }