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.

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

Rework client registration and clientId management #53

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

rtamalin
Copy link
Collaborator

@rtamalin rtamalin commented Feb 6, 2025

Previously the clientId was the Id assigned by the upstream telemetry service when a client was registered. This has been renamed to the registrationId, along with the accompanying request header and the associated fields in the registration response and re-authentication request.

Previously a client system would generated a client instance id value and store it in a file; this has now been renamed to a registration value. Where previously the client instance id was just a single string value derived from the timestamp when the value was generated, it now consists of three values, namely a clientId as a UUID string, an optional systemUUID (if available), and a UTC timestamp for when the registration was generated.

Where previously this registrationId from the client credentials was used as the clientId field in telemetry bundle and report structures, we now will use the clientId value from the client's registration.

Updated docs to reflect these changes.

Previously the clientId was the Id assigned by the upstream telemetry
service when a client was registered. This has been renamed to the
registrationId, along with the accompanying request header and the
associated fields in the registration response and re-authentication
request.

Previously a client system would generated a client instance id value
and store it in a file; this has now been renamed to a registration
value. Where previously the client instance id was just a single
string value derived from the timestamp when the value was generated,
it now consists of three values, namely a clientId as a UUID string,
an optional systemUUID (if available), and a UTC timestamp for when
the registration was generated.

Where previously this registrationId from the client credentials was
used as the clientId field in telemetry bundle and report structures,
we now will use the clientId value from the client's registration.

Updated docs to reflect these changes.
Need to ensure that the registration is loaded before checking for
the authentication creds.

Also correct some lingering references to client instance id.
Primarily ensuring that any existing registrations are loaded before
attempting to generate a new one.

Also add support for handling an HTTP StatusConflict (409) in the case
of a duplicate registration attempt by retrying registering the client
once after triggering a regeneration of the client registration.
@@ -25,7 +25,7 @@ func (c *ClientRegistrationRequest) String() string {

// ClientRegistrationResponse is the response payload body from the server
type ClientRegistrationResponse struct {
ClientId int64 `json:"clientId"`
RegistrationId int64 `json:"registrationId"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't RegistrationId supposed to be a string now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So the old clientId has become registrationId, and remains an int value.

There is now a separate clientId (UUID string) which is the first field in the clientRegistration structure, and is what is sent as the report and bundle structure clientId values.

@@ -38,8 +38,8 @@ func (c *ClientRegistrationResponse) String() string {

// Client Authenticate handling via /temelemtry/authenticate
type ClientAuthenticationRequest struct {
ClientId int64 `json:"clientId"`
InstIdHash types.ClientInstanceIdHash `json:"instIdHash"`
RegistrationId int64 `json:"registrationId"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as last comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The old clientId has become the new registrationId, while there is a new clientId (comes from first field in the clientRegistration structure) that is the UUID string value that is now sent in the bundle and report header client id fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants