Skip to content

Added support for overwriting the service version per classloader #1726

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

Merged
merged 54 commits into from
Jan 27, 2022

Conversation

tobiasstadler
Copy link
Contributor

@tobiasstadler tobiasstadler commented Mar 30, 2021

What does this PR do?

Fixes #1725

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

@ghost
Copy link

ghost commented Mar 30, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Reason: null

  • Start Time: 2022-01-27T07:15:41.553+0000

  • Duration: 48 min 20 sec

  • Commit: d76f2a5

Test stats 🧪

Test Results
Failed 0
Passed 2455
Skipped 16
Total 2471

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark tests.

  • run jdk compatibility tests : Run the JDK Compatibility tests.

  • run integration tests : Run the Agent Integration tests.

  • run end-to-end tests : Run the APM-ITs.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@tobiasstadler
Copy link
Contributor Author

@felixbarny Is there any chance to get this merged?

@felixbarny
Copy link
Member

I'm out for a bit, will leave this to the team to decide.

@tobiasstadler
Copy link
Contributor Author

Ok, Thank you for the info.

@SylvainJuge @eyalkoren What is your opinion?

@SylvainJuge
Copy link
Member

FYI, I will also be off for a couple of weeks.

From what I recall we previously agreed that fixing that more generally would be a better option (#1327), while it's not without challenges, especially when relying on packaging conventions, I think it would probably a better option to solve the general case.

@tobiasstadler
Copy link
Contributor Author

@SylvainJuge It would be great if #1327 is supported, but as far as I know nobody is working on this. I also think that #1327 can build in this PR.

@SylvainJuge SylvainJuge added the community Issues and PRs created by the community label Jul 27, 2021
@tobiasstadler
Copy link
Contributor Author

tobiasstadler commented Aug 23, 2021

@SylvainJuge If this was merged I could use it in https://github.com/tobiasstadler/apm-wildfly-deployment-plugin to set the service version (in addition to the service name)

@tobiasstadler
Copy link
Contributor Author

@SylvainJuge / @felixbarny Any News?

@AlexanderWert AlexanderWert added this to the 8.0-candidate milestone Sep 13, 2021
@SylvainJuge
Copy link
Member

Hi @tobiasstadler, we have added this to our short-term backlog.
While we can't provide guarantees nor ETA, we will try to handle it soon.

@tobiasstadler
Copy link
Contributor Author

Any chance this will make it into 1.29.0?

@felixbarny
Copy link
Member

After merging and consolidating with #1922, I don't have objections to this PR.

@tobiasstadler
Copy link
Contributor Author

I will use ServiceInfo instead of ServiceNameAndVersion once #1922 is merged.

@felixbarny
Copy link
Member

run elasticsearch-ci/docs

@felixbarny felixbarny enabled auto-merge (squash) January 27, 2022 07:12
@felixbarny
Copy link
Member

Thanks for the contribution and for gently pushing for it ❤️

auto-merge was automatically disabled January 27, 2022 07:15

Head branch was pushed to by a user without write access

@felixbarny
Copy link
Member

run elasticsearch-ci/docs

@felixbarny felixbarny enabled auto-merge (squash) January 27, 2022 07:18
@felixbarny felixbarny merged commit a43c21b into elastic:main Jan 27, 2022
@tobiasstadler
Copy link
Contributor Author

Thank you very much!

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Service Version per classloader
4 participants