Skip to content

Add support for Jedis 4 - #2626

Merged
eyalkoren merged 3 commits into
elastic:mainfrom
eyalkoren:jedis4
May 16, 2022
Merged

Add support for Jedis 4#2626
eyalkoren merged 3 commits into
elastic:mainfrom
eyalkoren:jedis4

Conversation

@eyalkoren

@eyalkoren eyalkoren commented May 10, 2022 •
edited by zube Bot
Loading

Copy link
Copy Markdown
Contributor

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

@ghost

ghost commented May 10, 2022 •
edited by ghost
Loading

Copy link
Copy Markdown

💚 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
Closed
@eyalkoren
eyalkoren requested a review from felixbarny May 10, 2022 13:44
felixbarny
felixbarny approved these changes May 16, 2022
View reviewed changes

@felixbarny felixbarny left a comment

Copy link
Copy Markdown
Member

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
@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

Reviewers

@felixbarny felixbarny felixbarny approved these changes

Assignees

No one assigned

Labels

agent-java

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Jedis 4 support?

3 participants

@eyalkoren @felixbarny @elastic-apm-tech