OnOpampConnectionSettings/Accepted callbacks need re-thinking

[tigrannajaryan]

trafficstars

The comments say that the Client implementation (the caller) will attempt to re-connect using new settings.

The example implementation works completely differently, the callback tries the re-connection.

Which is the right way?

To summarize this is what the example is supposed to do:

1. OnOpampConnectionSettings callback called when new settings are offered by the Server.
2. The callback implementation first pre-verifies the settings (e.g. check certificates, etc).
3. If checks in (2) pass the callback implementation logs that it is starting to reconnect, creates a reconnection goroutine and returns from the callback.
4. Reconnection goroutine stops the Client, creates a new Client with new connection settings and wait for successful OnConnect() callback.
5. If OnConnect() is not called within a predefined period of time reconnection goroutine assumes the new connection settings are bad, reverts to the old connection settings and re-creates the Client again.
6. We get rid of OnOpampConnectionSettingsAccepted(), it is not needed.

The example implementation of steps 4 and 5 is not complete today (e.g not waiting for OnConnect), but can be modified to match the above steps.

See also comment thread here: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/30237#discussion_r1509173487

[tigrannajaryan]

cc @andykellr @srikanthccv @evan-bradley

[srikanthccv]

I listened to the meeting recording, and this sounds good to me. However, making it non-blocking leaves us with one question: what would be the mechanism for the client to report back to the server whether it accepted or rejected the connection settings offer?

[tigrannajaryan]

what would be the mechanism for the client to report back to the server whether it accepted or rejected the connection settings offer?

It can be done using a health message. We don't have any other way to report errors from the agent currently.

[tigrannajaryan]

Steps 1-3 and 6 done in https://github.com/open-telemetry/opamp-go/pull/266

Steps 4-5 remaining.