Skip to content

Conversation

@fisherdarling
Copy link
Collaborator

@fisherdarling fisherdarling commented Dec 17, 2025

This PR adds a panic and sentry hook to foundations. The panic hook will increment a metric and log the fact a panic occurred, where the sentry hook only increments a metric. The panic hook uses foundations' logging functionality if it's initialized, otherwise, it will print a JSON formatted log to stderr.

The two metrics emitted by foundations are: panics_total and sentry_events_total.

To easily test foundations' global metrics, I CI migrated to cargo-nextest. Also, I moved the target build testing to its own job (just to separate build/test). Finally, I generated a simple AGENTS.md which we can start refining to help with future PRs.

The diff looks rather large, but the vast majority of lines changed are tests and doc comments. Basically, we add some builder helpers under alerts::_private and use those to setup the panic and sentry hook.

This commit adds a simple AGENTS.md. This wasn't human-curated, just the
output of opencode's `/init` with Opus 4.5.
Comment on lines 218 to 309
name: Build
runs-on: ${{ matrix.os }}
strategy:
matrix:
thing:
- x86_64-linux
- aarch64-linux
- arm64-android
- arm-android
- aarch64-ios
- x86_64-macos
- x86_64-windows
include:
- apt_packages: ""
- custom_env: {}
- build_only: false
- cargo_args: ""

- thing: x86_64-linux
target: x86_64-unknown-linux-gnu
rust: stable
os: ubuntu-latest

- thing: aarch64-linux
build_only: true
target: aarch64-unknown-linux-gnu
rust: stable
os: ubuntu-latest
apt_packages: crossbuild-essential-arm64
custom_env:
CC: aarch64-linux-gnu-gcc
CXX: aarch64-linux-gnu-g++
CARGO_TARGET_AARCH64_UNKNOWN_LINUX_GNU_LINKER: aarch64-linux-gnu-g++

- thing: arm64-android
build_only: true
target: aarch64-linux-android
rust: stable
os: ubuntu-latest
cargo_args: --no-default-features --features server-client-common-default

- thing: arm-android
build_only: true
target: armv7-linux-androideabi
rust: stable
os: ubuntu-latest
cargo_args: --no-default-features --features server-client-common-default

- thing: aarch64-ios
build_only: true
target: aarch64-apple-ios
rust: stable
os: macos-latest
cargo_args: --no-default-features --features server-client-common-default

- thing: x86_64-macos
target: x86_64-apple-darwin
rust: stable
os: macos-latest

- thing: x86_64-windows
build_only: true
target: x86_64-pc-windows-msvc
rust: stable
os: windows-latest
cargo_args: --no-default-features --features server-client-common-default

steps:
- uses: actions/checkout@v4
with:
submodules: "recursive"
- name: Install Rust
uses: dtolnay/rust-toolchain@master
with:
toolchain: ${{ matrix.rust }}
targets: ${{ matrix.target }}
- uses: Swatinem/rust-cache@v2
- name: Install target-specific APT dependencies
if: "matrix.apt_packages != ''"
run: sudo apt update && sudo apt install -y ${{ matrix.apt_packages }}
shell: bash
- name: Set Android Linker path
if: endsWith(matrix.thing, '-android')
run: echo "CARGO_TARGET_$(echo ${{ matrix.target }} | tr \\-a-z _A-Z)_LINKER=$ANDROID_NDK/toolchains/llvm/prebuilt/linux-x86_64/bin/$(echo ${{ matrix.target }} | sed s/armv7/armv7a/)21-clang++" >> "$GITHUB_ENV"
- name: Build tests
# We `build` because we want the linker to verify we are cross-compiling correctly for check-only targets.
run: cargo build --target ${{ matrix.target }} ${{matrix.cargo_args}}
- name: Build
run: cargo build -p foundations --target ${{ matrix.target }} ${{matrix.cargo_args}}
shell: bash
env: ${{ matrix.custom_env }}
- name: Run tests
if: "!matrix.build_only"
run: _RJEM_MALLOC_CONF=prof:true cargo test --target ${{ matrix.target }} ${{matrix.cargo_args}}

test:
name: Test
runs-on: ${{ matrix.os }}
strategy:
matrix:
thing:
- x86_64-linux
- x86_64-macos
include:
- thing: x86_64-linux
target: x86_64-unknown-linux-gnu
rust: stable
os: ubuntu-latest

- thing: x86_64-macos
target: x86_64-apple-darwin
rust: stable
os: macos-latest

Check warning

Code scanning / CodeQL

Workflow does not contain permissions Medium

Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {contents: read}

Copilot Autofix

AI 3 days ago

To address the Workflow does not contain permissions error, add an explicit permissions key setting minimal required permission to the workflow. Since the jobs shown merely check out code, build, and test, the job(s) only need contents: read; neither push nor PR write permissions are evident from the code.
Best Practice: Add the permissions: contents: read block at the top level of the workflow, immediately below the workflow name and before on:, so that it applies to all jobs unless overridden.
Steps:

  • Edit .github/workflows/ci.yml.
  • Add the following at line 2:
    permissions:
      contents: read
  • This ensures that all jobs use the minimum GITHUB_TOKEN privilege.

Suggested changeset 1
.github/workflows/ci.yml

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -1,4 +1,6 @@
 name: CI
+permissions:
+  contents: read
 
 on:
   pull_request:
EOF
@@ -1,4 +1,6 @@
name: CI
permissions:
contents: read

on:
pull_request:
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@fisherdarling fisherdarling force-pushed the fisher/panic-and-sentry-hook branch from f3e444a to 3773226 Compare December 17, 2025 14:59
@fisherdarling fisherdarling force-pushed the fisher/panic-and-sentry-hook branch 7 times, most recently from 9203b72 to 5887224 Compare December 17, 2025 15:47
#[cfg(feature = "metrics")]
self::metrics::init::init(config.service_info, &config.settings.metrics);

let _ = TELEMETRY_INITIALIZED.set(());
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure if there's a better way to do this

@fisherdarling fisherdarling force-pushed the fisher/panic-and-sentry-hook branch from 5887224 to 6ab4caf Compare December 17, 2025 16:11
.unwrap_or_else(|| "<unknown>".to_string());

// Use serde_json for proper JSON escaping of all control characters
let json_output = serde_json::json!({
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Used for escaping the input payload. A little heavy, but most foundations users will likely have serde_json in their dep tree.

@fisherdarling fisherdarling force-pushed the fisher/panic-and-sentry-hook branch from 6ab4caf to d19e279 Compare December 17, 2025 16:14
@fisherdarling fisherdarling marked this pull request as ready for review December 17, 2025 16:22
@TheJokr TheJokr self-requested a review December 18, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant