Skip to content

Commit 8486d56

Browse files
committed
Respect the provided log options regardless of tracing configuration
1 parent bee719b commit 8486d56

2 files changed

Lines changed: 132 additions & 16 deletions

File tree

nemoguardrails/rails/llm/llmrails.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,7 @@ async def generate_async(
911911
await streaming_handler.push_chunk(END_OF_STREAM)
912912

913913
# IF tracing is enabled we need to set GenerationLog attrs
914+
original_log_options = None
914915
if self.config.tracing.enabled:
915916
if options is None:
916917
options = GenerationOptions()
@@ -921,6 +922,7 @@ async def generate_async(
921922
else:
922923
# If options is a dict, convert it to GenerationOptions
923924
options = GenerationOptions(**options)
925+
original_log_options = options.log.model_copy(deep=True)
924926

925927
# enable log options
926928
# it is aggressive, but these are required for tracing
@@ -1038,6 +1040,26 @@ async def generate_async(
10381040
)
10391041
await tracer.export_async()
10401042

1043+
# respect original log specification, if tracing added information to the output
1044+
if original_log_options:
1045+
if not any((
1046+
original_log_options.internal_events,
1047+
original_log_options.activated_rails,
1048+
original_log_options.llm_calls,
1049+
original_log_options.colang_history
1050+
)):
1051+
del res.log
1052+
else:
1053+
if not original_log_options.internal_events:
1054+
res.log.internal_events = []
1055+
if not original_log_options.activated_rails:
1056+
res.log.activated_rails = []
1057+
if not original_log_options.llm_calls:
1058+
res.log.llm_calls = []
1059+
1060+
1061+
1062+
10411063
return res
10421064
else:
10431065
# If a prompt is used, we only return the content of the message.

tests/test_tracing.py

Lines changed: 110 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
# limitations under the License.
1515

1616
import asyncio
17+
import itertools
1718
import unittest
1819
from unittest.mock import AsyncMock, MagicMock, patch
1920

@@ -312,10 +313,7 @@ async def test_tracing_does_not_mutate_user_options():
312313
), "User's original options were modified! This causes instability."
313314

314315
# verify that tracing still works
315-
assert response.log is not None, "Tracing should still work correctly"
316-
assert (
317-
response.log.activated_rails is not None
318-
), "Should have activated rails data"
316+
assert "log" not in response, "Tracing should still work correctly, without affecting returned log"
319317

320318

321319
@pytest.mark.asyncio
@@ -358,17 +356,16 @@ async def test_tracing_with_none_options():
358356
messages=[{"role": "user", "content": "hello"}], options=None
359357
)
360358

361-
assert response.log is not None
362-
assert response.log.activated_rails is not None
363-
assert response.log.stats is not None
359+
assert "log" not in response
364360

365361

366362
@pytest.mark.asyncio
367363
async def test_tracing_aggressive_override_when_all_disabled():
368364
"""Test that tracing aggressively enables all logging when user disables all options.
369365
370366
When user disables all three tracing related options, tracing still enables
371-
ALL of them to ensure comprehensive logging data.
367+
ALL of them to ensure comprehensive logging data. However, this should not contaminate the
368+
returned response object
372369
"""
373370

