Skip to content

Commit

Permalink
fix(am1): Add span count metric (unrevert) (#75827)
Browse files Browse the repository at this point in the history
We ran into trouble with the previous PR due to a few issues; a typo in
the name, exception logging in the 'has' causing a thundering herd of
exceptions, and the condition order causing the problem to not trip
tests / soak time.

original PR for context: #75485

Notes:
- `am3_tier` vs `am3-tier` as a result of a bad copy paste (out of a
thread instead of the file it's registered)
- `logger.exception` circumventing the `try / except`, already fixed
[here](687086f#diff-9cd8ed7a349a276392e908cabcd613d3647a94eaa5956ae79e4efb604e2c9232L309)
- Conditional order has `organizations:dynamic-sampling` first, which
causes an early shortcut of the `and`
  • Loading branch information
k-fish committed Aug 8, 2024
1 parent 6b7c828 commit 864255d
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
24 changes: 23 additions & 1 deletion src/sentry/ingest/consumer/processors.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import functools
import logging
from collections.abc import Mapping
from collections.abc import Mapping, MutableMapping
from typing import Any

import orjson
Expand Down Expand Up @@ -187,6 +187,11 @@ def process_event(
event_id=event_id,
project_id=project_id,
)

try:
collect_span_metrics(project, data)
except Exception:
pass
elif data.get("type") == "feedback":
if features.has("organizations:user-feedback-ingest", project.organization, actor=None):
save_event_feedback.delay(
Expand Down Expand Up @@ -324,3 +329,20 @@ def process_userreport(message: IngestMessage, project: Project) -> bool:
# If you want to remove this make sure to have triaged all errors in Sentry
logger.exception("userreport.save.crash")
return False


def collect_span_metrics(
project: Project,
data: MutableMapping[str, Any],
):
if not features.has("organizations:am3-tier", project.organization) and not features.has(
"organizations:dynamic-sampling", project.organization
):
amount = (
len(data.get("spans", [])) + 1
) # Segment spans also get added to the total span count.
metrics.incr(
"event.save_event.unsampled.spans.count",
amount=amount,
tags={"organization": project.organization.slug},
)
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import zipfile
from io import BytesIO
from typing import Any
from unittest.mock import Mock
from unittest.mock import Mock, patch

import orjson
import pytest
Expand All @@ -19,6 +19,7 @@
from sentry import eventstore
from sentry.event_manager import EventManager
from sentry.ingest.consumer.processors import (
collect_span_metrics,
process_attachment_chunk,
process_event,
process_individual_attachment,
Expand All @@ -28,6 +29,7 @@
from sentry.models.eventattachment import EventAttachment
from sentry.models.userreport import UserReport
from sentry.options import set
from sentry.testutils.helpers.features import Feature
from sentry.testutils.pytest.fixtures import django_db_all
from sentry.testutils.skips import requires_snuba, requires_symbolicator
from sentry.usage_accountant import accountant
Expand Down Expand Up @@ -595,3 +597,19 @@ def test_individual_attachments_missing_chunks(default_project, factories, monke
attachments = list(EventAttachment.objects.filter(project_id=project_id, event_id=event_id))

assert not attachments


@django_db_all
def test_collect_span_metrics(default_project):
with Feature({"organizations:dynamic-sampling": True, "organization:am3-tier": True}):
with patch("sentry.ingest.consumer.processors.metrics") as mock_metrics:
assert mock_metrics.incr.call_count == 0
collect_span_metrics(default_project, {"spans": [1, 2, 3]})
assert mock_metrics.incr.call_count == 0

with Feature({"organizations:dynamic-sampling": False, "organization:am3-tier": False}):
with patch("sentry.ingest.consumer.processors.metrics") as mock_metrics:

assert mock_metrics.incr.call_count == 0
collect_span_metrics(default_project, {"spans": [1, 2, 3]})
assert mock_metrics.incr.call_count == 1

0 comments on commit 864255d

Please sign in to comment.