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

fix(cabi): Remove dynamic sampling abi #2515

Merged
merged 4 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 0 additions & 21 deletions py/sentry_relay/processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
"validate_sampling_configuration",
"validate_project_config",
"normalize_global_config",
"run_dynamic_sampling",
]


Expand Down Expand Up @@ -279,23 +278,3 @@ def normalize_global_config(config):
return json.loads(rv)
except json.JSONDecodeError:
raise ValueError(rv)


def run_dynamic_sampling(sampling_config, root_sampling_config, dsc, event):
"""
Runs dynamic sampling on an event and returns the merged rules together with the sample rate.
"""
assert isinstance(sampling_config, str)
assert isinstance(root_sampling_config, str)
assert isinstance(dsc, str)
assert isinstance(event, str)

result_json = rustcall(
lib.run_dynamic_sampling,
encode_str(sampling_config),
encode_str(root_sampling_config),
encode_str(dsc),
encode_str(event),
)

return json.loads(decode_str(result_json, free=True))
212 changes: 0 additions & 212 deletions py/tests/test_processing.py
Original file line number Diff line number Diff line change
Expand Up @@ -301,215 +301,3 @@ def test_global_config_unparsable():
str(e.value)
== "invalid value: integer `-5`, expected usize at line 1 column 45"
)


def test_run_dynamic_sampling_with_valid_params_and_match():
sampling_config = """{
"rules": [],
"rulesV2": [
{
"samplingValue":{
"type": "factor",
"value": 2.0
},
"type": "transaction",
"active": true,
"condition": {
"op": "and",
"inner": [
{
"op": "eq",
"name": "event.transaction",
"value": [
"/world"
],
"options": {
"ignoreCase": true
}
}
]
},
"id": 1000
}
],
"mode": "received"
}"""

root_sampling_config = """{
"rules": [],
"rulesV2": [
{
"samplingValue":{
"type": "sampleRate",
"value": 0.5
},
"type": "trace",
"active": true,
"condition": {
"op": "and",
"inner": []
},
"id": 1001
}
],
"mode": "received"
}"""

dsc = """{
"trace_id": "d0303a19-909a-4b0b-a639-b17a74c3533b",
"public_key": "abd0f232775f45feab79864e580d160b",
"release": "1.0",
"environment": "dev",
"transaction": "/hello",
"replay_id": "d0303a19-909a-4b0b-a639-b17a73c3533b"
}"""

event = """{
"type": "transaction",
"transaction": "/world"
}"""

result = sentry_relay.run_dynamic_sampling(
sampling_config,
root_sampling_config,
dsc,
event,
)
assert "merged_sampling_configs" in result
assert result["sampling_match"] == {
"sample_rate": 1.0,
"seed": "d0303a19-909a-4b0b-a639-b17a74c3533b",
"matched_rule_ids": [1000, 1001],
}


def test_run_dynamic_sampling_with_valid_params_and_no_match():
sampling_config = """{
"rules": [],
"rulesV2": [],
"mode": "received"
}"""

root_sampling_config = """{
"rules": [],
"rulesV2": [
{
"samplingValue":{
"type": "sampleRate",
"value": 0.5
},
"type": "trace",
"active": true,
"condition": {
"op": "and",
"inner": [
{
"op": "eq",
"name": "trace.transaction",
"value": [
"/foo"
],
"options": {
"ignoreCase": true
}
}
]
},
"id": 1001
}
],
"mode": "received"
}"""

dsc = """{
"trace_id": "d0303a19-909a-4b0b-a639-b17a74c3533b",
"public_key": "abd0f232775f45feab79864e580d160b",
"release": "1.0",
"environment": "dev",
"transaction": "/hello",
"replay_id": "d0303a19-909a-4b0b-a639-b17a73c3533b"
}"""

event = """{
"type": "transaction",
"transaction": "/world"
}"""

result = sentry_relay.run_dynamic_sampling(
sampling_config,
root_sampling_config,
dsc,
event,
)
assert "merged_sampling_configs" in result
assert result["sampling_match"] is None


def test_run_dynamic_sampling_with_valid_params_and_no_dsc_and_no_event():
sampling_config = """{
"rules": [],
"rulesV2": [],
"mode": "received"
}"""

