Skip to content

feat(explorer): allow client to pass in metadata#104606

Merged
roaga merged 1 commit intomasterfrom
explorer/client-metadata
Dec 9, 2025
Merged

feat(explorer): allow client to pass in metadata#104606
roaga merged 1 commit intomasterfrom
explorer/client-metadata

Conversation

@roaga
Copy link
Copy Markdown
Contributor

@roaga roaga commented Dec 9, 2025

Allows client to pass in arbitrary metadata when starting a new run and allow it to retrieve it later

@roaga roaga requested a review from aliu39 December 9, 2025 19:37
@roaga roaga requested a review from a team as a code owner December 9, 2025 19:37
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Dec 9, 2025
Comment on lines +252 to 254
payload["metadata"] = metadata

body = orjson.dumps(payload, option=orjson.OPT_NON_STR_KEYS)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The new metadata field might not be accepted by the external Seer backend, leading to runtime errors.
Severity: HIGH | Confidence: Low

🔍 Detailed Analysis

The client-side code sends a metadata field to an external Seer service, but there's no verification or integration tests to confirm the backend accepts this new parameter. If the backend has not been updated or rejects unknown fields, it could return a 400 Bad Request, causing the client to raise requests.HTTPError() and the feature to fail at runtime.

💡 Suggested Fix

Add integration tests that call the external Seer backend with the metadata parameter to verify its acceptance and proper handling. Coordinate with the Seer backend team to ensure metadata is supported.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/seer/explorer/client.py#L252-L254

Potential issue: The client-side code sends a `metadata` field to an external Seer
service, but there's no verification or integration tests to confirm the backend accepts
this new parameter. If the backend has not been updated or rejects unknown fields, it
could return a `400 Bad Request`, causing the client to raise `requests.HTTPError()` and
the feature to fail at runtime.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6582980

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 9, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/seer/explorer/client.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #104606      +/-   ##
===========================================
- Coverage   80.63%    80.52%   -0.12%     
===========================================
  Files        9335      9330       -5     
  Lines      402949    400718    -2231     
  Branches    25689     25689              
===========================================
- Hits       324921    322679    -2242     
- Misses      77562     77573      +11     
  Partials      466       466              

@roaga roaga merged commit 785c395 into master Dec 9, 2025
69 checks passed
@roaga roaga deleted the explorer/client-metadata branch December 9, 2025 20:22
@github-actions github-actions bot locked and limited conversation to collaborators Dec 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants