From aaba03cece6050c1d6331707c7277eb0c245bd6d Mon Sep 17 00:00:00 2001 From: Jeffrey Chien Date: Tue, 2 Apr 2024 17:41:45 -0400 Subject: [PATCH] Make unit conversion case-insensitive (#1113) --- internal/cloudwatch/unit.go | 110 ++++++++++++++++-------------- internal/cloudwatch/unit_test.go | 44 ++++++------ internal/util/unit/prefix.go | 27 ++++++-- internal/util/unit/prefix_test.go | 26 ++++--- 4 files changed, 121 insertions(+), 86 deletions(-) diff --git a/internal/cloudwatch/unit.go b/internal/cloudwatch/unit.go index b215cfe211..829ac604ff 100644 --- a/internal/cloudwatch/unit.go +++ b/internal/cloudwatch/unit.go @@ -5,12 +5,11 @@ package cloudwatch import ( "fmt" + "strings" "time" - "unicode" "github.com/aws/aws-sdk-go-v2/service/cloudwatch/types" - "github.com/aws/amazon-cloudwatch-agent/internal/util/collections" "github.com/aws/amazon-cloudwatch-agent/internal/util/unit" ) @@ -26,18 +25,18 @@ var baseUnits = map[string]types.StandardUnit{ "us": types.StandardUnitMicroseconds, "ms": types.StandardUnitMilliseconds, // bytes - "B": types.StandardUnitBytes, - "By": types.StandardUnitBytes, - "Bi": types.StandardUnitBits, + "b": types.StandardUnitBytes, + "by": types.StandardUnitBytes, + "bi": types.StandardUnitBits, // rates - "B/s": types.StandardUnitBytesSecond, - "By/s": types.StandardUnitBytesSecond, - "Bi/s": types.StandardUnitBitsSecond, + "b/s": types.StandardUnitBytesSecond, + "by/s": types.StandardUnitBytesSecond, + "bi/s": types.StandardUnitBitsSecond, } var uniqueConversions = map[string]struct { - unit types.StandardUnit - scale float64 + standardUnit types.StandardUnit + scale float64 }{ // time "ns": {types.StandardUnitMicroseconds, 1 / float64(time.Microsecond.Nanoseconds())}, @@ -76,72 +75,81 @@ var scaledBaseUnits = map[types.StandardUnit]map[unit.MetricPrefix]types.Standar // ToStandardUnit converts from the OTEL unit names to the corresponding names // supported by AWS CloudWatch. Some OTEL unit types are unsupported. func ToStandardUnit(unit string) (string, float64, error) { - if IsStandardUnit(unit) { - return unit, 1, nil + standardUnit, scale, err := toStandardUnit(unit) + return string(standardUnit), scale, err +} + +func toStandardUnit(unit string) (types.StandardUnit, float64, error) { + u := strings.ToLower(unit) + if standardUnit, ok := standardUnits[u]; ok { + return standardUnit, 1, nil } - if baseUnit, ok := baseUnits[unit]; ok { - return string(baseUnit), 1, nil + if standardUnit, ok := baseUnits[u]; ok { + return standardUnit, 1, nil } - if conversion, ok := uniqueConversions[unit]; ok { - return string(conversion.unit), conversion.scale, nil + if conversion, ok := uniqueConversions[u]; ok { + return conversion.standardUnit, conversion.scale, nil } - prefix, base := splitUnit(unit) - if baseUnit, ok := baseUnits[base]; ok { - return scaleBaseUnit(prefix, baseUnit) + prefix, baseUnit := splitUnit(u) + if standardUnit, ok := baseUnits[baseUnit]; ok && prefix != nil { + return scaleBaseUnit(prefix, standardUnit) } - return string(types.StandardUnitNone), 1, fmt.Errorf("non-convertible unit: %q", unit) + return types.StandardUnitNone, 1, fmt.Errorf("non-convertible unit: %q", unit) } -// splitUnit splits a unit and its prefix based on the second capital letter found. +// splitUnit splits a unit and its prefix based on available prefixes. // e.g. MiBy will split into prefix "Mi" and base "By". -func splitUnit(unit string) (string, string) { - var index int - if len(unit) > 1 { - for i, r := range unit[1:] { - if unicode.IsUpper(r) { - index = i + 1 - break - } +func splitUnit(unit string) (unit.Prefix, string) { + for _, prefix := range supportedPrefixes { + p := strings.ToLower(prefix.String()) + baseUnit, ok := strings.CutPrefix(unit, p) + if ok { + return prefix, baseUnit } } - return unit[:index], unit[index:] + return nil, unit } -// scaleBaseUnit takes a prefix and the CloudWatch base unit and finds the scaled CloudWatch unit and +// scaleBaseUnit takes a prefix and the CloudWatch standard unit and finds the scaled CloudWatch unit and // the scale factor if value adjustments are necessary. -func scaleBaseUnit(prefix string, baseUnit types.StandardUnit) (string, float64, error) { - scaledUnits, ok := scaledBaseUnits[baseUnit] +func scaleBaseUnit(prefix unit.Prefix, standardUnit types.StandardUnit) (types.StandardUnit, float64, error) { + scaledUnits, ok := scaledBaseUnits[standardUnit] if !ok { - return string(types.StandardUnitNone), 1, fmt.Errorf("non-scalable unit: %v", baseUnit) + return types.StandardUnitNone, 1, fmt.Errorf("non-scalable unit: %v", standardUnit) } + var metricPrefix unit.MetricPrefix scale := float64(1) - metricPrefix := unit.MetricPrefix(prefix) - if metricPrefix.Value() == -1 { + switch p := prefix.(type) { + case unit.MetricPrefix: + metricPrefix = p + case unit.BinaryPrefix: var err error - metricPrefix, scale, err = unit.ConvertToMetric(unit.BinaryPrefix(prefix)) + metricPrefix, scale, err = unit.ConvertToMetric(p) if err != nil { - return string(types.StandardUnitNone), 1, fmt.Errorf("unsupported prefix: %v", prefix) + return types.StandardUnitNone, 1, err } + default: + return types.StandardUnitNone, 1, fmt.Errorf("unsupported prefix: %v", prefix) } if scaledUnit, ok := scaledUnits[metricPrefix]; ok { - return string(scaledUnit), scale, nil + return scaledUnit, scale, nil } - return string(types.StandardUnitNone), 1, fmt.Errorf("unsupported prefix %v for %v", prefix, baseUnit) + return types.StandardUnitNone, 1, fmt.Errorf("unsupported prefix %v for %v", prefix, standardUnit) } -var standardUnits = collections.NewSet[string]() - -// IsStandardUnit determines if the unit is acceptable by CloudWatch. -func IsStandardUnit(unit string) bool { - if unit == "" { - return false - } - _, ok := standardUnits[unit] - return ok -} +var ( + standardUnits = make(map[string]types.StandardUnit) + supportedPrefixes []unit.Prefix +) func init() { for _, standardUnit := range types.StandardUnitNone.Values() { - standardUnits.Add(string(standardUnit)) + standardUnits[strings.ToLower(string(standardUnit))] = standardUnit + } + for _, binaryPrefix := range unit.BinaryPrefixes { + supportedPrefixes = append(supportedPrefixes, binaryPrefix) + } + for _, metricPrefix := range unit.MetricPrefixes { + supportedPrefixes = append(supportedPrefixes, metricPrefix) } } diff --git a/internal/cloudwatch/unit_test.go b/internal/cloudwatch/unit_test.go index c4f312ff8d..20ec570f9a 100644 --- a/internal/cloudwatch/unit_test.go +++ b/internal/cloudwatch/unit_test.go @@ -4,21 +4,21 @@ package cloudwatch import ( - "math" "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestSimpleUnit(t *testing.T) { - // Each element in the slice has the input and expectedOutput. - cases := [][2]string{ + // Each element in the slice has the input and expected output. + testCases := [][2]string{ {"", "None"}, {"1", "None"}, {"B", "Bytes"}, + {"By", "Bytes"}, + {"by", "Bytes"}, {"B/s", "Bytes/Second"}, - {"By/s", "Bytes/Second"}, + {"BY/S", "Bytes/Second"}, {"Bi/s", "Bits/Second"}, {"Bi", "Bits"}, {"None", "None"}, @@ -26,15 +26,15 @@ func TestSimpleUnit(t *testing.T) { {"%", "Percent"}, } - for _, c := range cases { - a, s, err := ToStandardUnit(c[0]) + for _, testCase := range testCases { + unit, scale, err := ToStandardUnit(testCase[0]) assert.NoError(t, err) - assert.Equal(t, c[1], a) - assert.EqualValues(t, 1, s) + assert.Equal(t, testCase[1], unit) + assert.EqualValues(t, 1, scale) } } -// If the unit cannot be converted then use None. +// If the unit cannot be converted then use None and return an error. func TestUnsupportedUnit(t *testing.T) { testCases := []string{"banana", "ks"} for _, testCase := range testCases { @@ -47,20 +47,24 @@ func TestUnsupportedUnit(t *testing.T) { func TestScaledUnits(t *testing.T) { testCases := []struct { - input string - unit string - scale float64 - epsilon float64 + input string + unit string + scale float64 }{ - {"MiBy", "Megabytes", 1.049, 0.001}, - {"kB", "Kilobytes", 1, 0}, - {"min", "Seconds", 60, 0}, - {"ns", "Microseconds", 0.001, 0}, + {"MiBy", "Megabytes", 1.048576}, + {"mby", "Megabytes", 1}, + {"kB", "Kilobytes", 1}, + {"kib/s", "Kilobytes/Second", 1.024}, + {"ms", "Milliseconds", 1}, + {"ns", "Microseconds", 0.001}, + {"min", "Seconds", 60}, + {"h", "Seconds", 60 * 60}, + {"d", "Seconds", 24 * 60 * 60}, } for _, testCase := range testCases { unit, scale, err := ToStandardUnit(testCase.input) - require.NoError(t, err) + assert.NoError(t, err) assert.Equal(t, testCase.unit, unit) - assert.GreaterOrEqual(t, testCase.epsilon, math.Abs(testCase.scale-scale)) + assert.Equal(t, testCase.scale, scale) } } diff --git a/internal/util/unit/prefix.go b/internal/util/unit/prefix.go index 834677078e..07b052ccde 100644 --- a/internal/util/unit/prefix.go +++ b/internal/util/unit/prefix.go @@ -5,6 +5,11 @@ package unit import "fmt" +type Prefix interface { + fmt.Stringer + Scale() float64 +} + // MetricPrefix is a base 10 prefix used by the metric system. type MetricPrefix string @@ -20,8 +25,10 @@ const ( MetricPrefixTera = "T" ) -// Value returns the scale from the base unit or -1 if invalid. -func (m MetricPrefix) Value() float64 { +var MetricPrefixes = []MetricPrefix{MetricPrefixKilo, MetricPrefixMega, MetricPrefixGiga, MetricPrefixTera} + +// Scale returns the scale from the base unit or -1 if invalid. +func (m MetricPrefix) Scale() float64 { switch m { case MetricPrefixKilo: return kilo @@ -35,6 +42,10 @@ func (m MetricPrefix) Value() float64 { return -1 } +func (m MetricPrefix) String() string { + return string(m) +} + // BinaryPrefix is a base 2 prefix for data storage. type BinaryPrefix string @@ -51,8 +62,10 @@ const ( BinaryPrefixTebi = "Ti" ) -// Value returns the scale from the base unit or -1 if invalid. -func (b BinaryPrefix) Value() float64 { +var BinaryPrefixes = []BinaryPrefix{BinaryPrefixKibi, BinaryPrefixMebi, BinaryPrefixGibi, BinaryPrefixTebi} + +// Scale returns the scale from the base unit or -1 if invalid. +func (b BinaryPrefix) Scale() float64 { switch b { case BinaryPrefixKibi: return kibi @@ -66,6 +79,10 @@ func (b BinaryPrefix) Value() float64 { return -1 } +func (b BinaryPrefix) String() string { + return string(b) +} + var binaryToMetricMapping = map[BinaryPrefix]MetricPrefix{ BinaryPrefixKibi: MetricPrefixKilo, BinaryPrefixMebi: MetricPrefixMega, @@ -79,6 +96,6 @@ func ConvertToMetric(binaryPrefix BinaryPrefix) (MetricPrefix, float64, error) { if !ok { return "", -1, fmt.Errorf("no valid conversion for %v", binaryPrefix) } - scale := binaryPrefix.Value() / metricPrefix.Value() + scale := binaryPrefix.Scale() / metricPrefix.Scale() return metricPrefix, scale, nil } diff --git a/internal/util/unit/prefix_test.go b/internal/util/unit/prefix_test.go index 786d62dd3e..68c126e4ec 100644 --- a/internal/util/unit/prefix_test.go +++ b/internal/util/unit/prefix_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestMetricPrefix(t *testing.T) { @@ -18,12 +17,16 @@ func TestMetricPrefix(t *testing.T) { }{ {"Ki", -1}, {"k", 1e3}, + {"M", 1e6}, {"G", 1e9}, + {"T", 1e12}, } for _, testCase := range testCases { got := MetricPrefix(testCase.prefix) - assert.Equal(t, testCase.value, got.Value()) + assert.Equal(t, testCase.value, got.Scale()) + assert.Equal(t, testCase.prefix, got.String()) } + assert.Len(t, MetricPrefixes, 4) } func TestBinaryPrefix(t *testing.T) { @@ -32,13 +35,17 @@ func TestBinaryPrefix(t *testing.T) { value float64 }{ {"k", -1}, - {"Ki", 1024}, - {"Gi", 1073741824}, + {"Ki", math.Pow(2, 10)}, + {"Mi", math.Pow(2, 20)}, + {"Gi", math.Pow(2, 30)}, + {"Ti", math.Pow(2, 40)}, } for _, testCase := range testCases { got := BinaryPrefix(testCase.prefix) - assert.Equal(t, testCase.value, got.Value()) + assert.Equal(t, testCase.value, got.Scale()) + assert.Equal(t, testCase.prefix, got.String()) } + assert.Len(t, BinaryPrefixes, 4) } func TestConvertBinaryToMetric(t *testing.T) { @@ -50,15 +57,14 @@ func TestConvertBinaryToMetric(t *testing.T) { prefix BinaryPrefix metricPrefix MetricPrefix scale float64 - epsilon float64 }{ - {BinaryPrefixKibi, MetricPrefixKilo, 1.024, 0}, - {BinaryPrefixGibi, MetricPrefixGiga, 1.073, 0.001}, + {BinaryPrefixKibi, MetricPrefixKilo, 1.024}, + {BinaryPrefixGibi, MetricPrefixGiga, 1.073741824}, } for _, testCase := range testCases { got, scale, err = ConvertToMetric(testCase.prefix) - require.NoError(t, err) + assert.NoError(t, err) assert.Equal(t, testCase.metricPrefix, got) - assert.GreaterOrEqual(t, testCase.epsilon, math.Abs(testCase.scale-scale)) + assert.Equal(t, testCase.scale, scale) } }