fix(daemon): aggregate packages throughout harvest

daemon/internal/newrelic/app.go

@@ -340,78 +340,3 @@ func (app *App) Inactive(threshold time.Duration) bool {
 	}
 	return time.Since(app.LastActivity) > threshold
 }
-
-// filter seen php packages data to avoid sending duplicates
-//
-// the `App` structure contains a map of PHP Packages the reporting
-// application has encountered.
-//
-// the map of packages should persist for the duration of the
-// current connection
-//
-// takes the `PhpPackages.data` byte array as input and unmarshals
-// into an anonymous interface array
-//
-// the JSON format received from the agent is:
-//
-//	[["package_name","version",{}],...]
-//
-// for each entry, assign the package name and version to the `PhpPackagesKey`
-// struct and use the key to verify data does not exist in the map. If the
-// key does not exist, add it to the map and the array of 'new' packages.
-//
-// convert the array of 'new' packages into a byte array representing
-// the expected data that should match input, minus the duplicates.
-func (app *App) filterPhpPackages(data []byte) []byte {
-	if data == nil {
-		return nil
-	}
-
-	var pkgKey PhpPackagesKey
-	var newPkgs []PhpPackagesKey
-	var x []interface{}
-
-	err := json.Unmarshal(data, &x)
-	if nil != err {
-		log.Errorf("failed to unmarshal php package json: %s", err)
-		return nil
-	}
-
-	for _, pkgJson := range x {
-		pkg, _ := pkgJson.([]interface{})
-		if len(pkg) != 3 {
-			log.Errorf("invalid php package json structure: %+v", pkg)
-			return nil
-		}
-		name, ok := pkg[0].(string)
-		version, ok := pkg[1].(string)
-		pkgKey = PhpPackagesKey{name, version}
-		_, ok = app.PhpPackages[pkgKey]
-		if !ok {
-			app.PhpPackages[pkgKey] = struct{}{}
-			newPkgs = append(newPkgs, pkgKey)
-		}
-	}
-
-	if newPkgs == nil {
-		return nil
-	}
-
-	buf := &bytes.Buffer{}
-	buf.WriteString(`[`)
-	for _, pkg := range newPkgs {
-		buf.WriteString(`["`)
-		buf.WriteString(pkg.Name)
-		buf.WriteString(`","`)
-		buf.WriteString(pkg.Version)
-		buf.WriteString(`",{}],`)
-	}
-
-	resJson := buf.Bytes()
-
-	// swap last ',' character with ']'
-	resJson = resJson[:len(resJson)-1]
-	resJson = append(resJson, ']')
-
-	return resJson
-}

daemon/internal/newrelic/app_test.go

@@ -613,54 +613,3 @@ func TestMaxPayloadSizeInBytesFromConnectReply(t *testing.T) {
 		t.Errorf("parseConnectReply(something), got [%v], expected [%v]", c.MaxPayloadSizeInBytes, expectedMaxPayloadSizeInBytes)
 	}
 }
-
-func TestFilterPhpPackages(t *testing.T) {
-	app := App{
-		PhpPackages: make(map[PhpPackagesKey]struct{}),
-	}
-	var nilData []byte = nil
-	emptyData := []byte(`[[{}]]`)
-	validData := []byte(`[["drupal","6.0",{}]]`)
-	moreValidData := []byte(`[["wordpress","7.0",{}],["symfony","5.1",{}]]`)
-	duplicateData := []byte(`[["drupal","6.0",{}]]`)
-	versionData := []byte(`[["drupal","9.0",{}]]`)
-	invalidData := []byte(`[[["1","2","3"],["4","5"]{}]]`)
-
-	filteredData := app.filterPhpPackages(nilData)
-	if filteredData != nil {
-		t.Errorf("expected 'nil' result on 'nil' input, got [%v]", filteredData)
-	}
-
-	filteredData = app.filterPhpPackages(emptyData)
-	if filteredData != nil {
-		t.Errorf("expected 'nil' result on empty data input, got [%v]", filteredData)
-	}
-
-	expect := []byte(`[["drupal","6.0",{}]]`)
-	filteredData = app.filterPhpPackages(validData)
-	if string(filteredData) != string(expect) {
-		t.Errorf("expected [%v], got [%v]", string(expect), string(filteredData))
-	}
-
-	expect = []byte(`[["wordpress","7.0",{}],["symfony","5.1",{}]]`)
-	filteredData = app.filterPhpPackages(moreValidData)
-	if string(filteredData) != string(expect) {
-		t.Errorf("expected [%v], got [%v]", string(expect), string(filteredData))
-	}
-
-	filteredData = app.filterPhpPackages(duplicateData)
-	if filteredData != nil {
-		t.Errorf("expected 'nil', got [%v]", filteredData)
-	}
-
-	expect = []byte(`[["drupal","9.0",{}]]`)
-	filteredData = app.filterPhpPackages(versionData)
-	if string(filteredData) != string(expect) {
-		t.Errorf("expected [%v], got [%v]", string(expect), string(filteredData))
-	}
-
-	filteredData = app.filterPhpPackages(invalidData)
-	if filteredData != nil {
-		t.Errorf("expected 'nil', go [%v]", filteredData)
-	}
-}

