Add fallback logic for job name and build tag in build data#368
Conversation
nikita-tkachenko-datadog
added
the
changelog/Fixed
| String jobName = job.getName(); | ||
| if (StringUtils.isNotBlank(jobName)) { | ||
| return jobName; | ||
| } | ||
| if (envVars != null) { | ||
| String envJobBaseName = envVars.get("JOB_BASE_NAME"); | ||
| if (StringUtils.isNotBlank(envJobBaseName)) { | ||
| return envJobBaseName; | ||
| } | ||
| String envJobName = envVars.get("JOB_NAME"); | ||
| if (StringUtils.isNotBlank(envJobName)) { | ||
| return envJobName; | ||
| } | ||
| } | ||
| return "unknown"; |
There was a problem hiding this comment.
For the JOB_BASE_NAME, wouldn't better to check if the env var is set, and if not, use the getJobName functions?
| String jobName = job.getName(); | |
| if (StringUtils.isNotBlank(jobName)) { | |
| return jobName; | |
| } | |
| if (envVars != null) { | |
| String envJobBaseName = envVars.get("JOB_BASE_NAME"); | |
| if (StringUtils.isNotBlank(envJobBaseName)) { | |
| return envJobBaseName; | |
| } | |
| String envJobName = envVars.get("JOB_NAME"); | |
| if (StringUtils.isNotBlank(envJobName)) { | |
| return envJobName; | |
| } | |
| } | |
| return "unknown"; | |
| if (envVars != null) { | |
| String envJobBaseName = envVars.get("JOB_BASE_NAME"); | |
| if (StringUtils.isNotBlank(envJobBaseName)) { | |
| return envJobBaseName; | |
| } | |
| } | |
| return getJobName(run, envVars); |
There was a problem hiding this comment.
I was afraid to change the logic too much, since I didn't know what the difference was between the API and the environment variables. That is why I added using the latter only as a fallback - to minimise the changes in behaviour.
If we can be sure that the env var never contradicts whatever is reported by the API, then surely we can change it.
What do you think?
There was a problem hiding this comment.
I would say that the API and env var should be aligned, but I don't have enough context of the Jenkins core implementation to put my finger on it 😅.
There was a problem hiding this comment.
Judging by the core code it does seem to be the case. Still I think we should err on the side of caution.
I have changed getBaseJobName to delegate to getJobName similarly to what you suggested, but still left the API as the higher priority option, so the behaviour is the following:
- Try to get the base name from the API
- Fall back to
JOB_BASE_NAME - Fall back to
getJobName
2 participants
Requirements for Contributing to this repository
What does this PR do?
Adds fallback behaviour to the logic that populates job name, job base name and build tag:
JOB_NAMEis used to fill itJOB_BASE_NAME(orJOB_NAMEif base name is not available) is used to fill itjenkins-${JOB_NAME}-${BUILD_NUMBER}is used to fill itCI Visibility backend requires these values to be non-empty. If any of them is not provided, corresponding events will be dropped.
Description of the Change
Alternate Designs
Possible Drawbacks
Verification Process
The new logic is covered with unit tests.
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.