From 6325ee913f108db8f6ebee3bcc480d9cde8f405f Mon Sep 17 00:00:00 2001 From: Tigran Najaryan Date: Fri, 15 Jul 2022 11:07:12 -0400 Subject: [PATCH] Correctly handle status compression on the server side This is an alternate draft PR to demonstrate how https://github.com/open-telemetry/opamp-spec/issues/109 can be resolved without adding a new "compressed/full" flag, suggested by @andykellr --- internal/examples/server/data/agent.go | 27 ++++++++++++++++++++------ internal/proto/opamp.proto | 4 ++++ protobufs/opamp.pb.go | 17 ++++++++++------ 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/internal/examples/server/data/agent.go b/internal/examples/server/data/agent.go index 65a86dfa..b1e0859a 100644 --- a/internal/examples/server/data/agent.go +++ b/internal/examples/server/data/agent.go @@ -198,13 +198,27 @@ 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. - response.Flags |= protobufs.ServerToAgent_ReportFullState - } + // 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) agentDescrChanged := agent.updateStatusField(newStatus) + // Check if any fields were omitted in the status report. + effectiveConfigOmitted := agent.Status.Capabilities&protobufs.AgentCapabilities_ReportsEffectiveConfig != 0 && newStatus.EffectiveConfig == nil + packageStatusesOmitted := agent.Status.Capabilities&protobufs.AgentCapabilities_ReportsPackageStatuses != 0 && newStatus.PackageStatuses == nil + remoteConfigStatusOmitted := agent.Status.Capabilities&protobufs.AgentCapabilities_AcceptsRemoteConfig != 0 && newStatus.RemoteConfigStatus == nil + healthOmitted := agent.Status.Capabilities&protobufs.AgentCapabilities_ReportsHealth != 0 && newStatus.Health == nil + + // True if the status was not fully reported. + statusIsCompressed := effectiveConfigOmitted || packageStatusesOmitted || remoteConfigStatusOmitted || healthOmitted + + if statusIsCompressed && 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 + } + configChanged := false if agentDescrChanged { // Agent description is changed. @@ -219,14 +233,15 @@ 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 } agent.updateEffectiveConfig(newStatus, response) + } // SetCustomConfig sets a custom config for this Agent. diff --git a/internal/proto/opamp.proto b/internal/proto/opamp.proto index 5536d8b7..972ea2b1 100644 --- a/internal/proto/opamp.proto +++ b/internal/proto/opamp.proto @@ -538,6 +538,10 @@ enum AgentCapabilities { AcceptsOtherConnectionSettings = 0x00000200; // The Agent can accept restart requests. AcceptsRestartCommand = 0x00000400; + // The Agent reports Health + ReportsHealth = 0x00000800; + // // The Agent will report RemoteConfig in AgentToServer. + // ReportsRemoteConfig = 0x00001000; // Add new capabilities here, continuing with the least significant unused bit. } diff --git a/protobufs/opamp.pb.go b/protobufs/opamp.pb.go index 4fe0c909..41f3c5b3 100644 --- a/protobufs/opamp.pb.go +++ b/protobufs/opamp.pb.go @@ -138,6 +138,8 @@ const ( AgentCapabilities_AcceptsOtherConnectionSettings AgentCapabilities = 512 // The Agent can accept restart requests. AgentCapabilities_AcceptsRestartCommand AgentCapabilities = 1024 + // The Agent reports Health + AgentCapabilities_ReportsHealth AgentCapabilities = 2048 ) // Enum value maps for AgentCapabilities. @@ -155,6 +157,7 @@ var ( 256: "AcceptsOpAMPConnectionSettings", 512: "AcceptsOtherConnectionSettings", 1024: "AcceptsRestartCommand", + 2048: "ReportsHealth", } AgentCapabilities_value = map[string]int32{ "UnspecifiedAgentCapability": 0, @@ -169,6 +172,7 @@ var ( "AcceptsOpAMPConnectionSettings": 256, "AcceptsOtherConnectionSettings": 512, "AcceptsRestartCommand": 1024, + "ReportsHealth": 2048, } ) @@ -2953,7 +2957,7 @@ var file_opamp_proto_rawDesc = []byte{ 0x63, 0x63, 0x65, 0x70, 0x74, 0x73, 0x50, 0x61, 0x63, 0x6b, 0x61, 0x67, 0x65, 0x73, 0x53, 0x74, 0x61, 0x74, 0x75, 0x73, 0x10, 0x10, 0x12, 0x1c, 0x0a, 0x18, 0x4f, 0x66, 0x66, 0x65, 0x72, 0x73, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x53, 0x65, 0x74, 0x74, 0x69, 0x6e, - 0x67, 0x73, 0x10, 0x20, 0x2a, 0xd4, 0x02, 0x0a, 0x11, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x43, 0x61, + 0x67, 0x73, 0x10, 0x20, 0x2a, 0xe8, 0x02, 0x0a, 0x11, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x43, 0x61, 0x70, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x69, 0x65, 0x73, 0x12, 0x1e, 0x0a, 0x1a, 0x55, 0x6e, 0x73, 0x70, 0x65, 0x63, 0x69, 0x66, 0x69, 0x65, 0x64, 0x41, 0x67, 0x65, 0x6e, 0x74, 0x43, 0x61, 0x70, 0x61, 0x62, 0x69, 0x6c, 0x69, 0x74, 0x79, 0x10, 0x00, 0x12, 0x11, 0x0a, 0x0d, 0x52, 0x65, @@ -2974,11 +2978,12 @@ var file_opamp_proto_rawDesc = []byte{ 0x63, 0x65, 0x70, 0x74, 0x73, 0x4f, 0x74, 0x68, 0x65, 0x72, 0x43, 0x6f, 0x6e, 0x6e, 0x65, 0x63, 0x74, 0x69, 0x6f, 0x6e, 0x53, 0x65, 0x74, 0x74, 0x69, 0x6e, 0x67, 0x73, 0x10, 0x80, 0x04, 0x12, 0x1a, 0x0a, 0x15, 0x41, 0x63, 0x63, 0x65, 0x70, 0x74, 0x73, 0x52, 0x65, 0x73, 0x74, 0x61, 0x72, - 0x74, 0x43, 0x6f, 0x6d, 0x6d, 0x61, 0x6e, 0x64, 0x10, 0x80, 0x08, 0x42, 0x2e, 0x5a, 0x2c, 0x67, - 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x6f, 0x70, 0x65, 0x6e, 0x2d, 0x74, - 0x65, 0x6c, 0x65, 0x6d, 0x65, 0x74, 0x72, 0x79, 0x2f, 0x6f, 0x70, 0x61, 0x6d, 0x70, 0x2d, 0x67, - 0x6f, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, - 0x74, 0x6f, 0x33, + 0x74, 0x43, 0x6f, 0x6d, 0x6d, 0x61, 0x6e, 0x64, 0x10, 0x80, 0x08, 0x12, 0x12, 0x0a, 0x0d, 0x52, + 0x65, 0x70, 0x6f, 0x72, 0x74, 0x73, 0x48, 0x65, 0x61, 0x6c, 0x74, 0x68, 0x10, 0x80, 0x10, 0x42, + 0x2e, 0x5a, 0x2c, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x6f, 0x70, + 0x65, 0x6e, 0x2d, 0x74, 0x65, 0x6c, 0x65, 0x6d, 0x65, 0x74, 0x72, 0x79, 0x2f, 0x6f, 0x70, 0x61, + 0x6d, 0x70, 0x2d, 0x67, 0x6f, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x73, 0x62, + 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var (