Skip to content

Commit

Permalink
fix metrics_test.go
Browse files Browse the repository at this point in the history
  • Loading branch information
@swi-jared
swi-jared committed May 2, 2024
1 parent 2b6a014 commit 64539f5
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 55 deletions.
11 changes: 4 additions & 7 deletions internal/metrics/metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,12 +456,6 @@ func (m *measurements) CopyAndReset(flushInterval int32) *measurements {
m.Lock()
defer m.Unlock()

if len(m.m) == 0 {
m.FlushInterval = flushInterval
m.transMap.Reset()
return nil
}

clone := m.Clone()
m.m = make(map[string]*Measurement)
m.transMap.Reset()
Expand Down Expand Up @@ -552,6 +546,10 @@ func addRuntimeMetrics(bbuf *bson.Buffer, index *int) {
// return metrics message in BSON format
func (r *registry) BuildBuiltinMetricsMessage(flushInterval int32, qs *EventQueueStats,
rcs map[string]*RateCounts, runtimeMetrics bool) []byte {
var m = r.apmMetrics.CopyAndReset(flushInterval)
if m == nil {
return nil
}

bbuf := bson.NewBuffer()

Expand Down Expand Up @@ -584,7 +582,6 @@ func (r *registry) BuildBuiltinMetricsMessage(flushInterval int32, qs *EventQueu
addRuntimeMetrics(bbuf, &index)
}

var m = r.apmMetrics.CopyAndReset(flushInterval)
for _, measurement := range m.m {
addMeasurementToBSON(bbuf, &index, measurement)
}
Expand Down
94 changes: 46 additions & 48 deletions internal/metrics/metrics_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,9 @@ func TestAddHistogramToBSON(t *testing.T) {
}

func TestGenerateMetricsMessage(t *testing.T) {
testMetrics := NewMeasurements(false, metricsTransactionsMaxDefault)
bbuf := bson.WithBuf(BuildBuiltinMetricsMessage(testMetrics, &EventQueueStats{},
reg := NewLegacyRegistry()
flushInterval := int32(60)
bbuf := bson.WithBuf(reg.BuildBuiltinMetricsMessage(flushInterval, &EventQueueStats{},
map[string]*RateCounts{ // requested, sampled, limited, traced, through
RCRegular: {10, 2, 5, 5, 1},
RCRelaxedTriggerTrace: {3, 0, 1, 2, 0},
Expand Down Expand Up @@ -482,14 +483,15 @@ func TestGenerateMetricsMessage(t *testing.T) {

assert.Nil(t, m["TransactionNameOverflow"])

testMetrics = NewMeasurements(false, metricsTransactionsMaxDefault)
reg = NewLegacyRegistry()
r := reg.(*registry)
for i := 0; i <= metricsTransactionsMaxDefault; i++ {
if !testMetrics.transMap.IsWithinLimit("Transaction-" + strconv.Itoa(i)) {
if !r.apmMetrics.transMap.IsWithinLimit("Transaction-" + strconv.Itoa(i)) {
break
}
}

m, err = bsonToMap(bson.WithBuf(BuildBuiltinMetricsMessage(testMetrics, &EventQueueStats{},
m, err = bsonToMap(bson.WithBuf(reg.BuildBuiltinMetricsMessage(flushInterval, &EventQueueStats{},
map[string]*RateCounts{RCRegular: {}, RCRelaxedTriggerTrace: {}, RCStrictTriggerTrace: {}}, true)))
require.NoError(t, err)

Expand Down Expand Up @@ -545,13 +547,6 @@ func TestRateCounts(t *testing.T) {
assert.Equal(t, &RateCounts{}, rc)
}

func resetHistograms() {
apmHistograms = &histograms{
histograms: make(map[string]*histogram),
precision: metricsHistPrecisionDefault,
}
}

func TestRecordSpan(t *testing.T) {
tr, teardown := testutils.TracerSetup()
defer teardown()
Expand All @@ -568,11 +563,12 @@ func TestRecordSpan(t *testing.T) {
),
)
span.End(trace.WithTimestamp(now.Add(1 * time.Second)))
reg := NewLegacyRegistry()
r := reg.(*registry)

// This affects global state (ApmMetrics below)
RecordSpan(span.(sdktrace.ReadOnlySpan), false)
reg.RecordSpan(span.(sdktrace.ReadOnlySpan), false)

m := ApmMetrics.CopyAndReset(60)
m := r.apmMetrics.CopyAndReset(60)
assert.NotEmpty(t, m.m)
v := m.m["ResponseTime&true&http.method:GET&http.status_code:200&sw.is_error:false&sw.transaction:my cool route&"]
assert.NotNil(t, v, fmt.Sprintf("Map: %v", m.m))
Expand All @@ -588,8 +584,9 @@ func TestRecordSpan(t *testing.T) {
v.Tags)
assert.Equal(t, responseTime, v.Name)

h := apmHistograms.histograms
resetHistograms()
h := r.apmHistograms.histograms
reg = NewLegacyRegistry()
r = reg.(*registry)
assert.NotEmpty(t, h)
globalHisto := h[""]
granularHisto := h["my cool route"]
Expand All @@ -602,9 +599,9 @@ func TestRecordSpan(t *testing.T) {
assert.Equal(t, int64(1), granularHisto.hist.TotalCount())

// Now test for AO
RecordSpan(span.(sdktrace.ReadOnlySpan), true)
reg.RecordSpan(span.(sdktrace.ReadOnlySpan), true)

m = ApmMetrics.CopyAndReset(60)
m = r.apmMetrics.CopyAndReset(60)
assert.NotEmpty(t, m.m)
k1 := "TransactionResponseTime&true&HttpMethod:GET&TransactionName:my cool route&"
k2 := "TransactionResponseTime&true&HttpStatus:200&TransactionName:my cool route&"
Expand All @@ -629,8 +626,9 @@ func TestRecordSpan(t *testing.T) {
m.m[k3].Tags,
)

h = apmHistograms.histograms
resetHistograms()
h = r.apmHistograms.histograms
reg = NewLegacyRegistry()
r = reg.(*registry)

Check failure on line 631 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / lint 

SA4006: this value of `r` is never used (staticcheck)
assert.NotEmpty(t, h)
globalHisto = h[""]
granularHisto = h["my cool route"]
Expand Down Expand Up @@ -660,10 +658,10 @@ func TestRecordSpanErrorStatus(t *testing.T) {
)
span.End(trace.WithTimestamp(now.Add(1 * time.Second)))

// This affects global state (ApmMetrics below)
RecordSpan(span.(sdktrace.ReadOnlySpan), false)
reg := NewLegacyRegistry().(*registry)
reg.RecordSpan(span.(sdktrace.ReadOnlySpan), false)

m := ApmMetrics.CopyAndReset(60)
m := reg.apmMetrics.CopyAndReset(60)
assert.NotEmpty(t, m.m)
v := m.m["ResponseTime&true&http.method:GET&http.status_code:500&sw.is_error:true&sw.transaction:my cool route&"]
assert.NotNil(t, v, fmt.Sprintf("Map: %v", m.m))
Expand All @@ -679,8 +677,8 @@ func TestRecordSpanErrorStatus(t *testing.T) {
v.Tags)
assert.Equal(t, responseTime, v.Name)

h := apmHistograms.histograms
resetHistograms()
h := reg.apmHistograms.histograms
reg = NewLegacyRegistry().(*registry)
assert.NotEmpty(t, h)
globalHisto := h[""]
granularHisto := h["my cool route"]
Expand All @@ -693,9 +691,9 @@ func TestRecordSpanErrorStatus(t *testing.T) {
assert.Equal(t, int64(1), granularHisto.hist.TotalCount())

// Now test for AO
RecordSpan(span.(sdktrace.ReadOnlySpan), true)
reg.RecordSpan(span.(sdktrace.ReadOnlySpan), true)

m = ApmMetrics.CopyAndReset(60)
m = reg.apmMetrics.CopyAndReset(60)
assert.NotEmpty(t, m.m)
k1 := "TransactionResponseTime&true&HttpMethod:GET&TransactionName:my cool route&"
k2 := "TransactionResponseTime&true&HttpStatus:500&TransactionName:my cool route&"
Expand All @@ -719,8 +717,8 @@ func TestRecordSpanErrorStatus(t *testing.T) {
map[string]string{"TransactionName": "my cool route"},
m.m[k3].Tags,
)
h = apmHistograms.histograms
resetHistograms()
h = reg.apmHistograms.histograms
reg = NewLegacyRegistry().(*registry)

Check failure on line 721 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / lint 

SA4006: this value of `reg` is never used (staticcheck)
assert.NotEmpty(t, h)
globalHisto = h[""]
granularHisto = h["my cool route"]
Expand Down Expand Up @@ -764,16 +762,16 @@ func TestRecordSpanOverflow(t *testing.T) {
)
span2.End(trace.WithTimestamp(now.Add(1 * time.Second)))

reg := NewLegacyRegistry().(*registry)
// The cap only takes affect after the following reset
ApmMetrics.SetCap(1)
ApmMetrics.CopyAndReset(60)
assert.Equal(t, int32(1), ApmMetrics.Cap())
reg.SetApmMetricsCap(1)
reg.apmMetrics.CopyAndReset(60)
assert.Equal(t, int32(1), reg.ApmMetricsCap())

// This affects global state (ApmMetrics below)
RecordSpan(span.(sdktrace.ReadOnlySpan), false)
RecordSpan(span2.(sdktrace.ReadOnlySpan), false)
reg.RecordSpan(span.(sdktrace.ReadOnlySpan), false)
reg.RecordSpan(span2.(sdktrace.ReadOnlySpan), false)

m := ApmMetrics.CopyAndReset(60)
m := reg.apmMetrics.CopyAndReset(60)
// We expect to have a record for `my cool route` and one for `other`
assert.Equal(t, 2, len(m.m))
v := m.m["ResponseTime&true&http.method:GET&http.status_code:200&sw.is_error:false&sw.transaction:my cool route&"]
Expand Down Expand Up @@ -804,8 +802,8 @@ func TestRecordSpanOverflow(t *testing.T) {
v.Tags)
assert.Equal(t, responseTime, v.Name)

h := apmHistograms.histograms
resetHistograms()
h := reg.apmHistograms.histograms
reg = NewLegacyRegistry().(*registry)

Check failure on line 806 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / lint 

SA4006: this value of `reg` is never used (staticcheck)
assert.NotEmpty(t, h)
globalHisto := h[""]
granularHisto := h["my cool route"]
Expand Down Expand Up @@ -851,15 +849,15 @@ func TestRecordSpanOverflowAppoptics(t *testing.T) {

// The cap only takes affect after the following reset
// Appoptics-style will generate 3 metrics, so we'll set the cap to that here
ApmMetrics.SetCap(3)
ApmMetrics.CopyAndReset(60)
assert.Equal(t, int32(3), ApmMetrics.Cap())
reg := NewLegacyRegistry().(*registry)
reg.SetApmMetricsCap(3)
reg.apmMetrics.CopyAndReset(60)
assert.Equal(t, int32(3), reg.ApmMetricsCap())

// This affects global state (ApmMetrics below)
RecordSpan(span.(sdktrace.ReadOnlySpan), true)
RecordSpan(span2.(sdktrace.ReadOnlySpan), true)
reg.RecordSpan(span.(sdktrace.ReadOnlySpan), true)
reg.RecordSpan(span2.(sdktrace.ReadOnlySpan), true)

m := ApmMetrics.CopyAndReset(60)
m := reg.apmMetrics.CopyAndReset(60)
// We expect to have 3 records for `my cool route` and 3 for `other`
assert.Equal(t, 6, len(m.m))

Expand All @@ -878,8 +876,8 @@ func TestRecordSpanOverflowAppoptics(t *testing.T) {
assert.Equal(t, 1, v.Count)
}

h := apmHistograms.histograms
resetHistograms()
h := reg.apmHistograms.histograms
reg = NewLegacyRegistry().(*registry)

Check failure on line 880 in internal/metrics/metrics_test.go

View workflow job for this annotation

GitHub Actions / lint 

SA4006: this value of `reg` is never used (staticcheck)
assert.NotEmpty(t, h)
globalHisto := h[""]
granularHisto := h["my cool route"]
Expand Down

0 comments on commit 64539f5

Please sign in to comment.