Added better logging/messaging around collector package updating

opamp/observiq/observiq_client.go

@@ -227,6 +227,8 @@ func (c *Client) onConnectHandler() {
 	// If the current version matches the server offered version, this implies a good install and so we should set the PackageStatuses and
 	// send it to the OpAMP Server. If the version does not match, just change the PackageStatues JSON so that the Updater can start rollback.
 	if lastMainPackageStatus.ServerOfferedVersion == version.Version() {
+		c.logger.Info(
+			fmt.Sprintf("Package %s update was successful", packagestate.CollectorPackageName))
 		lastMainPackageStatus.Status = protobufs.PackageStatus_Installed
 		lastMainPackageStatus.AgentHasVersion = version.Version()
 		lastMainPackageStatus.AgentHasHash = lastMainPackageStatus.ServerOfferedHash
@@ -240,7 +242,15 @@ func (c *Client) onConnectHandler() {
 			c.logger.Error("OpAMP client failed to set package statuses", zap.Error(err))
 		}
 	} else {
+		c.logger.Error(
+			fmt.Sprintf(
+				"Package %s update failed because of collector version mismatch: expected %s, actual %s",
+				packagestate.CollectorPackageName, lastMainPackageStatus.ServerOfferedVersion, version.Version()))
+
 		lastMainPackageStatus.Status = protobufs.PackageStatus_InstallFailed
+		lastMainPackageStatus.ErrorMessage =
+			fmt.Sprintf("Failed because of collector version mismatch: expected %s, actual %s",
+				lastMainPackageStatus.ServerOfferedVersion, version.Version())
 
 		if err := c.packagesStateProvider.SetLastReportedStatuses(lastPackageStatuses); err != nil {
 			c.logger.Error("Failed to set last reported package statuses", zap.Error(err))
@@ -319,6 +329,10 @@ func (c *Client) onPackagesAvailableHandler(packagesAvailable *protobufs.Package
 	// Don't respond to PackagesAvailable messages while currently installing. We use this in memory data rather than the
 	// PackageStatuses persistant data in order to ensure that we don't get stuck in a stuck state
 	if c.safeGetUpdatingPackage() {
+		c.logger.Warn(
+			fmt.Sprintf(
+				"Not starting new package update for AllPackagesHash %s as already installing new packages",
+				packagesAvailable.GetAllPackagesHash()))
 		curPackageStatuses.ErrorMessage = "Already installing new packages"
 		if err := c.opampClient.SetPackageStatuses(curPackageStatuses); err != nil {
 			c.logger.Error("OpAMP client failed to set package statuses", zap.Error(err))
@@ -395,6 +409,8 @@ func (c *Client) createPackageMaps(
 			}
 
 			if version.Version() == availPkg.GetVersion() {
+				c.logger.Info(
+					fmt.Sprintf("Package %s update ignored because no new version offered", packagestate.CollectorPackageName))
 				if agentHash == nil {
 					pkgStatusMap[name].AgentHasHash = availPkg.GetHash()
 				}
@@ -406,12 +422,16 @@ func (c *Client) createPackageMaps(
 					pkgStatusMap[name].Status = protobufs.PackageStatus_Installing
 					pkgFileMap[name] = availPkg.GetFile()
 				} else {
+					c.logger.Error(
+						fmt.Sprintf("Package %s update failed to determine valid downloadable file", packagestate.CollectorPackageName))
 					pkgStatusMap[name].Status = protobufs.PackageStatus_InstallFailed
 					pkgStatusMap[name].ErrorMessage = fmt.Sprintf("Package %s does not have a valid downloadable file", name)
 				}
 			}
 		// If it's not an expected package, return a failed status
 		default:
+			c.logger.Error(
+				fmt.Sprintf("Package %s update failed because it is not supported", packagestate.CollectorPackageName))
 			pkgStatusMap[name] = &protobufs.PackageStatus{
 				Name:                 name,
 				ServerOfferedVersion: availPkg.GetVersion(),
@@ -428,17 +448,22 @@ func (c *Client) createPackageMaps(
 // installPackageFromFile tries to download and extract the given tarball and then start up the new Updater binary that was
 // inside of it
 func (c *Client) installPackageFromFile(file *protobufs.DownloadableFile, curPackageStatuses *protobufs.PackageStatuses) {
+	c.logger.Info(fmt.Sprintf("Package update started for AllPackagesHash %s", curPackageStatuses.GetServerProvidedAllPackagesHash()))
 	// There should be no reason for us to exit this function unless we detected a problem with the Updater's installation
 	defer c.safeSetUpdatingPackage(false)
 
 	if fileManagerErr := c.downloadableFileManager.FetchAndExtractArchive(file); fileManagerErr != nil {
+		c.logger.Error(
+			fmt.Sprintf(
+				"Package %s update failed to download and verify downloadable file: %s",
+				packagestate.CollectorPackageName, fileManagerErr.Error()))
 		// Remove the update artifacts that may exist, depending on where FetchAndExtractArchive failed.
 		c.downloadableFileManager.CleanupArtifacts()
 
 		// Change existing status to show that install failed and get ready to send
 		curPackageStatuses.Packages[packagestate.CollectorPackageName].Status = protobufs.PackageStatus_InstallFailed
 		curPackageStatuses.Packages[packagestate.CollectorPackageName].ErrorMessage =
-			fmt.Sprintf("Failed to download and verify package %s's downloadable file: %s", packagestate.CollectorPackageName, fileManagerErr.Error())
+			fmt.Sprintf("Failed to download and verify downloadable file: %s", fileManagerErr.Error())
 
 		if err := c.packagesStateProvider.SetLastReportedStatuses(curPackageStatuses); err != nil {
 			c.logger.Error("Failed to save last reported package statuses", zap.Error(err))
@@ -452,6 +477,10 @@ func (c *Client) installPackageFromFile(file *protobufs.DownloadableFile, curPac
 	}
 
 	if monitorErr := c.updaterManager.StartAndMonitorUpdater(); monitorErr != nil {
+		c.logger.Error(
+			fmt.Sprintf(
+				"Package %s update failed because of issue with latest Updater: %s",
+				packagestate.CollectorPackageName, monitorErr))
 		// Remove the update artifacts
 		c.downloadableFileManager.CleanupArtifacts()
 
@@ -464,7 +493,7 @@ func (c *Client) installPackageFromFile(file *protobufs.DownloadableFile, curPac
 		// Change existing status to show that install failed and get ready to send
 		newPackageStatuses.Packages[packagestate.CollectorPackageName].Status = protobufs.PackageStatus_InstallFailed
 		if newPackageStatuses.Packages[packagestate.CollectorPackageName].ErrorMessage == "" {
-			newPackageStatuses.Packages[packagestate.CollectorPackageName].ErrorMessage = fmt.Sprintf("Failed to run the latest Updater: %s", monitorErr.Error())
+			newPackageStatuses.Packages[packagestate.CollectorPackageName].ErrorMessage = fmt.Sprintf("Failed to run the latest Updater: %s", monitorErr)
 		}
 
 		if err := c.packagesStateProvider.SetLastReportedStatuses(newPackageStatuses); err != nil {
@@ -491,6 +520,8 @@ func (c *Client) attemptFailedInstall(errMsg string) {
 		return
 	}
 
+	c.logger.Error(fmt.Sprintf("Package %s update failed: %s", packagestate.CollectorPackageName, errMsg))
+
 	lastMainPackageStatus := lastPackageStatuses.Packages[packagestate.CollectorPackageName]
 	lastMainPackageStatus.Status = protobufs.PackageStatus_InstallFailed
 	lastMainPackageStatus.ErrorMessage = errMsg

opamp/observiq/observiq_client_test.go

@@ -556,7 +556,7 @@ func TestClient_onConnectHandler(t *testing.T) {
 					assert.Equal(t, hash, status.Packages[packagestate.CollectorPackageName].AgentHasHash)
 					assert.Equal(t, newVersion, status.Packages[packagestate.CollectorPackageName].ServerOfferedVersion)
 					assert.Equal(t, newHash, status.Packages[packagestate.CollectorPackageName].ServerOfferedHash)
-					assert.Equal(t, "", status.Packages[packagestate.CollectorPackageName].ErrorMessage)
+					assert.Equal(t, "Failed because of collector version mismatch: expected 99.99.99, actual latest", status.Packages[packagestate.CollectorPackageName].ErrorMessage)
 					assert.Equal(t, protobufs.PackageStatus_InstallFailed, status.Packages[packagestate.CollectorPackageName].Status)
 				})
 
@@ -1379,7 +1379,7 @@ func TestClient_onPackagesAvailableHandler(t *testing.T) {
 					assert.Equal(t, 1, len(status.Packages))
 					assert.Equal(t, packagesAvailableNew.Packages[collectorPackageName].Version, status.Packages[collectorPackageName].ServerOfferedVersion)
 					assert.Equal(t, packagesAvailableNew.Packages[collectorPackageName].Hash, status.Packages[collectorPackageName].ServerOfferedHash)
-					assert.Equal(t, "Failed to download and verify package observiq-otel-collector's downloadable file: oops", status.Packages[collectorPackageName].ErrorMessage)
+					assert.Equal(t, "Failed to download and verify downloadable file: oops", status.Packages[collectorPackageName].ErrorMessage)
 					assert.Equal(t, protobufs.PackageStatus_InstallFailed, status.Packages[collectorPackageName].Status)
 					assert.Equal(t, collectorPackageName, status.Packages[collectorPackageName].Name)
 					assert.Equal(t, packageStatuses.Packages[collectorPackageName].AgentHasHash, status.Packages[collectorPackageName].AgentHasHash)