ess-dmsc / graylog-logger Goto Github PK

When quitting my application (CTRL+C), I am getting the following error.

terminate called after throwing an instance of 'std::system_error'
  what():  Resource deadlock avoided
Abort (core dumped)

It occurs in the GraylogConnection.cpp file on line 64, at connectionThread.join().

void GraylogConnection::EndThread() {
    closeThread = true;
    connectionThread.join();
    if (NULL != conAddresses) {
        freeaddrinfo(conAddresses);
        conAddresses = NULL;
    }
}

In my code, I am opening the Graylog connection like this;

void GrayLogging::open()
{
    handler = new GraylogInterface("myhost"), 11111);

    Log::RemoveAllHandlers();
    Log::AddLogHandler(handler);
    Log::SetMinimumSeverity(Severity::Informational);
}

And when receiving a quit signal, I am deleting the handler, which calls the destructor.

void GrayLogging::close()
{
    delete handler;
}

Do you have any idea how to fix this?

PIMPL could be used in GraylogConnection.hpp to avoid needing to expose asio in the library headers.

Perhaps I'm misunderstanding how this intended to be built but I found that when I build the latest version with conan it doesn't find the fmt library even when I have the fmt package in my conan dependencies.

If I add this to cmake/Findfmt.cmake then it works: https://github.com/ceph/ceph/blob/master/cmake/modules/Findfmt.cmake
Unfortunately that repo is LGPL which is possibly incompatible with your license but we can probably write something from scratch or source from another project to get the same result.

edit: I also get long compiler errors from the use of minimal::apply in FmtMsg using gcc 6.5.0 with -std=c++1z which I would have expected to work if only C++14 features are required.

I'm really more of a C programmer so once things get heavy with templates I'm often a bit lost. But is there a reason to use minimal::apply? My best guess was you had trouble capturing args so ended up using a tuple. But I found that this compiles under --std=gnu++14 and seems to run correctly. Is what I've done here OK or are there cases it wouldn't work?

  template <typename... Args>
  void fmt_log(const Severity Level, std::string Format, Args... args) {
    if (int(Level) > int(MinSeverity)) {
      return;
    }
    auto ThreadId = std::this_thread::get_id();
    Executor.SendWork([=]() {
      LogMessage cMsg(BaseMsg);
      cMsg.SeverityLevel = Level;
      cMsg.Timestamp = std::chrono::system_clock::now();
      try {
          cMsg.MessageString = fmt::format(Format, args...);
      } catch (fmt::format_error &e) {
          cMsg.SeverityLevel = Log::Severity::Error;
          cMsg.MessageString = fmt::format("graylog-logger internal error. Unable to format "
                              "the string \"{}\". The error was: \"{}\".",
                              Format, e.what());
      }
      std::ostringstream ss;
      ss << ThreadId;
      cMsg.ThreadId = ss.str();
      for (auto &ptr : Handlers) {
        ptr->addMessage(cMsg);
      }
    });
  }

Happy to prepare a PR if there's a solution that works for everyone here.

There is currently some messy code in the file writer and forwarder for logging to file, to console or to graylog. It seems reasonable to support different logging methods as institutions other than ESS may not want to be bound to Graylog.
One nice way of cleaning up that code would be to use https://github.com/gabime/spdlog and incorporate graylog-logger as a plugin. Any thoughts on this @SkyToGround?

It's probably better to just say "Information" or even "Info"

I was just debugging a startup issue in our program where it locks up. It was hard to figure out from the logs (configured to output to both Console and GELF) where it got to because several messages remain in the buffer and are never flushed out.

It would be nice if I could manually flush the logs out, or even better if the logging thread flushed them after a certain timeout. I would be happy to prepare a PR if you are interested but I would probably need a little guidance around the best way to add it to the current architecture.