Revert llm kind check removal, fix llmobs_service tests

Author: Yun-Kim

ddtrace/llmobs/_llmobs.py

@@ -120,6 +120,7 @@ def _on_span_finish(self, span):
     def _submit_llmobs_span(self, span: Span) -> None:
         """Generate and submit an LLMObs span event to be sent to LLMObs."""
         span_event = None
+        is_llm_span = span._get_ctx_item(SPAN_KIND) == "llm"
         is_ragas_integration_span = False
         try:
             span_event, is_ragas_integration_span = self._llmobs_span_event(span)
@@ -129,7 +130,7 @@ def _submit_llmobs_span(self, span: Span) -> None:
                 "Error generating LLMObs span event for span %s, likely due to malformed span", span, exc_info=True
             )
         finally:
-            if not span_event or is_ragas_integration_span:
+            if not span_event or not is_llm_span or is_ragas_integration_span:
                 return
             if self._evaluator_runner:
                 self._evaluator_runner.enqueue(span_event, span)

riotfile.py

@@ -2883,8 +2883,8 @@ def select_pys(min_version: str = MIN_PYTHON_VERSION, max_version: str = MAX_PYT
             name="llmobs",
             command="pytest {cmdargs} tests/llmobs",
             pkgs={"vcrpy": latest, "pytest-asyncio": "==0.21.1"},
-            pys=select_pys(min_version="3.7"),
             venvs=[
+                Venv(pys="3.7"),
                 Venv(pys=select_pys(min_version="3.8"), pkgs={"ragas": "==0.1.21", "langchain": latest}),
             ],
         ),

tests/llmobs/conftest.py

@@ -129,6 +129,13 @@ def mock_http_writer_logs():
         yield m
 
 
+@pytest.fixture
+def mock_llmobs_logs():
+    with mock.patch("ddtrace.llmobs._llmobs.log") as m:
+        yield m
+        m.reset_mock()
+
+
 @pytest.fixture
 def ddtrace_global_config():
     config = {}
@@ -243,15 +250,25 @@ def llmobs_span_writer():
 
 
 @pytest.fixture
-def llmobs(monkeypatch, tracer, llmobs_env, llmobs_span_writer):
+def llmobs(
+    ddtrace_global_config,
+    monkeypatch,
+    tracer,
+    llmobs_env,
+    llmobs_span_writer,
+    mock_llmobs_eval_metric_writer,
+    mock_llmobs_evaluator_runner,
+):
     for env, val in llmobs_env.items():
         monkeypatch.setenv(env, val)
-
+    global_config = default_global_config()
+    global_config.update(dict(_llmobs_ml_app=llmobs_env.get("DD_LLMOBS_ML_APP")))
+    global_config.update(ddtrace_global_config)
     # TODO: remove once rest of tests are moved off of global config tampering
-    with override_global_config(dict(_llmobs_ml_app=llmobs_env.get("DD_LLMOBS_ML_APP"))):
+    with override_global_config(global_config):
         llmobs_service.enable(_tracer=tracer)
         llmobs_service._instance._llmobs_span_writer = llmobs_span_writer
-        yield llmobs
+        yield llmobs_service
     llmobs_service.disable()
 
 

tests/llmobs/test_llmobs.py

@@ -1,4 +1,3 @@
-import mock
 import pytest
 
 from ddtrace.ext import SpanTypes
@@ -8,12 +7,6 @@
 from tests.llmobs._utils import _expected_llmobs_llm_span_event
 
 
-@pytest.fixture
-def mock_logs():
-    with mock.patch("ddtrace.llmobs._trace_processor.log") as mock_logs:
-        yield mock_logs
-
-
 class TestMLApp:
     @pytest.mark.parametrize("llmobs_env", [{"DD_LLMOBS_ML_APP": "<not-a-real-app-name>"}])
     def test_tag_defaults_to_env_var(self, tracer, llmobs_env, llmobs_events):
@@ -228,19 +221,19 @@ def test_model_and_provider_are_set(tracer, llmobs_events):
     assert span_event["meta"]["model_provider"] == "model_provider"
 
 
-def test_malformed_span_logs_error_instead_of_raising(mock_logs, tracer, llmobs_events):
+def test_malformed_span_logs_error_instead_of_raising(tracer, llmobs_events, mock_llmobs_logs):
     """Test that a trying to create a span event from a malformed span will log an error instead of crashing."""
     with tracer.trace("root_llm_span", span_type=SpanTypes.LLM) as llm_span:
         # span does not have SPAN_KIND tag
         pass
-    mock_logs.error.assert_called_once_with(
-        "Error generating LLMObs span event for span %s, likely due to malformed span", llm_span
+    mock_llmobs_logs.error.assert_called_with(
+        "Error generating LLMObs span event for span %s, likely due to malformed span", llm_span, exc_info=True
     )
     assert len(llmobs_events) == 0
 
 
-def test_processor_only_creates_llmobs_span_event(tracer, llmobs_events):
-    """Test that the LLMObsTraceProcessor only creates LLMObs span events for LLM span types."""
+def test_only_generate_span_events_from_llmobs_spans(tracer, llmobs_events):
+    """Test that we only generate LLMObs span events for LLM span types."""
     with tracer.trace("root_llm_span", service="tests.llmobs", span_type=SpanTypes.LLM) as root_span:
         root_span._set_ctx_item(const.SPAN_KIND, "llm")
         with tracer.trace("child_span"):
@@ -250,5 +243,5 @@ def test_processor_only_creates_llmobs_span_event(tracer, llmobs_events):
     expected_grandchild_llmobs_span["parent_id"] = str(root_span.span_id)
 
     assert len(llmobs_events) == 2
-    assert llmobs_events[0] == _expected_llmobs_llm_span_event(root_span, "llm")
-    assert llmobs_events[1] == expected_grandchild_llmobs_span
+    assert llmobs_events[1] == _expected_llmobs_llm_span_event(root_span, "llm")
+    assert llmobs_events[0] == expected_grandchild_llmobs_span