[Ns-developers] log time -> log context

[Tom Henderson]

Mathieu Lacage wrote:
> On Wed, 2008-05-21 at 19:35 +0100, Gustavo Carneiro wrote:
> 
>> I don't know what else to explain:
>>
>>   /**
>>    * Set the current node that is being simulated.
>>    * \warning this method has no effect on optimized NS-3 builds, as
>> it is used only for logging.
>>    * \param node   id of the current node, or -1 if none or not known
>>    */
>>   static void SetCurrentNode (int32_t node);
>>
>>
>> It's dead simple.  What might be missing perhaps is some guideline
>> explaining when the user is expected to call it.  Is that what you
>> mean?
> 
> What is "the current node" ? What is its relationship with the "nodeid
> context" embedded in each event ? How does each event get a "nodeid
> context" ? Where does it get it from ?
> 
> There are many types of "current node id". There is the one which is
> used to associate a "current node id" to the event scheduled with
> Simulator::Schedule. There is the one which is coming from the "current
> node id" of the event we are currently executing. There is the "current
> node id" used for logging. Explaining clearly the relationship between
> all these different types of "current node id" makes my brain hurt.
> 
>>         Using raw sockets outside of the context of an Application
>>         Start
>>         function makes little sense and is clearly not the way the
>>         system was
>>         designed to be used: yes, it can be made to work, yes, we have
>>         examples
>>         which do this, yes you like being able to do it but, as
>>         discussed a
>>         while ago in a bug report, _this is not the way the system was
>>         designed
>>         to be used_. If, when you do this, you do not get the best
>>         logging
>>         messages in the world without some extra work, such is life.
>>
>> About the extra work:
>>
>>  1. I am not convinced the extra work will be worth anything.  It
>> doesn't improve readability in my opinion;
> 
> It does not improve code readability. It makes the conceptual model of
> the API much more clear and defines precisely and uniquely what the
> "current node id" means by uniquely associating a single "current node
> id" to each event. 
> 
> What you are trying to suggest also goes against the design of the
> current API which is to move _all_ per-event arguments to the Schedule
> calls: we could have done what you suggested for all other arguments:
> 
> Simulator::SetNextDelay (Time delay);
> Simulator::SetNextArguments (...);
> 
> Whether or not the above is better than what we have now is not
> something I will argue about. What I will argue about is consistency of
> the design: we are not using this style of API in this header _now_ so,
> we should not use this style for new API in this header.
> 
>>  2. Adding extra Schedule calls will increasing the number of events
>> and decrease NS-3 performance.  I want the logging improvements to be
>> as little intrusive as possible, as all logging should be.
> 
> You are worried about minimizing the number of changes to the codebase.
> I am worried about making the API understandable and consistent.
> 
> Mathieu
> 
> 

What about asking each file to define an optional context that is 
meaningful to it, and leave it out where it doesn't make sense?

e.g.

in ipv4-l3-protocol.cc

NS_LOG_COMPONENT_DEFINE ("Ipv4L3Protocol");
#define APPEND_CONTEXT                                          \
   if (m_node)                                                   \
     {                                                           \
       std::clog << m_node->GetId () << " ";                     \
     }                                                           \

then in log.h

#define NS_LOG_FUNCTION_NOARGS()                                \
   do                                                            \
     {                                                           \
       if (g_log.IsEnabled (ns3::LOG_FUNCTION))                  \
         {                                                       \
           APPEND_TIME_PREFIX;                                   \
           APPEND_CONTEXT;                                       \
           std::clog << g_log.Name () << ":"                     \
                     << __FUNCTION__ << "()" << std::endl;       \
         }                                                       \
     }                                                           \
   while (false)