Repository: open-telemetry/opamp-go
Pull request #63 (2022-04-13 18:27:56)

Ensure client includes AgentDescription on Reconnect
Resolves: https://github.com/open-telemetry/opamp-go/issues/55

This PR ensures that the client includes AgentDescription every time it connects to the server, not just after `client.Start`.

Current behavior:
- Clients connection to server is interrupted.
- Client reconnects.
- Client does not prepare a "first status message" with an `AgentDescription`.

Expected behavior:
- Clients connection to server is interrupted.
- Client reconnects.
- Client prepares and sends a status including AgentDescription.



Patch:
diff --git a/client/clientimpl.go b/client/clientimpl.go
index f18a665..38d91ff 100644
--- a/client/clientimpl.go
+++ b/client/clientimpl.go
@@ -106,22 +106,6 @@ func (w *client) Start(settings StartSettings) error {
 		w.requestHeader["Authorization"] = []string{w.settings.AuthorizationHeader}
 	}
 
-	// Prepare the first status report.
-	w.sender.UpdateNextStatus(
-		func(statusReport *protobufs.StatusReport) {
-			statusReport.AgentDescription = w.settings.AgentDescription
-		},
-	)
-
-	w.sender.UpdateNextMessage(
-		func(msg *protobufs.AgentToServer) {
-			if msg.AddonStatuses == nil {
-				msg.AddonStatuses = &protobufs.AgentAddonStatuses{}
-			}
-			msg.AddonStatuses.ServerProvidedAllAddonsHash = w.settings.LastServerProvidedAllAddonsHash
-		},
-	)
-
 	w.startConnectAndRun()
 
 	w.isStarted = true
@@ -168,6 +152,9 @@ func (w *client) SetAgentDescription(descr *protobufs.AgentDescription) error {
 		return errAgentDescriptionMissing
 	}
 
+	// store the agent description to send on reconnect
+	w.settings.AgentDescription = descr
+
 	w.sender.UpdateNextStatus(func(statusReport *protobufs.StatusReport) {
 		statusReport.AgentDescription = descr
 	})
@@ -325,6 +312,22 @@ func (w *client) runOneCycle(ctx context.Context) {
 	// Create a cancellable context for background processors.
 	procCtx, procCancel := context.WithCancel(ctx)
 
+	// Prepare the first status report.
+	w.sender.UpdateNextStatus(
+		func(statusReport *protobufs.StatusReport) {
+			statusReport.AgentDescription = w.settings.AgentDescription
+		},
+	)
+
+	w.sender.UpdateNextMessage(
+		func(msg *protobufs.AgentToServer) {
+			if msg.AddonStatuses == nil {
+				msg.AddonStatuses = &protobufs.AgentAddonStatuses{}
+			}
+			msg.AddonStatuses.ServerProvidedAllAddonsHash = w.settings.LastServerProvidedAllAddonsHash
+		},
+	)
+
 	// Connected successfully. Start the sender. This will also send the first
 	// status report.
 	if err := w.sender.Start(procCtx, w.settings.InstanceUid, w.conn); err != nil {


Test patch:
diff --git a/client/clientimpl_test.go b/client/clientimpl_test.go
index 2360cd0..3f93b42 100644
--- a/client/clientimpl_test.go
+++ b/client/clientimpl_test.go
@@ -302,6 +302,54 @@ func TestFirstStatusReport(t *testing.T) {
 	assert.NoError(t, err)
 }
 
+func TestIncludesDetailsOnReconnect(t *testing.T) {
+	srv := internal.StartMockServer(t)
+
+	var receivedDetails int64
+	srv.OnMessage = func(msg *protobufs.AgentToServer) *protobufs.ServerToAgent {
+		// Track when we receive AgentDescription
+		if msg.StatusReport.AgentDescription != nil {
+			atomic.AddInt64(&receivedDetails, 1)
+		}
+
+		return &protobufs.ServerToAgent{
+			InstanceUid: msg.InstanceUid,
+		}
+	}
+
+	var connected int64
+	settings := StartSettings{
+		Callbacks: types.CallbacksStruct{
+			OnConnectFunc: func() {
+				atomic.AddInt64(&connected, 1)
+			},
+			OnRemoteConfigFunc: func(
+				ctx context.Context,
+				config *protobufs.AgentRemoteConfig,
+			) (effectiveConfig *protobufs.EffectiveConfig, configChanged bool, err error) {
+				return &protobufs.EffectiveConfig{}, false, nil
+			},
+		},
+		AgentDescription: &protobufs.AgentDescription{},
+	}
+
+	settings.OpAMPServerURL = "ws://" + srv.Endpoint
+	client := startClient(t, settings)
+
+	eventually(t, func() bool { return atomic.LoadInt64(&connected) == 1 })
+	eventually(t, func() bool { return atomic.LoadInt64(&receivedDetails) == 1 })
+
+	// close the agent connection. expect it to reconnect and send details again.
+	err := client.conn.Close()
+	assert.NoError(t, err)
+
+	eventually(t, func() bool { return atomic.LoadInt64(&connected) == 2 })
+	eventually(t, func() bool { return atomic.LoadInt64(&receivedDetails) == 2 })
+
+	err = client.Stop(context.Background())
+	assert.NoError(t, err)
+}
+
 func TestSetEffectiveConfig(t *testing.T) {
 	// Start a server.
 	srv := internal.StartMockServer(t)