fix: Send AgentToServer.agent_disconnect when client is stopped.

[minuk-dev]

• In the opamp-spec document,

  AgentDisconnect MUST be set in the last AgentToServer message sent from the Client to the Server.

• But, currently there is no implementation for it.

  • Because of it, the below issue occurs.
    • https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/39676

• To resolve it, ensure to send AgentToServer.agent_disconnect when the client's Stop().

[Edit] related to #179

[Edit - 2]

• To ensure to send last scheduled message, when client is stopped.

[tigrannajaryan]

Sorry for the delay. I suggest that for now we implement agent_disconnect for WebSocket only. If necessary we can add it for http transport later.

[agardnerIT]

I believe this logic should be added to HTTP too in order for the collector to be spec-compliant.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 80.19%. Comparing base (3521293) to head (28f76f3). :warning: Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #384      +/-   ##
==========================================
+ Coverage   80.10%   80.19%   +0.09%     
==========================================
  Files          27       27              
  Lines        2578     2590      +12     
==========================================
+ Hits         2065     2077      +12     
  Misses        395      395              
  Partials      118      118              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tigrannajaryan]

The build is failing because of a bug that will be fixed by https://github.com/open-telemetry/opamp-go/pull/425 Need to wait for that PR to be merged first.

[tigrannajaryan]

@minuk-dev please rebase. The build should now pass.

[minuk-dev]

I believe this logic should be added to HTTP too in order for the collector to be spec-compliant.

@agardnerIT I agree with you. But, I think the spec is ambiguous. But, if we try to send agent_disconnect, we should one more AgentToServer message http.

https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#websocket-transport-opamp-client-initiated

To close a connection the Client MUST first send an AgentToServer message with agent_disconnect field set. The Client MUST then send a WebSocket Close control frame and follow the procedure defined by WebSocket standard.

Should we suggest to add the spec like "The Client SHOULD send the AgentToServer message with agent_disconnect field set, if the Client is HTTP"? Honestly, I'm not sure about it. Because sometimes, the http client is hard to send additional message. So I agree https://github.com/open-telemetry/opamp-go/pull/384#discussion_r2080829073 this review.

Could you please open new issue to discuss it, if you need? Honestly, this PR is too longly opened... I'm not enough time to do it.