From 312d0363dcb01cd0f451fcd5fbf4abeff695ebc1 Mon Sep 17 00:00:00 2001 From: Lyn Nagara Date: Tue, 5 Mar 2024 12:43:18 -0800 Subject: [PATCH] ref: Remove references to deprecated settings settings.KAFKA_OUTCOMES and settings.KAFKA_OUTCOMES_BILLING are deprecated and will be removed soon --- src/sentry/testutils/pytest/kafka.py | 26 ------- src/sentry/utils/outcomes.py | 14 ++-- tests/sentry/utils/test_outcomes.py | 101 +++++++++++---------------- 3 files changed, 46 insertions(+), 95 deletions(-) diff --git a/src/sentry/testutils/pytest/kafka.py b/src/sentry/testutils/pytest/kafka.py index 151349f3bf6a9..aaaa8029b16ce 100644 --- a/src/sentry/testutils/pytest/kafka.py +++ b/src/sentry/testutils/pytest/kafka.py @@ -63,32 +63,6 @@ def inner(settings): return inner -@pytest.fixture -def kafka_topics_setter(): - """ - Returns a function that given a Django settings objects will setup the - kafka topics names to test names. - - :return: a function that given a settings object changes all kafka topic names - to "test-" - """ - - def set_test_kafka_settings(settings): - settings.KAFKA_INGEST_EVENTS = "ingest-events" - settings.KAFKA_TOPICS[settings.KAFKA_INGEST_EVENTS] = {"cluster": "default"} - - settings.INGEST_TRANSACTIONS = "ingest-transactions" - settings.KAFKA_TOPICS[settings.INGEST_TRANSACTIONS] = {"cluster": "default"} - - settings.KAFKA_INGEST_ATTACHMENTS = "ingest-attachments" - settings.KAFKA_TOPICS[settings.KAFKA_INGEST_ATTACHMENTS] = {"cluster": "default"} - - settings.KAFKA_OUTCOMES = "outcomes" - settings.KAFKA_TOPICS[settings.KAFKA_OUTCOMES] = {"cluster": "default"} - - return set_test_kafka_settings - - @pytest.fixture(scope="session") def scope_consumers(): """ diff --git a/src/sentry/utils/outcomes.py b/src/sentry/utils/outcomes.py index 19774c0a294a0..f7f1947797d35 100644 --- a/src/sentry/utils/outcomes.py +++ b/src/sentry/utils/outcomes.py @@ -4,8 +4,6 @@ from datetime import datetime from enum import IntEnum -from django.conf import settings - from sentry.conf.types.kafka_definition import Topic from sentry.constants import DataCategory from sentry.utils import json, kafka_config, metrics @@ -98,14 +96,10 @@ def track_outcome( timestamp = timestamp or to_datetime(time.time()) - # Send billing outcomes to a dedicated topic if there is a separate - # configuration for it. Otherwise, fall back to the regular outcomes topic. - # This does NOT switch the producer, if both topics are on the same cluster. - # - # In Sentry, there is no significant difference between the classes of - # outcome. In Sentry SaaS, they have elevated stability requirements as they - # are used for spike protection and quota enforcement. - topic_name = settings.KAFKA_OUTCOMES_BILLING if use_billing else settings.KAFKA_OUTCOMES + # Send billing outcomes to a dedicated topic. + topic_name = ( + billing_config["real_topic_name"] if use_billing else outcomes_config["real_topic_name"] + ) # Send a snuba metrics payload. publisher.publish( diff --git a/tests/sentry/utils/test_outcomes.py b/tests/sentry/utils/test_outcomes.py index c7f6a479c47f0..79f1c02d26f37 100644 --- a/tests/sentry/utils/test_outcomes.py +++ b/tests/sentry/utils/test_outcomes.py @@ -2,7 +2,6 @@ from unittest import mock import pytest -from django.conf import settings from sentry.conf.types.kafka_definition import Topic from sentry.utils import json, kafka_config, outcomes @@ -61,45 +60,38 @@ def test_parse_outcome(name, outcome): def test_track_outcome_default(setup): """ Asserts an outcomes serialization roundtrip with defaults. - - Additionally checks that non-billing outcomes are routed to the DEFAULT - outcomes cluster and topic, even if there is a separate cluster for billing - outcomes. """ + track_outcome( + org_id=1, + project_id=2, + key_id=3, + outcome=Outcome.INVALID, + reason="project_id", + ) - # Provide a billing cluster config that should be ignored - with mock.patch.dict( - settings.KAFKA_TOPICS, {settings.KAFKA_OUTCOMES_BILLING: {"cluster": "different"}} - ): - track_outcome( - org_id=1, - project_id=2, - key_id=3, - outcome=Outcome.INVALID, - reason="project_id", - ) - - cluster_args, _ = setup.mock_get_kafka_producer_cluster_options.call_args - assert cluster_args == (kafka_config.get_topic_definition(Topic.OUTCOMES)["cluster"],) - - assert outcomes.outcomes_publisher - (topic_name, payload), _ = setup.mock_publisher.return_value.publish.call_args - assert topic_name == settings.KAFKA_OUTCOMES + cluster_args, _ = setup.mock_get_kafka_producer_cluster_options.call_args + assert cluster_args == (kafka_config.get_topic_definition(Topic.OUTCOMES)["cluster"],) - data = json.loads(payload) - del data["timestamp"] - assert data == { - "org_id": 1, - "project_id": 2, - "key_id": 3, - "outcome": Outcome.INVALID.value, - "reason": "project_id", - "event_id": None, - "category": None, - "quantity": 1, - } + assert outcomes.outcomes_publisher + (topic_name, payload), _ = setup.mock_publisher.return_value.publish.call_args + + # not billing because it's not accepted/rate limited + assert topic_name == "outcomes" + + data = json.loads(payload) + del data["timestamp"] + assert data == { + "org_id": 1, + "project_id": 2, + "key_id": 3, + "outcome": Outcome.INVALID.value, + "reason": "project_id", + "event_id": None, + "category": None, + "quantity": 1, + } - assert outcomes.billing_publisher is None + assert outcomes.billing_publisher is None def test_track_outcome_billing(setup): @@ -120,7 +112,7 @@ def test_track_outcome_billing(setup): assert outcomes.outcomes_publisher (topic_name, _), _ = setup.mock_publisher.return_value.publish.call_args - assert topic_name == settings.KAFKA_OUTCOMES_BILLING + assert topic_name == "outcomes-billing" assert outcomes.billing_publisher is None @@ -130,30 +122,21 @@ def test_track_outcome_billing_topic(setup): Checks that outcomes are routed to the DEDICATED billing topic within the same cluster in default configuration. """ + track_outcome( + org_id=1, + project_id=1, + key_id=1, + outcome=Outcome.ACCEPTED, + ) - with mock.patch.dict( - settings.KAFKA_TOPICS, - { - settings.KAFKA_OUTCOMES_BILLING: { - "cluster": kafka_config.get_topic_definition(Topic.OUTCOMES)["cluster"], - } - }, - ): - track_outcome( - org_id=1, - project_id=1, - key_id=1, - outcome=Outcome.ACCEPTED, - ) - - cluster_args, _ = setup.mock_get_kafka_producer_cluster_options.call_args - assert cluster_args == (kafka_config.get_topic_definition(Topic.OUTCOMES)["cluster"],) + cluster_args, _ = setup.mock_get_kafka_producer_cluster_options.call_args + assert cluster_args == (kafka_config.get_topic_definition(Topic.OUTCOMES)["cluster"],) - assert outcomes.outcomes_publisher - (topic_name, _), _ = setup.mock_publisher.return_value.publish.call_args - assert topic_name == settings.KAFKA_OUTCOMES_BILLING + assert outcomes.outcomes_publisher + (topic_name, _), _ = setup.mock_publisher.return_value.publish.call_args + assert topic_name == "outcomes-billing" - assert outcomes.billing_publisher is None + assert outcomes.billing_publisher is None def test_track_outcome_billing_cluster(settings, setup): @@ -174,6 +157,6 @@ def test_track_outcome_billing_cluster(settings, setup): assert outcomes.billing_publisher (topic_name, _), _ = setup.mock_publisher.return_value.publish.call_args - assert topic_name == settings.KAFKA_OUTCOMES_BILLING + assert topic_name == "outcomes-billing" assert outcomes.outcomes_publisher is None