Skip to content

feat(webhooks): Add metrics for legacy webhook migration validation#115969

Closed
Christinarlong wants to merge 55 commits into
masterfrom
crl/webhooks-metrics
Closed

feat(webhooks): Add metrics for legacy webhook migration validation#115969
Christinarlong wants to merge 55 commits into
masterfrom
crl/webhooks-metrics

Conversation

@Christinarlong
Copy link
Copy Markdown
Contributor

@Christinarlong Christinarlong commented May 20, 2026

Adds the following metrics

  • legacy_webhook.plugin.send - track rate of old path sending
  • legacy_webhook.task.result - track rate of new path sending

Linear project: https://linear.app/getsentry/project/migrate-legacy-webhooks-away-from-plugins-c7549d6ed29b/overview

…ng migration

Instruments both the legacy plugin and the new webhook task with
`metrics.incr` counters so we can validate dry-run parity:
- `legacy_webhook.plugin.send` (old path, per-URL sent/error)
- `legacy_webhook.task.result` (new path, per-URL sent/dry_run/group_not_found)
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 20, 2026
Christinarlong and others added 28 commits May 20, 2026 16:08
Stacked on #115701.

`generatePlatformIconName` used to maintain its own `PLATFORM_ALIASES`
table — a strict subset of `LOGO_MAPPING` in `contextIcon.tsx`, kept in
sync by hand. With the migration to platformicons, the canonical mapping
now lives in `getLogoImage`. This change drops the duplicate table and
delegates to `getLogoImage`, also clearing the `@ts-expect-error` from
the previous keyed access.
Replace the legacy highlights JsonForm with AutoSaveForm text areas for
tags and context.

This keeps the existing autosave behavior, preserves the project cache
updates,
and tightens the tests around the autosave requests.

closes
https://linear.app/getsentry/issue/DE-1251/migrate-eventshighlightshighlightssettingsformtsx-from-legacy-form
…5783)

Migrates the project inbound filters settings page from the legacy form
system (`Form`, `JsonForm`, `FormField`, `FieldFromConfig`) to the new
scraps form system (`AutoSaveForm`, `useScrapsForm`, `FieldGroup`).

**Standard filter toggles & legacy browser filter**

Each filter toggle (browser-extensions, localhost, web-crawlers, etc.)
is now an `AutoSaveForm` with `field.Switch`. The legacy browser filter
uses `AutoSaveForm` with a `string[]` field value and a custom
`field.Base` render for the browser grid UI. Both share mutation options
via a `getFilterMutationOptions` factory. Optimistic updates go through
the filter list query cache instead of local `useState`, so
`AutoSaveForm`'s reset-after-save works correctly across rapid toggles.

**Project-level boolean filters**

Hydration errors and ChunkLoadError toggles use `AutoSaveForm` backed by
the detailed project query cache. Optimistic updates write to both the
query cache and `ProjectsStore` to keep the sidebar and other consumers
in sync.

**Custom filter textareas**

The `saveOnBlur: false` textarea fields (IP addresses, releases, error
messages, etc.) are unified into a single `useScrapsForm` with explicit
Save/Cancel buttons, replacing the old `JsonForm` + `inboundFilters.tsx`
config. `inboundFilters.tsx` is deleted since it was only imported here.

**FormSearch & field registry**

Wraps the migrated forms with `FormSearch` for settings search indexing
and regenerates `generatedFieldRegistry.ts`.

Refs DE-985

---------

