jenkinsci / opentelemetry-plugin

Monitor and observe Jenkins with OpenTelemetry.
https://plugins.jenkins.io/opentelemetry/
Apache License 2.0
93 stars 47 forks source link

Don't add a null attribute if Jenkins can't resolve Computer host name #748

Closed tobyp closed 7 months ago

tobyp commented 7 months ago

I've observed an error while testing using mvn hpi:run in a somewhat unusual network setup. It seems that Jenkins was unable to resolve a hostname for an agent in Computer.getHostName, and returned null. [This possibility is documented in the API](https://javadoc.jenkins.io/hudson/model/Computer.html#getHostName()).

This was attached as a span attribute, even though attributes may not be null, which caused an Exception.

Unfortunately I did not keep a copy of the exception I saw in the log listener I had attached to opentelemetry plugin, and I would rather not roll back (mvn hpi:run is very slow for me). I hope the two links above make the situation clear enough to accept the change regardless.

Testing done

Testing was done with mvn hpi:run, configuring the plugin, and running a build. Without this change, an error occurred. With this change, the error no longer occurred. I'm not sure how a testcase could be implemented, because I don't know how to force Jenkins to create a Computer with unresolvable hostname.

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [ ] Ensure you have provided tests - that demonstrates feature works or fixes the issue
tobyp commented 7 months ago

Oh, and also - since the MonitoringComputerListener only adds this attribute if there is no OpenTelemetryAttributesAction on the computer, I also added the JenkinsOtelSemanticAttributes.JENKINS_COMPUTER_NAME attribute, since that would have been set by MonitoringComputerListener, and is always available.