root_sampling_config = """{
"rules": [],
"rulesV2": [
{
"samplingValue":{
"type": "sampleRate",
"value": 0.5
},
"type": "trace",
"active": true,
"condition": {
"op": "and",
"inner": []
},
"id": 1001
}
],
"mode": "received"
}"""

dsc = "{}"

event = "{}"

result = sentry_relay.run_dynamic_sampling(
sampling_config,
root_sampling_config,
dsc,
event,
)
assert "merged_sampling_configs" in result
assert result["sampling_match"] is None


def test_run_dynamic_sampling_with_invalid_params():
sampling_config = """{
"rules": [],
"mode": "received"
}"""

root_sampling_config = """{
"rules": [],
"mode": "received"
}"""

dsc = """{
"trace_id": "d0303a19-909a-4b0b-a639-b17a74c3533b",
}"""

event = """{
"type": "transaction",
"test": "/test"
}"""

with pytest.raises(sentry_relay.InvalidJsonError):
sentry_relay.run_dynamic_sampling(
sampling_config,
root_sampling_config,
dsc,
event,
)
56 changes: 1 addition & 55 deletions relay-cabi/src/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use std::ffi::CStr;
use std::os::raw::c_char;
use std::slice;

use chrono::Utc;
use once_cell::sync::OnceCell;
use relay_common::glob::{glob_match_bytes, GlobOptions};
use relay_dynamic_config::{normalize_json, validate_json, GlobalConfig, ProjectConfig};
Expand All @@ -23,10 +22,7 @@ use relay_pii::{
};
use relay_protocol::{Annotated, Remark};
use relay_sampling::condition::RuleCondition;
use relay_sampling::config::SamplingRule;
use relay_sampling::evaluation::SamplingMatch;
use relay_sampling::{DynamicSamplingContext, SamplingConfig};
use serde::Serialize;
use relay_sampling::SamplingConfig;

use crate::core::{RelayBuf, RelayStr};

Expand Down Expand Up @@ -340,56 +336,6 @@ pub unsafe extern "C" fn normalize_global_config(value: *const RelayStr) -> Rela
}
}

#[derive(Debug, Serialize)]
struct EphemeralSamplingResult {
merged_sampling_configs: Vec<SamplingRule>,
sampling_match: Option<SamplingMatch>,
}

/// Runs dynamic sampling given the sampling config, root sampling config, DSC and event.
///
/// Returns the sampling decision containing the sample_rate and the list of matched rule ids.
#[no_mangle]
#[relay_ffi::catch_unwind]
pub unsafe extern "C" fn run_dynamic_sampling(
sampling_config: &RelayStr,
root_sampling_config: &RelayStr,
dsc: &RelayStr,
event: &RelayStr,
) -> RelayStr {
let sampling_config = serde_json::from_str::<SamplingConfig>(sampling_config.as_str())?;
let root_sampling_config =
serde_json::from_str::<SamplingConfig>(root_sampling_config.as_str())?;
// We can optionally accept a dsc and event.
let dsc = serde_json::from_str::<DynamicSamplingContext>(dsc.as_str());
let event = Annotated::<Event>::from_json(event.as_str());

// Instead of creating a new function, we decided to reuse the existing code here. This will have
// the only downside of not having the possibility to set the sample rate to a different value
// based on the `SamplingMode` but for this simulation it is not that relevant.
let rules: Vec<SamplingRule> = relay_sampling::evaluation::merge_rules_from_configs(
Some(&sampling_config),
Some(&root_sampling_config),
)
.cloned()
.collect();

// Only if we have both dsc and event we want to run dynamic sampling, otherwise we just return
// the merged sampling configs.
let match_result = if let (Ok(event), Ok(dsc)) = (event, dsc) {
SamplingMatch::match_against_rules(rules.iter(), event.value(), Some(&dsc), Utc::now())
} else {
None
};

let result = EphemeralSamplingResult {
merged_sampling_configs: rules,
sampling_match: match_result,
};

RelayStr::from(serde_json::to_string(&result).unwrap())
}

#[test]
fn pii_config_validation_invalid_regex() {
let config = r#"
Expand Down