WIP:Add OSGi support to OpenTelemetry project

[LWogan]

Use bnd plugin to add OSGi metdata to manifest files to allow them to be used within an OSGi framework

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: LWogan / name: Lorcan Wogan (72c25f0fda96868914f48bfef3708f8acf266cf3, 9ffc24a08f5574672b21b6efc72499e7af087ebe, 64fa0c50be95e213fae2d56fd65440d6b1159667, 3283d19210a7408fe0a41c4ab4ea0f93d810e678, 08c181fdf9a445d65ff2ae937de60cc5d633fcf4)

[jkwatson]

@LWogan before we can look at this or accept it, we need you to sign the CLA. Thanks!

[codecov[bot]]

Codecov Report

Merging #4627 (baf0eb4) into main (d84a111) will decrease coverage by 0.00%. The diff coverage is n/a.

:exclamation: Current head baf0eb4 differs from pull request most recent head a5110ce. Consider uploading reports for the commit a5110ce to get more accurate results

@@             Coverage Diff              @@
##               main    #4627      +/-   ##
============================================
- Coverage     90.04%   90.03%   -0.01%     
+ Complexity     5058     5057       -1     
============================================
  Files           581      581              
  Lines         15591    15591              
  Branches       1491     1491              
============================================
- Hits          14039    14038       -1     
  Misses         1100     1100              
- Partials        452      453       +1     

Impacted Files	Coverage Δ
...ava/io/opentelemetry/sdk/internal/RateLimiter.java	94.11% <0.00%> (-5.89%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d84a111...a5110ce. Read the comment docs.

[jkwatson]

I know virtually nothing about OSGi, but this looks ok to me on its surface. @jack-berg do you have any experience with OSGi, or know anyone we can rope in to help?

[LWogan]

To add a little explanation of what did does. The bnd instruction will generate OSGi metadata and add it to the manifest files of each jar. OSGi is a framework which relies on this metadata to functionally operate. The OpenTelemetry artifacts cannot be used by anyone running in an OSGi framework without this.

There was some logic already present in the project to set specific manifest attributes, I have preserved these by instructing the bnd operation to add them while adding its automatically generated OSGi metadata.

[LWogan]

@jack-berg Ive made some additional changes. I needed to add annotations to numerous package-info files. This informs the OSGi framework that these packages are available for use by other jars. Worth noting some packages marked with the namespace internal have been included as some of their classes are on the APIs of classes in other packages.

Another point to add is this PR will add support to this project for OSGi. However there are numerous dependencies such as io.grpc (and its transitive dependencies) which also do not have OSGi support. There are workarounds to this but I thought I should point that out.

Leaving as a draft for now as I am still integrating OpenTelemetry into an OSGi project so I might not have worked out all the issues yet.

[splatch]

I was able to test it and I believe it needs a second touch. Import of javax.annotation package should be marked as optional. Otherwise runtime environment is forced to host findbugs which is strictly speaking build time dependency/tool which does not serve any purpose for execution of byte code.

[splatch]

@jack-berg Looking at this again (on top of 0.16) I think there are more things to tackle within library design itself.

The SpanExporterConfiguration introduces a lookup of a specific implementations from a dependency consumed in opentelemetry-api module:

• SpanExporterConfiguration.configureSpanExporters
• SpanExporterConfiguration.configureExporter
• SpanExporterConfiguration.configureOtlp
• SpanExporterConfiguration.configureZipkin

Its not quite clear to me why such design exists. With such way in place you tie autconfigure module with all listed spi providers. Effectively it is not possible to separate api from specific implementations as bndtools which generates OSGi metadata is able to spot somehow Class.forName and refered class names. Effectively it causes direct listing of such packages as import statements for for sdk-extensions-autoconfigure, which is used by api. Wouldn't it be better to have some kind of ExporterFactory looked up through ServiceLoader first and then let it configure specific exporter through passed properties?

Any use of Class.forName(String) without a ClassLoader is a trouble marker under OSGi cause it will fail in most of the cases:

• ClasspathUtil.checkClassExists
• GlobalOpenTelemetry.maybeAutoConfigureAndSetGlobal

Such patterns work fine with single classloader which is used with flat classpath, but will certainly fail with hierarchical classloaders and osgi which has multiple classloaders. For simplicity of understanding - under OSGi each bundle (deployed jar) have its own classloader exposing only declared import statements.

[github-actions[bot]]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[jack-berg]

Its not quite clear to me why such design exists.

Hi @splatch - sorry for the delayed reply. Not sure what the best answer is here, but I can provide some context. The autoconfigure is responsible for configuring an OpenTelemetrySdk instance based on system properties and env vars. The components it configures includes exporters for specific protocols, such as zipkin, jaeger, prometheus, otlp, etc. Autoconfigure doesn't want to impose transitive dependencies on those exporters, since users will likely only need one or two and the others would just result in unnecessary bulk. So instead it places compileOnly dependencies on these components, and executes the configuration logic for each conditionally based on its presence on the classpath.

The advantage of this is that configuration lives in one place, instead of being scattered around each of the components. Not sure how you could accomplish this with the ExporterFactory pattern you describe, but admittedly I may not be fully grasping your suggestion.

[github-actions[bot]]

This PR was marked stale due to lack of activity. It will be closed in 14 days.

[github-actions[bot]]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[PascalDutch]

Hi, why is this never merged? We are also looking for functionality like this, since we want to use OpenTelemetry in an OSGi context.

[LWogan]

Hi, why is this never merged? We are also looking for functionality like this, since we want to use OpenTelemetry in an OSGi context.

From my perspective the pr is incomplete. Needs more testing and probably more changes.

The package structures of this Otel project means if you want to pull in a single lib then you pretty much have to pull in every other subproject due to the internal dependency tree with each subproject having some package import from another. This is a frustration when you only wish to use a subset of bundles.

Also, unfortunately, many of OpenTelems dependencies and also transient dependencies are not OSGi bundles which makes it very difficult to use this library within an OSGi context even with this PR applied.