-
Notifications
You must be signed in to change notification settings - Fork 72
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
OnOpampConnectionSettings/Accepted callbacks need re-thinking #261
Comments
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? |
It can be done using a health message. We don't have any other way to report errors from the agent currently. |
Contributes to open-telemetry#261 I intentionally left undefined the blocking vs nonblocking requirement for the callback. I suggest that we refine this after we make implementations and settle on a particular behavior. A continuation of this PR is needed that implements steps 4 and 5 of the flow described here: open-telemetry#261
Contributes to #261 I intentionally left undefined the blocking vs nonblocking requirement for the callback. I suggest that we refine this after we make implementations and settle on a particular behavior. A continuation of this PR is needed that implements steps 4 and 5 of the flow described here: #261
Steps 1-3 and 6 done in #266 Steps 4-5 remaining. |
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:
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: open-telemetry/opentelemetry-collector-contrib#30237 (comment)
The text was updated successfully, but these errors were encountered: