newrelic / newrelic-ruby-agent

New Relic RPM Ruby Agent
https://docs.newrelic.com/docs/apm/agents/ruby-agent/getting-started/introduction-new-relic-ruby/
Apache License 2.0
1.19k stars 599 forks source link

Add log forwarding support for for `semantic_logger` #2453

Open kaylareopelle opened 4 months ago

kaylareopelle commented 4 months ago

semantic_logger is a popular Ruby logging library. Currently, the Ruby agent does not support automatic forwarding for logging libraries that format their logs in JSON.

The work in #2399 is similar to this, since both libraries use JSON-style logs.

While working on this, also verify whether rails_semantic_logger needs additional work to support forwarding.

workato-integration[bot] commented 4 months ago

https://new-relic.atlassian.net/browse/NR-230847

jdelStrother commented 4 weeks ago

I don't suppose there's any progress or timeline on this? I'm looking at switching our regular Rails logs to one of the structured logging gems, and was leaning towards semantic_logger.

fallwith commented 4 weeks ago

I don't suppose there's any progress or timeline on this? I'm looking at switching our regular Rails logs to one of the structured logging gems, and was leaning towards semantic_logger.

Hi @jdelStrother! Your supposition is valid and we don't have any progress or estimates to share at this time.

But here are two things we can share:

  1. We have started looking at supporting Logstash via the logstasher gem as per issue #2399. I mention this because logstasher is in the same category of logger as semantic_logger in that it doesn't directly utilize the Ruby Logger class when delivering its logger functionality. My hope is that once the first non-Logger-class based gem has been instrumented, all subsequent ones will be a bit easier to go after.
  2. We factor in customer demand when prioritizing work. I will use your comment here as evidence of such demand for semantic_logger and put it in front of the team.
jon-rse commented 3 weeks ago

Semantic Logger has a New Relic appender which obviates the need for explicit support from New Relic. It was merged last year in 4.13.0. https://github.com/reidmorrison/semantic_logger/pull/248 It isn't mentioned on the Semantic Logger site, which made finding out about it difficult. Im not particularly happy with how the json is structured. @jdelStrother You could experiment with it while you wait for explicit support from NR.

Here's how I'm using it on Heroku:

   if ENV['RAILS_LOG_TO_STDOUT'].present?
      $stdout.sync = true
      config.rails_semantic_logger.add_file_appender = false
      config.semantic_logger.add_appender(io: $stdout, formatter: config.rails_semantic_logger.format)
      config.semantic_logger.add_appender(appender: :new_relic_logs, level: :info)
    end
jdelStrother commented 3 weeks ago

@jon-rse ooh, thanks - will take a look

jdelStrother commented 2 weeks ago

@jon-rse I see what you mean about the json mess. I find I need to switch to NewRelic's overly-verbose log_summary view to get the information I need, which is really clunky. I kind of wonder if it would be better if the message that was sent to NewRelic wasn't raw json, but instead was a string with interpolated payload values that you can later add query-time parsing rules for. I'm not entirely sure though - would welcome some guidance from NewRelic on the best approach.

hannahramadan commented 2 weeks ago

@jdelStrother One option that we're currently implementing with another json logging library, LogStasher, is adding key/values as attributes on the log event, which show up as filterable values in the UI. LogStasher events don't include certain attributes like message unless added by a user, so we omit things like message if there isn't one. What do you think of this approach? Here's an example of how this appears in the New Relic UI.

Screenshot 2024-07-03 at 1 28 14 PM
jdelStrother commented 1 week ago

(Apologies if we're getting off topic here - some of it is possibly more about the NewRelic Logs UI. But hopefully it's still relevant, since it might just be fixed by sending differently structured log data...)

I kind of miss my old-fashioned plain dumb text log workflow, where I could rely on having a single message column containing all the useful information, regardless of whether the log came from nginx or Rails:

dumb-text

When tracing an error in a specific transaction I often don't know what metadata columns might be needed ahead of time. With structured logs, if the app logs something like logger.error("Service failed", service: "foo", reason: "bar", retry_count: 3), the NewRelic 'message' column just gives me the bare "Service failed" message and I have to either click on it to open the details pane, or keep adding more and more columns to the log view. NewRelic's log_summary column does seem like the dumb plain-text replacement that I want, except it has so much extra irrelevant detail that each log spans 3 lines on my 4k display.


SemanticLogger's NewRelic formatter nests both the log message and payload metadata under the message key. So, if I call logger.info("Hello world", color: "red") the data sent to NewRelic is something along the lines of:

{
hostname:"app1-1",
level:"INFO",
logger.name:"Rails",
message.message:"Hello world",
message.payload.color: "red",
newrelic.source:"logs.APM",
timestamp:1720081806337
}

and I need to show the message.message column to view log messages. It sounds like that's maybe just a quirk in SemanticLogger's current implementation though... maybe to avoid collision with keys that NewRelic's already using at the top level? Not entirely sure.

hannahramadan commented 1 week ago

We hear you on the question of how to organize json logs in the UI—we ran into similar challenges with LogStasher instrumentation.

We considered a few different ways to strucutre the data, but decided we didn't want to make decisions for all customers on field reporting or appearance. So instead we decided it would be best to report logs as close are we could to their original form, which means key/value pairs and filterable log attributes. SemanticLogger's instrumentation is inline with how we'd intend nested keys to be reported. Here is where we do it for LogStasher.

We have an open feature request to add custom attributes to logs, which would allow you to create an interpolated string with the values you want, for example. If you'd like to show support for this, I'd encourage you to comment or 'react' on that PR. That could help bump it in priority. We also always welcome PRs ◡̈