Jedis 4.0.0

[tbradellis]

This adds instrumentation for Jedis 4.0.0+)
Maintains the same instrumentation points as previous versions. JedisPubSub and Connection instrumentation.

Related Github Issue

#641
https://github.com/redis/jedis/tree/4.0

Testing

Uses verifyInstrumentation in the usual way.
Additional integration test with local apps to ensure correct counts and no loss of instrumentation features.

Checks

[ NA] Are your contributions backwards compatible with relevant frameworks and APIs?
[ ] Does your code contain any breaking changes? Please describe.
[ ] Does your code introduce any new dependencies? Please describe.

[tbradellis]

This is ready for review. I have one question concerning a new method. In this new method there is a ProtocolCommand and a Keyword
https://github.com/redis/jedis/blob/ea18faa55728de4697ecf7f504aa826491a752d6/src/main/java/redis/clients/jedis/Connection.java#L139

Example:
https://github.com/redis/jedis/blob/ea18faa55728de4697ecf7f504aa826491a752d6/src/main/java/redis/clients/jedis/Jedis.java#L3377
This code already reports the command given. I suppose we could include the keyword as an attribute, but I don't know if it is necessary. I'm currently not tracing this method, but if we aren't going to report the Keyword, then tracing is not needed because it is handled in sendCommand(CommandArguments args)
What do you think? (previous instrumentation module versions do not report keyword)

[meiao]

Could you do like the old instrumentation and call getHostAndPort() when you need the info.
I worry that this will add an increase in memory due to the new classes that will be created for these fields and the maps they require.

The call to getHostAndPort is pretty straightforward and shouldn't add much overhead. Or it should be comparable with the map lookup that is needed with the (a)NewField.

And as an added benefit, you instrument one less method.

[meiao]

Also, the call to getHostAndPort could be done inside this method.

[meiao]

Any reason to trace this new method?
It may cause confusion when people upgrade Jedis and see a new span in their traces.

[tbradellis]

@aonuki I noticed that. maybe there is an opportunity in this JedisPubSub_Instrumentation class to also drop some @newfield usage and save a little memory (like in the Connection_Instrumentation).

newrelic-java-agent/instrumentation/jedis-4.0.0/src/main/java/redis/clients/jedis/JedisPubSub_Instrumentation.java

Line 35 in 4d9df06

this.host = client.getHostAndPort().getHost();

The only thing I see that might make a difference is the number of calls I’d need to make to get the host and port in each of the methods. Do you think this would be a good change here or would leaving it in this case make sense for any reason? Readability or anything else?

[meiao]

In JedisPubSub, you could move the call to inside the reportMethodAsExternal method and remove the host and port parameters.

[tbradellis]

Oh, derp...yes that could be done the same too. Thanks!!