Feature: Support for tracking for lettuce versions 6.5+ #760
Conversation
|
Please update the supported-list.md about your new versions. |
test/plugin/scenarios/lettuce-6.5.x-scenario/support-version.list
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
Add support for tracing Lettuce client version 6.5+ by refactoring common instrumentation code into a shared module, introducing a version-specific plugin, and providing an end-to-end test scenario.
- Extract common logic into a new
lettuce-commonplugin module - Create a
lettuce-6.5.x-pluginwith overrides for protocol keyword handling - Add the
lettuce-6.5.x-scenariointegration test scenario and update CI configuration
Reviewed Changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/plugin/scenarios/lettuce-6.5.x-scenario/support-version.list | Add supported version 6.5.0.RELEASE |
| test/plugin/scenarios/lettuce-6.5.x-scenario/src/main/java/.../Application.java | Bootstrap Spring Boot application for the new scenario |
| test/plugin/scenarios/lettuce-6.5.x-scenario/src/main/java/.../LettuceController.java | Implement sync/async Redis operations for tracing |
| apm-sniffer/apm-sdk-plugin/pom.xml | Replace lettuce-5.x-plugin with a new lettuce-plugins aggregator module |
| apm-sniffer/apm-sdk-plugin/lettuce-plugins/pom.xml | Declare lettuce-common and lettuce-6.5.x-plugin modules |
| apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/src/main/java/.../RedisChannelWriterInterceptor.java | Refactor and extract common instrumentation logic |
| apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-6.5.x-plugin/src/main/java/.../RedisChannelWriterInterceptorV6.java | Override command‐name extraction for Lettuce 6.5+ |
| CHANGES.md | Document release note for Lettuce 6.5+ support |
| .github/workflows/plugins-test.1.yaml | Enable CI job for the new lettuce-6.5.x-scenario |
Comments suppressed due to low confidence (1)
apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-6.5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v65/RedisChannelWriterInterceptorV6.java:26
- Add a unit test in the
lettuce-6.5.x-pluginmodule to verify thatgetCommandNamereturns the expected command name (e.g.,protocol.toString()), ensuring version-specific logic is covered.
protected String getCommandName(final ProtocolKeyword protocol) {
apm-sniffer/apm-sdk-plugin/lettuce-plugins/lettuce-common/pom.xml
Outdated
Show resolved
Hide resolved
...uce-6.5.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/lettuce/Application.java
Outdated
Show resolved
Hide resolved
test/plugin/scenarios/lettuce-6.5.x-scenario/support-version.list
Outdated
Show resolved
Hide resolved
|
Your change somehow impacts the existing tests.
Could you recheck? |
...org/apache/skywalking/apm/plugin/lettuce/v65/define/RedisChannelWriterInstrumentationV6.java
Outdated
Show resolved
Hide resolved
|
Could you give me more explanations here about what is difference between v5 and v6.5?, besides the different witness class. |
.../org/apache/skywalking/apm/plugin/lettuce/v5/define/RedisChannelWriterInstrumentationV5.java
Show resolved
Hide resolved
...5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v5/constant/Constants.java
Outdated
Show resolved
Hide resolved
.../main/java/org/apache/skywalking/apm/plugin/lettuce/v65/RedisChannelWriterInterceptorV6.java
Outdated
Show resolved
Hide resolved
....x-plugin/src/main/java/org/apache/skywalking/apm/plugin/lettuce/v65/constant/Constants.java
Outdated
Show resolved
Hide resolved
...org/apache/skywalking/apm/plugin/lettuce/v65/define/RedisChannelWriterInstrumentationV6.java
Outdated
Show resolved
Hide resolved
|
Most are good. Please fix some nits and we should be able to merge this. |
|
Hi, i fixed the PR suggestions . |
2 participants
CHANGESlog.