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
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
@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
t00mas pushed a commit to t00mas/opentelemetry-collector-contrib that referenced this pull request Jun 18, 2024
…etry#33462)

**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 open-telemetry#31641 up-to-date.

**Link to tracking Issue:** Resolves open-telemetry#30622

**Testing:** <Describe what testing was performed and which tests were
added.>
Unit tests

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Evan Bradley <[email protected]>
Co-authored-by: Evan Bradley <[email protected]>
Co-authored-by: Srikanth Chekuri <[email protected]>
Co-authored-by: Tigran Najaryan <[email protected]>
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jul 11, 2024
…etry#33462)

**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 open-telemetry#31641 up-to-date.

**Link to tracking Issue:** Resolves open-telemetry#30622

**Testing:** <Describe what testing was performed and which tests were
added.>
Unit tests

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Evan Bradley <[email protected]>
Co-authored-by: Evan Bradley <[email protected]>
Co-authored-by: Srikanth Chekuri <[email protected]>
Co-authored-by: Tigran Najaryan <[email protected]>
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