Consider adding a "status compressed/full" flag to AgentToServer

[tigrannajaryan]

The Server currently cannot tell if the status was compressed. As a consequence when it receives a message from an Agent it didn't see before it needs to ask for the full status. This unnecessary extra exchange when the Agent reports to the Server for the first time.
This can be avoided if we add a bit flag to indicate the that the status was compressed. If it is not (as is the case for newly starting Agents) the Server won't ask for full status.

[tigrannajaryan]

Here is a draft implementation with the new flag: open-telemetry/opamp-go#103

[andykellr]

I am on vacation right now, but I thought that the AgentToServer message would be one of the following:

For each submessage, either

1. The submessage is not nil because either
   a. It has changed since the last message
   b. The server requested the full state
2. The submessage is nil and has been omitted because
   a. It has not changed (compressed)
   b. It is not supported by the agent as indicated by the AgentCapabilities flags

A message would be considered compressed if any submessage supported by the agent is nil. The message is full if no submessages supported by the agent are nil.

If an agent contacts the server and sends a compressed message (as determined above), the server adds the ServerToAgent_ReportFullState flag to receive the full status of the agent.

[tigrannajaryan]

b. It is not supported by the agent as indicated by the AgentCapabilities flags

That's a good point. The problem is that we don't have all necessary capability flags to indicate this. There is none for AgentHealth and for RemoteConfigStatus. Perhaps that's what we are missing. If all compressible submessages have a corresponding capabilities flag then the fact of compression is possible to establish without this new flag I suggested.

Let me think a bit.

[andykellr]

BindPlane OP does not currently support AgentHealth (coming soon!), but for RemoteConfigStatus I check AgentCapabilities_AcceptsRemoteConfig. I figured that if it accepts remote config then it should support reporting RemoteConfigStatus.

For AgentHeath we could add AgentCapabilities_ReportsAgentHealth.

[tigrannajaryan]

@andykellr he is another draft PR that implements the approach you suggested: open-telemetry/opamp-go#105

It is a smaller change, but I am reluctant with this approach since it requires more checks to detect compression, which is a potential source of bug.

Thoughts?

[andykellr]

I don't feel that strongly about it, but in general my instinct is to avoid a second piece of information that can already be derived from the information that exists. Typically this leads to ambiguity when the flag says it is complete (but it isn't) or the flag says it is not complete (but it is).

In this particular case, as long as we consider the flag authoritative, I don't see an issue. This means that if the flag says it is complete, ReportFullState should not report additional information and if the flag says it is incomplete, ReportFullState should report additional information. It does simplify reasoning about the message.

[tigrannajaryan]

@andykellr I thought a bit more and I agree with you. Closing this in favour of #111 and open-telemetry/opamp-go#105