Skip to content

Commit

Permalink
Add "fully set" flag to AgentToServer message
Browse files Browse the repository at this point in the history
This is a draft PR to demonstrate how open-telemetry/opamp-spec#109
can be resolved.

The new StatusIsFullSet is set by the agent the status is fully set the message.
The server checks the flag to know if it needs to request the full status.

This is a backward compatible change. New servers that work with old agents may
make an extra request for the full status because old agents don't set this flag,
but this was already a problem in the past even before this change.
  • Loading branch information
tigrannajaryan committed Jul 15, 2022
1 parent 5911ef2 commit 2f70154
Show file tree
Hide file tree
Showing 6 changed files with 376 additions and 348 deletions.
3 changes: 3 additions & 0 deletions client/clientimpl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ func TestReportAgentDescription(t *testing.T) {
assert.EqualValues(t, 0, msg.SequenceNum)
// The first status report after Start must have full AgentDescription.
assert.True(t, proto.Equal(client.AgentDescription(), msg.AgentDescription))
assert.True(t, msg.Flags&protobufs.AgentToServer_StatusIsFullySet != 0)
return &protobufs.ServerToAgent{InstanceUid: msg.InstanceUid}
})

Expand All @@ -709,6 +710,7 @@ func TestReportAgentDescription(t *testing.T) {
assert.Nil(t, msg.AgentDescription)

assert.EqualValues(t, 1, msg.SequenceNum)
assert.True(t, msg.Flags&protobufs.AgentToServer_StatusIsFullySet == 0)

// Ask client for full AgentDescription.
return &protobufs.ServerToAgent{
Expand All @@ -725,6 +727,7 @@ func TestReportAgentDescription(t *testing.T) {
// The status report must again have full AgentDescription
// because the Server asked for it.
assert.True(t, proto.Equal(client.AgentDescription(), msg.AgentDescription))
assert.True(t, msg.Flags&protobufs.AgentToServer_StatusIsFullySet != 0)
return &protobufs.ServerToAgent{InstanceUid: msg.InstanceUid}
})

Expand Down
1 change: 1 addition & 0 deletions client/internal/clientcommon.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ func (c *ClientCommon) PrepareFirstMessage(ctx context.Context) error {
msg.EffectiveConfig = cfg
msg.RemoteConfigStatus = c.ClientSyncedState.RemoteConfigStatus()
msg.PackageStatuses = c.ClientSyncedState.PackageStatuses()
msg.Flags |= protobufs.AgentToServer_StatusIsFullySet

if c.PackagesStateProvider != nil {
// We have a state provider, so package related capabilities can work.
Expand Down
1 change: 1 addition & 0 deletions client/internal/receivedprocessor.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func (r *receivedProcessor) rcvFlags(
msg.Health = r.clientSyncedState.Health()
msg.RemoteConfigStatus = r.clientSyncedState.RemoteConfigStatus()
msg.PackageStatuses = r.clientSyncedState.PackageStatuses()
msg.Flags |= protobufs.AgentToServer_StatusIsFullySet

// The logic for EffectiveConfig is similar to the previous 4 sub-messages however
// the EffectiveConfig is fetched using GetEffectiveConfig instead of
Expand Down
17 changes: 13 additions & 4 deletions internal/examples/server/data/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,17 @@ func (agent *Agent) processStatusUpdate(
newStatus *protobufs.AgentToServer,
response *protobufs.ServerToAgent,
) {
if agent.Status != nil && agent.Status.SequenceNum+1 != newStatus.SequenceNum {
// We lost the previous status update. Request full status update from the agent.
// The status is not fully set in the message that we received. Agent compressed the status
// by omitting some fields.
receivedFullStatus := newStatus.Flags&protobufs.AgentToServer_StatusIsFullySet != 0

// We don't have any status for this Agent, or we lost the previous status update from the Agent, so our
// current status is not up-to-date.
lostPreviousUpdate := (agent.Status == nil) || (agent.Status != nil && agent.Status.SequenceNum+1 != newStatus.SequenceNum)

if !receivedFullStatus && lostPreviousUpdate {
// The status message is not fully set in the message that we received, but we lost the previous
// status update. Request full status update from the agent.
response.Flags |= protobufs.ServerToAgent_ReportFullState
}

Expand All @@ -219,8 +228,8 @@ func (agent *Agent) processStatusUpdate(
// If remote config is changed and different from what the Agent has then
// send the new remote config to the Agent.
if configChanged ||
(newStatus.RemoteConfigStatus != nil &&
bytes.Compare(newStatus.RemoteConfigStatus.LastRemoteConfigHash, agent.remoteConfig.ConfigHash) != 0) {
(agent.Status.RemoteConfigStatus != nil &&
bytes.Compare(agent.Status.RemoteConfigStatus.LastRemoteConfigHash, agent.remoteConfig.ConfigHash) != 0) {
// The new status resulted in a change in the config of the Agent or the Agent
// does not have this config (hash is different). Send the new config the Agent.
response.RemoteConfig = agent.remoteConfig
Expand Down
6 changes: 6 additions & 0 deletions internal/proto/opamp.proto
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ message AgentToServer {
// The Agent requests Server go generate a new instance_uid, which will
// be sent back in ServerToAgent message
RequestInstanceUid = 0x00000001;

// Indicates that the Agent included full status in this message, that none
// of the the status fields is omitted. This tells the Server that it is
// that this message is full and there is no need to request full status.
// See https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agent-status-compression
StatusIsFullySet = 0x00000002;
}
// Bit flags as defined by AgentToServerFlags bit masks.
AgentToServerFlags flags = 10;
Expand Down
Loading

0 comments on commit 2f70154

Please sign in to comment.