374371
config = RailsConfig.from_content(
@@ -425,11 +422,10 @@ async def test_tracing_aggressive_override_when_all_disabled():
425422

426423
assert response.log is not None
427424
assert (
428-
response.log.activated_rails is not None
429-
and len(response.log.activated_rails) > 0
425+
response.log.activated_rails == []
430426
)
431-
assert response.log.llm_calls is not None
432-
assert response.log.internal_events is not None
427+
assert response.log.llm_calls == []
428+
assert response.log.internal_events == []
433429

434430
assert user_options.log.activated_rails == original_activated_rails
435431
assert user_options.log.llm_calls == original_llm_calls
@@ -439,6 +435,104 @@ async def test_tracing_aggressive_override_when_all_disabled():
439435
assert user_options.log.internal_events == False
440436

441437

438+
@pytest.mark.asyncio
439+
@pytest.mark.parametrize(
440+
"activated_rails,llm_calls,internal_events,colang_history",
441+
list(itertools.product([False, True], repeat=4)),
442+
)
443+
async def test_tracing_preserves_specific_log_fields(
444+
activated_rails, llm_calls, internal_events, colang_history
445+
):
446+
"""Test that tracing does not modify the returned log fields
447+
"""
448+
449+
config = RailsConfig.from_content(
450+
colang_content="""
451+
define user express greeting
452+
"hello"
453+
454+
define flow
455+
user express greeting
456+
bot express greeting
457+
458+
define bot express greeting
459+
"Hello! How can I assist you today?"
460+
""",
461+
config={
462+
"models": [],
463+
"tracing": {"enabled": True, "adapters": [{"name": "FileSystem"}]},
464+
},
465+
)
466+
467+
chat = TestChat(
468+
config,
469+
llm_completions=[
470+
"user express greeting",
471+
"bot express greeting",
472+
"Hello! How can I assist you today?",
473+
],
474+
)
475+
476+
# user enables some subset of log options
477+
user_options = GenerationOptions(
478+
log=GenerationLogOptions(
479+
activated_rails=activated_rails,
480+
llm_calls=llm_calls,
481+
internal_events=internal_events,
482+
colang_history=colang_history,
483+
)
484+
)
485+
486+
original_activated_rails = user_options.log.activated_rails
487+
original_llm_calls = user_options.log.llm_calls
488+
original_internal_events = user_options.log.internal_events
489+
original_colang_history = user_options.log.colang_history
490+
491+
with patch.object(Tracer, "export_async", return_value=None):
492+
response = await chat.app.generate_async(
493+
messages=[{"role": "user", "content": "hello"}], options=user_options
494+
)
495+
496+
assert user_options.log.activated_rails == original_activated_rails
497+
assert user_options.log.llm_calls == original_llm_calls
498+
assert user_options.log.internal_events == original_internal_events
499+
assert user_options.log.colang_history == original_colang_history
500+
501+
# verify that only the requested log options are returned in the response
502+
if not any((
503+
user_options.log.activated_rails,
504+
user_options.log.llm_calls,
505+
user_options.log.internal_events,
506+
user_options.log.colang_history
507+
)):
508+
assert "log" not in response
509+
else:
510+
assert response.log is not None
511+
512+
if user_options.log.activated_rails:
513+
assert len(response.log.activated_rails) > 0
514+
else:
515+
assert len(response.log.activated_rails) == 0
516+
517+
if user_options.log.llm_calls:
518+
assert len(response.log.llm_calls) > 0
519+
else:
520+
assert len(response.log.llm_calls) == 0
521+
522+
if user_options.log.internal_events:
523+
assert len(response.log.internal_events) > 0
524+
else:
525+
assert len(response.log.internal_events) == 0
526+
527+
528+
assert user_options.log.activated_rails == original_activated_rails
529+
assert user_options.log.llm_calls == original_llm_calls
530+
assert user_options.log.internal_events == original_internal_events
531+
assert user_options.log.activated_rails == activated_rails
532+
assert user_options.log.llm_calls == llm_calls
533+
assert user_options.log.internal_events == internal_events
534+
535+
442536
@pytest.mark.asyncio
443537
async def test_tracing_aggressive_override_with_dict_options():
444538
"""Test that tracing works correctly when options are passed as a dict.
@@ -502,11 +596,11 @@ async def test_tracing_aggressive_override_with_dict_options():
502596

503597
assert response.log is not None
504598
assert (
505-
response.log.activated_rails is not None
506-
and len(response.log.activated_rails) > 0
599+
response.log.activated_rails == []
600+
and len(response.log.activated_rails) == 0
507601
)
508-
assert response.log.llm_calls is not None
509-
assert response.log.internal_events is not None
602+
assert response.log.llm_calls == []
603+
assert response.log.internal_events == []
510604

511605

512606
if __name__ == "__main__":

0 commit comments

Comments
 (0)