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.

Sign up for GitHub

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

Jump to bottom

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

Open
flivni opened this issue Dec 13, 2022 · 6 comments
Open

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

flivni opened this issue Dec 13, 2022 · 6 comments
Assignees
@kaylareopelle
Labels
community To tag external issues and PRs submitted by the community med impact new functionality trivial
Milestone
Feature smash! Too!

Comments

@flivni
Copy link

flivni commented Dec 13, 2022 •
edited
Loading

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"). #1141

You can see the offending code here:

newrelic-ruby-agent/lib/new_relic/agent/logging.rb

Line 51 in 74737a4

add_key_value(message, HOSTNAME_KEY, Hostname.get)

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
Copy link

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

@github-actions github-actions bot added the community To tag external issues and PRs submitted by the community label Dec 13, 2022
@hannahramadan
Copy link
Contributor

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
Copy link
Author

flivni commented Dec 15, 2022

Thanks @hannahramadan. I look forward to the updates.

@fallwith fallwith added the estimate Issue needing estimation label Jun 6, 2023
@kbford56 kbford56 added 3 Story Point Estimate and removed estimate Issue needing estimation labels Aug 10, 2023
@kaylareopelle kaylareopelle self-assigned this Mar 25, 2025
@kaylareopelle kaylareopelle moved this from Triage to In Progress in Ruby Engineering Board Mar 25, 2025
kaylareopelle added a commit that referenced this issue Mar 25, 2025
@kaylareopelle
Set hostname for logs using config
f754c80 
The default value of 'process_host.display_name' is the same as
NewRelic::Agent::Hostname.get, so this change will only impact users who
have set a unique value on the config.

Resolves #1696
@kaylareopelle kaylareopelle mentioned this issue Mar 25, 2025
Closed
@kaylareopelle
Copy link
Contributor

Hi @flivni! We're coming back to this issue. Thank you for your patience. Is this still a problem you'd like to see resolved?

@flivni
Copy link
Author

flivni commented Mar 25, 2025

Yup. I can create a PR if you'd like.

@kaylareopelle
Copy link
Contributor

Thank you, @flivni! I drafted #3096 for this feature, please take a look when you can.

Unfortuantely, it needs to stay in draft mode for a while. I'm starting the architectural conversations my colleague mentioned in her last comment, and will keep you posted on the progress.

WRT this quote:

"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"). #1141

I misspoke here. The process_host.display_name configuration option has specific UI ramifications, and is currently included only on Transactions as displayHost. The intention was not to use it for the service metadata on logs. I apologize for sharing inaccurate information.

While we're having the internal conversations, there are some ways you can add the process_host.display_name value to your log events if you use APM log forwarding.

You could update one of these configs using your newrelic.yml file:

  • application_logging.forwarding.custom_attributes: { displayHost: <%= NewRelic::Agent.config[:'process_host.display_name'] %> } (docs)
  • application_logging.forwarding.forwarding.labels.enabled could be used to attach all labels to your logs if you choose to add the process host as a label with the :labels configuration option (docs). There is also an option to exclude certain labels from forwarding.

Unfortunately, these options do not apply to the DecoratingLogger linked above, so if you are using that strategy to send logs to New Relic, I don't have any workarounds to recommend at the moment.

@kaylareopelle kaylareopelle moved this from In Progress to In Review / Blocked in Ruby Engineering Board Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kaylareopelle kaylareopelle

Labels
community To tag external issues and PRs submitted by the community med impact new functionality trivial
Projects
Ruby Engineering Board
Status: In Review / Blocked
Milestone
Feature smash! Too!
Development

Successfully merging a pull request may close this issue.

Set hostname for logs using config newrelic/newrelic-ruby-agent
6 participants
@fallwith @flivni @kbford56 @hannahramadan @kaylareopelle @chynesNR