fix(tracing): disallow v0.5 api usage for windows machines (backports #4830 to 1.7) (#4837)

Backports: #4830

Co-authored-by: Brett Langdon <[email protected]>

Author: mabdinur

ddtrace/internal/writer.py

@@ -279,9 +279,26 @@ def __init__(
         if headers:
             self._headers.update(headers)
         self._timeout = timeout
+
+        # Default to v0.4 if we are on Windows since there is a known compatibility issue
+        # https://github.com/DataDog/dd-trace-py/issues/4829
+        # DEV: sys.platform on windows should be `win32` or `cygwin`, but using `startswith`
+        #      as a safety precaution.
+        #      https://docs.python.org/3/library/sys.html#sys.platform
+        is_windows = sys.platform.startswith("win") or sys.platform.startswith("cygwin")
+        default_api_version = "v0.4" if is_windows else "v0.5"
+
         self._api_version = (
-            api_version or os.getenv("DD_TRACE_API_VERSION") or ("v0.5" if priority_sampler is not None else "v0.3")
+            api_version
+            or os.getenv("DD_TRACE_API_VERSION")
+            or (default_api_version if priority_sampler is not None else "v0.3")
         )
+        if is_windows and self._api_version == "v0.5":
+            raise RuntimeError(
+                "There is a known compatibiltiy issue with v0.5 API and Windows, "
+                "please see https://github.com/DataDog/dd-trace-py/issues/4829 for more details."
+            )
+
         try:
             Encoder = MSGPACK_ENCODERS[self._api_version]
         except KeyError:

releasenotes/notes/v05-encoding-eac70f23850a445e.yaml

@@ -1,5 +1,8 @@
 ---
 upgrade:
   - |
-    tracing: upgrades the default trace API version to ``v0.5``. The ``v0.5`` trace API version generates 
+    tracing: upgrades the default trace API version to ``v0.5`` for non-Windows systems. The ``v0.5`` trace API version generates
     smaller payloads, thus increasing the throughput to the Datadog agent especially with larger traces.
+  - |
+    tracing: configuring the ``v0.5`` trace API version on Windows machines will raise a ``RuntimeError``
+    due to known compatibility issues. Please see https://github.com/DataDog/dd-trace-py/issues/4829 for more details.

tests/tracer/test_writer.py

@@ -1,5 +1,7 @@
+import contextlib
 import os
 import socket
+import sys
 import tempfile
 import threading
 import time
@@ -20,12 +22,23 @@
 from ddtrace.internal.writer import LogWriter
 from ddtrace.internal.writer import Response
 from ddtrace.internal.writer import _human_size
+from ddtrace.sampler import RateByServiceSampler
 from ddtrace.span import Span
 from tests.utils import AnyInt
 from tests.utils import BaseTestCase
 from tests.utils import override_env
 
 
+@contextlib.contextmanager
+def mock_sys_platform(new_value):
+    old_value = sys.platform
+    try:
+        sys.platform = new_value
+        yield
+    finally:
+        sys.platform = old_value
+
+
 class DummyOutput:
     def __init__(self):
         self.entries = []
@@ -552,6 +565,86 @@ def test_writer_recreate_api_version(init_api_version, api_version, endpoint, en
     assert isinstance(writer._encoder, encoder_cls)
 
 
+@pytest.mark.parametrize(
+    "sys_platform, api_version, ddtrace_api_version, priority_sampler, raises_error, expected",
+    [
+        # -- win32
+        # Defaults on windows
+        ("win32", None, None, None, False, "v0.3"),
+        # Default with priority sampler
+        ("win32", None, None, RateByServiceSampler(), False, "v0.4"),
+        # Explicitly passed in API version is always used
+        ("win32", "v0.3", None, RateByServiceSampler(), False, "v0.3"),
+        ("win32", "v0.3", "v0.4", None, False, "v0.3"),
+        ("win32", "v0.3", "v0.4", RateByServiceSampler(), False, "v0.3"),
+        # Env variable is used if explicit value is not given
+        ("win32", None, "v0.4", None, False, "v0.4"),
+        ("win32", None, "v0.4", RateByServiceSampler(), False, "v0.4"),
+        # v0.5 is not supported on windows
+        ("win32", "v0.5", None, None, True, None),
+        ("win32", "v0.5", None, RateByServiceSampler(), True, None),
+        ("win32", "v0.5", "v0.4", RateByServiceSampler(), True, None),
+        ("win32", None, "v0.5", RateByServiceSampler(), True, None),
+        # -- cygwin
+        # Defaults on windows
+        ("cygwin", None, None, None, False, "v0.3"),
+        # Default with priority sampler
+        ("cygwin", None, None, RateByServiceSampler(), False, "v0.4"),
+        # Explicitly passed in API version is always used
+        ("cygwin", "v0.3", None, RateByServiceSampler(), False, "v0.3"),
+        ("cygwin", "v0.3", "v0.4", None, False, "v0.3"),
+        ("cygwin", "v0.3", "v0.4", RateByServiceSampler(), False, "v0.3"),
+        # Env variable is used if explicit value is not given
+        ("cygwin", None, "v0.4", None, False, "v0.4"),
+        ("cygwin", None, "v0.4", RateByServiceSampler(), False, "v0.4"),
+        # v0.5 is not supported on windows
+        ("cygwin", "v0.5", None, None, True, None),
+        ("cygwin", "v0.5", None, RateByServiceSampler(), True, None),
+        ("cygwin", "v0.5", "v0.4", RateByServiceSampler(), True, None),
+        ("cygwin", None, "v0.5", RateByServiceSampler(), True, None),
+        # -- Non-windows
+        # defaults
+        ("darwin", None, None, None, False, "v0.3"),
+        # Default with priority sample
+        ("darwin", None, None, RateByServiceSampler(), False, "v0.5"),
+        # Explicitly setting api version
+        ("darwin", "v0.4", None, RateByServiceSampler(), False, "v0.4"),
+        # Explicitly set version takes precedence
+        ("darwin", "v0.4", "v0.5", RateByServiceSampler(), False, "v0.4"),
+        # Via env variable
+        ("darwin", None, "v0.4", RateByServiceSampler(), False, "v0.4"),
+        ("darwin", None, "v0.5", RateByServiceSampler(), False, "v0.5"),
+    ],
+)
+def test_writer_api_version_selection(
+    sys_platform, api_version, ddtrace_api_version, priority_sampler, raises_error, expected, monkeypatch
+):
+    """test to verify that we are unable to select v0.5 api version when on a windows machine.
+
+    https://docs.python.org/3/library/sys.html#sys.platform
+
+    The possible ``sys.platform`` values when on windows are ``win32`` or ``cygwin``.
+    """
+
+    # Mock the value of `sys.platform` to be a specific value
+    with mock_sys_platform(sys_platform):
+
+        # If desired, set the DD_TRACE_API_VERSION env variable
+        if ddtrace_api_version is not None:
+            monkeypatch.setenv("DD_TRACE_API_VERSION", ddtrace_api_version)
+
+        try:
+            # Create a new writer
+            writer = AgentWriter(
+                agent_url="http://dne:1234", api_version=api_version, priority_sampler=priority_sampler
+            )
+            assert writer._api_version == expected
+        except RuntimeError:
+            # If we were not expecting a RuntimeError, then cause the test to fail
+            if not raises_error:
+                pytest.fail("Raised RuntimeError when it was not expected")
+
+
 def test_writer_reuse_connections_envvar(monkeypatch):
     monkeypatch.setenv("DD_TRACE_WRITER_REUSE_CONNECTIONS", "false")
     writer = AgentWriter(agent_url="http://localhost:9126")