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.

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

[cmd/opampsupervisor] Receive and report effective config #33462

Merged

Conversation

BinaryFissionGames
Copy link
Contributor

@BinaryFissionGames BinaryFissionGames commented Jun 10, 2024

Description:
Adds the ability to receive and run remote configurations from an OpAMP server, as well as to report the remote configuration status.

This PR is just bringing #31641 up-to-date.

Link to tracking Issue: Resolves #30622

Testing:
Unit tests

Documentation:

Copy link
Contributor

@evan-bradley evan-bradley left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @BinaryFissionGames.

cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
@evan-bradley evan-bradley requested a review from andykellr June 11, 2024 15:51
@evan-bradley evan-bradley requested a review from srikanthccv June 11, 2024 15:51
@evan-bradley
Copy link
Contributor

I requested a review from everyone who had a review requested on the previous PR. I think I'm a little too close to this one, so I wouldn't mind a second pair of eyes. If nobody has comments by the end of the week, I'll merge this.

cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
@BinaryFissionGames
Copy link
Contributor Author

@tigrannajaryan I think I've got all your feedback incorporated, when you get a chance to take another pass.

cmd/opampsupervisor/supervisor/supervisor.go Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
cmd/opampsupervisor/supervisor/supervisor.go Outdated Show resolved Hide resolved
BinaryFissionGames and others added 2 commits June 12, 2024 15:56
Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
@evan-bradley
Copy link
Contributor

Thanks @tigrannajaryan for the comprehensive review. The dual use of "effective config" was bothering me too, but I hadn't gone through and changed the usage everywhere. I like "merged config" to represent the config given to the Collector much more.

@evan-bradley evan-bradley merged commit 6c15654 into open-telemetry:main Jun 13, 2024
154 checks passed
@evan-bradley
Copy link
Contributor

Thank you @BinaryFissionGames for getting this across the finish line!

@github-actions github-actions bot added this to the next release milestone Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cmd/opampsupervisor] Get the effective config from the OpAMP extension
5 participants