Avoid calculating _dd.hostname for Jenkins workers #184
Conversation
drodriguezhdez
added
the
changelog/Fixed
| // If the NodeName == master, we don't set _dd.hostname. It will be overridden by the Datadog Agent. (Traces are only available using Datadog Agent) | ||
| // If the NodeName != master, we set _dd.hostname to 'none' explicitly, cause we cannot calculate the worker hostname. | ||
| if(!"master".equalsIgnoreCase(nodeName)) { | ||
| buildSpan.setTag(CITags._DD_HOSTNAME, HOSTNAME_NONE); |
There was a problem hiding this comment.
What's the impact of setting none here? This has a pretty big impact for metrics for instance (as it would create a new host with the "none" name), but this should be for traces only in that situation. Are we ok with setting a dummy value?
There was a problem hiding this comment.
As far as we know, APM doesn't create hosts based on the _dd.hostname tag. In this case, the value none is a special value for APM that is excluded from per-host billing. Additionally, if we don't set it and we let the agent put the master hostname, it'd be worse from a user perspective because we will be showing wrong information.
There was a problem hiding this comment.
Perfect, I didn't know about billing implications
|
|
||
| final String nodeHostname = buildData.getHostname("").isEmpty() ? pipelineData.getHostname("") : buildData.getHostname(""); | ||
| buildSpan.setTag(CITags._DD_HOSTNAME, nodeHostname); | ||
| // If the NodeName == master, we don't set _dd.hostname. It will be overridden by the Datadog Agent. (Traces are only available using Datadog Agent) |
There was a problem hiding this comment.
That's not true when the plugin is configured in HTTP mode. In that case, no hostname will be reported.
There was a problem hiding this comment.
Yeah, Jenkins traces collection is only available if the plugin is configured in a Datadog Agent mode: (link), so we can assume that's correct, right?
There was a problem hiding this comment.
Oh right, completely forgot about that. Was there any plan to allow sending traces via HTTP? If yes we should add some additional logic here to fail if the plugin runs in http mode. That will help in the future
Requirements for Contributing to this repository
What does this PR do?
This PR changes the logic of how we calculate the
_dd.hostnamefor the traces:master, we don't set_dd.hostname. It will be overridden by the Datadog Agent.master, we set_dd.hostnametononeexplicitly, cause we cannot calculate the worker hostname.This change is needed because we cannot always calculate the hostname of a Jenkins worker. For the main node, we will use the Datadog Agent logic to calculate the correct value.
Description of the Change
Alternate Designs
Possible Drawbacks
Verification Process
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.