Move non-core components out of opentelemetry-java

[jack-berg]

Following the spirit of moving opentelemetry-extension-annotations to a more appropriate home in opentelemetry-java-instrumentation, and prompted by a discussion in the 7/28 Office Hours, I've done a scan of the project for components that are candidates to live elsewhere. The list includes all artifacts that contain code that isn't explicitly included in the spec, with exceptions for opentelemetry-micrometer1-shim and opentelemetry-extension-kotlin.

Artifact	Status	Description
opentelemetry-extension-aws	stable	Consists of AwsXRayPropagator and AwsConfigurablePropagator, which implements the ConfigurablePropagatorProvider SPI.
opentelemetry-extension-incubator	alpha	Consists of PassThroughPropagator and ExtendedTracer.
opentelemetry-extension-noop-api	alpha	Includes a completely noop OpenTelemetry implementation.
opentelemetry-sdk-extension-resources	stable	Resource detectors for container, host, os, and process
opentelemetry-sdk-extension-aws	stable	AWS resource providers.
opentelemetry-sdk-extension-jfr-events	alpha	SpanProcessor which records spans as JFR events.
opentelemetry-sdk-extension-metric-incubator	alpha	Includes file based view configuration.
opentelemetry-sdk-extension-tracing-incubator	alpha	Includes the LeadDetectingSpanProcessor
opentelemetry-sdk-extension-zpages	alpha	zPages SpanProcessor

Comments

• opentelemetry-extension-aws - Not sure what the history is here, but xray propagator isn't included in the list of propagators defined the spec and including it in the core sets a precedent of making exceptions. I'd like to see this deprecated and moved to opentelemetry-java-contrib/aws-xray.
• opentelemetry-extension-incubator - I like the idea of having places to incubate API / SDK concepts, but think if they're going to live in the core project there needs to be a path to inclusion the stable API / SDK artifacts. Not clear that that path exists for PassThroughPropagator or ExtendedTracer.
• opentelemetry-extension-noop-api - Should probably move this toopentelemetry-java-contrib. If we think its important enough to keep in core, perhaps opentelemetry-extension-incubator, since there may be a chance for a completely noop implementation to be added to the spec some day.
• opentelemetry-sdk-extension-resources - I don't feel especially strongly about this, but this probably makes more sense in opentelemetry-java-instrumentation.
• opentelemetry-sdk-extension-aws - I think this makes most sense in opentelemetry-java-instrumentation, especially if the standard resource providers end up there.
• opentelemetry-sdk-extension-jfr-events - Move to opentelemetry-java-contrib.
• opentelemetry-sdk-extension-metric-incubator - Can probably retain since file based SDK configuration is being very seriously discussed in the specification.
• opentelemetry-sdk-extension-tracing-incubator - If we're confident enough with the LeakDetectingSpanProcessor, we should consider promoting it to the stable opentelemetry-sdk-testing artifact, since it makes sense as a testing utility.
• opentelemetry-sdk-extension-zpages - zPages are mentioned in the experimental section of the specification, but I think this could be merged into opentelemetry-sdk-extension-tracing-incubator, since there are no transitive dependencies required to run.

[anuraaga]

I'd like to see this deprecated and moved to opentelemetry-java-contrib/aws-xray.

Just for some history, actually the name of the propagator isn't really correct but stuck unfortunately - the propagator isn't really related to x-ray (besides the x-ray sdk defaulting to it which is probably where the name came from), but it's the propagation format used within aws. Notably, many propagation scenarios require it regardless of tracing backend. So aws-xray would not be a good artifact for it, which is for x-ray backend specific code.

Also, as this is already used in instrumentation, if moving it to contrib, it then requires deciding on the pattern for depending on contrib from the instrumentation repo. Compared to the others on the list, this one is actually probably being used the most in production by users, and I suspect most aren't x-ray users :P

[jack-berg]

This text from the spec seems pretty unambiguous about this propagator not belonging in the core repo:

Additional Propagators implementing vendor-specific protocols such as AWS X-Ray trace header protocol MUST NOT be maintained or distributed as part of the Core OpenTelemetry repositories.

if moving it to contrib, it then requires deciding on the pattern for depending on contrib from the instrumentation repo.

Yes, we need to decide this :)

[willarmiros]

Hi @jack-berg, would moving these components to a new home change their artifact name in Maven?

[jack-berg]

Yes. And for stable artifacts we would have to continue to publish the old coordinates until a 2.0.0 release. Alpha artifacts don't have the same restriction, and if moved would likely be published in both places for one version (while deprecated in opentelemetry-java) before being removed.

[mateuszrzeszutek]

I think that the AWS Xray propagator should probably stay in the SDK; and that we should change the spec to reflect that.
I view it as a component that is similar in nature to the OpenTracing shim, Micrometer shim, Jaeger propagator, etc. in that it enables OpenTelemetry to talk to legacy systems, and thus makes the transition to OTel (and W3C propagation) smoother.
Perhaps it should appear in the MAY be maintained list next to the OTTrace propagator: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/context/api-propagators.md#propagators-distribution

[trask]

• opentelemetry-extension-aws - Not sure what the history is here, but xray propagator isn't included in the list of propagators defined the spec and including it in the core sets a precedent of making exceptions. I'd like to see this deprecated and moved to opentelemetry-java-contrib/aws-xray.

+1 to @mateuszrzeszutek's comment. And if this change is made to the specification, it would probably make sense to move the aws xray propagator to https://github.com/open-telemetry/opentelemetry-java/tree/main/extensions/trace-propagators, though that would change their its coordinates.

• opentelemetry-extension-incubator - I like the idea of having places to incubate API / SDK concepts, but think if they're going to live in the core project there needs to be a path to inclusion the stable API / SDK artifacts. Not clear that that path exists for PassThroughPropagator or ExtendedTracer.

PassThroughPropagator seems interesting, I could see that maybe being spec'd someday

I don't see much of a path forward for ExtendedTracer either, I'm not sure it makes a lot of sense in contrib either, so I'd suggest just dropping it and if anyone notices then we've found a potential maintainer for it in contrib 😄

• opentelemetry-extension-noop-api - Should probably move this toopentelemetry-java-contrib. If we think its important enough to keep in core, perhaps opentelemetry-extension-incubator, since there may be a chance for a completely noop implementation to be added to the spec some day.

👍 for opentelemetry-extension-incubator

• opentelemetry-sdk-extension-resources - I don't feel especially strongly about this, but this probably makes more sense in opentelemetry-java-instrumentation.

yeah, this is interesting, I hadn't thought about it previously, but I think keeping semantic convention stuff out of the core repo is very reasonable (since for Java we have a dedicated "instrumentation" repo). this could include the semconv artifact too as we discussed in this week's SIG meeting. we can update https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/scope.md if we move forward with this plan.

• opentelemetry-sdk-extension-aws - I think this makes most sense in opentelemetry-java-instrumentation, especially if the standard resource providers end up there.

👍

• opentelemetry-sdk-extension-jfr-events - Move to opentelemetry-java-contrib.

👍 (I'm ok looking for component owner for this either before or after the move)

• opentelemetry-sdk-extension-metric-incubator - Can probably retain since file based SDK configuration is being very seriously discussed in the specification.

👍

• opentelemetry-sdk-extension-tracing-incubator - If we're confident enough with the LeakDetectingSpanProcessor, we should consider promoting it to the stable opentelemetry-sdk-testing artifact, since it makes sense as a testing utility.

LeakDetectingSpanProcessor feels different to me from the opentelemetry-sdk-testing artifact which is designed around JUnit / AssertJ. although I don't have a better suggestion though, other than contrib

• opentelemetry-sdk-extension-zpages - zPages are mentioned in the experimental section of the specification, but I think this could be merged into opentelemetry-sdk-extension-tracing-incubator, since there are no transitive dependencies required to run.

👍

[rapphil]

Yes. And for stable artifacts we would have to continue to publish the old coordinates until a 2.0.0 release. Alpha artifacts don't have the same restriction, and if moved would likely be published in both places for one version (while deprecated in opentelemetry-java) before being removed.

@jack-berg besides maven coordinates, would package names remain the same during the transition?

[jack-berg]

@rapphil it depends on where they move. Artifacts in opentelemetry-java have a package prefix of io.opentelemetry. Artifacts in opentelemetry-java-instrumentation have a package prefix of io.opentelemetry.instrumentation. Artifacts in opentelemetry-java-contrib have a package prefix of io.opentelemetry.contrib. So a move to a different repository would have a package name change. A move within this repository as I've proposed with some of the artifacts may or may not have a package name change.

[trask]

Should micrometer shim be moved out?

And settle on criteria for core repo: spec'd (or on its way to spec'd) AND not semantic convention related

[jack-berg]

On the subject of the micrometer shim:

I think we should be stricter and exclude components from here unless they are explicitly spec'd. I know we made an exception for micrometer. And that seemed like the right decision at the time, but micrometer tracing doesn't share the same usage and yet a good user experience requires that it belongs next to the micrometer metrics shim.

I believe that opentelemetry-java-instrumentation is the right home for both the micrometer metrics and tracing shim. These components are conceptually similar to the kafka metrics bridge, the logback appender, and the log4j appender. Each of these components bridges some other telemetry API into the opentelemetry API. And each is related to very popular java specific libraries that are unlikely to be part of the specification.

We could take it one step further and move even the spec'd shims to opentelemetry-java-instrumentation (opentracing and opencensus). That has the added benefit of having all the shims in one place, whether or not they are explicitly spec'd.

[trask]

My preference would be to move "bridges to other telemetry libraries" to the contrib repository, where experts in the target libraries can be component owners for them.

[trask]

We could probably reverse the dependency between instrumentation and contrib repos, making instrumentation repo depend on contrib repo. We have already discussed moving runtime attach and static instrumenter to the instrumentation repo, which I think are the only contrib components that depend on the instrumentation repo today.

[mateuszrzeszutek]

We could take it one step further and move even the spec'd shims to opentelemetry-java-instrumentation (opentracing and opencensus).

One possible problem with the OpenCensus bridge is that it's impossible to use it together with the javaagent, since it's an SDK bridge. Maybe it's just me, but it would be a bit strange to have it in the javaagent repo when it's impossible to use it together with the javaagent.

We could probably reverse the dependency between instrumentation and contrib repos, making instrumentation repo depend on contrib repo.

This is a pretty good idea 👍

[jack-berg]

My preference would be to move "bridges to other telemetry libraries" to the contrib repository, where experts in the target libraries can be component owners for them.

Doesn't the instrumentation repo depend on library experts chiming in from time to time as well? I think we should use this as an opportunity to really clarify the delineation between core, instrumentation, and contrib. I think we're coalescing on consensus that core only contains spec'd components which don't involve the semantic convention. I'm not as confident in the difference between contrib and instrumentation. Maybe contrib is the place for components that aren't spec'd, and don't involve the semantic convention. This definition would mean that contrib becomes the home for the micrometer metrics & tracing shims, as well as the kafka client metrics stuff.

One possible problem with the OpenCensus bridge is that it's impossible to use it together with the javaagent, since it's an SDK bridge. Maybe it's just me, but it would be a bit strange to have it in the javaagent repo when it's impossible to use it together with the javaagent.

Due to the presence of a growing number of library instrumentation artifacts, I've been viewing the instrumentation repo as a general home for instrumentation, which also publishes an agent which automatically weaves lots of instrumentation into apps at runtime. I think you're suggesting that instrumentation which can't be used by the agent should live in contrib, which seems like a strange delineation to me.

[trask]

Due to the presence of a growing number of library instrumentation artifacts, I've been viewing the instrumentation repo as a general home for instrumentation

This is correct, and has been the case since we renamed the repo from opentelemetry-java-auto-instr to opentelemetry-java-instrumentation a long while back.

I think you're suggesting that instrumentation which can't be used by the agent should live in contrib

I don't believe @mateuszrzeszutek is suggesting this, as he has been one of the people pushing the instrumentation repo forward for library (non-javaagent) instrumentation.

[mateuszrzeszutek]

