Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set instance_uid by Server on conflict or request for generation #63

Merged
merged 4 commits into from
Mar 22, 2022
Merged

Set instance_uid by Server on conflict or request for generation #63

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d9ad3b2
Set instance_uid by Server on conflict or request for generation
pmm-sumo Feb 22, 2022
22fc208
Update specification.md
pmm-sumo Feb 23, 2022
1bbcce1
Use separate field and explicit flag for server-assigned instance_uid
pmm-sumo Mar 9, 2022
1ae9135
Use separate field for agent identification
pmm-sumo Mar 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 46 additions & 9 deletions specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -159,6 +159,7 @@ message AgentToServer {
AgentAddonStatuses addon_statuses = 3;
AgentInstallStatus agent_install_status = 4;
AgentDisconnect agent_disconnect = 5;
AgentToServerFlags flags = 6;
pmm-sumo marked this conversation as resolved.
Show resolved Hide resolved
}
```

Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -248,6 +269,7 @@ message ServerToAgent {
AgentPackageAvailable agent_package_available = 6;
Flags flags = 7;
ServerCapabilities capabilities = 8;
AgentIdentification agent_identification = 9;
}
```

Expand All @@ -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
Expand Down Expand Up @@ -357,6 +383,17 @@ enum ServerCapabilities {
}
```

#### agent_identification
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that "new_instance_uid" would be too specific field and wanted something with a bit broader scope so I thought about "AgentIdentification", though also wondering about "AgentIdentifcationOverride", "AgentIdentificationOffer" or so. If you had anything else in mind @tigrannajaryan I will gladly accept it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything else that we may want to add to AgentIdentification message in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything else that we may want to add to AgentIdentification message in the future?

I don't have anything on my mind right now though I'm actually wondering if instead creating AgentIdentification, a model used in AgentDescription could be followed (so repeated KeyValue). I don't really have a strong use case but wondering if this would give an opportunity for the server to send some attributes back (both identifying and non-identifying) that would be persisted by the agent. Not sure if it makes much sense though (since server could always attach them later itself if needed)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add keyvalue list as an additional field later if needed. I think what we have for now is fine.


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>

Expand Down Expand Up @@ -2136,10 +2173,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>

Expand Down Expand Up @@ -2438,9 +2475,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?
Expand Down