-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Improve build scan metadata #44247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve build scan metadata #44247
Conversation
Pinging @elastic/es-core-infra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but left some suggestions.
While at it it would be great to have information about the CI worker this is what I most often have to go back to jenkins for.
Jenkins sets NODE_LABELS
we could prefix them and add them as tags here.
Instead of an included file, I'm wondering if it would be better to start a base plugin that would apply configuration to the root plugin and have this live in buildSrc along with everything else. We're starting to have more and more configuration that applies to the root project only.
gradle/build-scan.gradle
Outdated
} | ||
|
||
// Jenkins-specific build scan metadata | ||
if (System.getenv('JENKINS_URL')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could check if this contains elastic.co
to make sure it's our CI and accept the terms of service if so so no additional property needs to be passed in Jenkins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventually we'll be pushing scans to Gradle Enterprise which doesn't require accepting the ToS so I think that might be unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone ahead and implemented this. I agree, it'd be nice to ditch the need to pass the ToS param in Jenkins for the time being.
Indeed. Is it the labels specifically that you find yourself looking for? Is the information in the infrastructure section of the build scan insufficient? |
Yeah, I definitely didn't want to expand the root |
I've added these as custom values rather than tags to try and minimize a bit of noise as some of these are long (like the worker name) and there can be lots of them. We can still search and filter builds in Gradle Enterprise with this method though. So we could say "show me all build that ran on a worker with the |
@elasticmachine run elasticsearch-ci/packaging-sample |
@atorok here's an example of the node labels: https://scans.gradle.com/s/hnaify7kg33vg/custom-values#L2-L5 |
def jenkinsUrl = System.getenv('JENKINS_URL') ? new URL(System.getenv('JENKINS_URL')) : null | ||
|
||
// Accept Gradle ToS when project property org.elasticsearch.acceptScanTOS=true or this is an Elastic CI build | ||
if (jenkinsUrl?.host?.endsWith('elastic.co') || Boolean.valueOf(project.findProperty('org.elasticsearch.acceptScanTOS') ?: "false")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could just ditch the property, I don't see a reason to opt out of CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use this property to opt-in locally.
(cherry picked from commit 2797b24)
(cherry picked from commit 2797b24)
(cherry picked from commit 2797b24)
(cherry picked from commit 2797b24)
This PR does a couple of things:
First, we move the logic for some of our custom build scan into the build itself vs having this live in our Jenkins configuration. Having the conditional tagging logic in our Jenkins config is awkward for a few reasons.
Second, we've added some addition bits of information, mainly to fill in context gaps with the goal of making it less necessary to have to go back to the original Jenkins job page to get that information.