Due to the presence of a growing number of library instrumentation artifacts, I've been viewing the instrumentation repo as a general home for instrumentation, which also publishes an agent which automatically weaves lots of instrumentation into apps at runtime. I think you're suggesting that instrumentation which can't be used by the agent should live in contrib, which seems like a strange delineation to me.

I was just thinking out loud; you're right that it seems kinda strange.

I think that the decision here might just boil down to deciding the difference between the instrumentation and contrib repos:

I'm not as confident in the difference between contrib and instrumentation. Maybe contrib is the place for components that aren't spec'd, and don't involve the semantic convention. This definition would mean that contrib becomes the home for the micrometer metrics & tracing shims, as well as the kafka client metrics stuff.

The instrumentation repo already contains a bunch of features/components that are not specced and/or experimental, and I think it's been mostly working fine for it. The contrib repo contains plenty of instrumentations, if we define an instrumentation as "something that produces telemetry" - for example, the maven extension, which I believe is in its right place and I don't think I'd like to move it to the instrumentation repo. Also, some time ago we've been thinking about moving the less important/popular/supported instrumentations over to the contrib repo - and if we reverse the dependency between contrib and instrumentation we'll finally be able to do that.

How about we define the instrumentation repo as: instrumentation artifacts that are supported by OpenTelemetry maintainers. Which sounds super vague, but I think somewhat makes sense - we'd like to support things like OpenCensus and OpenTracing, or the most popular Java frameworks (servlet, spring, micrometer, ...). So, for example, Kafka instrumentation would stay instrumentation; Apache Dubbo or RocketMQ would be moved to contrib and we'd have to find owners for them (we have no idea how they work anyway 😂 ).
Everything that does not fit that definition goes to contrib.

