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

solves #3345: Add osgi headers #3346

Closed
wants to merge 4 commits into from

Conversation

royteeuwen
Copy link

@royteeuwen royteeuwen commented Oct 3, 2023

What does this PR do?

Add osgi headers to the apm-agent-api
Fixes #3345

Checklist

  • This is an enhancement of existing features, or a new feature in existing plugins
    • I have updated CHANGELOG.asciidoc
    • I have added tests that prove my fix is effective or that my feature works
    • Added an API method or config option? Document in which version this will be introduced
    • I have made corresponding changes to the documentation
  • This is a bugfix
  • This is a new plugin
    • I have updated CHANGELOG.asciidoc
    • My code follows the style guidelines of this project
    • I have made corresponding changes to the documentation
    • I have added tests that prove my fix is effective or that my feature works
    • New and existing unit tests pass locally with my changes
    • I have updated supported-technologies.asciidoc
    • Added an API method or config option? Document in which version this will be introduced
    • Added an instrumentation plugin? Describe how you made sure that old, non-supported versions are not instrumented by accident.
  • This is something else

@cla-checker-service
Copy link

cla-checker-service bot commented Oct 3, 2023

❌ Author of the following commits did not sign a Contributor Agreement:
68d5497, 0e21c74, 55fffa5, 20a39c3

Please, read and sign the above mentioned agreement if you want to contribute to this project

@github-actions github-actions bot added agent-java community Issues and PRs created by the community triage labels Oct 3, 2023
@github-actions
Copy link

github-actions bot commented Oct 3, 2023

👋 @royteeuwen Thanks a lot for your contribution!

It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it.

Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it.

@SylvainJuge
Copy link
Member

Hi !

Thanks for opening this PR @royteeuwen , after discussing this with the team we don't think this is the best approach and will likely close this PR and the related issue.

I understand this might be realy frustrating on your side, but we have a few things that make us thing this is the best option here:

  • OSGi is very rarely used in modern environments, thus we get very few feedback on its usage (thus it's harder to test in the field)
  • we don't have OSGi integration tests, thus we can't ensure this manifest will stay relevant in the future, nor if it works as expected in all the OSGi runtimes (each would require a dedicated integration test, further compounding the effort)
  • From what I understand this is just a manifest modification, thus it should be quite easy to statically modify the manifest in your project build like you very probably have to for other libraries that do not have this manifest.
  • This adds an unknown (at least to us) maven plugin, which seems a big ask for "just modifying a manifest".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-java community Issues and PRs created by the community triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide manifest headers to load apm-agent-api in an OSGi environment
2 participants