Skip to content

DO NOT MERGE: verify Sentry upload on CI #5750

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.

Sign up for GitHub

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

Draft
mokagio wants to merge 1 commit into
base: ainfra-2790-remove-sentryproperties-in-favor-of-public-values-and-env
Choose a base branch
from
+39 −0
Draft

DO NOT MERGE: verify Sentry upload on CI #5750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .buildkite/commands/verify-sentry-upload.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/bash -eu

# Throwaway: proves the Sentry mapping upload works with the auth token coming from the
# environment rather than `secret.properties`. No other CI job uploads, because they all
# pass `-PskipSentryProguardMappingUpload=true`.
# Delete this script and its pipeline step once AINFRA-2790 is verified.

"$(dirname "${BASH_SOURCE[0]}")/restore-cache.sh"

echo "--- :rubygems: Setting up Gems"

install_gems

echo "--- :closed_lock_with_key: Installing Secrets"

bundle exec fastlane run configure_apply

echo "--- :key: Checking SENTRY_AUTH_TOKEN"

if [ -n "${SENTRY_AUTH_TOKEN:-}" ]; then
echo "SENTRY_AUTH_TOKEN is present"
else
echo "SENTRY_AUTH_TOKEN is absent"
fi
Comment on lines +18 to +24

@claude claude Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The token check itself is right: ${SENTRY_AUTH_TOKEN:-} keeps set -u happy and the value is never echoed, only its presence.

It's one variable short of covering the failure modes, though. build.gradle.kts:484-486 gates includeProguardMapping on System.getenv()["CI"].toBoolean(), and the plugin only registers the upload tasks when that is true — so if CI is unset or not literally true, the run fails with Task ... not found, indistinguishable from a wrong task name and nothing to do with the token. Buildkite exports CI=true, so this is about keeping the log self-diagnosing rather than a likely break, and it costs one line:

Suggested change
echo "--- :key: Checking SENTRY_AUTH_TOKEN"
if [ -n "${SENTRY_AUTH_TOKEN:-}" ]; then
echo "SENTRY_AUTH_TOKEN is present"
else
echo "SENTRY_AUTH_TOKEN is absent"
fi
echo "--- :key: Checking Sentry upload preconditions"
if [ -n "${SENTRY_AUTH_TOKEN:-}" ]; then
echo "SENTRY_AUTH_TOKEN is present"
else
echo "SENTRY_AUTH_TOKEN is absent"
fi
# The upload tasks only exist when `CI` is truthy, because that is what gates
# `includeProguardMapping` in `applyCommonSentryConfiguration`. An unexpected value here surfaces as
# "task not found", which looks nothing like a token problem.
echo "CI=${CI:-<unset>}"


# Minifies with R8 to produce each mapping, then uploads it. Deliberately not `assembleRelease`:
# packaging and signing are not needed. All three modules run because each declares its own
# Sentry project slug, and the token has to authenticate against all of them.
echo "--- :sentry: Uploading ProGuard mappings"

./gradlew \
:app:uploadSentryProguardMappingsRelease \
:automotive:uploadSentryProguardMappingsRelease \
:wear:uploadSentryProguardMappingsRelease
Comment on lines +31 to +34

@claude claude Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Without --continue, the first failing task aborts the build and the other two modules never run — which defeats the stated reason for running all three ("a typo in one would otherwise surface only at release"). If the app slug were wrong, this step tells you nothing about automotive or wear, and you'd need a second round trip through CI to find out.

--continue still fails the step, but it reports every module's outcome in one run:

Suggested change
./gradlew \
:app:uploadSentryProguardMappingsRelease \
:automotive:uploadSentryProguardMappingsRelease \
:wear:uploadSentryProguardMappingsRelease
./gradlew --continue \
:app:uploadSentryProguardMappingsRelease \
:automotive:uploadSentryProguardMappingsRelease \
:wear:uploadSentryProguardMappingsRelease

Two related notes on this invocation:

The task name is unverified anywhere in the repo. grep over the tree and git log -S both turn up nothing but this script — no CI path has ever named a Sentry task, since they all skip the upload. If uploadSentryProguardMappingsRelease is not what the plugin (sentry-plugin = "6.18.0") registers, the step dies with Task 'uploadSentryProguardMappingsRelease' not found in project ':app', which reads like broken wiring rather than a typo in this script, and the PR's question goes unanswered. Worth confirming once with CI=true ./gradlew :app:tasks --all | grep -i sentry before trusting a red build here.

The source-context upload is not exercised. build.gradle.kts:487 sets includeSourceContext = shouldUploadDebugFiles from the same flag, and the real release path (build_bundle → gradle(task: ":#{app}:bundle", build_type: 'Release') in fastlane/Fastfile:596) passes no skipSentryProguardMappingUpload, so at code freeze the source-bundle upload runs against the same org and slugs for the first time too. Requesting only the mapping task leaves that half untested. Adding it would mean uploading PR-branch source code to the three production projects, which is a heavier side effect than the spare mapping the description accounts for — so it may well be the right call to leave out. If so, worth saying explicitly in the description, since "step passes ⇒ #5708 is safe to merge" is then narrower than it reads.

5 changes: 5 additions & 0 deletions .buildkite/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ steps:
artifact_paths:
- "**/build/outputs/apk/**/*"

# Throwaway, delete with `.buildkite/commands/verify-sentry-upload.sh`.
- label: ":sentry: Verify Sentry Upload"
command: ".buildkite/commands/verify-sentry-upload.sh"
plugins: [ $CI_TOOLKIT ]
Comment on lines +90 to +93

@claude claude Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cosmetic, but it costs nothing to avoid: the step lands immediately above the ########## Optional Prototype Builds ########## banner (indented under the group that follows it), so the banner now reads as documentation for this step. Appending the step at the end of the file instead would both keep that banner attached to its group and make the eventual deletion a single trailing hunk.

The rest of the step is consistent with its neighbours — plugins: [ $CI_TOOLKIT ], and no agents: override so it inherits the top-level queue: "android", which is the queue the description wants to test the token on.

Omitting the should-skip-job.sh --job-type build guard that assemble-release-apk.sh and prototype-build.sh open with is also the right call here: .buildkite/** isn't in COMMON_PATTERNS, so it would never skip on this branch anyway, and you want this step to run unconditionally.


##########
# Optional Prototype Builds
#
Expand Down