From b0f7c5a5c64c3ef2d3df356320c8d5ff287f2aa3 Mon Sep 17 00:00:00 2001
From: Przemek Maciolek <58699843+pmm-sumo@users.noreply.github.com>
Date: Tue, 22 Mar 2022 15:33:36 +0100
Subject: [PATCH] Set instance_uid by Server on conflict or request for
 generation (#63)

This is a first approach towards #56

It assumes that Agent's `instance_uid` can always be updated by Server (since resolving potential issues with conflicts is important).

There's a gap with multiplexed websocket connections which perhaps could be handled by adding Agent capability flag indicating if overrides can be accepted or not
---
 specification.md | 55 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/specification.md b/specification.md
index 090bc51..2385a13 100644
--- a/specification.md
+++ b/specification.md
@@ -105,9 +105,9 @@ ServerToAgent Protobuf messages:
 
 
 Typically a single Server accepts WebSocket connections from many agents. Agents
-are identified by self-assigned globally unique instance identifiers (or
-instance_uid for short). The instance_uid is recorded in each message sent from
-the Agent to the Server and from the Server to the Agent.
+are identified by self-assigned or server-assigned globally unique instance
+identifiers (or instance_uid for short). The instance_uid is recorded in each
+message sent from the Agent to the Server and from the Server to the Agent.
 
 The default URL path for the initial WebSocket's HTTP connection is /v1/opamp.
 The URL path MAY be configurable on the Agent and on the Server.
@@ -159,6 +159,7 @@ message AgentToServer {
     AgentAddonStatuses addon_statuses = 3;
     AgentInstallStatus agent_install_status = 4;
     AgentDisconnect agent_disconnect = 5;
+    AgentToServerFlags flags = 6;
 }
 ```
 
@@ -175,6 +176,9 @@ created by other Agents. The instance_uid SHOULD remain unchanged for the
 lifetime of the agent process. The recommended format for the instance_uid is
 [ULID](https://github.com/ulid/spec).
 
+In case the Agent wants to use an identifier generated by the Server, the field
+SHOULD be set with a temporary value and RequestInstanceUid flag MUST be set.
+
 #### status_report
 
 The status of the Agent. MUST be set in the first AgentToServer message that the
@@ -199,6 +203,23 @@ the last AgentToServer message.
 AgentDisconnect MUST be set in the last AgentToServer message sent from the
 agent to the server.
 
+#### flags
+
+
+Bit flags as defined by AgentToServerFlags bit masks.
+
+```protobuf
+enum AgentToServerFlags {
+    FlagsUnspecified = 0;
+
+    // Flags is a bit mask. Values below define individual bits.
+
+    // The agent requests server go generate a new instance_uid, which will
+    // be sent back in ServerToAgent message
+    RequestInstanceUid     = 0x00000001;
+}
+```
+
 
 ## ServerToAgent Message
 
@@ -248,6 +269,7 @@ message ServerToAgent {
     AgentPackageAvailable agent_package_available = 6;
     Flags flags = 7;
     ServerCapabilities capabilities = 8;
+    AgentIdentification agent_identification = 9;
 }
 ```
 
@@ -260,6 +282,10 @@ is multiplexed into one WebSocket connection (for example when a terminating
 proxy is used) the instance_uid field allows to distinguish which Agent the
 ServerToAgent message is addressed to.
 
+Note: the value can be overriden by server by sending a new one in the
+AgentIdentification field. When this happens then Agent MUST update its
+instance_uid to the value provided and use it for all further communication.
+
 #### error_response
 
 error_response is set if the Server wants to indicate that something went wrong
@@ -357,6 +383,17 @@ enum ServerCapabilities {
 }
 ```
 
+#### agent_identification
+
+Properties related to identification of the agent, which can be overriden by the
+server if needed. When new_instance_uid is set, Agent MUST update instance_uid
+to the value provided and use it for all further communication.
+
+```protobuf
+message AgentIdentification {
+  string new_instance_uid = 1;
+}
+```
 
 <h2 id="servererrorresponse-message">ServerErrorResponse Message</h2>
 
@@ -2145,10 +2182,10 @@ The Server MAY disconnect or deny serving requests if it detects that the same
 Agent instance has more than one simultaneous connection or if multiple Agent
 instances are using the same instance_uid.
 
-Open Question: does the Server need to actively detect duplicate instance_uids,
-which may happen due to Agents using bad UID generators which create globally
-non-unique UIDs or for example because of cloning of the VMs where the Agent
-runs?
+The Server SHOULD detect duplicate instance_uids (which may happen for example
+when Agents are using bad UID generators or due to cloning of the VMs where the
+Agent runs). When a duplicate instance_uid is detected, Server SHOULD generate
+a new instance_uid, and send it as new_instance_uid value of AgentIdentification.
 
 <h2 id="authentication">Authentication</h2>
 
@@ -2451,9 +2488,9 @@ TBD
 * ~~Do we need the sequence_num concept?~~ Deleted for now, not necessary for
   current feature set, but may need to be restored for other features (e.g.
   custom "extensions").
-* Does the Server need to actively detect duplicate instance_uids, which may
+* ~~Does the Server need to actively detect duplicate instance_uids, which may
   happen due to Agents using bad UID generators which create globally non-unique
-  UIDs?
+  UIDs?~~ Added.
 * ~~Do we need to split the AddonStatus and AgentStatus from the general
   StatusReport?~~ Yes, splitted.
 * Does WebSocket frame compression work for us or do we need our own mechanism?