address comments

Author: jj22ee

incubator/opentelemetry-sampler-aws-xray/src/fallback-sampler.ts

@@ -47,6 +47,6 @@ export class FallbackSampler implements Sampler {
   }
 
   public toString(): string {
-    return 'FallbackSampler{fallback sampling with sampling config of 1 req/sec and 5% of additional requests';
+    return 'FallbackSampler{fallback sampling with sampling config of 1 req/sec and 5% of additional requests}';
   }
 }

incubator/opentelemetry-sampler-aws-xray/src/rule-cache.ts

@@ -44,6 +44,9 @@ export class RuleCache {
   public getMatchedRule(
     attributes: Attributes
   ): SamplingRuleApplier | undefined {
+    // `this.ruleAppliers` should be sorted by priority, so `find()` is used here
+    // to determine the first highest priority rule that is matched. The last rule
+    // in the list should be the 'Default' rule with hardcoded priority of 10000.
     return this.ruleAppliers.find(
       rule =>
         rule.matches(attributes, this.samplerResource) ||
@@ -65,17 +68,16 @@ export class RuleCache {
   }
 
   public updateRules(newRuleAppliers: SamplingRuleApplier[]): void {
-    const oldRuleAppliersMap: { [key: string]: SamplingRuleApplier } = {};
+    const oldRuleAppliersMap = new Map<string, SamplingRuleApplier>();
 
     this.ruleAppliers.forEach((rule: SamplingRuleApplier) => {
-      oldRuleAppliersMap[rule.samplingRule.RuleName] = rule;
+      oldRuleAppliersMap.set(rule.samplingRule.RuleName, rule);
     });
 
     newRuleAppliers.forEach((newRule: SamplingRuleApplier, index: number) => {
       const ruleNameToCheck: string = newRule.samplingRule.RuleName;
-      if (ruleNameToCheck in oldRuleAppliersMap) {
-        const oldRule: SamplingRuleApplier =
-          oldRuleAppliersMap[ruleNameToCheck];
+      const oldRule = oldRuleAppliersMap.get(ruleNameToCheck);
+      if (oldRule) {
         if (newRule.samplingRule.equals(oldRule.samplingRule)) {
           newRuleAppliers[index] = oldRule;
         }

incubator/opentelemetry-sampler-aws-xray/src/sampling-rule-applier.ts

@@ -24,6 +24,7 @@ import {
   Context,
   Link,
   SpanKind,
+  diag,
 } from '@opentelemetry/api';
 import { Resource } from '@opentelemetry/resources';
 import {
@@ -119,11 +120,16 @@ export class SamplingRuleApplier {
       // Per spec, url.full is always populated with scheme://
       // If scheme is not present, assume it's bad instrumentation and ignore.
       if (schemeEndIndex > -1) {
-        // urlparse("scheme://netloc/path;parameters?query#fragment")
-        httpTarget = new URL(httpUrl).pathname;
-        if (httpTarget === '') httpTarget = '/';
+        try {
+          httpTarget = new URL(httpUrl).pathname;
+          if (httpTarget === '') httpTarget = '/';
+        } catch (e: unknown) {
+          diag.debug(`Unable to create URL object from url: ${httpUrl}`, e);
+        }
       }
-    } else if (httpTarget === undefined && httpUrl === undefined) {
+    }
+
+    if (httpTarget === undefined) {
       // When missing, the URL Path is assumed to be '/'
       httpTarget = '/';
     }

incubator/opentelemetry-sampler-aws-xray/test/fallback-sampler.test.ts

@@ -26,7 +26,7 @@ describe('FallBackSampler', () => {
 
   it('toString()', () => {
     expect(new FallbackSampler().toString()).toEqual(
-      'FallbackSampler{fallback sampling with sampling config of 1 req/sec and 5% of additional requests'
+      'FallbackSampler{fallback sampling with sampling config of 1 req/sec and 5% of additional requests}'
     );
   });
 });