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] Forward custom messages to/from agent #33576

Conversation

BinaryFissionGames
Copy link
Contributor

@BinaryFissionGames BinaryFissionGames commented Jun 14, 2024

Description:

  • Forwards custom messages through the supervisor to/from the agent

Link to tracking Issue: Closes #33575

Testing:

  • Tested against BindPlane
  • E2E tests aren't possible since there is no component using custom messages in contrib right now.
  • Added a couple unit tests

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

This generally looks good to me. I'd appreciate a review from @evan-bradley as well though.

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.

Sorry for the delay, this looks good to me. Thanks @djaglowski for taking a look.

@tigrannajaryan I'll leave this open in case you want to take a look, otherwise I'll merge this by Friday if I am able, or next Monday at the latest.

@BinaryFissionGames BinaryFissionGames force-pushed the feat/supervisor-passthrough-custom-capabilities branch from bb75b96 to 76a82ba Compare July 10, 2024 19:26
@BinaryFissionGames
Copy link
Contributor Author

@tigrannajaryan Gentle bump on this, it's ready for re-review when you get a chance 😄

@tigrannajaryan
Copy link
Member

@tigrannajaryan Gentle bump on this, it's ready for re-review when you get a chance 😄

@BinaryFissionGames I am out this week, won't be able to review. Please feel free to merge, no need to wait for me.

@djaglowski djaglowski merged commit be85cc8 into open-telemetry:main Jul 16, 2024
154 checks passed
@github-actions github-actions bot added this to the next release milestone Jul 16, 2024
mx-psi pushed a commit that referenced this pull request Sep 11, 2024
**Description:**

Follow-up to
#33576.

Boolean short-circuiting was causing the `onMessage` handler to not
process the whole `ServerToAgent` message if it included multiple
fields. I noticed this was causing the Collector to fail to start when
using the opamp-go demo server; the own metrics section is required for
the Collector to start since it provides the only pipelines by default.

I included a new unit test that verifies everything looks as we would
expect when getting a message like what would be sent by the example
server.

---------

Co-authored-by: Evan Bradley <evan-bradley@users.noreply.github.com>
jriguera pushed a commit to springernature/opentelemetry-collector-contrib that referenced this pull request Oct 4, 2024
…telemetry#34349)

**Description:**

Follow-up to
open-telemetry#33576.

Boolean short-circuiting was causing the `onMessage` handler to not
process the whole `ServerToAgent` message if it included multiple
fields. I noticed this was causing the Collector to fail to start when
using the opamp-go demo server; the own metrics section is required for
the Collector to start since it provides the only pipelines by default.

I included a new unit test that verifies everything looks as we would
expect when getting a message like what would be sent by the example
server.

---------

Co-authored-by: Evan Bradley <evan-bradley@users.noreply.github.com>
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] Forward Custom Messages to/from agent
5 participants