RUM-7562: Provide SDK commit SHA1 to the Shopist App#2993
Conversation
|
🎯 Code Coverage 🔗 Commit SHA: 3918198 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2993 +/- ##
===========================================
- Coverage 71.34% 71.27% -0.06%
===========================================
Files 859 859
Lines 31309 31314 +5
Branches 5275 5275
===========================================
- Hits 22335 22319 -16
- Misses 7505 7522 +17
- Partials 1469 1473 +4 🚀 New features to boost your workflow:
|
|
LGTM, but question is - will you have access to this BuildConfig field from the Shopist app ? |
kikoveiga
changed the title
kikoveiga
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| buildConfigField( | ||
| "String", | ||
| "SDK_COMMIT_SHA1", | ||
| "\"${providers.exec { | ||
| commandLine("git", "rev-parse", "HEAD") | ||
| }.standardOutput.asText.get().trim()}\"" |
There was a problem hiding this comment.
The Git SHA is resolved eagerly by calling providers.exec { … }.standardOutput.asText.get() while configuring SDK_COMMIT_SHA1. When the configuration cache is enabled, Gradle will cache the generated value and skip rerunning this block on subsequent builds as long as the script itself is unchanged. After checking out a different commit, the build will therefore reuse the old cached value and embed an outdated SHA in BuildConfig, which defeats the purpose of exposing the current commit to the app. Consider computing the value at task execution time or wiring the current HEAD into the cache key so configuration cache reuse cannot return a stale SHA.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is actually a valid note. Although we care only about CI builds, so we shouldn't be hit by Configuration Cache there since we don't reuse it so far between the builds (unlike Build Cache), but we may do it in the future.
On another hand, we don't want to run git command for every local build as well, since it may steal some time (although on my machine it takes 0.03s).
Did you consider any other options of adding commit hash to the build?
There was a problem hiding this comment.
This is actually a valid note. Although we care only about CI builds, so we shouldn't be hit by Configuration Cache there since we don't reuse it so far between the builds (unlike Build Cache), but we may do it in the future.
I tested what Codex mentioned and it didn't happen. The command is rerun everytime the app is built: even with ./gradlew assembleDebug --configuration-cache and when there are no changes to the dd-sdk-android-core module, which is where we do this BuildConfig field.
Did you consider any other options of adding commit hash to the build?
The first implementation was different, it added a property in the .pom file of the SDK release. This was better on the SDK side, as it only ran the git command once per release, but it looked much more complex to parse this .pom file from the Shopist app.
There was a problem hiding this comment.
Do you think the current solution needs to be optimized, or is the time difference insignificant?
There was a problem hiding this comment.
Time is insignificant, so let's keep it like that. My concern is only about the caching.
We could write hash to the file using a dedicated task (it can be invoked only from publish jobs, it is not really needed for the local builds) and read it from file declaring it as an input, so that Gradle know that input is invalidated.
But let's keep it simple for now.
5 participants
What does this PR do?
Adds the latest Git commit SHA1 to the
dd-sdk-android-corebuild.gradlefile as aBuildConfigconstant, so the Shopist app can send it as a global RUM attribute.Motivation
The Shopist app uses SNAPSHOT versions of the SDK. When multiple snapshot builds are published to the repository, it becomes difficult to determine which exact commit was used in a particular release.
Review checklist (to be filled by reviewers)