open-telemetry / opamp-spec

OpAMP Specification
Apache License 2.0
108 stars 34 forks source link

Decide if AgentDescription must be included in every StatusReport #76

Closed tigrannajaryan closed 2 years ago

tigrannajaryan commented 2 years ago

See the comment here: https://github.com/open-telemetry/opamp-go/pull/63#issuecomment-1098526891

The spec says:

The Agent MUST send a status report:
* First time immediately after connecting to the Server. The status report MUST
  be the first message sent by the Agent.
* Subsequently every time the status of the Agent changes.

Nowhere does it says that the AgentDescription must be also included in the status report. On the contrary, the spec says:

<h4 id="agent_description">agent_description</h4>

The description of the agent, its type, where it runs, etc. See
[AgentDescription](#agentdescription-message) message for details.

This field SHOULD be unset if no Agent description fields have changed since the
last StatusReport was sent.

The question is if AgentDescription must be included in every StatusReport. We need to consider what happens both for WebSocket and plain HTTP transports.

tigrannajaryan commented 2 years ago

Some thoughts on this topic:

WebSocket specifics:

  1. WebSocket connections are persistent. We can assume that if the connection is open and maintained then all AgentToServer messages sent via that connection were processed by the Server and the cumulative state described by all those AgentToServer messages is correctly stored by the Server.
  2. If the Server loses the state of the Agent it MUST NOT keep the WebSocket connection open, so that the assertion in point 1 above is valid.

Plain HTTP specifics:

  1. The connections are not persistent (they may be kept alive, but that is unrelated to this discussion).
  2. We cannot make assumptions about how long the Server keeps the Agent's state and whether the state is lost between AgentToServer messages sent by this Agent using HTTP requests.

Give the above, I believe the following is necessary:

Upon receiving AgentToServer message one of the following can happen:

More specifically:

Alternatively, we just make everything uniform and all 4 parts keep hashes and have 4 flags for the Server to request the them.

@open-telemetry/opamp-spec-approvers @andykellr @dsvanlani what do you think?

andykellr commented 2 years ago

I think your summary makes sense. In general, either the state needs to be supplied with every message or a hash needs to be supplied and the server needs a flag to be able to request the entire state.

At the protocol level, I wouldn't be opposed to some flexibility in the agent implementation. There could be a flag for each of the 4 parts and for each of the 4 components we could allow the agent to either send the entire state with each message or to send a hash.

In the reference implementation it would send the entire AgentDescription and RemoteConfigStatus and hashes of the EffectiveConfig and AgentAddonStatuses.

andykellr commented 2 years ago

After thinking about this some more, I think having all 4 flags is useful, even if most implementations pass smaller components on every request.