Skip to content

Commit

Permalink
metrics count should be positive
Browse files Browse the repository at this point in the history
  • Loading branch information
jiwen624 committed Apr 20, 2020
1 parent 7dee89d commit 106e00b
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 11 deletions.
26 changes: 25 additions & 1 deletion v1/ao/internal/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ import (
"github.com/pkg/errors"
)

// Linux distributions and their identifying files
const (
metricsTransactionsMaxDefault = 200 // default max amount of transaction names we allow per cycle
metricsHistPrecisionDefault = 2 // default histogram precision

metricsTagNameLengthMax = 64 // max number of characters for tag names
metricsTagValueLengthMax = 255 // max number of characters for tag values

// MaxTagsCount is the maximum number of tags allowed
MaxTagsCount = 50
)

// Special transaction names
Expand Down Expand Up @@ -59,6 +61,10 @@ const (
var (
// ErrExceedsMetricsCountLimit indicates there are too many distinct metrics.
ErrExceedsMetricsCountLimit = errors.New("exceeds metrics count limit per flush interval")
// ErrExceedsTagsCountLimit indicates there are too many tags
ErrExceedsTagsCountLimit = errors.New("exceeds tags count limit")
// ErrMetricsWithNonPositiveCount indicates the count is negative or zero
ErrMetricsWithNonPositiveCount = errors.New("metrics with non-positive count")
)

// SpanMessage defines a span message
Expand Down Expand Up @@ -409,11 +415,17 @@ func (m *Measurements) Clone() *Measurements {

// Summary submits the summary measurement to the reporter.
func (m *Measurements) Summary(name string, value float64, opts MetricOptions) error {
if err := opts.validate(); err != nil {
return err
}
return m.recordWithSoloTags(name, opts.Tags, value, opts.Count, true)
}

// Increment submits the incremental measurement to the reporter.
func (m *Measurements) Increment(name string, opts MetricOptions) error {
if err := opts.validate(); err != nil {
return err
}
return m.recordWithSoloTags(name, opts.Tags, 0, opts.Count, false)
}

Expand All @@ -424,6 +436,18 @@ type MetricOptions struct {
Tags map[string]string
}

func (mo *MetricOptions) validate() error {
if len(mo.Tags) > MaxTagsCount {
return ErrExceedsTagsCountLimit
}

if mo.Count <= 0 {
return ErrMetricsWithNonPositiveCount
}

return nil
}

func addRuntimeMetrics(bbuf *bson.Buffer, index *int) {
// category runtime
addMetricsValue(bbuf, index, "trace.go.runtime.NumGoroutine", runtime.NumGoroutine())
Expand Down
12 changes: 12 additions & 0 deletions v1/ao/internal/reporter/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,18 @@ func TestCustomMetrics(t *testing.T) {
customMetrics: metrics.NewMeasurements(true, grpcMetricIntervalDefault, 500),
}

// Test non-positive count
assert.NotNil(t, r.CustomSummaryMetric("Summary", 1.1, metrics.MetricOptions{
Count: 0,
HostTag: true,
Tags: map[string]string{"hello": "world"},
}))
assert.NotNil(t, r.CustomIncrementMetric("Incremental", metrics.MetricOptions{
Count: -1,
HostTag: true,
Tags: map[string]string{"hi": "globe"},
}))

r.CustomSummaryMetric("Summary", 1.1, metrics.MetricOptions{
Count: 1,
HostTag: true,
Expand Down
14 changes: 4 additions & 10 deletions v1/ao/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
package ao

import (
"errors"

"github.com/appoptics/appoptics-apm-go/v1/ao/internal/metrics"
"github.com/appoptics/appoptics-apm-go/v1/ao/internal/reporter"
)
Expand All @@ -14,31 +12,27 @@ type MetricOptions = metrics.MetricOptions

const (
// MaxTagsCount is the maximum number of tags allowed.
MaxTagsCount = 50
MaxTagsCount = metrics.MaxTagsCount
)

// The measurements submission errors
var (
// ErrExceedsTagsCountLimit indicates the count of tags exceeds the limit
ErrExceedsTagsCountLimit = errors.New("exceeds tags count limit")
ErrExceedsTagsCountLimit = metrics.ErrExceedsTagsCountLimit
// ErrExceedsMetricsCountLimit indicates there are too many distinct measurements in a flush cycle.
ErrExceedsMetricsCountLimit = metrics.ErrExceedsMetricsCountLimit
// ErrMetricsWithNonPositiveCount indicates the count is negative or zero
ErrMetricsWithNonPositiveCount = metrics.ErrMetricsWithNonPositiveCount
)

// SummaryMetric submits a summary type measurement to the reporter. The measurements
// will be collected in the background and reported periodically.
func SummaryMetric(name string, value float64, opts MetricOptions) error {
if len(opts.Tags) > MaxTagsCount {
return ErrExceedsTagsCountLimit
}
return reporter.SummaryMetric(name, value, opts)
}

// IncrementMetric submits a incremental measurement to the reporter. The measurements
// will be collected in the background and reported periodically.
func IncrementMetric(name string, opts MetricOptions) error {
if len(opts.Tags) > MaxTagsCount {
return ErrExceedsTagsCountLimit
}
return reporter.IncrementMetric(name, opts)
}

0 comments on commit 106e00b

Please sign in to comment.