opentelemetry-java icon indicating copy to clipboard operation
opentelemetry-java copied to clipboard

Adding opamp client subproject

Open LikeTheSalad opened this issue 1 year ago • 6 comments

Setting up the project dir for the OpAMP implementation.

LikeTheSalad avatar Aug 22 '24 13:08 LikeTheSalad

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.07%. Comparing base (97b3fa4) to head (0e2a53e). Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #6665   +/-   ##
=========================================
  Coverage     90.07%   90.07%           
  Complexity     6278     6278           
=========================================
  Files           697      697           
  Lines         18951    18951           
  Branches       1858     1858           
=========================================
  Hits          17071    17071           
  Misses         1306     1306           
  Partials        574      574           

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

codecov[bot] avatar Aug 22 '24 13:08 codecov[bot]

Does client code belong in this repo? I would hope the opamp project could publish a client that we could consume, rather than us having to become maintainers of it.

jkwatson avatar Aug 23 '24 01:08 jkwatson

Does client code belong in this repo? I would hope the opamp project could publish a client that we could consume, rather than us having to become maintainers of it.

I created it here based on a decision made in the SIG meeting on Aug 1st when we talked about contributing it given that it's a work that we had to do internally at Elastic anyway. I don't have a strong opinion on where it should live, although I'm not sure I can commit to maintaining it for a long time after the contribution is done, so in case you'd like to rediscuss where should it live and we decide that this isn't the right place, we should also try to find people willing to maintain it for a long time.

LikeTheSalad avatar Aug 23 '24 09:08 LikeTheSalad

@LikeTheSalad Is the goal going to be to create a generic java OpAmp client, or a thing that is specifically for configuring the OTel Java SDK? If the latter, then 👍🏽 . If the former, I think it shouldn't live here.

jkwatson avatar Aug 25 '24 20:08 jkwatson

@LikeTheSalad Is the goal going to be to create a generic java OpAmp client, or a thing that is specifically for configuring the OTel Java SDK? If the latter, then 👍🏽 . If the former, I think it shouldn't live here.

I'm planning to create a generic tool. What place would you recommend for it to live?

LikeTheSalad avatar Aug 27 '24 13:08 LikeTheSalad

@LikeTheSalad Is the goal going to be to create a generic java OpAmp client, or a thing that is specifically for configuring the OTel Java SDK? If the latter, then 👍🏽 . If the former, I think it shouldn't live here.

I think we need both. A generic OpAmp client for generically sending and receiving messages to an OpAmp server. And a specific message "protocol" built on top of that generic client specific for configuring / updating the SDK. We could bundle them together, but I think we'd later want to pull them apart so someone could use the generic version in a stand alone capacity.

jack-berg avatar Aug 27 '24 19:08 jack-berg

@LikeTheSalad Is the goal going to be to create a generic java OpAmp client, or a thing that is specifically for configuring the OTel Java SDK? If the latter, then 👍🏽 . If the former, I think it shouldn't live here.

I think we need both. A generic OpAmp client for generically sending and receiving messages to an OpAmp server. And a specific message "protocol" built on top of that generic client specific for configuring / updating the SDK. We could bundle them together, but I think we'd later want to pull them apart so someone could use the generic version in a stand alone capacity.

The question is... do we want to maintain a generic client in this codebase, or does it belong elsewhere? My guess is that it's not something we want to have to maintain publicly, but if it were internal, and not directly published, that could be fine.

jkwatson avatar Aug 28 '24 00:08 jkwatson

I added a topic for this week's SIG meeting to discuss the details.

LikeTheSalad avatar Aug 28 '24 14:08 LikeTheSalad

We decided to proceed in contrib for the time being until the OpAMP spec defines a set of API guidelines. So I'm closing this PR.

LikeTheSalad avatar Aug 30 '24 07:08 LikeTheSalad