(This definition kinda contradicts the maven extension example - maven is super popular, but I'd argue that instrumenting build systems is not so for the time being its place is in contrib)

[jack-berg]

Which sounds super vague, but I think somewhat makes sense

Ideally the definition we land on is unambiguous so we don't have to keep revisiting the decision in the future. I think your idea of "instrumentation artifacts that are supported by OpenTelemetry maintainers" could be made unambiguous by defining a process for maintainer support - if no maintainer is willing to "sponsor" the instrumentation, its a candidate to move to contrib. Still a bit tenuous.

On the note of opentelemetry-extension-aws aws propagator, we discussed the spec issue today in spec SIG. The consensus was pretty much to treat it as a bug in the java implementation, calling it out in the readme / an issue that this was a mistake. But the conversation was interesting because there was also consensus that we don't need to keep publishing stable artifacts to be compliant with semantic versioning. As long as the we don't break the API those components depend, folks can use an old version of a deprecated artifact with the latest API / SDK artifacts. We've been operating under this assumption with jaeger-proto, extension-annotations,opentelemetry-extension-aws, opentelemetry-sdk-extension-resources. Changing that assumption would open up the door for more options for aligning components with their proper long term home. Folks in the spec SIG noted that there are reasons we might want to continue maintaining deprecated code, such as to provide fixes for security vulnerabilities. But we don't need to necessarily keep publishing artifacts to do so, since we could provide patches to release branches.

[trask]

Here's a proposal based on the above:

• Publish an opentelemetry-javaagent-contrib-{version}.jar from the contrib repository
• Keep the current repo dependencies:

  opentelemetry-java > opentelemetry-java-instrumentation > opentelemetry-java-contrib

• Use @jack-berg's heuristic above:

  if no maintainer is willing to "sponsor" the instrumentation, it's a candidate to move to contrib

Then the javaagent published from the instrumentation repo would only contain components that are "sponsored" by a maintainer (either via the core repo or the instrumentation repo), while the javaagent published from the contrib repo would be equivalent to the javaagent that we publish currently.

While this definition of having a maintainer willing to "sponsor" a component is wishy-washy, it's not clear to me that we're going to come up with anything better.

[jack-berg]

We spoke in the 8/25/2022 Java SIG about a variety of topics related to where components should live within the three otel java repositorys opentelemetry-java, opentelemetry-java-instrumentation, and opentelemetry-java-contrib. To summarize:

• opentelemetry-java should only contain spec'd components which do not related to the semantic conventions. Its going to continue to publish opentelemetry-semconv for now, but the consumption of those conventions is an indication that the component is some sort of instrumentation and belongs elsewhere. Note that spec'd shims like the opentelemetry-opencensus-shim and opentelemetry-opentracing-shim should stay in opentelemetry-java: although they have some similarities with other components like the micrometer shim and kafka client metrics, they are different in that they should be temporary utilities to assist in the transition to opentelemetry. Additionally, opentelemetry was formed as the replacement for opentracing and opencensus, further reasoning for special status.
• opentelemetry-java-instrumentation is the home of instrumentation, including resource providers and shim-like things, which have sponsorship from a maintainer. As a maintainer of opentelemetry-java I'll avoid elaborating further because I'm sure the instrumentation maintainers will come up with a more precise definition that suits them.
• opentelemetry-java-contrib is the home of other contributions. This can include instrumentation, resource providers, shims that lack sponsorship from the opentelemetry-java-instrumentation maintainers; implementations of SDK extension points (Processors, Exporters, Propagators, etc) that are not explicitly of the specified. We agreed that following some instrumentation moving from opentelemetry-java-instrumentation to opentelemetry-java-contrib, we'd like to publish a opentelemetry-javaagent-contrib-{version}.jar (originally proposed here).

Changes to this repository as a result of this discussion include:

• Move resource providers out of this repository. Discussed separately and in more detail here.
• Move the micrometer1-shim (back) to opentelemetry-java-instrumentation, since the instrumentation maintainers agreed to sponsor it.

We still need to make a decision on:

• The opentelemetry-extension-aws extension. As mentioned here, we can leave it where it is as an exception. However, if we don't need to continue publishing stable artifacts forever, it seems like quite an odd exception. In that case, would be better to move it to opentelemetry-java-contrib and include it in the proposed opentelemetry-javaagent-contrib-{version}.jar.
• What to do with opentelemetry-extension-noop-api. We like the idea of moving it to opentelemetry-extension-incubator, but after some research that's not actually a good idea. The problem is it implements the ContextStorageProvider SPI, which means that if its included on the classpath the noop context storage provider will get used automatically. If we move to opentelemetry-extension-incubator, users will find context mysteriously not working. It has to either stay where it is, or move to opentelemetry-java-contrib. I'm in favor of moving to opentelemetry-java-contrib.

[trask]

• What to do with opentelemetry-extension-noop-api. We like the idea of moving it to opentelemetry-extension-incubator, but after some research that's not actually a good idea. The problem is it implements the ContextStorageProvider SPI, which means that if its included on the classpath the noop context storage provider will get used automatically. If we move to opentelemetry-extension-incubator, users will find context mysteriously not working. It has to either stay where it is, or move to opentelemetry-java-contrib. I'm in favor of moving to opentelemetry-java-contrib.

👍 can always move it to core repo if it gets specd some day

[guizmaii]

Hi,

After the move of the noop-api to io.opentelemetry.contrib, I don't find it in Maven Central:

See https://search.maven.org/search?q=io.opentelemetry.contrib

Where can I find it?

Thanks 🙂

[jack-berg]

The artifact was moved to opentelemetry-java-contrib/noop-api. There's a one week gap between the release of opentelemetry-java and opentelemetry-java-contrib. The noop-api will be first published this coming Friday, Sept 16th with maven coordinates io.opentelemetry.contrib:opentelemetry-noop-api:{version}.

[guizmaii]

Hi @jack-berg,

Is the io.opentelemetry.contrib:opentelemetry-noop-api being published already? I still don't find it in Maven Central

[mateuszrzeszutek]

Hey @guizmaii ,
We haven't released the 1.18.0 version of contrib yet, we've had an unexpected delay, sorry for that 🙏
You can expect it to be released early this week.

[jkwatson]

@guizmaii this has now been released, so should be available. I have verified it can be downloaded via those maven coordinates.

[jack-berg]

All items except opentelemetry-extension-aws have been addressed. Moving that conversation to #4831 and closing this issue.