Re-factor network_event_log

chromeos/network/network_handler_callbacks.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chromeos/network/network_handler_callbacks.h"
 
 #include "base/logging.h"
 #include "base/values.h"
 #include "chromeos/network/network_event_log.h"
 
 namespace chromeos {
 namespace network_handler {
 
-const char kLogModule[] = "ShillError";
-
 // These are names of fields in the error data dictionary for ErrorCallback.
 const char kErrorName[] = "errorName";
 const char kErrorMessage[] = "errorMessage";
 const char kServicePath[] = "servicePath";
 
 base::DictionaryValue* CreateErrorData(const std::string& service_path,
                                        const std::string& error_name,
                                        const std::string& error_message) {
   base::DictionaryValue* error_data(new base::DictionaryValue);
   error_data->SetString(kErrorName, error_name);
   error_data->SetString(kErrorMessage, error_message);
   if (!service_path.empty())
     error_data->SetString(kServicePath, service_path);
   return error_data;
 }
 
-void ShillErrorCallbackFunction(const std::string& module,
-                                const std::string& path,
+void ShillErrorCallbackFunction(const std::string& path,
                                 const ErrorCallback& error_callback,
                                 const std::string& error_name,
                                 const std::string& error_message) {
-  std::string error = "Shill Error in " + module;
+  std::string error = "Shill Error";
   if (!path.empty())
     error += " For " + path;
-  error += ": " + error_name + " : " + error_message;
-  LOG(ERROR) << error;
-  network_event_log::AddEntry(kLogModule, module, error);
+  error += ": " + error_name;
+  NET_LOG_ERROR(error, error_message);

[pneubeck (no reviews), 2013/05/13 13:16:25]

the file/line that is logged by NET_LOG_ERROR will not be helpful in this case
and is not a replacement for |module|.
Instead |module| should be replaced by base::Location and callers should use
FROM_HERE.

[stevenjb, 2013/05/13 19:36:39]

I considered that, but I really think that is overkill. We pretty rarely make
the same shill call from multiple places; I do not think there is likely to be
any ambiguity. We can always add info to the callback and call
network_event_log::AddEntry directly if we need to.

[pneubeck (no reviews), 2013/05/16 08:21:07]

I don't see a overkill on the perfomance side: the copy of a base::Location
should be negligible compared to the string operations.

Code complexity wise: arguable.

Since you seem to use the log messages currently more than I, please feel free
to leave it as is.

+  LOG(ERROR) << error << " : " << error_message;
   if (error_callback.is_null())
     return;
   scoped_ptr<base::DictionaryValue> error_data(
       CreateErrorData(path, error_name, error_message));
   error_callback.Run(error_name, error_data.Pass());
 }
 
 } // namespace network_handler
 }  // namespace chromeos