Co-authored-by: Claude Opus 4 <noreply@anthropic.com>
…in config retrieval, just once (#115983)

- Retrieve projects centrally and write onto the config
- Use the project information & ids from the config everywhere else
…yield individual rows and adjust tests accordingly (#115995)

Contributes to TET-2306

### Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.
…' header value (#115917)

Currently the `integration-proxy` endpoint can only accept
`application/json` and `*/*` `Accepts` headers. This change allows any
`Accepts` header. Additionally the `integration-proxy2` endpoint streams
the response to the connected proxy client.

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
The goal of this PR is to be a bit more strict when it comes to URLs so
that we don't allow link cell actions, etc. to take place for a URL like
`http://*/api/cool-endpoint`.

Closes EXP-957

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: Codex <noreply@openai.com>
`RouteContextInterface` was a legacy react-router 3 type that ended up
with only two consumers, both of which were trivially removable:

- `overrides.tsx` used `Pick<RouteContextInterface, 'location' |
'matches'>` to type the `react-hook:route-activated` override prop.
Replaced with an inline literal `{location: Location; matches:
UIMatch[]}` — same shape, no indirection through a legacy interface.
- `DeprecatedAsyncComponent` had a `declare context: {router:
RouteContextInterface}` field. Nothing in the codebase reads
`this.context.router` on any subclass — it was a leftover type
annotation for a class context that's never wired up.

With those gone the interface itself has no consumers, so it's deleted
from `legacyReactRouter.tsx`.

Small step toward eventually killing the rest of the legacy router types
(`PlainRoute`, `InjectedRouter`, `RouteComponentProps`), but each of
those still has real consumers and is its own track.
…alidation errors" (#115987)

## Summary

Reapplies #115736 which was reverted in 950ef17.

The original PR improved snapshot validation error formatting so that:
- **jsonschema errors** use `absolute_path` for full context when
`best_match` selects a nested error
- **Pydantic errors** include the image key and field path (e.g.
`Validation error in image "screen.png", field "width": ...`)

This makes error messages actionable when uploading hundreds of images
via sentry-cli.
This PR brings the Sentry conventions package up to date. This dep
update brings in the new `visibility` field on attributes that we'll be
using to remove those that are internal from non-staff users from seeing
in-app.
…115993)

This is a tricky one that I stumbled upon while fixing `any` types with
`no-unsafe-member-access`. TypeScript didn’t catch this because
`item.props` comes directly from `react-aria` and is hardcoded to `any`
which is really bad.

Basically, `TabList.Item` allows passing in tooltips as `tooltipProps`,
so we can do:

```
<TabList.Item tooltip={{ title: 'hello '}}>
```

This renders the tooltip on the TabItem fine, however when we have many
tabs, the overflowing ones are rendered into a `MenuItemList`, which has
a different type for the `tooltip` prop:


https://github.com/getsentry/sentry/blob/f9bda0535267decaccdedefa0d30cd423d1a2d80/static/app/components/core/menuListItem/menuListItem.tsx#L205-L214

Basically `tooltip` is a ReactNode or a function that produces a
ReactNode, and `tooltipOptions` are separate.

So what happens at runtime is that we pass `tooltip={{ title: 'hello
'}}` to the `MenuitemList`, which renders it as-is because it's not a
function, which makes react crash the page because it cannot render
objects.

---

This fix is a hotfix at best to prevent this from happening. I think a
proper fix would consolidate how we pass `tooltips` around: Do we just
want `tooltip` to be the props object everywhere? The extra `options`
are a weird API. However, the functional syntax to produce a tooltip
doesn’t go well with extra options in one object. Best I could come up
with is:

```
tooltip: Omit<TooltipProps, 'title'> & { title: ReactNode | (state) => ReactNode) }
```

This would be doable in a follow-up but has a larger surface area, would
disallow passing string tooltips (which we do a lot, would need to
change to `{title: 'hello'}` _and_ with `react-select` and `react-aria`
being `any` it's pretty hard to figure out where we produce `options`
Adds a patterns page for `useRenderToString`, documenting the recipe for
rendering React components to HTML strings. The primary use case is
eCharts tooltip/label formatters that require string return values but
should use design system components.

Thanks @nikkikapadia for surfacing that this wasn't documented yet!

[See Preview
Deploy](https://sentry-git-nm-storiespattern-rendertostring.sentry.dev/stories/patterns/render-to-html/)
I added a few ignore comments as I didn't want to modify the runtime
code to satisfy the typechecker when the code already behaves correctly.

Refs ENG-6444
Set loglevel to WARNING so that we don't get options.seen logs captured
in the RPC schema file.
This adds an introspection after each autofix step that uses a LLM to
judge if we should continue. For now this is behind a feature flag and
will only emit analytics as we fine tune this.
I'm somewhat low confidence I've understood the current/new query key
plumbing correct. But I think this corrects the `doRefresh` callback to
properly clear the replay & replay-segments data?

Fixes REPLAY-910.
@Christinarlong Christinarlong requested review from a team as code owners May 21, 2026 18:01
@Christinarlong Christinarlong marked this pull request as draft May 21, 2026 18:01
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 21, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@github-actions
Copy link
Copy Markdown
Contributor

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.62%

Comment on lines +237 to +239
let value = dataRow[column.key];
const externalLinkTarget =
to && !isInternalNavigationTarget(to) && isValidUrl(to) ? to : undefined;
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 switch to using column.key for data lookup will fail for equation columns, as the key does not match the API response's index, causing cell actions to break.
Severity: HIGH

Suggested Fix

The data lookup for cell values needs to use the correct alias provided by the backend API for equation columns (e.g., equation[0]) instead of column.key. This may require a mapping from the column identifier to the correct data row alias before performing the lookup.

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: static/app/views/discover/table/cellAction.tsx#L237-L239

Potential issue: In the Discover table, the change from `dataRow[column.name]` to
`dataRow[column.key]` for cell action data lookups introduces a bug for equation
columns. For equations, the `column.key` is a string like `equation|count() / 100`, but
the data from the API is indexed by an alias, such as `equation[0]`. As a result,
`dataRow[column.key]` resolves to `undefined` for any equation column. This breaks all
cell actions for these columns, including 'Add to filter' and 'Exclude from filter', as
they will be operating on an undefined value.

"service": service_name,
"method": method_name,
"request_type": type(request).__name__,
"request": MessageToDict(request),
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.

Full billing service request payload logged via MessageToDict

Serializing the entire protobuf request with MessageToDict(request) and writing it to logger.info dumps all billing-request fields—potentially including contract values, plan tiers, invoice amounts, or other customer-confidential billing data—into durable operational logs. Remove the "request" key; the already-selective organization_id and contract_id fields provide sufficient context.

Evidence
  • extras["request"] = MessageToDict(request) is added unconditionally for every billing service method call.
  • extras is passed directly to logger.info("billing.service.method.start", extra=extras) at line 99, making the full request payload a durable log entry.
  • The same extras dict (via **extras) is also included in the billing.service.method.success log and likely error logs, multiplying the exposure surface.
  • The surrounding context already selectively extracts organization_id and contract_id—indicating awareness that full payload logging was not the original intent.
  • Billing service requests can carry customer-confidential fields (contract values, plan details, seat counts, etc.) beyond just the two explicitly extracted IDs.

Identified by Warden wrdn-pii · 3SU-H6H

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.