feat(tracing): Gate SQL query params behind data collection options

Record SQL params and paramstyle in `record_sql_queries` based on the new
`data_collection.database_query_data` option instead of the
`_experiments.record_sql_params` flag. When data collection is enabled, it
takes precedence over the legacy flag; otherwise the legacy behavior is
preserved until the experiment is removed.

Refs PY-2587

Author: ericapisani

sentry_sdk/tracing_utils.py

@@ -27,6 +27,7 @@
     _module_in_list,
     capture_internal_exceptions,
     filename_for_module,
+    has_data_collection_enabled,
     is_sentry_url,
     is_valid_sample_rate,
     logger,
@@ -145,15 +146,27 @@ def record_sql_queries(
 ) -> "Generator[Union[sentry_sdk.tracing.Span, sentry_sdk.traces.StreamedSpan], None, None]":
     # TODO: Bring back capturing of params by default
     client = sentry_sdk.get_client()
-    if client.options["_experiments"].get("record_sql_params", False):
-        if not params_list or params_list == [None]:
-            params_list = None
+    if has_data_collection_enabled(client.options):
+        if client.options["data_collection"]["database_query_data"]:
+            if not params_list or params_list == [None]:
+                params_list = None
 
-        if paramstyle == "pyformat":
-            paramstyle = "format"
+            if paramstyle == "pyformat":
+                paramstyle = "format"
+        else:
+            params_list = None
+            paramstyle = None
     else:
-        params_list = None
-        paramstyle = None
+        # TODO: remove this else block once data collection is released
+        if client.options["_experiments"].get("record_sql_params", False):
+            if not params_list or params_list == [None]:
+                params_list = None
+
+            if paramstyle == "pyformat":
+                paramstyle = "format"
+        else:
+            params_list = None
+            paramstyle = None
 
     query = _format_sql(cursor, query)
 

tests/test_tracing_utils.py

@@ -1,12 +1,15 @@
 from dataclasses import asdict, dataclass
-from typing import List, Optional
+from typing import Any, Dict, List, Optional
+from unittest import mock
 
 import pytest
 
+import sentry_sdk
 from sentry_sdk.tracing_utils import (
     Baggage,
     _should_be_included,
     _should_continue_trace,
+    record_sql_queries,
 )
 from tests.conftest import TestTransportWithOptions
 
@@ -286,3 +289,128 @@ def test_baggage_from_incoming_header_value_with_equals_sign():
     header = "sentry-release=v1.0==1,sentry-trace_id=abc123"
     baggage = Baggage.from_incoming_header(header)
     assert baggage.sentry_items == {"release": "v1.0==1", "trace_id": "abc123"}
+
+
+def _get_query_breadcrumb_data(
+    sentry_init,
+    capture_events,
+    sentry_options: "Dict[str, Any]",
+    params_list: "Any" = [1, 2],
+    paramstyle: "Optional[str]" = "pyformat",
+) -> "Dict[str, Any]":
+    sentry_init(**sentry_options)
+    events = capture_events()
+
+    with record_sql_queries(
+        cursor=mock.MagicMock(),
+        query="SELECT * FROM users WHERE id IN (%s, %s)",
+        params_list=params_list,
+        paramstyle=paramstyle,
+        executemany=False,
+    ):
+        pass
+
+    sentry_sdk.capture_message("hi")
+    (event,) = events
+    (crumb,) = event["breadcrumbs"]["values"]
+    assert crumb["category"] == "query"
+    return crumb["data"]
+
+
+@pytest.mark.parametrize(
+    "sentry_options, expected_data",
+    (
+        pytest.param(
+            {"_experiments": {"data_collection": {"database_query_data": True}}},
+            {"db.params": [1, 2], "db.paramstyle": "format"},
+            id="data_collection_on_records_params",
+        ),
+        pytest.param(
+            {"_experiments": {"data_collection": {"database_query_data": False}}},
+            {},
+            id="data_collection_off_strips_params",
+        ),
+        pytest.param(
+            {"_experiments": {"data_collection": {}}},
+            {"db.params": [1, 2], "db.paramstyle": "format"},
+            id="data_collection_default_records_params",
+        ),
+        pytest.param(
+            {"_experiments": {"record_sql_params": True}},
+            {"db.params": [1, 2], "db.paramstyle": "format"},
+            id="legacy_record_sql_params_on_records_params",
+        ),
+        pytest.param(
+            {"_experiments": {"record_sql_params": False}},
+            {},
+            id="legacy_record_sql_params_off_strips_params",
+        ),
+        pytest.param(
+            {},
+            {},
+            id="no_options_strips_params",
+        ),
+        pytest.param(
+            {
+                "_experiments": {
+                    "record_sql_params": True,
+                    "data_collection": {"database_query_data": False},
+                }
+            },
+            {},
+            id="data_collection_off_takes_precedence_over_legacy_on",
+        ),
+        pytest.param(
+            {
+                "_experiments": {
+                    "record_sql_params": False,
+                    "data_collection": {"database_query_data": True},
+                }
+            },
+            {"db.params": [1, 2], "db.paramstyle": "format"},
+            id="data_collection_on_takes_precedence_over_legacy_off",
+        ),
+    ),
+)
+def test_record_sql_queries_data_collection(
+    sentry_init, capture_events, sentry_options, expected_data
+):
+    assert (
+        _get_query_breadcrumb_data(sentry_init, capture_events, sentry_options)
+        == expected_data
+    )
+
+
+@pytest.mark.parametrize("params_list", (None, [], [None]))
+def test_record_sql_queries_empty_params_not_recorded(
+    sentry_init, capture_events, params_list
+):
+    data = _get_query_breadcrumb_data(
+        sentry_init,
+        capture_events,
+        {"_experiments": {"data_collection": {"database_query_data": True}}},
+        params_list=params_list,
+    )
+    assert "db.params" not in data
+
+
+def test_record_sql_queries_paramstyle_passthrough(sentry_init, capture_events):
+    data = _get_query_breadcrumb_data(
+        sentry_init,
+        capture_events,
+        {"_experiments": {"data_collection": {"database_query_data": True}}},
+        paramstyle="qmark",
+    )
+    assert data["db.paramstyle"] == "qmark"
+
+
+def test_record_sql_queries_paramstyle_dropped_when_collection_off(
+    sentry_init, capture_events
+):
+    data = _get_query_breadcrumb_data(
+        sentry_init,
+        capture_events,
+        {"_experiments": {"data_collection": {"database_query_data": False}}},
+        paramstyle="qmark",
+    )
+    assert "db.paramstyle" not in data