daemon/internal/newrelic/integration/test.go

@@ -733,6 +733,9 @@ func (t *Test) comparePhpPackages(harvest *newrelic.Harvest) {
 		}
 	}
 
+	// we don't currently test multiple requests that could result in needing to track package history
+	// or filter packages. An empty map here will allow all packages to be reported unfiltered.
+	harvest.PhpPackages.Filter(make(map[newrelic.PhpPackagesKey]struct{}))
 	audit, err := newrelic.IntegrationData(harvest.PhpPackages, newrelic.AgentRunID("?? agent run id"), time.Now())
 	if nil != err {
 		t.Fatal(err)
@@ -763,25 +766,15 @@ func (t *Test) comparePhpPackages(harvest *newrelic.Harvest) {
 				len(expectedPackages), len(actualPackages), expectedPackages, actualPackages))
 			return
 		}
-		for i, _ := range expectedPackages {
+		for i := range expectedPackages {
 			var matchingIdx int = -1
 			for j, pkg := range actualPackages {
-				//fmt.Printf("Comparing %s to %s\n", pkg.Name, expectedPackages[i].Name)
 				if pkg.Name == expectedPackages[i].Name {
-					//fmt.Printf("Match - index = %d\n", j)
 					matchingIdx = j
 					break
 				}
 			}
 
-			//fmt.Printf("MatchingIdx: %d\n", matchingIdx)
-			//fmt.Printf("expectedPatckages[%d]: %+v\n", i, expectedPackages[i])
-			// if -1 != matchingIdx {
-			// 	fmt.Printf("actualPackages[%d]: %+v\n", matchingIdx, actualPackages[matchingIdx])
-			// } else {
-			// 	fmt.Printf("no match in actualPackages!\n")
-			// }
-
 			if -1 != matchingIdx {
 				testPackageNameOnly := false
 				if nil != expectedPkgsCollection.config.packageNameOnly {
@@ -823,7 +816,7 @@ func (t *Test) comparePhpPackages(harvest *newrelic.Harvest) {
 		}
 
 		// create notes for all packages in the actual list not in the expected list
-		for ii, _ := range actualPackages {
+		for ii := range actualPackages {
 			var found bool = false
 			for _, pkg := range expectedPackages {
 				if pkg.Name == actualPackages[ii].Name {

daemon/internal/newrelic/php_packages.go

@@ -7,78 +7,146 @@ package newrelic
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"time"
-
-	"github.com/newrelic/newrelic-php-agent/daemon/internal/newrelic/log"
 )
 
 type PhpPackagesKey struct {
 	Name    string
 	Version string
 }
 
-// phpPackages represents all detected packages reported by an agent.
+// PhpPackages represents all detected packages reported by an agent
+// for a harvest cycle, as well as the filtered list of packages not
+// yet seen by the daemon for the lifecycle of the current daemon
+// process to be reported to the backend.
 type PhpPackages struct {
-	numSeen int
-	data    JSONString
+	numSeen      int
+	data         map[PhpPackagesKey]struct{}
+	filteredPkgs []PhpPackagesKey
 }
 
-// NumSeen returns the total number PHP packages payloads stored.
-// Should always be 0 or 1.  The agent reports all the PHP
-// packages as a single JSON string.
+// NumSaved returns whether PHP packages payloads are stored by
+// the daemon for the current harvest. Should always be 0 or 1.
+// The agent reports all the PHP packages as a single JSON string.
 func (packages *PhpPackages) NumSaved() float64 {
 	return float64(packages.numSeen)
 }
 
-// newPhpPackages returns a new PhpPackages struct.
+// NewPhpPackages returns a new PhpPackages struct.
 func NewPhpPackages() *PhpPackages {
 	p := &PhpPackages{
-		numSeen: 0,
-		data:    nil,
+		numSeen:      0,
+		data:         make(map[PhpPackagesKey]struct{}),
+		filteredPkgs: nil,
 	}
 
 	return p
 }
 
-// SetPhpPackages sets the observed package list.
-func (packages *PhpPackages) SetPhpPackages(data []byte) error {
+// Filter seen php packages data to avoid sending duplicates
+//
+// the `App` structure contains a map of PHP Packages the reporting
+// application has encountered.
+//
+// the map of packages should persist for the duration of the
+// current connection
+//
+// the JSON format received from the agent is:
+//
+//	[["package_name","version",{}],...]
+//
+// for each entry, assign the package name and version to the `PhpPackagesKey`
+// struct and use the key to verify data does not exist in the map. If the
+// key does not exist, add it to the map and the slice of filteredPkgs to be
+// sent in the current harvest.
+func (packages *PhpPackages) Filter(pkgHistory map[PhpPackagesKey]struct{}) {
+	if packages == nil || len(packages.data) == 0 {
+		return
+	}
 
-	if nil == packages {
-		return fmt.Errorf("packages is nil!")
+	for pkgKey := range packages.data {
+		_, ok := pkgHistory[pkgKey]
+		if !ok {
+			pkgHistory[pkgKey] = struct{}{}
+			packages.filteredPkgs = append(packages.filteredPkgs, pkgKey)
+		}
 	}
-	if nil != packages.data {
-		log.Debugf("SetPhpPackages - data field was not nil |^%s| - overwriting data", packages.data)
+}
+
+// AddPhpPackagesFromData observes the PHP packages info from the agent.
+func (packages *PhpPackages) AddPhpPackagesFromData(data []byte) error {
+	if packages == nil {
+		return fmt.Errorf("packages is nil")
 	}
-	if nil == data {
-		return fmt.Errorf("data is nil!")
+	if len(data) == 0 {
+		return fmt.Errorf("data is nil")
 	}
+
+	var x []any
+
+	err := json.Unmarshal(data, &x)
+	if err != nil {
+		return fmt.Errorf("failed to unmarshal php package json: %s", err.Error())
+	}
+
+	for _, pkgJSON := range x {
+		pkg, _ := pkgJSON.([]any)
+		if len(pkg) != 3 {
+			return fmt.Errorf("invalid php package json structure: %+v", pkg)
+		}
+
+		name, ok := pkg[0].(string)
+		if !ok || len(name) == 0 {
+			return fmt.Errorf("unable to parse package name")
+		}
+
+		version, ok := pkg[1].(string)
+		if !ok || len(version) == 0 {
+			return fmt.Errorf("unable to parse package version")
+		}
+
+		pkgKey := PhpPackagesKey{name, version}
+		_, ok = packages.data[pkgKey]
+		if !ok {
+			packages.data[pkgKey] = struct{}{}
+		}
+	}
+
 	packages.numSeen = 1
-	packages.data = data
 
 	return nil
 }
 
-// AddPhpPackagesFromData observes the PHP packages info from the agent.
-func (packages *PhpPackages) AddPhpPackagesFromData(data []byte) error {
-	return packages.SetPhpPackages(data)
-}
-
 // CollectorJSON marshals events to JSON according to the schema expected
 // by the collector.
 func (packages *PhpPackages) CollectorJSON(id AgentRunID) ([]byte, error) {
 	if packages.Empty() {
 		return []byte(`["Jars",[]]`), nil
 	}
 
-	buf := &bytes.Buffer{}
+	var buf bytes.Buffer
 
 	estimate := 512
 	buf.Grow(estimate)
 	buf.WriteByte('[')
-	buf.WriteString("\"Jars\",")
-	if 0 < packages.numSeen {
-		buf.Write(packages.data)
+	buf.WriteString(`"Jars",`)
+	if len(packages.filteredPkgs) > 0 {
+		buf.WriteByte('[')
+		for _, pkg := range packages.filteredPkgs {
+			buf.WriteString(`["`)
+			buf.WriteString(pkg.Name)
+			buf.WriteString(`","`)
+			buf.WriteString(pkg.Version)
+			buf.WriteString(`",{}],`)
+		}
+
+		// swap last ',' character with ']'
+		buf.Truncate(buf.Len() - 1)
+		buf.WriteByte(']')
+	} else {
+		buf.WriteString("[]")
 	}
 	buf.WriteByte(']')
 
@@ -94,7 +162,7 @@ func (packages *PhpPackages) FailedHarvest(newHarvest *Harvest) {
 
 // Empty returns true if the collection is empty.
 func (packages *PhpPackages) Empty() bool {
-	return nil == packages || nil == packages.data || 0 == packages.numSeen
+	return nil == packages || len(packages.data) == 0 || 0 == packages.numSeen
 }
 
 // Data marshals the collection to JSON according to the schema expected