adriancole commented on a change in pull request #917: Refactors to reduce
injector complexity and duplicate code
URL:
https://github.com/apache/incubator-zipkin-brave/pull/917#discussion_r290095715
##########
File path:
instrumentation/messaging/src/main/java/brave/messaging/MessagingOperation.java
##########
@@ -16,16 +16,9 @@
*/
package brave.messaging;
-public interface ChannelAdapter<Channel> {
- /**
- * Messaging channel, e.g. kafka topic, jms queue, jms topic, etc.
- */
- String channel(Channel channel);
-
- String channelTagKey(Channel channel);
-
- /**
- * Messaging broker service, e.g. kafka-cluster, jms-server.
- */
- String remoteServiceName(Channel channel);
+public enum MessagingOperation {
+ SEND,
+ BULK_SEND,
+ RECEIVE,
+ BULK_RECEIVE;
Review comment:
imho the point of abstraction is to eliminate unnecessary naming
differences. there's no difference between receive and poll. Later in this
change, you'll see how someone can go back and match the names of kafka.
Why is this important? well, is the span any different in remote nature if
we had one named "poll" and another "receive" if not, why would we split
aggregations on that. Basically from a service perspective, I don't think we
intend to have different names by default on accident of what libraries choose
to name things.
iotw, I would prefer to rename RECEIVE to POLL of receive isn't a good name
:) however, we used to use send and receive in our v1 messaging model, so
thought maybe this is a better default name.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]