solves #768: Add OSGi support by exporting all java packages except internal packages

[royteeuwen]

Pull request to add OSGi support

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: royteeuwen / name: Roy Teeuwen (ff2f9cacd1d8ee24bf41e96840c6959310847fa1)
• :white_check_mark: login: bdhoine / name: Barry d'Hoine (769923c383169737605b221b5d7ba1d74dd2ec29, 0b080b2ef3d0c4f47644bb4d43bd8e87d06ccc59)

[codecov[bot]]

Codecov Report

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

Project coverage is 90.06%. Comparing base (97b3fa4) to head (769923c). Report is 217 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6669      +/-   ##
============================================
- Coverage     90.07%   90.06%   -0.02%     
  Complexity     6278     6278              
============================================
  Files           697      697              
  Lines         18951    18951              
  Branches       1858     1858              
============================================
- Hits          17071    17068       -3     
- Misses         1306     1307       +1     
- Partials        574      576       +2     

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

[royteeuwen]

@jack-berg @bdhoine and myself are from the same organization, that's why you see a similar PR in the other repo's ;) we need it there as well.

Of course I don't mind adding an integration test / sample application at all, but I would like to see what the expected result is:

• OSGi is based on manifest headers in the resulting JAR. We can create a unit test that checks the manifest headers to see that the correct headers are available. If yes, then this will work in an OSGi environment. This is a very fast test and would not be hard to create. I can also link to the OSGi docs to showcase what is required.
• Another approach would be to start up an entire OSGi application in the integration-tests folder that contains the opentelemetry api and sdk jar. This way we can test that the jar files work by creating some sample traces or something of the sorts. But I would like to avoid adding an integration tests to this project, that make the build inherently a bit slower, and then get it declined because of that ;)

So what do you expect / propose? I will also join the Java SIG as mentioned in the linked github issue, so maybe it's best to combine the two discussion lists and continue the talk in the issue instead of having two separate discussions in the pr and the issue about the same topic at the same time.

[jack-berg]

We can create a unit test that checks the manifest headers to see that the correct headers are available. If yes, then this will work in an OSGi environment. This is a very fast test and would not be hard to create. I can also link to the OSGi docs to showcase what is required.

What's your confidence level that this would in fact ensure that OSGi support is valid? Have you tried building your branch locally and running your app? There was a comment on #4627 suggesting that some of our Class.forName usage would get in the way of OSGi.. an integration test seems like the surest way to know its going to behave like we think.

We have to do something similar to ensure support for graal. Is there a lightweight way to do the same with OSGi? You mention that integration tests are inherently a little slower - how slow are we talking? We have precedent for running some fairly expensive but valuable tests as a part of our build, so could be worth it.

[royteeuwen]

So far I have tested it by wrapping all OpenTelemetry jars into one jar together and adding the correct manifest headers and then running it in an OSGi application, this was the most efficient way of doing it without having to lose too much time, but it becomes a hassle because I have to build my own wrapper every time when a new release comes out. That's the reason for making this PR.

But thanks for pointing the issue out of the Class.forName, it will give issues when having all jars separate instead of one uber jar, without adding extra headers to fix it. The integration test will be perfect to point that out!

The integration test will still be lightweight enough, just slower than a unit test. I'll try something out and come back with the results!

[jkwatson]

@royteeuwen any update on getting an integration test working?

[github-actions[bot]]

This has been automatically marked as stale because it has been marked as needing author feedback and has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[royteeuwen]

@royteeuwen any update on getting an integration test working?

I got everything working in my separate repo but had to wait for bnd 7.1 to be released, which is now the case.

The only thing left is that in the repo where I set everything up, I’m using maven (the builder that I’m used to) and now I’ll have to see how to port it to gradle, because I’m using a plugin that doesn’t exist in gradle, so not sure yet how to easily port it.

[jkwatson]

@royteeuwen any update on getting an integration test working?

I got everything working in my separate repo but had to wait for bnd 7.1 to be released, which is now the case.

The only thing left is that in the repo where I set everything up, I’m using maven (the builder that I’m used to) and now I’ll have to see how to port it to gradle, because I’m using a plugin that doesn’t exist in gradle, so not sure yet how to easily port it.

Keep us updated! thanks!