Publish autogravity preview failures to Sentry with span context - #13681
Open
TorstenDittmann wants to merge 1 commit into
Open
TorstenDittmann wants to merge 1 commit into
TorstenDittmann wants to merge 1 commit into
Conversation
Autogravity 4xx responses (for example 413 on oversized payloads) were attached to the request span with that HTTP status. Sentry only exports Appwrite exceptions that are publishable, or other errors with code 0 or 5xx, so those failures never arrived. File, crop, and timing attributes were also added only after a successful transform. Record identifying preview attributes before detection, stamp autogravity cache/bytes/status on the span, and wrap detector failures as a publishable 500 so oversized-image 500s can be debugged in Sentry. Co-authored-by: Torsten Dittmann <TorstenDittmann@users.noreply.github.com>
TorstenDittmann
marked this pull request as ready for review
September 14, 2026 19:01
✨ Benchmark resultsComparing
Per-scenario breakdown & investigation detailsMetrics below reflect the current branch (after). Δ P95 compares against the base.
Top API waits (after)
|
Contributor
 |
Comment on lines
+82
to
+83
| $this->assertSame(\strlen('source-image'), $span->get('autogravity.bytes')); | ||
|
|
Contributor
There was a problem hiding this comment.
These tests assert private telemetry details, including exact span keys, byte-counting values, and internal span error state. This violates the repository directive to test observable behavior rather than mirror source code or configuration. Replace these assertions with coverage at the exported Sentry-event boundary; this repository requirement must be satisfied before merging. The same pattern also appears in the failure assertions on lines 108–113.
Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/Autogravity/DetectorTest.php
Line: 82-83
Comment:
**Tests Mirror Internal Telemetry**
These tests assert private telemetry details, including exact span keys, byte-counting values, and internal span error state. This violates the repository directive to test observable behavior rather than mirror source code or configuration. Replace these assertions with coverage at the exported Sentry-event boundary; this repository requirement must be satisfied before merging. The same pattern also appears in the failure assertions on lines 108–113.
**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
When
gravity=autofailed — including oversized images that came back as 4xx from Autogravity and were remapped to a user-facing 500 — Sentry never received a usable event.Two things were going wrong:
413,400, …). Sentry only exports Appwrite exceptions that are publishable, or other errors with code0or>= 500. A 413 from a too-large payload was rewritten to500 Server Errorfor the client, then dropped by the sampler.This change:
autogravity.cache,autogravity.bytes,autogravity.status, andautogravity.erroron the current span when detection runs or fails.GENERAL_SERVER_ERROR(keeping the original exception asprevious).storage.bucket.id,storage.file.id, andautogravity.statusto Sentry tags so oversized-image 500s can be filtered.Test Plan
tests/unit/Autogravity/DetectorTest.php(cache hit/miss and 413 failure attributes on the current span)tests/unit/Autogravity/ClientTest.php(testAnalyzePayloadTooLarge)composer linton the touched PHP files (environment here has no PHP/Composer; CI should run these)Related PRs and Issues
gravity=auto) preview workChecklist