search

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.2k stars 598 forks source link

Logger does not use `process_host.display_name` when setting `hostname` in the logs #1696

Open flivni opened 1 year ago

flivni commented 1 year ago

The logger does not use process_host.display_name when setting hostname in the logs. Instead it uses Socket.gethostname via a call to NewRelic::Agent::Hostname.get.

I believe it is a bug. (In another issue, a New Relic developer assumes the feature already acts this way when she says "Alternatively, we have the ability to set the app name (entity.name in the default attributes) and hostname by configuration in either the newrelic.yml file or by environment variable"). https://github.com/newrelic/newrelic-ruby-agent/issues/1141

You can see the offending code here: https://github.com/newrelic/newrelic-ruby-agent/blob/74737a46a141d1d7e251c06f8db157743b02f3d8/lib/new_relic/agent/logging.rb#L51

I think the fix is to just replace with: Agent.config[:'process_host.display_name']

I can create a PR if that would be helpful.

Thanks!

workato-integration[bot] commented 1 year ago

https://issues.newrelic.com/browse/NEWRELIC-5867

hannahramadan commented 1 year ago

Hi @flivni! Thanks for calling attention to this and providing a suggested fix. process_host.display_name isn’t being applied to log events. This is a change we’d like to make!

The features for log events need to be consistent across all New Relic’s agents. Since this is a new feature, we will need to bring this back to our team before making updates as there are a few other changes that need to take place and processes to go through. We will follow up with updates.

Thanks again for reporting this!

flivni commented 1 year ago

Thanks @hannahramadan. I look forward to the updates.