Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrating from cerr/cout #412

Open
matelich opened this issue Mar 30, 2024 · 1 comment
Open

Migrating from cerr/cout #412

matelich opened this issue Mar 30, 2024 · 1 comment
Labels
question Further information is requested

Comments

@matelich
Copy link

matelich commented Mar 30, 2024

It's late in the project and I don't feel like rewriting all my cerr/couts at this time. I've adapted CrowCpp's logging scheme to work with quill and I'd like feedback on improvements or major concerns.

namespace Pungi
{
   //inspired by crow::logger, for quill
   class logger
   {
   public:
      logger(const quill::MacroMetadata* m) :
         metadata_(m)
      {}
      ~logger()
      {
         quill::get_root_logger()->log<false>(quill::LogLevel::None, metadata_, stringstream_.str());
      }

      template<typename T>
      logger& operator<<(T const& value)
      {
         stringstream_ << value;
         return *this;
      }

   private:
      std::ostringstream stringstream_;
      const quill::MacroMetadata* metadata_;
   };
}
#define COMBINE1(X,Y) X##Y  // helper macro
#define COMBINE(X,Y) COMBINE1(X,Y)
#define MY_MACRO_METADATA(log_level) \
  static constexpr quill::MacroMetadata COMBINE(macro_metadata,__LINE__)                     \
  {                                                                                  \
    __FILE__ ":" QUILL_STRINGIFY(__LINE__), __FUNCTION__, "{}", nullptr, log_level,  \
      quill::MacroMetadata::Event::Log, false, false                                 \
  }
#define CERR MY_MACRO_METADATA(quill::LogLevel::Warning); if (true) Pungi::logger(&COMBINE(macro_metadata,__LINE__))
#define COUT MY_MACRO_METADATA(quill::LogLevel::Info); if (true) Pungi::logger(&COMBINE(macro_metadata,__LINE__))

Bulk replaces like s/std::cerr/CERR/ and s/ << std::endl;/;/ seem to be doing the trick.
Issues with things like ostream_iterator need special handling and use in headers is risky with name conflicts, but this is just a stepping stone to just using Quill directly.

Also can't be used like

if (foo)
  COUT << "bar";

due to scoping but that's easily fixed with {}.

At this point, I don't care about disabling them by log levels, but that is an obvious extension.

   LOG_INFO("Starting up");
   COUT << "Starting up";

outputs

06:47:07.432243191 [58624] main.cpp:195 LOG_INFO root Starting up
06:47:07.432397237 [58624] main.cpp:196 LOG_INFO root Starting up

@odygrd
Copy link
Owner

odygrd commented May 16, 2024

sorry for the delay ...

The problem doing this is that you are formatting everything in the thread that is issuing the log. The library is designed to reduce the overhead by taking a binary copy of the types and dedicating the formatting to a backend logging thread.

If you are okay with the additional overhead of formatting the types to strings on your caller thread then what you have done is okay

@odygrd odygrd added the question Further information is requested label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants