Skip to content

Commit

Permalink
ref(profiling): remove support for metrics with profile namespace (#4391
Browse files Browse the repository at this point in the history
)

As we moved away from `generic-metrics` we won't need to support metrics
with profile namespace anymore.
  • Loading branch information
viglia authored Dec 16, 2024
1 parent a8305c8 commit 3f7fc88
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 89 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
- Add data categories for Uptime and Attachment Items. ([#4363](https://github.com/getsentry/relay/pull/4363), [#4374](https://github.com/getsentry/relay/pull/4374))
- Add ability to rate limit attachments by count (not just by the number of bytes). ([#4377](https://github.com/getsentry/relay/pull/4377))

**Internal**:

- Remove support for metrics with profile namespace. ([#4391](https://github.com/getsentry/relay/pull/4391))

## 24.11.2

**Breaking Changes**:
Expand Down
7 changes: 1 addition & 6 deletions relay-base-schema/src/metrics/mri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ pub enum MetricNamespace {
Transactions,
/// Metrics extracted from spans.
Spans,
/// Metrics extracted from profile functions.
Profiles,
/// User-defined metrics directly sent by SDKs and applications.
Custom,
/// Metric stats.
Expand All @@ -134,12 +132,11 @@ pub enum MetricNamespace {

impl MetricNamespace {
/// Returns all namespaces/variants of this enum.
pub fn all() -> [Self; 7] {
pub fn all() -> [Self; 6] {
[
Self::Sessions,
Self::Transactions,
Self::Spans,
Self::Profiles,
Self::Custom,
Self::Stats,
Self::Unsupported,
Expand All @@ -157,7 +154,6 @@ impl MetricNamespace {
Self::Sessions => "sessions",
Self::Transactions => "transactions",
Self::Spans => "spans",
Self::Profiles => "profiles",
Self::Custom => "custom",
Self::Stats => "metric_stats",
Self::Unsupported => "unsupported",
Expand All @@ -173,7 +169,6 @@ impl std::str::FromStr for MetricNamespace {
"sessions" => Ok(Self::Sessions),
"transactions" => Ok(Self::Transactions),
"spans" => Ok(Self::Spans),
"profiles" => Ok(Self::Profiles),
"custom" => Ok(Self::Custom),
"metric_stats" => Ok(Self::Stats),
_ => Ok(Self::Unsupported),
Expand Down
3 changes: 0 additions & 3 deletions relay-cogs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ pub enum AppFeature {
MetricsTransactions,
/// Metrics in the spans namespace.
MetricsSpans,
/// Metrics in the profiles namespace.
MetricsProfiles,
/// Metrics in the sessions namespace.
MetricsSessions,
/// Metrics in the custom namespace.
Expand Down Expand Up @@ -167,7 +165,6 @@ impl AppFeature {
Self::Replays => "replays",
Self::MetricsTransactions => "metrics_transactions",
Self::MetricsSpans => "metrics_spans",
Self::MetricsProfiles => "metrics_profiles",
Self::MetricsSessions => "metrics_sessions",
Self::MetricsCustom => "metrics_custom",
Self::MetricsStats => "metrics_metric_stats",
Expand Down
21 changes: 11 additions & 10 deletions relay-dynamic-config/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,6 @@ fn is_err_or_empty(filters_config: &ErrorBoundary<GenericFiltersConfig>) -> bool
#[derive(Default, Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
pub struct Options {
/// Kill switch for shutting down profile function metrics
/// ingestion in the generic-metrics platform
#[serde(
rename = "profiling.generic_metrics.functions_ingestion.enabled",
deserialize_with = "default_on_error",
skip_serializing_if = "is_default"
)]
pub profiles_function_generic_metrics_enabled: bool,

/// Kill switch for controlling the cardinality limiter.
#[serde(
rename = "relay.cardinality-limiter.mode",
Expand Down Expand Up @@ -211,6 +202,17 @@ pub struct Options {
)]
pub deprecated2: f32,

/// Deprecated, still forwarded for older downstream Relays.
/// Kill switch for shutting down profile function metrics
/// ingestion in the generic-metrics platform.
#[doc(hidden)]
#[serde(
rename = "profiling.generic_metrics.functions_ingestion.enabled",
deserialize_with = "default_on_error",
skip_serializing_if = "is_default"
)]
pub deprecated3: bool,

/// All other unknown options.
#[serde(flatten)]
other: HashMap<String, Value>,
Expand Down Expand Up @@ -249,7 +251,6 @@ impl BucketEncodings {
match namespace {
MetricNamespace::Transactions => self.transactions,
MetricNamespace::Spans => self.spans,
MetricNamespace::Profiles => self.profiles,
MetricNamespace::Custom => self.custom,
MetricNamespace::Stats => self.metric_stats,
// Always force the legacy encoding for sessions,
Expand Down
27 changes: 10 additions & 17 deletions relay-metrics/src/aggregator/cost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,29 +145,26 @@ mod tests {
}
"#);
cost_tracker.add_cost(MetricNamespace::Custom, project_key1, 50);
cost_tracker.add_cost(MetricNamespace::Profiles, project_key1, 50);
insta::assert_debug_snapshot!(cost_tracker, @r#"
CostTracker {
total_cost: 100,
total_cost: 50,
cost_per_project_key: {
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 100,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 50,
},
cost_per_namespace: {
Profiles: 50,
Custom: 50,
},
}
"#);
cost_tracker.add_cost(namespace, project_key2, 200);
insta::assert_debug_snapshot!(cost_tracker, @r#"
CostTracker {
total_cost: 300,
total_cost: 250,
cost_per_project_key: {
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 100,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 50,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fee"): 200,
},
cost_per_namespace: {
Profiles: 50,
Custom: 250,
},
}
Expand All @@ -176,13 +173,12 @@ mod tests {
cost_tracker.subtract_cost(namespace, project_key3, 666);
insta::assert_debug_snapshot!(cost_tracker, @r#"
CostTracker {
total_cost: 300,
total_cost: 250,
cost_per_project_key: {
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 100,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 50,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fee"): 200,
},
cost_per_namespace: {
Profiles: 50,
Custom: 250,
},
}
Expand All @@ -191,34 +187,31 @@ mod tests {
cost_tracker.subtract_cost(namespace, project_key1, 666);
insta::assert_debug_snapshot!(cost_tracker, @r#"
CostTracker {
total_cost: 300,
total_cost: 250,
cost_per_project_key: {
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 100,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 50,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fee"): 200,
},
cost_per_namespace: {
Profiles: 50,
Custom: 250,
},
}
"#);
cost_tracker.subtract_cost(namespace, project_key2, 20);
insta::assert_debug_snapshot!(cost_tracker, @r#"
CostTracker {
total_cost: 280,
total_cost: 230,
cost_per_project_key: {
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 100,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fed"): 50,
ProjectKey("a94ae32be2584e0bbd7a4cbb95971fee"): 180,
},
cost_per_namespace: {
Profiles: 50,
Custom: 230,
},
}
"#);

// Subtract all
cost_tracker.subtract_cost(MetricNamespace::Profiles, project_key1, 50);
cost_tracker.subtract_cost(MetricNamespace::Custom, project_key1, 50);
cost_tracker.subtract_cost(MetricNamespace::Custom, project_key2, 180);
insta::assert_debug_snapshot!(cost_tracker, @r#"
Expand Down
1 change: 0 additions & 1 deletion relay-metrics/src/cogs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ fn to_app_feature(ns: MetricNamespace) -> AppFeature {
MetricNamespace::Sessions => AppFeature::MetricsSessions,
MetricNamespace::Transactions => AppFeature::MetricsTransactions,
MetricNamespace::Spans => AppFeature::MetricsSpans,
MetricNamespace::Profiles => AppFeature::MetricsProfiles,
MetricNamespace::Custom => AppFeature::MetricsCustom,
MetricNamespace::Stats => AppFeature::MetricsStats,
MetricNamespace::Unsupported => AppFeature::MetricsUnsupported,
Expand Down
2 changes: 0 additions & 2 deletions relay-server/src/services/processor/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub fn is_valid_namespace(bucket: &Bucket, source: BucketSource) -> bool {
MetricNamespace::Sessions => true,
MetricNamespace::Transactions => true,
MetricNamespace::Spans => true,
MetricNamespace::Profiles => true,
MetricNamespace::Custom => true,
MetricNamespace::Stats => source == BucketSource::Internal,
MetricNamespace::Unsupported => false,
Expand Down Expand Up @@ -81,7 +80,6 @@ fn is_metric_namespace_valid(state: &ProjectInfo, namespace: MetricNamespace) ->
MetricNamespace::Sessions => true,
MetricNamespace::Transactions => true,
MetricNamespace::Spans => state.config.features.produces_spans(),
MetricNamespace::Profiles => true,
MetricNamespace::Custom => state.has_feature(Feature::CustomMetrics),
MetricNamespace::Stats => true,
MetricNamespace::Unsupported => false,
Expand Down
12 changes: 0 additions & 12 deletions relay-server/src/services/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,17 +663,6 @@ impl StoreService {
);
return Ok(());
}
MetricNamespace::Profiles => {
if !self
.global_config
.current()
.options
.profiles_function_generic_metrics_enabled
{
return Ok(());
}
KafkaTopic::MetricsGeneric
}
_ => KafkaTopic::MetricsGeneric,
};
let headers = BTreeMap::from([("namespace".to_string(), namespace.to_string())]);
Expand Down Expand Up @@ -1346,7 +1335,6 @@ impl Message for KafkaMessage<'_> {
MetricNamespace::Sessions => "metric_sessions",
MetricNamespace::Transactions => "metric_transactions",
MetricNamespace::Spans => "metric_spans",
MetricNamespace::Profiles => "metric_profiles",
MetricNamespace::Custom => "metric_custom",
MetricNamespace::Stats => "metric_metric_stats",
MetricNamespace::Unsupported => "metric_unsupported",
Expand Down
52 changes: 14 additions & 38 deletions tests/integration/test_metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -1905,44 +1905,6 @@ def test_missing_global_filters_enables_metric_extraction(
assert metrics_consumer.get_metrics()


def test_profiles_metrics(mini_sentry, relay):
relay = relay(mini_sentry, options=TEST_CONFIG)

project_id = 42
mini_sentry.add_basic_project_config(project_id)

timestamp = int(datetime.now(tz=timezone.utc).timestamp())
metrics_payload = f"profiles/foo:42|c|T{timestamp}\nprofiles/bar:17|c|T{timestamp}"

relay.send_metrics(project_id, metrics_payload)

envelope = mini_sentry.captured_events.get(timeout=3)
assert len(envelope.items) == 1

metrics_item = envelope.items[0]
assert metrics_item.type == "metric_buckets"

received_metrics = metrics_without_keys(
json.loads(metrics_item.get_bytes().decode()), keys={"metadata"}
)
assert received_metrics == [
{
"timestamp": time_after(timestamp),
"width": 1,
"name": "c:profiles/bar@none",
"value": 17.0,
"type": "c",
},
{
"timestamp": time_after(timestamp),
"width": 1,
"name": "c:profiles/foo@none",
"value": 42.0,
"type": "c",
},
]


def test_metrics_with_denied_names(
mini_sentry, relay_with_processing, metrics_consumer
):
Expand Down Expand Up @@ -2204,3 +2166,17 @@ def test_metrics_extraction_with_computed_context_filters(
metrics = metrics_consumer.get_metrics()
for metric, _ in metrics:
assert metric["name"] not in metric_names


def test_profiles_metrics(mini_sentry, relay):
relay = relay(mini_sentry, options=TEST_CONFIG)

project_id = 42
mini_sentry.add_basic_project_config(project_id)

timestamp = int(datetime.now(tz=timezone.utc).timestamp())
metrics_payload = f"profiles/foo:42|c|T{timestamp}\nprofiles/bar:17|c|T{timestamp}"

relay.send_metrics(project_id, metrics_payload)

assert mini_sentry.captured_events.empty()

0 comments on commit 3f7fc88

Please sign in to comment.