turns out that the synchronization is handled already

newrelic · May 31, 2022 · 2857de1 · 2857de1
1 parent 2a78e04
commit 2857de1
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 166 deletions.
diff --git a/v3/newrelic/log_events.go b/v3/newrelic/log_events.go
@@ -6,7 +6,6 @@ package newrelic
 import (
 	"bytes"
 	"container/heap"
-	"sync"
 	"time"
 
 	"github.com/newrelic/go-agent/v3/internal/jsonx"
@@ -24,34 +23,26 @@ type logEvents struct {
 	numSeen        int
 	failedHarvests int
 	severityCount  map[string]int
-	rwMutex        sync.RWMutex
 	commonAttributes
 	config loggingConfig
 	logs   logEventHeap
 }
 
 // NumSeen returns the number of events seen
-func (events *logEvents) NumSeen() int {
-	events.rwMutex.RLock()
-	defer events.rwMutex.RUnlock()
-	return events.numSeen
+func (events *logEvents) NumSeen() float64 {
+	return float64(events.numSeen)
 }
 
 // NumSaved returns the number of events that will be harvested for this cycle
-func (events *logEvents) NumSaved() int {
-	events.rwMutex.RLock()
-	defer events.rwMutex.RUnlock()
-	return len(events.logs)
+func (events *logEvents) NumSaved() float64 {
+	return float64(len(events.logs))
 }
 
 // Adds logging metrics to a harvest metric table if appropriate
 func (events *logEvents) RecordLoggingMetrics(metrics *metricTable) {
-	events.rwMutex.RLock()
-	defer events.rwMutex.RUnlock()
-
 	// This is done to avoid accessing locks 3 times instead of once
-	seen := float64(events.numSeen)
-	saved := float64(len(events.logs))
+	seen := events.NumSeen()
+	saved := events.NumSaved()
 
 	if events.config.collectMetrics && metrics != nil {
 		metrics.addCount(logsSeen, seen, forced)
@@ -91,8 +82,6 @@ func (events *logEvents) capacity() int {
 }
 
 func (events *logEvents) Add(e *logEvent) {
-	events.rwMutex.Lock()
-	defer events.rwMutex.Unlock()
 	// always collect this but do not report logging metrics when disabled
 	events.numSeen++
 	events.severityCount[e.severity]++
@@ -140,13 +129,10 @@ func (events *logEvents) Merge(other *logEvents) {
 		events.Add(&e)
 	}
 
-	events.numSeen = allSeen
+	events.numSeen = int(allSeen)
 }
 
 func (events *logEvents) CollectorJSON(agentRunID string) ([]byte, error) {
-	events.rwMutex.RLock()
-	defer events.rwMutex.RUnlock()
-
 	if 0 == len(events.logs) {
 		return nil, nil
 	}

diff --git a/v3/newrelic/log_events_test.go b/v3/newrelic/log_events_test.go
@@ -5,7 +5,6 @@ package newrelic
 
 import (
 	"fmt"
-	"sync"
 	"testing"
 
 	"github.com/newrelic/go-agent/v3/internal"
@@ -386,150 +385,6 @@ func TestLogEventCollectionDisabled(t *testing.T) {
 	}
 }
 
-func TestAsyncAddLogEvent(t *testing.T) {
-	numThreads := 8
-	capacity := numThreads - 1
-
-	events := newLogEvents(testCommonAttributes, loggingConfigEnabled(capacity))
-	group := new(sync.WaitGroup)
-	group.Add(numThreads)
-
-	// Add a bunch of log events aynchronously
-	for n := 0; n < numThreads/2; n++ {
-		p := priority(float32(n) / 10.0)
-		event := &logEvent{
-			priority:  p,
-			timestamp: 123456,
-			severity:  "INFO",
-			message:   fmt.Sprintf("info message %.2f", p),
-		}
-		go func(event *logEvent) {
-			events.Add(event)
-			group.Done()
-		}(event)
-	}
-
-	for n := 0; n < numThreads/2; n++ {
-		p := priority(float32(n+numThreads/2) / 10.0)
-		event := &logEvent{
-			priority:  p,
-			timestamp: 123456,
-			severity:  "WARN",
-			message:   fmt.Sprintf("warn message %.2f", p),
-		}
-		go func(event *logEvent) {
-			events.Add(event)
-			group.Done()
-		}(event)
-	}
-
-	group.Wait()
-
-	expectMap := map[string]int{
-		"INFO": numThreads / 2,
-		"WARN": numThreads / 2,
-	}
-
-	metricErrors := events.assertMetrics(8, capacity, expectMap)
-	if metricErrors != nil {
-		t.Error(metricErrors)
-	}
-
-	// Test Heap Data
-	// Assumes that heap implementation is correct when executed synchronously
-	expectEvents := newLogEvents(testCommonAttributes, loggingConfigEnabled(capacity))
-	for n := 0; n < numThreads/2; n++ {
-		p := priority(float32(n) / 10.0)
-		event := &logEvent{
-			priority:  p,
-			timestamp: 123456,
-			severity:  "INFO",
-			message:   fmt.Sprintf("info message %.2f", p),
-		}
-		expectEvents.Add(event)
-	}
-
-	for n := 0; n < numThreads/2; n++ {
-		p := priority(float32(n+numThreads/2) / 10.0)
-		event := &logEvent{
-			priority:  p,
-			timestamp: 123456,
-			severity:  "WARN",
-			message:   fmt.Sprintf("warn message %.2f", p),
-		}
-		expectEvents.Add(event)
-	}
-
-	heapError := events.assertHeapContains(expectEvents)
-	if heapError != nil {
-		t.Error(heapError)
-	}
-}
-
-// verifies that each log events heap contains the same elements
-// heaps must be composed of unique messages
-func (events *logEvents) assertHeapContains(expect *logEvents) error {
-	expectLogs := make(map[string]bool, len(expect.logs))
-
-	for _, event := range expect.logs {
-		expectLogs[event.message] = false
-	}
-
-	for _, event := range events.logs {
-		expectLogs[event.message] = true
-	}
-
-	missing := []string{}
-	for msg, contains := range expectLogs {
-		if !contains {
-			missing = append(missing, msg)
-		}
-	}
-
-	if len(missing) != 0 {
-		return fmt.Errorf("expected logs were missing from the event heap: %v", missing)
-	}
-
-	return nil
-}
-
-func (events *logEvents) assertMetrics(expectSeen, expectSaved int, expectSeverity map[string]int) error {
-	err := assertInt(expectSeen, int(events.NumSeen()))
-	if err != nil {
-		return fmt.Errorf("incorrect number of events seen: %v", err)
-	}
-
-	err = assertInt(expectSaved, int(events.NumSaved()))
-	if err != nil {
-		return fmt.Errorf("incorrect number of events saved: %v", err)
-	}
-
-	if len(expectSeverity) != len(events.severityCount) {
-		return fmt.Errorf("incorrect number of severities seen: expect %d, actual %d", len(expectSeverity), len(events.severityCount))
-	}
-
-	for k, v := range expectSeverity {
-		val, ok := events.severityCount[k]
-		if !ok {
-			return fmt.Errorf("expected severity %s is missing from actual severity count", k)
-		}
-
-		err := assertInt(v, val)
-		if err != nil {
-			return fmt.Errorf("incorrect severity count for %s: expect %d, actual %d", k, v, val)
-		}
-	}
-
-	return nil
-}
-
-func assertInt(expect int, actual int) error {
-	if expect != actual {
-		return fmt.Errorf("expected %d, actual %d", expect, actual)
-	}
-	return nil
-}
-
 func BenchmarkLogEventsAdd(b *testing.B) {
 	events := newLogEvents(testCommonAttributes, loggingConfigEnabled(internal.MaxLogEvents))
 	event := &logEvent{