diff --git a/.github/file-filters.yml b/.github/file-filters.yml index 2b81e2f0b6d..a6372fc4840 100644 --- a/.github/file-filters.yml +++ b/.github/file-filters.yml @@ -10,3 +10,11 @@ high_risk_code: &high_risk_code # Class used by hybrid SDKs - "sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java" + +config_kt: + - "buildSrc/src/main/java/Config.kt" + +sdk_name_check_scripts: + - ".github/workflows/sdk-name-check.yml" + - "scripts/detect_sdk_name_changes.py" + - "scripts/test_detect_sdk_name_changes.py" diff --git a/.github/workflows/sdk-name-check.yml b/.github/workflows/sdk-name-check.yml new file mode 100644 index 00000000000..45565292662 --- /dev/null +++ b/.github/workflows/sdk-name-check.yml @@ -0,0 +1,158 @@ +# When sdk.name values are added, removed, or updated in Config.kt, posts a PR comment +# reminding authors to update the sdk_map spreadsheet for Looker/Hex reporting. +# Warn-only: does not block merge. See scripts/detect_sdk_name_changes.py. +# Fork PRs cannot post comments (read-only GITHUB_TOKEN on pull_request). +name: 'SDK Name Check' +run-name: sdk.name change check (Config.kt) + +on: + pull_request: + types: [opened, synchronize, reopened] + +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + files-changed: + name: Detect changed files + runs-on: ubuntu-latest + outputs: + config_kt: ${{ steps.changes.outputs.config_kt }} + sdk_name_check_scripts: ${{ steps.changes.outputs.sdk_name_check_scripts }} + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - name: Get changed files + id: changes + uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 + with: + token: ${{ github.token }} + filters: .github/file-filters.yml + + sdk-name-check: + name: SDK name check + if: needs.files-changed.outputs.config_kt == 'true' || needs.files-changed.outputs.sdk_name_check_scripts == 'true' + needs: files-changed + runs-on: ubuntu-latest + permissions: + pull-requests: write + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 0 + + - name: Run detector unit tests + run: python3 -m unittest discover -s scripts -p 'test_detect_sdk_name_changes.py' + + - name: Detect SDK name changes + if: needs.files-changed.outputs.config_kt == 'true' + continue-on-error: true + id: detect + run: | + git fetch --no-tags origin \ + "${{ github.event.pull_request.base.sha }}" \ + "${{ github.event.pull_request.head.sha }}" + changes=$(python3 scripts/detect_sdk_name_changes.py \ + --base "${{ github.event.pull_request.base.sha }}" \ + --head "${{ github.event.pull_request.head.sha }}") + echo "changes=${changes}" >> "$GITHUB_OUTPUT" + + - name: Update PR comment + if: needs.files-changed.outputs.config_kt == 'true' && (steps.detect.outcome == 'success' || steps.detect.outcome == 'failure') + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + SDK_NAME_CHANGES: ${{ steps.detect.outputs.changes }} + DETECT_OUTCOME: ${{ steps.detect.outcome }} + with: + script: | + const marker = ''; + + const comments = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + + const existing = comments.find( + (comment) => + comment.body?.includes(marker) && + comment.user?.type === 'Bot', + ); + + const upsertComment = async (body) => { + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + return; + } + + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); + }; + + if (process.env.DETECT_OUTCOME === 'failure') { + await upsertComment( + [ + '### ⚠️ SDK name check failed', + '', + 'Could not compare `*SDK_NAME` declarations in `Config.kt`. Use `val FOO_SDK_NAME = "..."` with an optional `$OTHER_SDK_NAME` prefix.', + '', + 'See the **Detect SDK name changes** step logs for details.', + '', + marker, + ].join('\n'), + ); + return; + } + + const changes = JSON.parse(process.env.SDK_NAME_CHANGES || '{}'); + const added = changes.added || []; + const removed = changes.removed || []; + const sheetUrl = changes.sdk_map_sheet_url; + + if (added.length === 0 && removed.length === 0) { + if (existing) { + await github.rest.issues.deleteComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + }); + } + return; + } + + const formatList = (names) => names.map((name) => `- \`${name}\``).join('\n'); + const sections = ['### ⚠️ SDK name changes detected', '']; + + if (added.length > 0) { + sections.push( + 'This PR adds new `sdk.name` value(s) that will appear in production telemetry:', + '', + formatList(added), + '', + `Please add them to the [sdk_map spreadsheet](${sheetUrl}) so Looker/Hex reporting stays accurate.`, + '', + ); + } + + if (removed.length > 0) { + sections.push( + 'This PR removes `sdk.name` value(s) from production telemetry:', + '', + formatList(removed), + '', + `Please remove them from the [sdk_map spreadsheet](${sheetUrl}).`, + '', + ); + } + + sections.push(marker); + await upsertComment(sections.join('\n')); diff --git a/buildSrc/src/main/java/Config.kt b/buildSrc/src/main/java/Config.kt index f0e2e9baf86..61d1d12ae0f 100644 --- a/buildSrc/src/main/java/Config.kt +++ b/buildSrc/src/main/java/Config.kt @@ -40,12 +40,14 @@ object Config { } object Sentry { + // .github/workflows/sdk-name-check.yml expects all SDK name declarations to end in `*SDK_NAME`. val SENTRY_JAVA_SDK_NAME = "sentry.java" val SENTRY_ANDROID_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.android" val SENTRY_TIMBER_SDK_NAME = "$SENTRY_ANDROID_SDK_NAME.timber" val SENTRY_LOGBACK_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.logback" val SENTRY_JUL_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.jul" val SENTRY_LOG4J2_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.log4j2" + val SENTRY_LOG4J3_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.log4j3" val SENTRY_SPRING_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.spring" val SENTRY_SPRING_JAKARTA_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.spring.jakarta" val SENTRY_SPRING_7_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.spring-7" diff --git a/scripts/detect_sdk_name_changes.py b/scripts/detect_sdk_name_changes.py new file mode 100644 index 00000000000..946f1866caf --- /dev/null +++ b/scripts/detect_sdk_name_changes.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python3 +"""Detect added and removed sdk.name values in Config.kt between two revisions.""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +from pathlib import Path + +CONFIG_PATH = "buildSrc/src/main/java/Config.kt" +SDK_MAP_SHEET_URL = ( + "https://docs.google.com/spreadsheets/d/1hqFhytQuHMvuOz1XD0kCXg6x0ViflHrpjW7nNhvzYmU/" + "edit?gid=334165604" +) +SDK_NAME_PATTERN = re.compile(r"^\s*val (\w+SDK_NAME) = (.+)$") +STRING_LITERAL = re.compile(r'^"((?:[^"\\]|\\.)*)"$') +INTERPOLATION_PATTERN = re.compile(r"\$([A-Za-z0-9_]+)") + + +def parse_sdk_constants(content: str) -> dict[str, str]: + """Return mapping of constant name to resolved sdk.name string.""" + resolved: dict[str, str] = {} + for line in content.splitlines(): + match = SDK_NAME_PATTERN.match(line) + if not match: + continue + name, rhs = match.group(1), match.group(2).strip() + resolved[name] = resolve_rhs(rhs, resolved) + return resolved + + +def resolve_rhs(rhs: str, resolved: dict[str, str]) -> str: + literal = STRING_LITERAL.match(rhs) + if not literal: + raise ValueError(f"Cannot parse SDK name assignment: {rhs}") + return expand_interpolation(literal.group(1), resolved) + + +def expand_interpolation(value: str, resolved: dict[str, str]) -> str: + if "${" in value: + raise ValueError(f"Unsupported brace interpolation in SDK name value: {value}") + + def replace(match: re.Match[str]) -> str: + name = match.group(1) + if name not in resolved: + raise ValueError(f"Unknown SDK name constant: {name}") + return resolved[name] + + return INTERPOLATION_PATTERN.sub(replace, value) + + +def find_sdk_name_changes( + base: dict[str, str], head: dict[str, str] +) -> tuple[list[str], list[str]]: + base_values = set(base.values()) + head_values = set(head.values()) + added = sorted(head_values - base_values) + removed = sorted(base_values - head_values) + return added, removed + + +def git_show(ref: str, path: str) -> str: + try: + return subprocess.check_output( + ["git", "show", f"{ref}:{path}"], + text=True, + stderr=subprocess.PIPE, + ) + except subprocess.CalledProcessError as error: + if error.returncode == 128 and "exists on disk, but not in" in error.stderr: + return "" + raise + + +def read_config_source(base: str | None, head: str | None, config_path: str) -> tuple[str, str]: + if base is None or head is None: + raise ValueError("Both base and head refs are required") + return git_show(base, config_path), git_show(head, config_path) + + +def format_changes(added: list[str], removed: list[str]) -> str: + return json.dumps( + { + "added": added, + "removed": removed, + "sdk_map_sheet_url": SDK_MAP_SHEET_URL, + }, + separators=(",", ":"), + ) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Print added and removed sdk.name values as JSON." + ) + parser.add_argument("--base", help="Git ref for the PR base revision") + parser.add_argument("--head", help="Git ref for the PR head revision") + parser.add_argument("--base-file", type=Path, help="Base Config.kt file") + parser.add_argument("--head-file", type=Path, help="Head Config.kt file") + parser.add_argument("--config-path", default=CONFIG_PATH) + args = parser.parse_args(argv) + + try: + if args.base_file and args.head_file: + base_content = args.base_file.read_text(encoding="utf-8") + head_content = args.head_file.read_text(encoding="utf-8") + elif args.base and args.head: + base_content, head_content = read_config_source( + args.base, args.head, args.config_path + ) + else: + parser.error("Provide --base/--head or --base-file/--head-file") + + base_sdk = parse_sdk_constants(base_content) + head_sdk = parse_sdk_constants(head_content) + except ValueError as error: + print(f"error: {error}", file=sys.stderr) + return 1 + except subprocess.CalledProcessError as error: + print(f"error: git command failed: {' '.join(error.cmd)}", file=sys.stderr) + return 1 + + added, removed = find_sdk_name_changes(base_sdk, head_sdk) + print(format_changes(added, removed)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_detect_sdk_name_changes.py b/scripts/test_detect_sdk_name_changes.py new file mode 100644 index 00000000000..50c21c21c54 --- /dev/null +++ b/scripts/test_detect_sdk_name_changes.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 + +import io +import json +import tempfile +import unittest +import unittest.mock +from pathlib import Path + +from detect_sdk_name_changes import ( + SDK_MAP_SHEET_URL, + find_sdk_name_changes, + main, + parse_sdk_constants, +) + + +class DetectSdkNameChangesTest(unittest.TestCase): + BASE_CONFIG = """ +object Config { + object Sentry { + val SENTRY_JAVA_SDK_NAME = "sentry.java" + val SENTRY_ANDROID_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.android" + } +} +""" + + HEAD_WITH_NEW_CONSTANT = """ +object Config { + object Sentry { + val SENTRY_JAVA_SDK_NAME = "sentry.java" + val SENTRY_ANDROID_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.android" + val SENTRY_FOO_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.foo" + } +} +""" + + HEAD_WITH_NON_SENTRY_PREFIX = """ +object Config { + object Sentry { + val SENTRY_JAVA_SDK_NAME = "sentry.java" + val SENTRY_ANDROID_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.android" + val LEGACY_SDK_NAME = "sentry-java" + } +} +""" + + HEAD_WITH_CHANGED_VALUE = """ +object Config { + object Sentry { + val SENTRY_JAVA_SDK_NAME = "sentry.java" + val SENTRY_ANDROID_SDK_NAME = "$SENTRY_JAVA_SDK_NAME.android-renamed" + } +} +""" + + HEAD_WITH_REMOVED_CONSTANT = """ +object Config { + object Sentry { + val SENTRY_JAVA_SDK_NAME = "sentry.java" + } +} +""" + + HEAD_WITH_INVALID_RHS = """ +object Config { + object Sentry { + val SENTRY_JAVA_SDK_NAME = "sentry.java" + val SENTRY_FOO_SDK_NAME = buildSdkName("foo") + } +} +""" + + HEAD_WITH_BRACE_INTERPOLATION = """ +object Config { + object Sentry { + val SENTRY_JAVA_SDK_NAME = "sentry.java" + val SENTRY_FOO_SDK_NAME = "${SENTRY_JAVA_SDK_NAME}.foo" + } +} +""" + + def test_no_changes(self) -> None: + base = parse_sdk_constants(self.BASE_CONFIG) + head = parse_sdk_constants(self.BASE_CONFIG) + self.assertEqual(find_sdk_name_changes(base, head), ([], [])) + + def test_new_interpolated_constant(self) -> None: + base = parse_sdk_constants(self.BASE_CONFIG) + head = parse_sdk_constants(self.HEAD_WITH_NEW_CONSTANT) + self.assertEqual(find_sdk_name_changes(base, head), (["sentry.java.foo"], [])) + + def test_new_constant_without_sentry_prefix(self) -> None: + base = parse_sdk_constants(self.BASE_CONFIG) + head = parse_sdk_constants(self.HEAD_WITH_NON_SENTRY_PREFIX) + self.assertEqual(find_sdk_name_changes(base, head), (["sentry-java"], [])) + + def test_changed_value_detects_add_and_remove(self) -> None: + base = parse_sdk_constants(self.BASE_CONFIG) + head = parse_sdk_constants(self.HEAD_WITH_CHANGED_VALUE) + self.assertEqual( + find_sdk_name_changes(base, head), + (["sentry.java.android-renamed"], ["sentry.java.android"]), + ) + + def test_removed_constant(self) -> None: + base = parse_sdk_constants(self.BASE_CONFIG) + head = parse_sdk_constants(self.HEAD_WITH_REMOVED_CONSTANT) + self.assertEqual(find_sdk_name_changes(base, head), ([], ["sentry.java.android"])) + + def test_parse_failure_returns_error(self) -> None: + with self.assertRaises(ValueError): + parse_sdk_constants(self.HEAD_WITH_INVALID_RHS) + + def test_brace_interpolation_is_rejected(self) -> None: + with self.assertRaises(ValueError): + parse_sdk_constants(self.HEAD_WITH_BRACE_INTERPOLATION) + + def test_cli_reports_parse_failure(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + base_file = temp_path / "base.kt" + head_file = temp_path / "head.kt" + base_file.write_text(self.BASE_CONFIG, encoding="utf-8") + head_file.write_text(self.HEAD_WITH_INVALID_RHS, encoding="utf-8") + + stderr = io.StringIO() + with unittest.mock.patch("sys.stderr", stderr): + exit_code = main( + ["--base-file", str(base_file), "--head-file", str(head_file)] + ) + + self.assertEqual(exit_code, 1) + self.assertIn("Cannot parse SDK name assignment", stderr.getvalue()) + + def test_cli_prints_json_changes(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = Path(temp_dir) + base_file = temp_path / "base.kt" + head_file = temp_path / "head.kt" + base_file.write_text(self.BASE_CONFIG, encoding="utf-8") + head_file.write_text(self.HEAD_WITH_NEW_CONSTANT, encoding="utf-8") + + buffer = io.StringIO() + with unittest.mock.patch("sys.stdout", buffer): + exit_code = main( + ["--base-file", str(base_file), "--head-file", str(head_file)] + ) + + self.assertEqual(exit_code, 0) + self.assertEqual( + json.loads(buffer.getvalue()), + { + "added": ["sentry.java.foo"], + "removed": [], + "sdk_map_sheet_url": SDK_MAP_SHEET_URL, + }, + ) + + +if __name__ == "__main__": + unittest.main()