Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[EPIC] Generic Inbound Filters #57799

Closed
6 tasks done
iambriccardo opened this issue Oct 10, 2023 · 8 comments
Closed
6 tasks done

[EPIC] Generic Inbound Filters #57799

iambriccardo opened this issue Oct 10, 2023 · 8 comments
Assignees

Comments

@iambriccardo
Copy link
Member

iambriccardo commented Oct 10, 2023

Problem

The inbound filters configuration is currently quite limited, since it's based on a pre-defined set of filters, that are defined statically in both Sentry and Relay. Such manual configuration is a tedious process and is quite limited, since it requires quite a lot of manual work to satisfy even the simplest of constraints.

An example of inbound filters config:

"filterSettings": {
  "legacyBrowsers": {
    "isEnabled": true,
    "options": [
    "ie_pre_9",
    "safari_pre_6",
    "opera_pre_15",
    "ie9",
    "ie10",
    "android_pre_4",
    "ie11",
    "opera_mini_pre_8"
    ]
  },
  "webCrawlers": {
    "isEnabled": true
  },
},

Possible Solution

Relay offers a DSL for matching incoming events which is now used in both dynamic sampling and metric extraction. Such DSL enables developers to express composite conditions on the incoming event's payload, which can already satisfy a lot of existing matching logic implemented for inbound filters.

The idea would be to introduce a new generic field that will contain key-value pairs of inbound filters and their condition for filtering. For efficiency reasons the definition of rules could leverage the new global config system of Relay and just propagate the active status as project scoped configuration.

The implementation of inbound filters could look something like:

"filterSettings": {
  # All non-generic filters are expressed like before for backwards compatibility.
  "legacyBrowsers": {
    "isEnabled": true,
    "options": [
    "ie_pre_9",
    "safari_pre_6",
    "opera_pre_15",
    "ie9",
    "ie10",
    "android_pre_4",
    "ie11",
    "opera_mini_pre_8"
    ]
  },
  "webCrawlers": {
    "isEnabled": true
  },
  # Here all the generic inbound filters should go.
  “generic”: {
  		“hydrationErrors”: {
  			“condition”: # ds condition,
  			“isEnabled”: true
  		}
  	}
},

Such a new configuration will be backward compatible by having Sentry generate inbound filters with both DSLs for a fixed amount of time. After that time the old configuration will be faded out resulting in old custom Relays not inbound filtering.

Work To Do

Tasks

  1. iambriccardo
  2. iker-barriocanal
  3. Scope: Backend
    iker-barriocanal
  4. Scope: Backend
    iker-barriocanal
  5. Scope: Backend
    iker-barriocanal
@jjbayer
Copy link
Member

jjbayer commented Oct 30, 2023

Remaining steps depend on getsentry/relay#2584: When the filter config moves to global config, we cannot apply filters as long as global config is not yet loaded.

@bruno-garcia
Copy link
Member

Related: Replay customers currently suffer from issues when sampling OnError replays and inbound filters drop the event that triggers the replay to start:

@sentaur-athena
Copy link
Member

@iambriccardo wanted to follow-up about the work of moving filters from project endpoint to inbound filters endpoint. We have request from partner to support filter for IP address, Releases and Error Message like what's available in the UI. Currently this is happening using project endpoint so I hesitate to publish it as is. The endpoint is very bloated.

Was wondering if you're adding them all to a filter endpoint and if so what are the timelines?
cc: @Dhrumil-Sentry

@jernejstrasner
Copy link
Contributor

This work is on the Ingest plate. We're all busy with metrics but the config merge part we will do asap. It could get done in the next couple of weeks but we'll keep you posted.
After that you all can then add whichever filters you need.
How does this sound @sentaur-athena ?

@sentaur-athena
Copy link
Member

@jernejstrasner sounds good. Since the partner need this faster and we don't want to get your focus away from metrics work either, I will write private documentation for them to use the Project Update Endpoint meanwhile. When we do the refactory, before re-moving the inbound filter from projects endpoint we should make sure to let them know to move to the new solution.

@iambriccardo I will send over the doc to you to review before sending to them. Thank you.

@sentaur-athena
Copy link
Member

@jernejstrasner to be clear, is the work that is on ingest plate moving all filters out of project details endpoint and to a filters endpoint?
Would be great to have timelines when you have them and make sure we're all aligned on a rollout plan. Appreciate it.

@jjbayer
Copy link
Member

jjbayer commented Feb 8, 2024

@sentaur-athena creating a new sentry API endpoint for inbound filters is out of scope for this epic IMO, because it's a different concern (API <-> client communication vs. sentry <-> relay communication).

@iker-barriocanal
Copy link
Contributor

Support for generic inbound filters is now complete! Filters are defined in Sentry and automagically forwarded to and applied in Relay (no Relay changes required).

Creating new filters is as easy as expanding the project filter list to apply filters to a specific project, or the global filter list to apply filters to all projects. Global filters are overwritten by project filters, making it easy to apply filters to some/most projects with minimal config size overhead.

If you'd like to add new filters but are unsure of how to leverage both configs, don't hesitate to reach out.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants