[
https://issues.apache.org/jira/browse/GEODE-10016?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17486932#comment-17486932
]
ASF GitHub Bot commented on GEODE-10016:
----------------------------------------
gaussianrecurrence commented on a change in pull request #918:
URL: https://github.com/apache/geode-native/pull/918#discussion_r799326466
##########
File path: cppcache/src/Log.cpp
##########
@@ -382,7 +383,8 @@ std::string Log::formatLogLine(LogLevel level) {
<< std::put_time(&tm_val, "%Y/%m/%d %H:%M:%S") << '.' <<
std::setfill('0')
<< std::setw(6) << microseconds.count() << ' '
<< std::put_time(&tm_val, "%z ") << g_hostName << ":"
- << boost::this_process::get_id() << " " << std::this_thread::get_id()
+ << boost::this_process::get_id() << " "
+ << DistributedSystemImpl::getThreadName(std::this_thread::get_id())
Review comment:
I am not totally sure about this, but changing the logging format might
break the GNMSG tool that @pdxcodemonkey implemented some time ago. I am not
sure what level of support you are intending to provide for this tool, but
might be good to make sure that the tool could handle both log formats
##########
File path: cppcache/src/ThreadPool.cpp
##########
@@ -16,6 +16,8 @@
*/
#include "ThreadPool.hpp"
+#include <sstream>
Review comment:
I think this might be a remaining of some dev debugging. Is this needed
anymore?
##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int
appdomainId) {
}
}
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {
Review comment:
Is there any particular reason why you would want to obtain the thread
name from another thread which is not the caller thread? Because if there is no
reason, I'd say it'd be best to get the thread ID of the current thread within
the function, and that would make syntax a little bit simpler.
##########
File path: cppcache/src/DistributedSystemImpl.cpp
##########
@@ -143,11 +147,24 @@ void DistributedSystemImpl::unregisterCliCallback(int
appdomainId) {
}
}
-void DistributedSystemImpl::setThreadName(const std::string& threadName) {
+std::string DistributedSystemImpl::getThreadName(std::thread::id id) {
+ std::string threadName = m_threadNames[id];
+ if (threadName == "") {
+ std::stringstream ss;
+ ss << id;
+ threadName = ss.str();
+ }
+ return threadName;
+}
+
+void DistributedSystemImpl::setThreadName(const std::string& threadName,
Review comment:
Same comment regarding thread ID as for getThreadName, IMHO I don't see
a reason why you would need to pass a thread ID here
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Use Thread Name In Log Messages
> -------------------------------
>
> Key: GEODE-10016
> URL: https://issues.apache.org/jira/browse/GEODE-10016
> Project: Geode
> Issue Type: Improvement
> Components: native client
> Reporter: Michael Martell
> Priority: Minor
> Labels: pull-request-available
>
> The native client logging system currently prints the threadId in all log
> messages. Since all internally created native client threads are named, we
> should print the threadName instead of threadId. This will be extremely
> helpful to understanding the flow of messages since there are many background
> threads in the native client.
> Note: Lots of log messages are running on an application thread which was not
> created internally by the native client. Messages running on these threads
> should continue to print the threadId.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
