Attempt to resolve #137 (#138)

* Attempt to resolve #137

* I found a reason for omitting the fractional part.

It was my mistake. 🙄

collector/converter.go

@@ -21,11 +21,11 @@ func convArtiToPromBool(b bool) float64 {
 }
 
 const (
-	pattOneNumber = `^(?P<number>[[:digit:]]{1,3}(?:,[[:digit:]]{3})*)(?:\.[[:digit:]]{1,2})? ?(?P<multiplier>%|bytes|[KMGT]B)?$`
+	pattNumber = `^(?P<number>[[:digit:]]{1,3}(?:,[[:digit:]]{3})*(?:\.[[:digit:]]{1,2})?) ?(?P<multiplier>%|bytes|[KMGT]B)?$`
 )
 
 var (
-	reOneNumber = regexp.MustCompile(pattOneNumber)
+	reNumber = regexp.MustCompile(pattNumber)
 )
 
 func (e *Exporter) convArtiToPromNumber(artiNum string) (float64, error) {
@@ -34,20 +34,20 @@ func (e *Exporter) convArtiToPromNumber(artiNum string) (float64, error) {
 		logDbgKeyArtNum, artiNum,
 	)
 
-	if !reOneNumber.MatchString(artiNum) {
+	if !reNumber.MatchString(artiNum) {
 		e.logger.Debug(
 			"The arti number did not match known templates.",
 			logDbgKeyArtNum, artiNum,
 		)
 		err := fmt.Errorf(
 			`The string '%s' does not match pattern '%s'.`,
 			artiNum,
-			pattOneNumber,
+			pattNumber,
 		)
 		return 0, err
 	}
 
-	groups := extractNamedGroups(artiNum, reOneNumber)
+	groups := extractNamedGroups(artiNum, reNumber)
 
 	// The following `strings.replace` is for those cases that contain a comma
 	// thousands separator.  In other cases, unnecessary, but cheaper than if.
@@ -72,47 +72,59 @@ func (e *Exporter) convArtiToPromNumber(artiNum string) (float64, error) {
 }
 
 const (
-	pattTBytesPercent = `^(?P<tbytes>[[:digit:]]+(?:\.[[:digit:]]{1,2})?) TB \((?P<percent>[[:digit:]]{1,2}(?:\.[[:digit:]]{1,2})?)%\)$`
+	pattFileStoreData = `^(?P<size>[[:digit:]]+(?:\.[[:digit:]]{1,2})? [KMGT]B) \((?P<usage>[[:digit:]]{1,2}(?:\.[[:digit:]]{1,2})?%)\)$`
 )
 
 var (
-	reTBytesPercent = regexp.MustCompile(pattTBytesPercent)
+	reFileStoreData = regexp.MustCompile(pattFileStoreData)
 )
 
-func (e *Exporter) convArtiToPromSizeAndUsage(artiSize string) (float64, float64, error) {
+// convArtiToPromFileStoreData tries to interpret the string from artifactory
+// as filestore data.
+// Usually the inscription has two parts. Size and percentage of use. However,
+// it happens that artifactory only gives the size.
+// Please look at the cases in the unit test `TestConvFileStoreData`.
+func (e *Exporter) convArtiToPromFileStoreData(artiSize string) (float64, float64, error) {
 	e.logger.Debug(
-		"Attempting to convert a string from artifactory representing a number.",
+		"Attempting to convert a string from artifactory representing a file store data.",
 		logDbgKeyArtNum, artiSize,
 	)
 
-	if !reTBytesPercent.MatchString(artiSize) {
+	if !strings.Contains(artiSize, `%`) {
+		b, err := e.convArtiToPromNumber(artiSize)
+		if err != nil {
+			return 0, 0, fmt.Errorf(
+				"The string '%s' not recognisable as known artifactory filestore size: %w",
+				artiSize,
+				err,
+			)
+		}
+		return b, 0, nil
+	}
+
+	if !reFileStoreData.MatchString(artiSize) {
 		e.logger.Debug(
-			"The arti number did not match known templates.",
+			fmt.Sprintf(
+				"The arti number did not match template '%s'.",
+				pattFileStoreData,
+			),
 			logDbgKeyArtNum, artiSize,
 		)
 		err := fmt.Errorf(
 			`The string '%s' does not match '%s' pattern.`,
 			artiSize,
-			pattTBytesPercent,
+			pattFileStoreData,
 		)
 		return 0, 0, err
 	}
-
-	groups := extractNamedGroups(artiSize, reTBytesPercent)
-
-	b, err := e.convNumber(groups["tbytes"])
+	groups := extractNamedGroups(artiSize, reFileStoreData)
+	size, err := e.convArtiToPromNumber(groups["size"])
 	if err != nil {
 		return 0, 0, err
 	}
-	mulTB, _ := e.convMultiplier(`TB`)
-	size := b * mulTB
-
-	p, err := e.convNumber(groups["percent"])
+	usage, err := e.convArtiToPromNumber(groups["usage"])
 	if err != nil {
 		return 0, 0, err
 	}
-	mulPercent, _ := e.convMultiplier(`%`)
-	percent := p * mulPercent
-
-	return size, percent, nil
+	return size, usage, nil
 }

collector/converter_internals.go

@@ -25,7 +25,7 @@ func (e *Exporter) convMultiplier(m string) (float64, error) {
 		"The string was not recognized as a known multiplier.",
 		"artifactory.number.multiplier", m,
 	)
-	return 0, fmt.Errorf(`Could not recognise '%s; as multiplier`, m)
+	return 0, fmt.Errorf(`Could not recognise '%s' as multiplier`, m)
 }
 
 func (e *Exporter) convNumber(n string) (float64, error) {

collector/converter_test.go

@@ -7,6 +7,7 @@ package collector
 // Fell free to make them better.
 
 import (
+	"math"
 	"testing"
 
 	l "github.com/peimanja/artifactory_exporter/logger"
@@ -16,11 +17,17 @@ var fakeExporter = Exporter{
 	logger: l.New(
 		l.Config{
 			Format: l.FormatDefault,
-			Level:  l.LevelDefault,
+			Level:  "debug",
 		},
 	),
 }
 
+const float64EqualityThreshold = 1e-6
+
+func almostEqual(a, b float64) bool {
+	return math.Abs(a-b) <= float64EqualityThreshold
+}
+
 func TestConvNum(t *testing.T) {
 	tests := []struct {
 		input string
@@ -32,29 +39,29 @@ func TestConvNum(t *testing.T) {
 		},
 		{
 			input: `8,888.88 MB`,
-			want:  9319743488.00,
+			want:  9320666234.879999,
 		},
 		{
 			input: `88.88 GB`,
-			want:  94489280512.00,
+			want:  95434173317.119995,
 		},
 		{
 			input: `888.88 GB`,
-			want:  953482739712.00,
+			want:  954427632517.119995,
 		},
 	}
 	for _, tc := range tests {
 		got, err := fakeExporter.convArtiToPromNumber(tc.input)
 		if err != nil {
 			t.Fatalf(`An error '%v' occurred during conversion.`, err)
 		}
-		if tc.want != got {
+		if !almostEqual(tc.want, got) {
 			t.Fatalf(`Want %f, but got %f.`, tc.want, got)
 		}
 	}
 }
 
-func TestConvTwoNum(t *testing.T) {
+func TestConvFileStoreData(t *testing.T) {
 	tests := []struct {
 		input string
 		want  []float64
@@ -63,33 +70,42 @@ func TestConvTwoNum(t *testing.T) {
 			input: `3.33 TB (3.3%)`,
 			want:  []float64{3661373720494.080078, 0.033},
 		},
-
 		{
 			input: `6.66 TB (6.66%)`,
 			want:  []float64{7322747440988.160156, 0.0666},
 		},
-
 		{
 			input: `11.11 TB (11.1%)`,
 			want:  []float64{12215574184591.359375, 0.111},
 		},
-
 		{
 			input: `99.99 TB (99.99%)`,
 			want:  []float64{109940167661322.234375, 0.9999},
 		},
+		{
+			input: `499.76 GB`,
+			want:  []float64{536613213962.23999, 0},
+		},
+		{
+			input: `4.82 GB (0.96%)`,
+			want:  []float64{5175435591.68, 0.0096},
+		},
+		{
+			input: `494.94 GB (99.04%)`,
+			want:  []float64{531437778370.559998, 0.9904},
+		},
 	}
 	for _, tc := range tests {
-		gotSize, gotPercent, err := fakeExporter.convArtiToPromSizeAndUsage(tc.input)
+		gotSize, gotPercent, err := fakeExporter.convArtiToPromFileStoreData(tc.input)
 		if err != nil {
 			t.Fatalf(`An error '%v' occurred during conversion.`, err)
 		}
 		wantSize := tc.want[0]
-		if wantSize != gotSize {
+		if !almostEqual(wantSize, gotSize) {
 			t.Fatalf(`Problem with size. Want %f, but got %f.`, wantSize, gotSize)
 		}
 		wantPercent := tc.want[1]
-		if wantPercent != gotPercent {
+		if !almostEqual(wantPercent, gotPercent) {
 			t.Fatalf(`Problem with percentage. Want %f, but got %f.`, wantPercent, gotPercent)
 		}
 	}

collector/storage.go

@@ -61,7 +61,7 @@ func (e *Exporter) exportFilestore(metricName string, metric *prometheus.Desc, s
 		e.jsonParseFailures.Inc()
 		return
 	}
-	value, percent, err := e.convArtiToPromSizeAndUsage(size)
+	value, percent, err := e.convArtiToPromFileStoreData(size)
 	/*
 	 * What should you use the percentage for?
 	 * Maybe Issue #126?