Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/sentry/features/temporary.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ def register_temporary_features(manager: FeatureManager) -> None:
# Enable higher limit for workflows
manager.add("organizations:more-workflows", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
# Normalize segment names during span enrichment
manager.add("organizations:normalize_segment_names_in_span_enrichment", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
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.

Dangling comment left after removing feature flag

Low Severity

The comment # Normalize segment names during span enrichment on line 186 was left behind after removing the manager.add("organizations:normalize_segment_names_in_span_enrichment", ...) call it described. It now sits as an orphaned comment between the more-workflows feature and the # Extract on demand metrics comment, which could confuse future readers.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ae5a9b5. Configure here.

# Extract on demand metrics
manager.add("organizations:on-demand-metrics-extraction", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
# Extract on demand metrics (experimental features)
Expand Down
30 changes: 1 addition & 29 deletions src/sentry/spans/consumers/process_segments/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,10 @@
from sentry_conventions.attributes import ATTRIBUTE_NAMES
from sentry_kafka_schemas.schema_types.ingest_spans_v1 import SpanEvent

from sentry import features, options
from sentry import options
from sentry.constants import DataCategory
from sentry.dynamic_sampling.rules.helpers.latest_releases import record_latest_release
from sentry.event_manager import INSIGHT_MODULE_TO_PROJECT_FLAG_NAME
from sentry.ingest.transaction_clusterer.datasource import TRANSACTION_SOURCE_URL
from sentry.ingest.transaction_clusterer.datasource.redis import record_segment_name
from sentry.ingest.transaction_clusterer.normalization import normalize_segment_name
from sentry.insights import FilterSpan
from sentry.insights import modules as insights_modules
from sentry.issue_detection.performance_detection import detect_performance_problems
Expand All @@ -41,7 +38,6 @@
from sentry.utils.dates import to_datetime
from sentry.utils.outcomes import Outcome, OutcomeAggregator
from sentry.utils.projectflags import set_project_flag_and_signal
from sentry.utils.safe import safe_execute

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -103,7 +99,6 @@ def _process_segment(
):
return []

safe_execute(_normalize_segment_name, segment_span, project)
_add_segment_name(segment_span, spans)
_compute_breakdowns(segment_span, spans, project)
_create_models(segment_span, project)
Expand Down Expand Up @@ -183,29 +178,6 @@ def _enrich_spans(
return segment, spans


@metrics.wraps("spans.consumers.process_segments.normalize_segment_name")
@sentry_sdk.trace
def _normalize_segment_name(segment_span: CompatibleSpan, project: Project) -> None:
if not features.has(
"organizations:normalize_segment_names_in_span_enrichment", project.organization
):
return

segment_name = attribute_value(
segment_span, ATTRIBUTE_NAMES.SENTRY_SEGMENT_NAME
) or segment_span.get("name")
if not segment_name:
return

source = attribute_value(segment_span, ATTRIBUTE_NAMES.SENTRY_SPAN_SOURCE)
unknown_if_parameterized = not source
known_to_be_unparameterized = source == TRANSACTION_SOURCE_URL
if unknown_if_parameterized or known_to_be_unparameterized:
normalize_segment_name(project, segment_span)

record_segment_name(project, segment_span)


@metrics.wraps("spans.consumers.process_segments.add_segment_name")
def _add_segment_name(segment: CompatibleSpan, spans: Sequence[CompatibleSpan]) -> None:
segment_name = segment.get("name")
Expand Down
45 changes: 0 additions & 45 deletions tests/sentry/spans/consumers/process_segments/test_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
from unittest import mock

import pytest
from sentry_conventions.attributes import ATTRIBUTE_NAMES

from sentry.issues.grouptype import PerformanceStreamedSpansGroupTypeExperimental
from sentry.models.environment import Environment
from sentry.models.release import Release
from sentry.spans.consumers.process_segments.message import _verify_compatibility, process_segment
from sentry.spans.consumers.process_segments.shim import build_shim_event_data
from sentry.testutils.cases import TestCase
from sentry.testutils.helpers.features import Feature
from sentry.testutils.helpers.options import override_options
from sentry.testutils.issue_detection.experiments import exclude_experimental_detectors
from tests.sentry.spans.consumers.process import build_mock_span
Expand Down Expand Up @@ -305,49 +303,6 @@ def test_segment_name_propagation_when_name_missing(self) -> None:
child_attributes = child_span["attributes"] or {}
assert child_attributes.get("sentry.segment.name") is None

@mock.patch("sentry.spans.consumers.process_segments.message.record_segment_name")
def test_segment_name_normalization_with_feature(
self, mock_record_segment_name: mock.MagicMock
):
_, segment_span = self.generate_basic_spans()
segment_span["name"] = "/foo/2fd4e1c67a2d28fced849ee1bb76e7391b93eb12/user/123/0"

with self.feature("organizations:normalize_segment_names_in_span_enrichment"):
processed_spans = process_segment([segment_span])

assert processed_spans[0]["name"] == "/foo/*/user/*/0"
mock_record_segment_name.assert_called_once()

@mock.patch("sentry.spans.consumers.process_segments.message.record_segment_name")
def test_segment_name_normalization_without_feature(
self, mock_record_segment_name: mock.MagicMock
):
_, segment_span = self.generate_basic_spans()
segment_span["name"] = "/foo/2fd4e1c67a2d28fced849ee1bb76e7391b93eb12/user/123/0"

with Feature({"organizations:normalize_segment_names_in_span_enrichment": False}):
processed_spans = process_segment([segment_span])

assert (
processed_spans[0]["name"] == "/foo/2fd4e1c67a2d28fced849ee1bb76e7391b93eb12/user/123/0"
)
mock_record_segment_name.assert_not_called()

def test_segment_name_normalization_checks_source(self) -> None:
_, segment_span = self.generate_basic_spans()
segment_span["name"] = "/foo/2fd4e1c67a2d28fced849ee1bb76e7391b93eb12/user/123/0"
segment_span["attributes"][ATTRIBUTE_NAMES.SENTRY_SPAN_SOURCE] = {
"type": "string",
"value": "route",
}

with self.feature("organizations:normalize_segment_names_in_span_enrichment"):
processed_spans = process_segment([segment_span])

assert (
processed_spans[0]["name"] == "/foo/2fd4e1c67a2d28fced849ee1bb76e7391b93eb12/user/123/0"
)


def test_verify_compatibility() -> None:
spans: list[dict[str, Any]] = [
Expand Down
Loading