Skip to content

Fix hostname not being set for worker nodes in freestyle jobs#328

Merged
albertvaka merged 3 commits into
masterfrom
albertvaka/worker-hostname-env-var-fallback
Feb 10, 2023
Merged

Fix hostname not being set for worker nodes in freestyle jobs#328
albertvaka merged 3 commits into
masterfrom
albertvaka/worker-hostname-env-var-fallback

Conversation

@albertvaka

Copy link
Copy Markdown
Collaborator

What does this PR do?

Try to read hostname from the environment if not in updatedBuildData.

If a job has no child stages (eg: in a freestyle job), updatedBuildData in DatadogWebhookBuildLogic will never actually be updated because DatadogWebhookPipelineLogic didn't run. Because of this even if the job runs in a worker node everything we read from updatedBuildData will be the main node's info.

When this happens, this PR adds a workaround to read the hostname from the DD_CI_HOSTNAME env var directly from DatadogWebhookBuildLogic.

Alternate Designs

Ideally we should have a single way to get the workers info that works both for pipelines and for single jobs. But the logic is too convoluted and I'm not confident enough to do a bigger change.

Verification Process

Run a freestyle job on a worker with the DD_CI_HOSTNAME variable set, check that the host is set properly in the payloads.

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.

If a job has no child stages (eg: in a freestyle job), updatedBuildData
will never actually be updated, so even if we are in a worker node
everything we read from it will be the main node's info. When this
happens, try to read the hostname from the DD_CI_HOSTNAME env var directly.
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/model/StepData.java Outdated
buildSpan.putMeta(CITags._DD_HOSTNAME, HOSTNAME_NONE);
String envHostnameOrNone = HOSTNAME_NONE;
try {
Map<String, String> env = run.getEnvironment(new LogTaskListener(logger, Level.INFO));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is trick to have the env from the worker? I didn't have this LogTaskListener class in my radar.

@albertvaka albertvaka Feb 9, 2023

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

You can also do getEnv() which does this under the hood but it is now deprecated in favor of this. The listener thing is only used to log if there's an error.

@albertvaka
albertvaka merged commit e4b2e23 into master Feb 10, 2023
@albertvaka
albertvaka deleted the albertvaka/worker-hostname-env-var-fallback branch February 10, 2023 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants