Merge branch 'master' into ogi/feat/span_attribute_extraction_counter

src/sentry/api/endpoints/organization_release_health_data.py

@@ -11,6 +11,7 @@ from sentry.exceptions import InvalidParams
 from sentry.sentry_metrics.use_case_utils import get_use_case_id
 from sentry.snuba.metrics import DerivedMetricException, QueryDefinition, get_series
 from sentry.types.ratelimit import RateLimit, RateLimitCategory
+from sentry.utils import metrics
 from sentry.utils.cursors import Cursor, CursorResult
 
 
@@ -59,6 +60,9 @@ class OrganizationReleaseHealthDataEndpoint(OrganizationEndpoint):
                     use_case_id=get_use_case_id(request),
                     tenant_ids={"organization_id": organization.id},
                 )
+                # due to possible data corruption crash_free_rate value can be less than 0,
+                # which is not valid behavior, so those values have to be bottom capped at 0
+                metrics.ensure_non_negative_crash_free_rate_value(data, request, organization)
                 data["query"] = query.query
             except (
                 InvalidParams,

src/sentry/snuba/metrics/span_attribute_extraction.py

@@ -175,13 +175,11 @@ def _map_span_attribute_name(span_attribute: str) -> str:
         return span_attribute
 
     if span_attribute in _SENTRY_TAGS:
-        prefix = "span.sentry_tags"
-    else:
-        prefix = "span.data"
+        return f"span.sentry_tags.{span_attribute}"
 
     sanitized_span_attr = span_attribute.replace(".", "\\.")
 
-    return f"{prefix}.{sanitized_span_attr}"
+    return f"span.data.{sanitized_span_attr}"
 
 
 def _is_counter(extraction_rule: MetricsExtractionRule) -> bool:

src/sentry/utils/metrics.py

@@ -11,7 +11,9 @@ from random import random
 from threading import Thread
 from typing import Any, TypeVar
 
+import sentry_sdk
 from django.conf import settings
+from rest_framework.request import Request
 
 from sentry.metrics.base import MetricsBackend, MutableTags, Tags
 from sentry.metrics.middleware import MiddlewareWrapper, add_global_tags, global_tags
@@ -28,6 +30,7 @@ __all__ = [
     "gauge",
     "backend",
     "MutableTags",
+    "ensure_non_negative_crash_free_rate_value",
 ]
 
 
@@ -259,3 +262,57 @@ def event(
     except Exception:
         logger = logging.getLogger("sentry.errors")
         logger.exception("Unable to record backend metric")
+
+
+def ensure_non_negative_crash_free_rate_value(
+    data: Any, request: Request, organization, CRASH_FREE_RATE_METRIC_KEY="session.crash_free_rate"
+):
+    """
+    Ensures that crash_free_rate metric will never have negative
+    value returned to the customer by replacing all the negative values with 0.
+    Negative value of crash_free_metric can happen due to the
+    corrupted data that is used to calculate the metric
+    (see: https://github.com/getsentry/sentry/issues/73172)
+
+    Example format of data argument:
+    {
+        ...
+        "groups" : [
+            ...
+            "series": {..., "session.crash_free_rate": [..., None, 0.35]},
+            "totals": {..., "session.crash_free_rate": 0.35}
+        ]
+    }
+    """
+    groups = data["groups"]
+    for group in groups:
+        if "series" in group:
+            series = group["series"]
+            if CRASH_FREE_RATE_METRIC_KEY in series:
+                for i, value in enumerate(series[CRASH_FREE_RATE_METRIC_KEY]):
+                    try:
+                        value = float(value)
+                        if value < 0:
+                            with sentry_sdk.push_scope() as scope:
+                                scope.set_tag("organization", organization.id)
+                                scope.set_extra("crash_free_rate_in_series", value)
+                                scope.set_extra("request_query_params", request.query_params)
+                                sentry_sdk.capture_message("crash_free_rate in series is negative")
+                            series[CRASH_FREE_RATE_METRIC_KEY][i] = 0
+                    except TypeError:
+                        # value is not a number
+                        continue
+
+        if "totals" in group:
+            totals = group["totals"]
+            if (
+                CRASH_FREE_RATE_METRIC_KEY in totals
+                and totals[CRASH_FREE_RATE_METRIC_KEY] is not None
+                and totals[CRASH_FREE_RATE_METRIC_KEY] < 0
+            ):
+                with sentry_sdk.push_scope() as scope:
+                    scope.set_tag("organization", organization.id)
+                    scope.set_extra("crash_free_rate", totals[CRASH_FREE_RATE_METRIC_KEY])
+                    scope.set_extra("request_query_params", request.query_params)
+                    sentry_sdk.capture_message("crash_free_rate is negative")
+                totals[CRASH_FREE_RATE_METRIC_KEY] = 0

src/sentry/utils/sdk.py

@@ -76,7 +76,7 @@ SAMPLED_TASKS = {
     "sentry.tasks.derive_code_mappings.derive_code_mappings": settings.SAMPLED_DEFAULT_RATE,
     "sentry.monitors.tasks.clock_pulse": 1.0,
     "sentry.tasks.auto_enable_codecov": settings.SAMPLED_DEFAULT_RATE,
-    "sentry.dynamic_sampling.tasks.boost_low_volume_projects": 1.0,
+    "sentry.dynamic_sampling.tasks.boost_low_volume_projects": 0.2,
     "sentry.dynamic_sampling.tasks.boost_low_volume_transactions": 0.2,
     "sentry.dynamic_sampling.tasks.recalibrate_orgs": 0.2,
     "sentry.dynamic_sampling.tasks.sliding_window_org": 0.2,
@@ -223,13 +223,6 @@ def before_send_transaction(event: Event, _: Hint) -> Event | None:
     ):
         return None
 
-    # This code is added only for debugging purposes, as such, it should be removed once the investigation is done.
-    if event.get("transaction") in {
-        "sentry.dynamic_sampling.boost_low_volume_projects_of_org",
-        "sentry.dynamic_sampling.tasks.boost_low_volume_projects",
-    }:
-        logger.info("transaction_logged", extra=event)
-
     # Occasionally the span limit is hit and we drop spans from transactions, this helps find transactions where this occurs.
     num_of_spans = len(event["spans"])
     event["tags"]["spans_over_limit"] = str(num_of_spans >= 1000)

static/app/types/project.tsx

@@ -23,6 +23,7 @@ export type Project = {
   eventProcessing: {
     symbolicationDegraded: boolean;
   };
+  extrapolateMetrics: boolean;
   features: string[];
   firstEvent: string | null;
   firstTransactionEvent: boolean;
@@ -52,8 +53,8 @@ export type Project = {
   isMember: boolean;
   name: string;
   organization: Organization;
-  plugins: Plugin[];
 
+  plugins: Plugin[];
   processingIssues: number;
   relayCustomMetricCardinalityLimit: number | null;
   relayPiiConfig: string;

static/app/utils/metrics/features.tsx

@@ -17,6 +17,10 @@ export function hasCustomMetricsExtractionRules(organization: Organization) {
   return organization.features.includes('custom-metrics-extraction-rule');
 }
 
+export function hasMetricsExtrapolationFeature(organization: Organization) {
+  return organization.features.includes('metrics-extrapolation');
+}
+
 /**
  * Returns the forceMetricsLayer query param for the alert
  * wrapped in an object so it can be spread into existing query params

static/app/views/settings/projectMetrics/extrapolationField.tsx

@@ -0,0 +1,76 @@
+import {useEffect, useState} from 'react';
+
+import {
+  addErrorMessage,
+  addLoadingMessage,
+  addSuccessMessage,
+} from 'sentry/actionCreators/indicator';
+import BooleanField from 'sentry/components/forms/fields/booleanField';
+import ExternalLink from 'sentry/components/links/externalLink';
+import Panel from 'sentry/components/panels/panel';
+import PanelBody from 'sentry/components/panels/panelBody';
+import {t, tct} from 'sentry/locale';
+import ProjectsStore from 'sentry/stores/projectsStore';
+import type {Project} from 'sentry/types/project';
+import {useMutation} from 'sentry/utils/queryClient';
+import type RequestError from 'sentry/utils/requestError/requestError';
+import useApi from 'sentry/utils/useApi';
+import useOrganization from 'sentry/utils/useOrganization';
+
+interface ExtrapolationFieldProps {
+  project: Project;
+}
+
+export function ExtrapolationField({project}: ExtrapolationFieldProps) {
+  const organization = useOrganization();
+  const api = useApi();
+
+  const [isToggleEnabled, setIsToggleEnabled] = useState(!!project.extrapolateMetrics);
+
+  // Reload from props if new project state is received
+  useEffect(() => {
+    setIsToggleEnabled(!!project.extrapolateMetrics);
+  }, [project.extrapolateMetrics]);
+
+  const {mutate: handleToggleChange} = useMutation<Project, RequestError, boolean>({
+    mutationFn: value => {
+      return api.requestPromise(`/projects/${organization.slug}/${project.slug}/`, {
+        method: 'PUT',
+        data: {
+          extrapolateMetrics: value,
+        },
+      });
+    },
+    onMutate: () => {
+      addLoadingMessage(t('Toggling metrics extrapolation'));
+    },
+    onSuccess: updatedProject => {
+      addSuccessMessage(t('Successfully toggled metrics extrapolation'));
+      ProjectsStore.onUpdateSuccess(updatedProject);
+    },
+    onError: () => {
+      addErrorMessage(t('Failed to toggle metrics extrapolation'));
+    },
+  });
+
+  return (
+    <Panel>
+      <PanelBody>
+        <BooleanField
+          onChange={handleToggleChange}
+          value={isToggleEnabled}
+          name="metrics-extrapolation-toggle"
+          disabled={!project.access.includes('project:write')} // admin, manager and owner of an organization will be able to edit this field
+          label={t('Metrics Extrapolation')}
+          help={tct(
+            'Enables metrics extrapolation from sampled data, providing more reliable and comprehensive metrics for your project. To learn more about metrics extrapolation, [link:read the docs]',
+            {
+              // TODO(telemetry-experience): Add link to metrics extrapolation docs when available
+              link: <ExternalLink href="https://docs.sentry.io/product/metrics/" />,
+            }
+          )}
+        />
+      </PanelBody>
+    </Panel>
+  );
+}

static/app/views/settings/projectMetrics/projectMetrics.tsx

@@ -9,7 +9,10 @@ import {t, tct} from 'sentry/locale';
 import type {Organization} from 'sentry/types/organization';
 import type {Project} from 'sentry/types/project';
 import {METRICS_DOCS_URL} from 'sentry/utils/metrics/constants';
-import {hasCustomMetricsExtractionRules} from 'sentry/utils/metrics/features';
+import {
+  hasCustomMetricsExtractionRules,
+  hasMetricsExtrapolationFeature,
+} from 'sentry/utils/metrics/features';
 import routeTitleGen from 'sentry/utils/routeTitle';
 import useOrganization from 'sentry/utils/useOrganization';
 import {useMetricsOnboardingSidebar} from 'sentry/views/metrics/ddmOnboarding/useMetricsOnboardingSidebar';
@@ -17,6 +20,7 @@ import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHea
 import TextBlock from 'sentry/views/settings/components/text/textBlock';
 import PermissionAlert from 'sentry/views/settings/project/permissionAlert';
 import {CustomMetricsTable} from 'sentry/views/settings/projectMetrics/customMetricsTable';
+import {ExtrapolationField} from 'sentry/views/settings/projectMetrics/extrapolationField';
 import {MetricsExtractionRulesTable} from 'sentry/views/settings/projectMetrics/metricsExtractionRulesTable';
 
 type Props = {
@@ -65,6 +69,10 @@ function ProjectMetrics({project}: Props) {
 
       <PermissionAlert project={project} />
 
+      {hasMetricsExtrapolationFeature(organization) ? (
+        <ExtrapolationField project={project} />
+      ) : null}
+
       {hasExtractionRules ? <MetricsExtractionRulesTable project={project} /> : null}
 
       {!hasExtractionRules || project.hasCustomMetrics ? (

tests/js/fixtures/project.ts

@@ -59,6 +59,7 @@ export function ProjectFixture(params: Partial<Project> = {}): Project {
     sensitiveFields: [],
     subjectTemplate: '',
     verifySSL: false,
+    extrapolateMetrics: false,
     ...params,
   };
 }

tests/sentry/api/endpoints/test_organization_release_health_data.py

@@ -2413,3 +2413,59 @@ class DerivedMetricsDataTest(MetricsAPIBaseTestCase):
             "totals": {"p50(session.duration)": 6.0},
             "series": {"p50(session.duration)": [6.0]},
         }
+
+    def test_do_not_return_negative_crash_free_rate_value_to_the_customer(self):
+        """
+        Bug: https://github.com/getsentry/sentry/issues/73172
+        Assert that negative value is never returned to the user, even
+        in case when there is a problem with the data, instead of negative value we return 0.
+        This problem happens during ingestion when 'e:session/crashed' metric is
+        ingested, but 'e:session/all' for some reason is not. That would cause
+        crash_free_rate value to be negative since it is calculated as:
+
+        crash_free_rate = 1 - (count('e:session/crashed') / count('e:session/all'))
+        """
+        # ingesting 'e:session/all' and 'e:session/crashed'
+        self.build_and_store_session(
+            project_id=self.project.id,
+            minutes_before_now=1,
+            status="crashed",
+        )
+
+        # manually ingesting only 'e:session/crashed' metric
+        # to make sure that there are more 'e:session/crashed' metrics ingested
+        # than 'e:session/all'
+        for i in range(2):
+            session = self.build_session(
+                started=self.adjust_timestamp(
+                    self.now
+                    - timedelta(
+                        minutes=i,
+                    )
+                ).timestamp()
+            )
+            # ingesting only 'e:session/crashed'
+            self.store_metric(
+                self.organization.id,
+                self.project.id,
+                "counter",
+                SessionMRI.RAW_SESSION.value,
+                {"session.status": "crashed"},
+                int(session["started"]),
+                +1,
+                use_case_id=UseCaseID.SESSIONS,
+            )
+
+        response = self.get_success_response(
+            self.organization.slug,
+            field=["session.crash_free_rate", "session.all", "session.crashed"],
+            statsPeriod="6m",
+            interval="1m",
+        )
+
+        group = response.data["groups"][0]
+        assert group["totals"]["session.all"] == 1.0
+        assert group["totals"]["session.crashed"] == 3.0
+        assert group["totals"]["session.crash_free_rate"] == 0.0
+        for value in group["series"]["session.crash_free_rate"]:
+            assert value is None or value >= 0