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

OTeL Java splitting SDK into multiple artifacts #779

Closed
kenfinnigan opened this issue Aug 11, 2020 · 8 comments · Fixed by #931
Closed

OTeL Java splitting SDK into multiple artifacts #779

kenfinnigan opened this issue Aug 11, 2020 · 8 comments · Fixed by #931
Labels
area:sdk Related to the SDK priority:p3 Lowest priority level question Question for discussion release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:miscellaneous For issues that don't match any other spec label

Comments

@kenfinnigan
Copy link
Member

What are you trying to achieve?

I opened open-telemetry/opentelemetry-java#1460 to split the current single SDK artifact into multiple pieces for Correlation Context, Metrics, Tracing, and then combine them all into a single artifact.

The reason for this is I'd like to use Micrometer for the metrics implementation, as it's a mature and popular framework for handling metrics in Java frameworks. Ideally, I don't want to re-implement the other pieces of the OTeL API to use Micrometer. If the SDK is split into multiple pieces it is easier to switch in Micrometer just for metrics.

Additional context.

There was a concern raised on the issue that this goes against the goal of OTeL to bring metrics, tracing, etc together into a single implementation. I was asked to raise an issue here to hear the perspective of the specification group on such an implementation split for Java.

@kenfinnigan kenfinnigan added the spec:miscellaneous For issues that don't match any other spec label label Aug 11, 2020
@arminru arminru added area:sdk Related to the SDK question Question for discussion labels Aug 11, 2020
@Oberon00
Copy link
Member

IMHO, in how many artifacts some OpenTelemetry implementation is split up is an implementation detail, as long as the API artifact(s) are separate from the SDK artifact(s).

@andrewhsu andrewhsu added priority:p3 Lowest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Aug 11, 2020
@andrewhsu
Copy link
Member

from the spec sig mtg today, marking as required for ga to make a decision on what to do based on the question

@reyang
Copy link
Member

reyang commented Aug 11, 2020

I think each language should have freedom to decide how the SDK is componentized/organized:

  1. In C/C++, the linker will do the job. We don't ship *.dll/*.so at this moment so it is not a concern.
  2. In JavaScript, the web scenario would normally want the script to be combined and minimized to improve load speed.

@carlosalberto
Copy link
Contributor

+1 to having the SDK packages separation as an implementation detail.

@jkwatson
Copy link
Contributor

Is there value in noting this in the specs, or should we just assume that wasn't isn't forbidden is explicitly ok, wrt implementation details?

@arminru
Copy link
Member

arminru commented Aug 19, 2020

+1 to having the SDK packages separation as an implementation detail.

@jkwatson I think explicitly stating this in the spec would be fine.

@tigrannajaryan
Copy link
Member

I suggest that we close this issue as a "wont-fix" since it is an implementation detail and does not need to be in the specs.

@arminru
Copy link
Member

arminru commented Sep 9, 2020

I think adding/adapting one or two sentences to explicitly allow this to the spec would still make sense since some statements make it sound like the spec would prescribe the exact structure, e.g.:

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/library-guidelines.md#expected-usage

The OpenTelemetry Language Library is composed of 2 packages: API package and SDK package.

I added two extra lines making it clear in #931.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK priority:p3 Lowest priority level question Question for discussion release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:miscellaneous For issues that don't match any other spec label
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants