Merge 553f04c into 91f2817

CHANGELOG.md

@@ -5,9 +5,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Added
+- Support for empty buckets tag, which will generate nil buckets for the prometheus Histogram and use default prometheus buckets
+
 ### Changed
 - *Breaking*: `prometheus.Histogram` is now used to build histograms, instead of `prometheus.Observer`, which means that previous code building `prometheus.Observer` won't compile anymore.
 
+### Removed
+- *Breaking*: default buckets on histograms. All histogram should explicitly specify their buckets now or they will fail to build.
+
 ## [0.3.0] - 2019-10-10
 ### Added
 - Add objectives to summaries through struct tag and set default values when none specified

README.md

@@ -64,7 +64,7 @@ Define your metrics:
 ```go
 var metrics struct {
 	SomeCounter                      func() prometheus.Counter   `name:"some_counter" help:"some counter"`
-	SomeHistogram                    func() prometheus.Histogram `name:"some_histogram" help:"Some histogram with default buckets"`
+	SomeHistogram                    func() prometheus.Histogram `name:"some_histogram" help:"Some histogram with default prometheus buckets" buckets:""`
 	SomeHistogramWithSpecificBuckets func() prometheus.Histogram `name:"some_histogram_with_buckets" help:"Some histogram with custom buckets" buckets:".01,.05,.1"`
 	SomeGauge                        func() prometheus.Gauge     `name:"some_gauge" help:"Some gauge"`
 	SomeSummaryWithSpecificMaxAge    func() prometheus.Summary   `name:"some_summary_with_specific_max_age" help:"Some summary with custom max age" max_age:"20m"`
@@ -170,7 +170,7 @@ So you can later define it as:
 
 ```go
 var metrics struct {
-	DurationSeconds func() prometheusx.TimeHistogram `name:"duration_seconds" help:"Duration in seconds"`
+	DurationSeconds func() prometheusx.TimeHistogram `name:"duration_seconds" help:"Duration in seconds" buckets:".001,.005,.01,.025,.05,.1"`
 }
 
 func init() {

metrics_test.go

@@ -21,7 +21,7 @@ func Test_InitHappyCase(t *testing.T) {
 	}
 
 	var metrics struct {
-		HTTPRequestTime           func(labels) prometheus.Histogram `name:"http_request_count" help:"Time taken to serve a HTTP request" metricsbuckets:"0.001,0.005,0.01,0.05,0.1,0.5,1,5,10"`
+		HTTPRequestTime           func(labels) prometheus.Histogram `name:"http_request_count" help:"Time taken to serve a HTTP request" buckets:"0.001,0.005,0.01,0.05,0.1,0.5,1,5,10"`
 		DuvelsEmptied             func(labels) prometheus.Counter   `name:"duvels_emptied" help:"Delirium floor sweep count"`
 		RubberDuckInTherapy       func(labels) prometheus.Gauge     `name:"rubber_ducks_in_therapy" help:"Number of rubber ducks who need help after some intense coding"`
 		BrokenDeploysAccomplished func(labels) prometheus.Summary   `name:"broken_deploys_accomplished" help:"Number of deploys that broke production"`
@@ -124,7 +124,7 @@ func Test_LabelsWithBooleans(t *testing.T) {
 	}
 
 	var metrics struct {
-		WithLabels func(labelsWithBools) prometheus.Histogram `name:"with_booleans" help:"Parse booleans as strings"`
+		WithLabels func(labelsWithBools) prometheus.Histogram `name:"with_booleans" help:"Parse booleans as strings" buckets:""`
 	}
 
 	gotoprom.MustInit(&metrics, "testbooleans")
@@ -153,7 +153,7 @@ func Test_LabelsWithInts(t *testing.T) {
 	}
 
 	var metrics struct {
-		WithLabels func(labelsWithInts) prometheus.Histogram `name:"with_ints" help:"Parse ints as strings"`
+		WithLabels func(labelsWithInts) prometheus.Histogram `name:"with_ints" help:"Parse ints as strings" buckets:""`
 	}
 
 	gotoprom.MustInit(&metrics, "testints")
@@ -186,7 +186,7 @@ func Test_DefaultLabelValues(t *testing.T) {
 	}
 
 	var metrics struct {
-		WithLabels func(labelsWithEmptyValues) prometheus.Histogram `name:"with_labels" help:"Assign default values"`
+		WithLabels func(labelsWithEmptyValues) prometheus.Histogram `name:"with_labels" help:"Assign default values" buckets:"10,20,30"`
 	}
 	gotoprom.MustInit(&metrics, "testdefault")
 
@@ -202,7 +202,7 @@ func Test_DefaultLabelValues(t *testing.T) {
 
 func Test_HistogramWithUnsupportedBuckets(t *testing.T) {
 	var metrics struct {
-		Histogram func() prometheus.Histogram `name:"with_broken_buckets" help:"Wrong buckets" buckets:"0.005, +inf"`
+		Histogram func() prometheus.Histogram `name:"with_broken_buckets" help:"Wrong buckets" buckets:"0.005, whatever"`
 	}
 	err := gotoprom.Init(&metrics, "test")
 	assert.NotNil(t, err)

prometheusvanilla/builders.go

@@ -106,12 +106,19 @@ func BuildSummary(name, help, namespace string, labelNames []string, tag reflect
 func bucketsFromTag(tag reflect.StructTag) ([]float64, error) {
 	bucketsString, ok := tag.Lookup("buckets")
 	if !ok {
-		return DefaultBuckets(), nil
+		return nil, fmt.Errorf("buckets not specified")
 	}
+
+	s := strings.TrimSpace(bucketsString)
+	if len(s) == 0 {
+		return nil, nil
+	}
+
 	bucketSlice := strings.Split(bucketsString, ",")
 	buckets := make([]float64, len(bucketSlice))
+
 	var err error
-	for i := 0; i < len(bucketSlice); i++ {
+	for i := range bucketSlice {
 		buckets[i], err = strconv.ParseFloat(bucketSlice[i], 64)
 		if err != nil {
 			return nil, fmt.Errorf("invalid bucket specified: %s", err)
@@ -121,11 +128,6 @@ func bucketsFromTag(tag reflect.StructTag) ([]float64, error) {
 	return buckets, nil
 }
 
-// DefaultBuckets provides a list of buckets you can use when you don't know what to use yet.
-func DefaultBuckets() []float64 {
-	return []float64{.05, .1, 0.25, .5, 1, 5, 10}
-}
-
 func maxAgeFromTag(tag reflect.StructTag) (time.Duration, error) {
 	maxAgeString, ok := tag.Lookup("max_age")
 	if !ok {

prometheusvanilla/builders_test.go

@@ -17,16 +17,21 @@ const (
 )
 
 var (
-	labels                                   = make(prometheus.Labels, 2)
-	keys                                     = make([]string, 0, len(labels))
-	defaultTag             reflect.StructTag = `name:"some_name" help:"some help for the metric"`
-	bucketsTag             reflect.StructTag = `name:"some_name" help:"some help for the metric" buckets:"0.001,0.005,0.01,0.05,0.1,0.5,1,5,10"`
+	labels                       = make(prometheus.Labels, 2)
+	keys                         = make([]string, 0, len(labels))
+	defaultTag reflect.StructTag = `name:"some_name" help:"some help for the metric"`
+
+	bucketsTag          reflect.StructTag = `name:"some_name" help:"some help for the metric" buckets:"0.001,0.005,0.01,0.05,0.1,0.5,1,5,10"`
+	emptyBucketsTag     reflect.StructTag = `name:"some_name" help:"some help for the metric" buckets:""`
+	malformedBucketsTag reflect.StructTag = `name:"some_name" help:"some help for the metric" buckets:"fourtytwo"`
+
 	maxAgeTag              reflect.StructTag = `name:"some_name" help:"some help for the metric" max_age:"1h"`
 	objectivesTag          reflect.StructTag = `name:"some_name" help:"some help for the metric" max_age:"1h" objectives:"0.55,0.95,0.98"`
 	objectivesMalformedTag reflect.StructTag = `name:"some_name" help:"some help for the metric" max_age:"1h" objectives:"notFloat"`
-	expectedBuckets                          = []float64{0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10}
-	expectedMaxAge                           = time.Hour
-	expectedObjectives                       = map[float64]float64{0.55: 0.045, 0.95: 0.005, 0.98: 0.002}
+
+	expectedBuckets    = []float64{0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10}
+	expectedMaxAge     = time.Hour
+	expectedObjectives = map[float64]float64{0.55: 0.045, 0.95: 0.005, 0.98: 0.002}
 )
 
 func TestBuilders(t *testing.T) {
@@ -47,7 +52,7 @@ func TestBuilders(t *testing.T) {
 	})
 
 	t.Run("Test building a histogram", func(t *testing.T) {
-		f, c, err := BuildHistogram(name, help, nameSpace, keys, "")
+		f, c, err := BuildHistogram(name, help, nameSpace, keys, `buckets:""`)
 		assert.NoError(t, err)
 		assert.Implements(t, (*prometheus.Collector)(nil), c)
 		assert.Implements(t, (*prometheus.Histogram)(nil), f(labels))
@@ -78,10 +83,20 @@ func TestBuckets(t *testing.T) {
 		assert.ElementsMatch(t, expectedBuckets, buckets)
 	})
 
-	t.Run("Test it returns default buckets when none are found", func(t *testing.T) {
-		buckets, err := bucketsFromTag(defaultTag)
+	t.Run("Test empty string generates empty buckets slice", func(t *testing.T) {
+		buckets, err := bucketsFromTag(emptyBucketsTag)
 		assert.NoError(t, err)
-		assert.ElementsMatch(t, DefaultBuckets(), buckets)
+		assert.Len(t, buckets, 0)
+	})
+
+	t.Run("Test it returns error when buckets are malformed", func(t *testing.T) {
+		_, err := bucketsFromTag(malformedBucketsTag)
+		assert.Error(t, err)
+	})
+
+	t.Run("Test it returns error when none are found", func(t *testing.T) {
+		_, err := bucketsFromTag(defaultTag)
+		assert.Error(t, err)
 	})
 }