Cleanup lettuce instrumentation and add tests #872
Conversation
...n/lettuce-5.0/src/test/java/com/nr/lettuce5/instrumentation/Lettuce5InstrumentationTest.java
Outdated
Show resolved
Hide resolved
...on/lettuce-6.0/src/main/java/io/lettuce/core/AbstractRedisAsyncCommands_Instrumentation.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Left some specific comments but there's also a few general things to address.
All source code files need the New Relic copyright header but modified for 2022.
What’s up with the .cache-tests files being checked in? Can we delete them from this PR?
instrumentation/scala-2.12.0/.cache-testsinstrumentation/slick-2.12_3.2.0/.cache-testsinstrumentation/akka-http-core-10.2.0/.cache-tests
There's some inconsistent formatting throughout, make sure that all files have been formatted using the java-agent-code-style.
...umentation/lettuce-5.0/src/main/java/com/nr/fit/lettuce/instrumentation/NRErrorConsumer.java
Outdated
Show resolved
Hide resolved
instrumentation/lettuce-4.3/src/main/java/com/lambdaworks/redis/AbstractRedisAsyncCommands.java
Outdated
Show resolved
Hide resolved
instrumentation/lettuce-4.3/src/main/java/com/nr/fit/lettuce/instrumentation/NRBiConsumer.java
Outdated
Show resolved
Hide resolved
instrumentation/lettuce-5.0/src/main/java/io/lettuce/core/protocol/CommandHandler.java
Outdated
Show resolved
Hide resolved
instrumentation/lettuce-5.0/src/main/java/io/lettuce/core/protocol/CommandHandler.java
Outdated
Show resolved
Hide resolved
instrumentation/lettuce-5.0/src/main/java/io/netty/channel/AbstractChannel_Instrumentation.java
Outdated
Show resolved
Hide resolved
instrumentation/lettuce-6.0/src/main/java/com/nr/fit/lettuce/instrumentation/NRBiConsumer.java
Outdated
Show resolved
Hide resolved
|
Looks like I have some work. I'll try to get started this weekend in my "copious free time" but might have to pick up first thing Monday. |
twcrone
force-pushed
the
tcrone/lettuce-instrumentation
branch
from
33cffb8 to
1ff6734
Compare
twcrone
force-pushed
the
tcrone/lettuce-instrumentation
branch
3 times, most recently
from
5de5c4f to
bae6a6b
Compare
|
@jasonjkeller I think I've addressed your concerns. I'm wondering if you wanted more in the README.md about Testcontainers? Right now, I think only kafka and lettuce use them for instrumentation tests, so just noted that Docker might be needed to run instrumentation tests with links to Testcontainer documentation and Docker install. Let me know your thoughts! |
Yea, just making it clear that Docker is a requirement to run all tests is what I was getting at. |
instrumentation/lettuce-4.3/src/test/java/com/nr/lettuce43/instrumentation/helper/Data.java
Show resolved
Hide resolved
...tion/lettuce-4.3/src/test/java/com/nr/lettuce43/instrumentation/helper/RedisDataService.java
Show resolved
Hide resolved
...ation/lettuce-5.0/src/test/java/com/nr/lettuce5/instrumentation/helper/RedisDataService.java
Show resolved
Hide resolved
...ation/lettuce-6.0/src/main/java/io/lettuce/core/protocol/CommandHandler_Instrumentation.java
Show resolved
Hide resolved
...ation/lettuce-6.0/src/test/java/com/nr/lettuce6/instrumentation/helper/RedisDataService.java
Show resolved
Hide resolved
twcrone
force-pushed
the
tcrone/lettuce-instrumentation
branch
from
bae6a6b to
7ae8a34
Compare
Provides Java instrumentation for the Lettuce driver for Redis. Instrumentation is being migrated from New Relic Experimental to be included in the product