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.

Sign up for GitHub

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

Jump to bottom

Allow OpAMP server to request an agent restart or shutdown #53

Merged
merged 2 commits into from
Mar 29, 2022
Merged

Allow OpAMP server to request an agent restart or shutdown #53

merged 2 commits into from
Mar 29, 2022

Conversation

andykellr
Copy link
Contributor

@andykellr andykellr commented Jan 18, 2022 •
edited by tigrannajaryan
Loading

  • Adds ServerToAgentCommand to allow the server to request a restart or shutdown.
  • Adds AcceptsRestartRequests to AgentCapabilities so that agents can indicate if they support restart.
  • Adds OnCommand to the client Callbacks which is called when the server requests shutdown via the ServerToAgentCommand
  • Moved client.CallbacksStruct to types.CallbacksStruct so that it could be used it in receiver_test.go without an import cycle.

@andykellr andykellr requested a review from a team January 18, 2022 01:34
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 18, 2022 •
edited
Loading

CLA Signed

The committers are authorized under a signed CLA.

@andykellr andykellr mentioned this pull request Jan 18, 2022
Open
tigrannajaryan
tigrannajaryan reviewed Jan 19, 2022
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

@andykellr thanks for the proposal.
It may useful to discuss this in the spec repo in the issue and only after agreeing to a specific approach work on the implementation.

internal/proto/opamp.proto Outdated Show resolved Hide resolved
@andykellr
Copy link
Contributor Author

I was traveling and finally had a chance to refactor this PR to use a ServerToAgentCommand.

@andykellr andykellr changed the title Allow OpAMP server to request an agent restart Allow OpAMP server to request an agent restart or shutdown Feb 4, 2022
@tigrannajaryan
Copy link
Member

I was traveling and finally had a chance to refactor this PR to use a ServerToAgentCommand.

I think this is a reasonable approach. Can you please submit a PR against https://github.com/open-telemetry/opamp-spec and also make sure the change is visible/discussed in the next workgroup meeting?

This was referenced Feb 22, 2022
Merged
Merged
@andykellr @dsvanlani
Add ServerToAgent_RequestRestart flag to request agent restart
d61387f 
Refactor to use a new field ServerToAgentCommand for Restart and Shutdown

Removed reference to shutdown command for now
@tigrannajaryan
Copy link
Member

@andykellr Please update this PR to only implement changes accepted in open-telemetry/opamp-spec#64 for now.

@andykellr
Copy link
Contributor Author

@tigrannajaryan I think this PR is ready to merge. Please let me know if any other changes are needed.

@andykellr andykellr requested a review from pmm-sumo March 28, 2022 19:52
tigrannajaryan
tigrannajaryan approved these changes Mar 28, 2022
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM.
Let's wait for @pmm-sumo to also approve.

pmm-sumo
pmm-sumo approved these changes Mar 29, 2022
View reviewed changes
Copy link
Contributor

@pmm-sumo pmm-sumo left a comment

Choose a reason for hiding this comment

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

LGTM too :)

@tigrannajaryan tigrannajaryan merged commit 528e802 into open-telemetry:main Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@pmm-sumo pmm-sumo pmm-sumo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@andykellr @tigrannajaryan @pmm-sumo @dsvanlani