opamp-go icon indicating copy to clipboard operation
opamp-go copied to clipboard

Add CustomMessage to AgentToServer and ServerToAgent

Open andykellr opened this issue 2 years ago • 3 comments

See https://github.com/open-telemetry/opamp-spec/issues/49

It was easiest for me to start here. If we agree on the direction of this, I plan to do the following:

  • update the opamp-spec
  • add OnCustomMessage method to the client Callbacks interfaces
  • add SendCustomMessage to the OpAMPClient
  • add implementation to the client and server
  • add tests for that implementation

andykellr avatar Sep 17 '22 03:09 andykellr

Codecov Report

Base: 74.61% // Head: 74.61% // No change to project coverage :thumbsup:

Coverage data is based on head (13c5f71) compared to base (89eeb30). Patch has no changes to coverable lines.

:exclamation: Current head 13c5f71 differs from pull request most recent head 44c4718. Consider uploading reports for the commit 44c4718 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #130   +/-   ##
=======================================
  Coverage   74.61%   74.61%           
=======================================
  Files          23       23           
  Lines        1765     1765           
=======================================
  Hits         1317     1317           
  Misses        334      334           
  Partials      114      114           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Sep 17 '22 03:09 codecov[bot]

@tigrannajaryan Let me know what you think of this direction.

andykellr avatar Sep 17 '22 03:09 andykellr

I realized overnight that we should add a hash so it is clear that the response is appropriately associated with the last message.

I also stubbed the client callback, which I don't think is perfect, so feedback is welcome. Obviously none if this compiles because I added the method and didn't implement it anywhere. I also didn't generate the protobuf code.

andykellr avatar Sep 17 '22 13:09 andykellr

Discussed 3 topics today:

  • Describe uses cases to make the motivation clear
  • Explain possible communication patterns (request/response only or can stream unidirectionally or bidirectionally?)
  • Consider using sequence ids instead of hashes

tigrannajaryan avatar Sep 20 '22 21:09 tigrannajaryan

Closing while spec is being finalized and discussion is happening in the spec repo.

andykellr avatar Oct 11 '22 15:10 andykellr