Skip to content

Don't log SEVERE exception if /bin/hostname is missing#339

Merged
sarah-witt merged 1 commit into
jenkinsci:masterfrom
Vlatombe:missing-hostname-bin-polluting-logs
May 15, 2023
Merged

Don't log SEVERE exception if /bin/hostname is missing#339
sarah-witt merged 1 commit into
jenkinsci:masterfrom
Vlatombe:missing-hostname-bin-polluting-logs

Conversation

@Vlatombe

@Vlatombe Vlatombe commented May 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Instead, log to FINE level, swallow the exception, and try the next method.

I'm running the plugin in a container where /bin/hostname is not present, and the plugin is spamming the logs with the following

SEVERE	o.d.j.p.datadog.DatadogUtilities#severe: Failed to obtain UNIX hostname
INFO	o.d.j.p.datadog.DatadogUtilities#severe: Failed to obtain UNIX hostname: java.io.IOException: Cannot run program "/bin/hostname": error=2, No such file or directory
 	at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1128)
 	at java.base/java.lang.ProcessBuilder.start(ProcessBuilder.java:1071)
 	at java.base/java.lang.Runtime.exec(Runtime.java:592)
 	at java.base/java.lang.Runtime.exec(Runtime.java:451)
 	at org.datadog.jenkins.plugins.datadog.DatadogUtilities.getHostname(DatadogUtilities.java:514)
 	at org.datadog.jenkins.plugins.datadog.events.AbstractDatadogSimpleEvent.<init>(AbstractDatadogSimpleEvent.java:37)
 	at org.datadog.jenkins.plugins.datadog.events.ConfigChangedEventImpl.<init>(ConfigChangedEventImpl.java:38)
 	at org.datadog.jenkins.plugins.datadog.listeners.DatadogSaveableListener.onChange(DatadogSaveableListener.java:79)
 	at hudson.model.listeners.SaveableListener.fireOnChange(SaveableListener.java:82)
 	at hudson.model.Descriptor.save(Descriptor.java:913)
[...]

Requirements for Contributing to this repository

  • Fill out the template below. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • The pull request must only fix one issue at the time.
  • The pull request must update the test suite to demonstrate the changed functionality.
  • After you create the pull request, all status checks must be pass before a maintainer reviews your contribution. For more details, please see CONTRIBUTING.

What does this PR do?

Description of the Change

Alternate Designs

Possible Drawbacks

Verification Process

Additional Notes

Release Notes

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@Vlatombe
Don't log SEVERE exception if /bin/hostname is missing
a2fc22b 
Instead, log to FINE level, swallow the exception, and try the next
method.
@Vlatombe Vlatombe changed the title Don't log SEVERE exception if /bin/hostname is missing Don't log SEVERE exception if /bin/hostname is missing May 9, 2023
jglick
jglick reviewed May 9, 2023
View reviewed changes

@jglick jglick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it not just

return InetAddress.getLocalHost().getHostName();

to begin with?

@Vlatombe

Vlatombe commented May 10, 2023

Copy link
Copy Markdown
Member Author

From what I read InetAddress.getLocalHost().getHostName() can sometimes return the loopback device, which is not really useful. So I can understand why calling hostname would be preferred.
The other problem here is that there are plenty of calls in the plugin where EnvVars refers to agent environment variables, but this lookup never looks at the controller environment variables, which would be also an easy way to look up the host name if available.

@sarah-witt

sarah-witt commented May 10, 2023

Copy link
Copy Markdown
Collaborator

Thanks for the contribution @Vlatombe!

this lookup never looks at the controller environment variables, which would be also an easy way to look up the host name if available.

That sounds like a good addition. Do you have a specific controller environment variable in mind?

@Vlatombe

Vlatombe commented May 10, 2023

Copy link
Copy Markdown
Member Author

@sarah-witt I filed it as a separate PR -- #340

@sarah-witt
sarah-witt merged commit c1d5c6c into jenkinsci:master May 15, 2023
@Vlatombe
Vlatombe deleted the missing-hostname-bin-polluting-logs branch May 16, 2023 07:29
@Dohbedoh

Dohbedoh commented May 22, 2023

Copy link
Copy Markdown

@sarah-witt any chance to get this released ?

@sarah-witt

sarah-witt commented May 22, 2023

Copy link
Copy Markdown
Collaborator

Hi @Dohbedoh, we plan to make a release this week!

@Dohbedoh

Dohbedoh commented May 22, 2023

Copy link
Copy Markdown

Hi @sarah-witt, awesome! thanks

@sarah-witt sarah-witt added the changelog/Fixed Fixed features results into a bug fix version bump label May 23, 2023
@sarah-witt

sarah-witt commented May 24, 2023

Copy link
Copy Markdown
Collaborator

@Dohbedoh datadog plugin version 5.4.1 was released earlier today: https://plugins.jenkins.io/datadog/releases/!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sarah-witt sarah-witt sarah-witt approved these changes
+1 more reviewer
@jglick jglick jglick left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

changelog/Fixed Fixed features results into a bug fix version bump

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Vlatombe @sarah-witt @Dohbedoh @jglick