more comments

pkg/sampling/common.go

@@ -50,15 +50,10 @@ func (cts commonTraceState) ExtraValues() []KV {
 }
 
 // trimOws removes optional whitespace on both ends of a string.
+// this uses the strict definition for optional whitespace tiven
+// in https://www.w3.org/TR/trace-context/#tracestate-header-field-values
 func trimOws(input string) string {
-	// Hard-codes the value of owsCharset
-	for len(input) > 0 && (input[0] == ' ' || input[0] == '\t') {
-		input = input[1:]
-	}
-	for len(input) > 0 && (input[len(input)-1] == ' ' || input[len(input)-1] == '\t') {
-		input = input[:len(input)-1]
-	}
-	return input
+	return strings.Trim(input, " \t")
 }
 
 // scanKeyValues is common code to scan either W3C or OTel tracestate
@@ -98,7 +93,12 @@ func (s keyValueScanner) scanKeyValues(input string, f func(key, value string) e
 
 		eq := strings.IndexByte(member, s.equality)
 		if eq < 0 {
-			// A regexp should have rejected this input.
+			// We expect to find the `s.equality`
+			// character in this string because we have
+			// already validated the whole input syntax
+			// before calling this parser.  I.e., this can
+			// never happen, and if it did, the result
+			// would be to skip malformed entries.
 			continue
 		}
 		if err := f(member[:eq], member[eq+1:]); err != nil {

pkg/sampling/oteltracestate.go

@@ -57,10 +57,18 @@
 	}
 
 	// ErrInconsistentSampling is returned when a sampler update
-	// is illogical.  It is safe to ignore.  Samplers should avoid
-	// this condition using a ThresholdLessThan() test.
+	// is illogical, indicating that the tracestate was not
+	// modified.  Preferrably Samplers will avoid seeing this
+	// error by using a ThresholdGreater() test, which allows them
+	// to report a more clear error to the user.  For example, if
+	// data arrives sampled at 1/100 and an eqalizing sampler is
+	// configureed for 1/2 sampling, the Sampler should detect the
+	// illogical condition itself and skip the call to
+	// UpdateTValueWithSampling.  This condition should be
+	// avoided, but if it arises it is safe to ignore.  In the example,
+	// the span will continue to indicate it was sampled at 1/100 even
+	// after passing through the 1/2 equalizing sampler.
 	ErrInconsistentSampling = fmt.Errorf("cannot raise existing sampling probability")
-	ErrInconsistentZero     = fmt.Errorf("cannot zero sampling probability")
 )
 
 // NewOTelTraceState returns a parsed reprseentation of the