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

Add support for Jedis 4 #2626

Merged
merged 3 commits into from May 16, 2022
Merged

Add support for Jedis 4 #2626

merged 3 commits into from May 16, 2022

Conversation

eyalkoren
Copy link
Contributor

@eyalkoren eyalkoren commented May 10, 2022

What does this PR do?

Closes #2616

The proposed instrumentation hereby seems simpler and is more suitable for easily supporting Jedis 4 as well.
All existing tests pass, so I can't say if I missed anything with this simplification.

Checklist

  • This is a new plugin
    • I have updated CHANGELOG.asciidoc - add also as a refactoring change for older Jedis versions
    • 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 - the old versions instrumentation is disabled for Jedis 4 based on class loader matching and the new one is specifically enabled for Jedis 4 similarly

@apmmachine
Copy link
Collaborator

apmmachine commented May 10, 2022

💚 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

  • Start Time: 2022-05-16T12:21:19.131+0000

  • Duration: 48 min 51 sec

Test stats 🧪

Test Results
Failed 0
Passed 2872
Skipped 22
Total 2894

💚 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 windows tests : Build & tests on windows.

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

@eyalkoren eyalkoren mentioned this pull request May 10, 2022
@eyalkoren eyalkoren requested a review from felixbarny May 10, 2022 13:44
Copy link
Member

@felixbarny felixbarny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I like the simpler approach of instrumenting just one entry point instead of all the command method implementations!

@eyalkoren eyalkoren enabled auto-merge (squash) May 16, 2022 11:46
@eyalkoren eyalkoren merged commit 68f28cd into elastic:main May 16, 2022
APM-Agents (OLD) automation moved this from In Progress to Done May 16, 2022
@eyalkoren eyalkoren deleted the jedis4 branch May 16, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Jedis 4 support?
3 participants