[Feature] Support message trace feature by hugoasdf · Pull Request #162 · apache/rocketmq-client-cpp

hugoasdf · 2019-07-01T05:05:12Z

What is the purpose of the change

[Feature] Support message trace feature

Brief changelog

add message trace feature for cpp sdk,add hook,dispatcher

Verifying this change

can send message trace message.
TBD:web console show.

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.

Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when a cross-module dependency exists.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

…variable. declare correct string::size_type by auto. (apache#143)

* update network interface. - feature: use only one event loop for all TcpTransport. - update: network components. * remove boost mutex, timed_mutex and condition_variable in TcpRemotingClient, TcpTransport and ReponseFunture.

ifplusor · 2019-07-01T22:21:49Z

src/consumer/MQConsumer.cpp

+#include "AsyncTraceDispatcher.h"
+namespace rocketmq {
+
+      MQConsumer::MQConsumer(PullRequest* request) {


where the param request is used?

ifplusor · 2019-07-01T22:24:39Z

include/MQConsumer.h

+
+
+  private:
+  std::shared_ptr<TraceDispatcher> traceDispatcher;


rename to 'm_traceDispatcher'

ifplusor · 2019-07-01T22:24:53Z

include/MQConsumer.h

+   //DefaultMQProducerImpl defaultMQProducerImpl;
+
+
+  std::vector<std::shared_ptr<ConsumeMessageHook> > consumeMessageHookList;


same as above.

ifplusor · 2019-07-01T22:27:20Z

src/consumer/MQConsumer.cpp

+
+
+   MQConsumer::~MQConsumer() {
+	if (traceDispatcher.use_count()>0) {


don't use 'use_count() > 0' as condition, but 'traceDispatcher != nullptr'.

ifplusor · 2019-07-01T22:32:26Z

src/producer/DefaultMQProducer.cpp


+
+#include "AsyncTraceDispatcher.h"
+#include <boost/shared_ptr.hpp>


where use boost::shared_ptr or boost::weak_ptr ?

ifplusor · 2019-07-01T22:46:47Z

include/AsyncTraceDispatcher.h

@@ -0,0 +1,165 @@
+#ifndef __AsyncTraceDispatcher_H__


add Apache License declaration.

ifplusor · 2019-07-01T22:47:21Z

include/AsyncTraceDispatcher.h

+  //DefaultMQProducer* traceProducer;
+  std::string TraceTopicName;
+  AsyncRunnable_run_context(bool stoppedv, int batchSizev,
+	  //AsyncTraceDispatcher* atdv,


format all changed files with clang-format.

ifplusor · 2019-07-01T22:49:52Z

src/trace/hook/SendMessageTraceHookImpl.h

+  public:
+  SendMessageTraceHookImpl(std::shared_ptr<TraceDispatcher>& localDispatcher);
+  virtual std::string hookName();
+  //virtual void sendMessageBefore(SendMessageContext* context);


remove unnecessary functions.

ifplusor · 2019-07-01T22:52:25Z

src/trace/hook/ConsumeMessageTraceHookImpl.cpp

+
+			// Caculate the cost time for processing messages
+			int costTime = 0;//(int)((System.currentTimeMillis() - subBeforeContext.getTimeStamp()) / context.getMsgList().size());
+			subAfterContext.setCostTime(costTime);//


calculate real costTime.

ifplusor · 2019-07-01T22:53:08Z

src/trace/hook/ConsumeMessageTraceHookImpl.cpp

+ */
+
+/*
+package org.apache.rocketmq.client.trace.hook;


remove unnecessary code.

review recommends are read,checks will become,thanks.

remove unnecessary code.

jonnxu · 2019-07-08T16:37:15Z

include/AsyncTraceDispatcher.h

@@ -0,0 +1,118 @@
+#ifndef __AsyncTraceDispatcher_H__


please use big alphabet for macro definition, the same for following files
AsyncTraceDispatcher_H change to ASYNCTRACEDISPATCHER_H

jonnxu · 2019-07-08T16:38:36Z

include/AsyncTraceDispatcher.h

+
+class AsyncTraceDispatcher : public TraceDispatcher, public enable_shared_from_this<AsyncTraceDispatcher> {
+ private:
+  // static InternalLogger log;  //= ClientLogger.getLog();


delete the unnecessary code for new class

jonnxu · 2019-07-08T16:39:56Z

include/AsyncTraceDispatcher.h

+  std::condition_variable m_appenderQueuenotEmpty;
+
+  std::thread* m_shutDownHook;
+  bool m_stopped = false;


not assignment class variant here

jonnxu · 2019-07-08T16:40:28Z

include/AsyncTraceDispatcher.h

+  // std::thread* worker;
+  std::shared_ptr<std::thread> m_worker;
+
+ public:


why here is public class members ?

jonnxu · 2019-07-08T16:43:12Z

include/ConsumeMessageHook.h

+
+class ConsumeMessageHook {
+public:
+	virtual std::string hookName() { return ""; }


here format, please check if there is format problem for others

jonnxu · 2019-07-08T16:46:00Z

include/DefaultMQProducer.h

 class ROCKETMQCLIENT_API DefaultMQProducer : public MQProducer {
 public:
-  DefaultMQProducer(const std::string& groupname);
+  DefaultMQProducer(const std::string& groupname, bool Withouttrace = false, void* rpcHook = nullptr);


notice the name format for funtion variant, such as Withouttrace, please check others

jonnxu · 2019-07-08T16:49:23Z

include/TraceHelper.h

+  static std::string LOCAL_ADDRESS;  //= "/*UtilAll.ipToIPv4Str(UtilAll.getIP())*/";
+
+ private:
+  std::string m_topic = "";


no need assignment here

jonnxu · 2019-07-08T16:51:53Z

src/message/MQMessage.cpp

 const string MQMessage::PROPERTY_TRANSACTION_PREPARED_QUEUE_OFFSET = "TRAN_PREPARED_QUEUE_OFFSET";
 const string MQMessage::PROPERTY_TRANSACTION_CHECK_TIMES = "TRANSACTION_CHECK_TIMES";
 const string MQMessage::PROPERTY_CHECK_IMMUNITY_TIME_IN_SECONDS = "CHECK_IMMUNITY_TIME_IN_SECONDS";
+const string MQMessage::CONSUME_CONTEXT_TYPE = "ConsumeContextType";


keep the same style for parameter name

keep the same style for parameter name

updated,-:)

…nt-cpp into tracedev3

jovany-wang

Basically, I have 2 suggestions:

Add doc comments for the methods.
Show some usages.

jovany-wang · 2019-07-29T11:49:03Z

include/AsyncTraceDispatcher.h

+#include "TraceDispatcher.h"
+namespace rocketmq {
+
+class AsyncTraceDispatcher : public TraceDispatcher, public enable_shared_from_this<AsyncTraceDispatcher> {


public enable_shared_from_this<AsyncTraceDispatcher> seems can be private here right?

jovany-wang · 2019-07-29T11:49:30Z

include/AsyncTraceDispatcher.h

+  std::atomic<long> m_discardCount;
+  std::shared_ptr<std::thread> m_worker;
+
+  // public:


remove this line?

jovany-wang · 2019-07-29T11:49:51Z

include/AsyncTraceDispatcher.h

+  std::atomic<bool> m_delydelflag;
+
+ public:
+  bool get_stopped() { return m_stopped; }


Suggested change

bool get_stopped() { return m_stopped; }

bool get_stopped() const { return m_stopped; }

jovany-wang · 2019-07-29T11:51:35Z

include/AsyncTraceDispatcher.h

+  virtual void start(std::string nameSrvAddr, AccessChannel accessChannel = AccessChannel::LOCAL);
+
+  virtual void setdelydelflag(bool v) { m_delydelflag = v; }
+  bool getdelydelflag() { return m_delydelflag; }


Suggested change

bool getdelydelflag() { return m_delydelflag; }

bool getdelydelflag() const { return m_delydelflag; }

jovany-wang · 2019-07-29T11:51:59Z

include/AsyncTraceDispatcher.h

+  std::string& getTraceTopicName() { return m_traceTopicName; }
+
+  void setTraceTopicName(std::string traceTopicNamev) { m_traceTopicName = traceTopicNamev; }
+  bool getisStarted() { return m_isStarted.load(); };


Suggested change

bool getisStarted() { return m_isStarted.load(); };

bool getisStarted() const { return m_isStarted.load(); };

jovany-wang · 2019-07-29T11:54:05Z

include/AsyncTraceDispatcher.h

+
+}  // namespace rocketmq
+
+#endif


Please add new blank line at the end of this file.

jovany-wang · 2019-07-29T11:54:18Z

include/ConsumeMessageContext.h

+  std::string msgnamespace;
+
+ public:
+  std::string getConsumerGroup() { return consumerGroup; };


Suggested change

std::string getConsumerGroup() { return consumerGroup; };

std::string getConsumerGroup() const { return consumerGroup; };

jovany-wang · 2019-07-29T11:54:29Z

include/ConsumeMessageContext.h

+
+  void setConsumerGroup(std::string consumerGroup) { consumerGroup = consumerGroup; };
+
+  std::list<MQMessageExt> getMsgList() { return msgList; };


Suggested change

std::list<MQMessageExt> getMsgList() { return msgList; };

std::list<MQMessageExt> getMsgList() const { return msgList; };

jovany-wang · 2019-07-29T11:54:41Z

include/ConsumeMessageContext.h

+
+  void setMsgList(std::list<MQMessageExt> msgList) { msgList = msgList; };
+  void setMsgList(std::vector<MQMessageExt> pmsgList) { msgList.assign(pmsgList.begin(), pmsgList.end()); };
+  MQMessageQueue getMq() { return mq; };


Suggested change

MQMessageQueue getMq() { return mq; };

MQMessageQueue getMq() const { return mq; };

jovany-wang · 2019-07-29T11:55:09Z

include/ConsumeMessageHook.h

+
+class ConsumeMessageHook {
+ public:
+  virtual std::string hookName() { return ""; }


hookName -> getHookname?

ShannonDing · 2020-03-17T02:31:15Z

thanks for your PR,
according to the conflicts, the code was refactored in #276, this PR will be closed.
welcome testing this feature.
the message trace, for the async sending, is not supported now, and welcome to add this functionality.

ifplusor and others added 5 commits July 1, 2019 00:30

[ISSUE apache#142] save string::find result into a string::size_type …

d7cdc29

…variable. declare correct string::size_type by auto. (apache#143)

batch issue

a7d130b

Update: update network interface. (apache#60)

f344d78

* update network interface. - feature: use only one event loop for all TcpTransport. - update: network components. * remove boost mutex, timed_mutex and condition_variable in TcpRemotingClient, TcpTransport and ReponseFunture.

[feature]message trace

4b48bf0

Merge branch 'master' into tracedev3

110e60d

ifplusor requested changes Jul 1, 2019

View reviewed changes

trace message code reshape

38bd98b

jonnxu reviewed Jul 8, 2019

View reviewed changes

hugoasdf added 5 commits July 9, 2019 21:19

trace message code reshape2

877b477

trace message code reshape2

93bc31e

message trace code reshape3

4b8b43a

Merge branch 'tracedev3' of https://github.com/hugoasdf/rocketmq-clie…

ca145c3

…nt-cpp into tracedev3

Format changes with .clang-format

82079a7

hugoasdf changed the title ~~[feature]message trace~~ [Feature] Support message trace feature Jul 16, 2019

hugoasdf added 2 commits July 18, 2019 22:51

change members to private

08989a6

Merge branch 'tracedev3' of https://github.com/hugoasdf/rocketmq-clie…

96df212

…nt-cpp into tracedev3

jovany-wang suggested changes Jul 29, 2019

View reviewed changes

ShannonDing added the enhancement New feature or request label Jan 10, 2020

ShannonDing mentioned this pull request Feb 28, 2020

[FEATURE]Try to support trace messages. #267

Closed

ShannonDing linked an issue Feb 28, 2020 that may be closed by this pull request

[FEATURE]Try to support trace messages. #267

Closed

ShannonDing closed this Mar 17, 2020

		//DefaultMQProducerImpl defaultMQProducerImpl;


		std::vector<std::shared_ptr<ConsumeMessageHook> > consumeMessageHookList;



		MQConsumer::~MQConsumer() {
		if (traceDispatcher.use_count()>0) {



		#include "AsyncTraceDispatcher.h"
		#include <boost/shared_ptr.hpp>

	bool get_stopped() { return m_stopped; }
	bool get_stopped() const { return m_stopped; }

	bool getdelydelflag() { return m_delydelflag; }
	bool getdelydelflag() const { return m_delydelflag; }

	bool getisStarted() { return m_isStarted.load(); };
	bool getisStarted() const { return m_isStarted.load(); };

	std::string getConsumerGroup() { return consumerGroup; };
	std::string getConsumerGroup() const { return consumerGroup; };


		void setConsumerGroup(std::string consumerGroup) { consumerGroup = consumerGroup; };

		std::list<MQMessageExt> getMsgList() { return msgList; };

	std::list<MQMessageExt> getMsgList() { return msgList; };
	std::list<MQMessageExt> getMsgList() const { return msgList; };

	MQMessageQueue getMq() { return mq; };
	MQMessageQueue getMq() const { return mq; };


		} // namespace rocketmq

		#endif No newline at end of file

Conversation

hugoasdf commented Jul 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief changelog

Verifying this change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jovany-wang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShannonDing commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

hugoasdf commented Jul 1, 2019 •

edited

Loading

ShannonDing commented Mar 17, 2020 •

edited

Loading