Adds RocketMQ plugin#1449
Conversation
CodePrometheus
force-pushed
the
add-rocketmq-plugin
branch
from
74abe15 to
8cbd9dc
Compare
|
@codefromthecrypt Could you kindly review this when you have some time? Thank you! |
codefromthecrypt
left a comment
codefromthecrypt
left a comment
There was a problem hiding this comment.
Thanks for taking this over @CodePrometheus. I think it needs to be in brave-bom and also added to beans to complete. Consider this approved when comments addressed. @reta if you don't mind covering next review pass.
| @BenchmarkMode(Mode.SampleTime) | ||
| @OutputTimeUnit(TimeUnit.MICROSECONDS) | ||
| @State(Scope.Thread) | ||
| public class RocketMQProducerBenchmarks { |
There was a problem hiding this comment.
please add an example of this run in the PR desc in triple backtick
| @@ -0,0 +1,98 @@ | |||
| # brave-instrumentation-rocketmq-client | |||
There was a problem hiding this comment.
look at the other README examples, I don't recall if we are pasting everything including imports, or just the pertinant parts. I think the latter as it keeps the thing more readable and also we don't compile check the markdown. When reviewing, see if you may have any other ideas to improve the README, but nothing besides this comes to mind.
| */ | ||
| final class TracingMessageListenerConcurrently extends AbstractMessageListener implements MessageListenerConcurrently { | ||
| final MessageListenerConcurrently messageListenerConcurrently; | ||
| final BiFunction<ConsumeConcurrentlyStatus, ConsumeConcurrentlyStatus, Boolean> CONCURRENT_CHECK = |
There was a problem hiding this comment.
I assume rocketmq is minimum java 8 so this will be fine
There was a problem hiding this comment.
yep, rocketmq requires at least Java8.
| static final int BROKER_PORT = 10911; | ||
|
|
||
| RocketMQContainer() { | ||
| super(DockerImageName.parse("apache/rocketmq:5.3.1")); |
There was a problem hiding this comment.
I looked and this is multi-arch now. good! https://hub.docker.com/r/apache/rocketmq/tags
Thanks a lot @CodePrometheus , a few very minor comments from me, solid work! |
|
@codefromthecrypt @reta Thanks for your time! I made a round of fixes for the current issue. Benchmark results on my local are as follows: |
|
nice work! |
3 participants
Resolves #1043