Skip to content

fix:Use JSON string instead of dic for span attributes when content capture is disabled #4101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DineshThumma9 wants to merge 5 commits into
base: main
Choose a base branch
from
+38 −74
Open

fix:Use JSON string instead of dic for span attributes when content capture is disabled #4101

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
78e6962
fix:Use JSON string instead of dic for span attributes when content c…
DineshThumma9 Jan 8, 2026
c743d2d
Trigger CLA Check
DineshThumma9 Jan 8, 2026
19f3b63
refactored:Added helper function and improved maintainability
DineshThumma9 Jan 8, 2026
7096757
fixed formatting
DineshThumma9 Jan 9, 2026
40a9daf
Merge branch 'main' into fix/issue-span-attributes
DineshThumma9 Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/google/adk/telemetry/tracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def trace_tool_call(
_safe_json_serialize(args),
)
else:
span.set_attribute('gcp.vertex.agent.tool_call_args', {})
span.set_attribute('gcp.vertex.agent.tool_call_args', '{}')
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To improve maintainability and avoid using a magic string, consider defining a constant for '{}' at the module level and reusing it here and in other similar places.

For example:

EMPTY_JSON_STRING = '{}'
...
span.set_attribute('gcp.vertex.agent.tool_call_args', EMPTY_JSON_STRING)

This change would apply to all 6 locations modified in this PR (lines 152, 182, 222, 268, 293, 349) and could also replace other existing instances of the '{}' literal in this file for consistency.


# Tracing tool response
tool_call_id = '<not specified>'
Expand Down Expand Up @@ -179,7 +179,7 @@ def trace_tool_call(
_safe_json_serialize(tool_response),
)
else:
span.set_attribute('gcp.vertex.agent.tool_response', {})
span.set_attribute('gcp.vertex.agent.tool_response', '{}')


def trace_merged_tool_calls(
Expand Down Expand Up @@ -219,7 +219,7 @@ def trace_merged_tool_calls(
function_response_event_json,
)
else:
span.set_attribute('gcp.vertex.agent.tool_response', {})
span.set_attribute('gcp.vertex.agent.tool_response', '{}')
# Setting empty llm request and response (as UI expect these) while not
# applicable for tool_response.
span.set_attribute('gcp.vertex.agent.llm_request', '{}')
Expand Down Expand Up @@ -265,7 +265,7 @@ def trace_call_llm(
_safe_json_serialize(_build_llm_request_for_trace(llm_request)),
)
else:
span.set_attribute('gcp.vertex.agent.llm_request', {})
span.set_attribute('gcp.vertex.agent.llm_request', '{}')
# Consider removing once GenAI SDK provides a way to record this info.
if llm_request.config:
if llm_request.config.top_p:
Expand All @@ -290,7 +290,7 @@ def trace_call_llm(
llm_response_json,
)
else:
span.set_attribute('gcp.vertex.agent.llm_response', {})
span.set_attribute('gcp.vertex.agent.llm_response', '{}')

if llm_response.usage_metadata is not None:
span.set_attribute(
Expand Down Expand Up @@ -346,7 +346,7 @@ def trace_send_data(
]),
)
else:
span.set_attribute('gcp.vertex.agent.data', {})
span.set_attribute('gcp.vertex.agent.data', '{}')


def _build_llm_request_for_trace(llm_request: LlmRequest) -> dict[str, Any]:
Expand Down
107 changes: 60 additions & 47 deletions tests/unittests/telemetry/test_spans.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,24 +473,31 @@ async def test_call_llm_disabling_request_response_content(
# Act
trace_call_llm(invocation_context, 'test_event_id', llm_request, llm_response)

# Assert
assert not any(
arg_name == 'gcp.vertex.agent.llm_request' and arg_value != {}
for arg_name, arg_value in (
call_obj.args
for call_obj in mock_span_fixture.set_attribute.call_args_list
)
), "Attribute 'gcp.vertex.agent.llm_request' was incorrectly set on the span."

assert not any(
arg_name == 'gcp.vertex.agent.llm_response' and arg_value != {}
for arg_name, arg_value in (
call_obj.args
for call_obj in mock_span_fixture.set_attribute.call_args_list
)
), (
"Attribute 'gcp.vertex.agent.llm_response' was incorrectly set on the"
' span.'
# Assert - Check attributes are set to JSON string '{}' not dict {}
llm_request_calls = [
call
for call in mock_span_fixture.set_attribute.call_args_list
if call.args[0] == 'gcp.vertex.agent.llm_request'
]
assert (
len(llm_request_calls) == 1
), "Expected 'gcp.vertex.agent.llm_request' to be set exactly once"
assert llm_request_calls[0].args[1] == '{}', (
"Expected JSON string '{}' for llm_request when content capture is"
f' disabled, got {llm_request_calls[0].args[1]!r}'
)

llm_response_calls = [
call
for call in mock_span_fixture.set_attribute.call_args_list
if call.args[0] == 'gcp.vertex.agent.llm_response'
]
assert (
len(llm_response_calls) == 1
), "Expected 'gcp.vertex.agent.llm_response' to be set exactly once"
assert llm_response_calls[0].args[1] == '{}', (
"Expected JSON string '{}' for llm_response when content capture is"
f' disabled, got {llm_response_calls[0].args[1]!r}'
)
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The new assertions are much clearer than the previous ones. To further improve the tests and reduce code duplication, you could extract this assertion logic into a helper function. This pattern is repeated for llm_request and llm_response here, and also in test_trace_tool_call_disabling_request_response_content and test_trace_merged_tool_disabling_request_response_content.

A helper could look something like this:

def _assert_span_attribute_set_to_empty_json(mock_span, attribute_name: str):
    calls = [
        call
        for call in mock_span.set_attribute.call_args_list
        if call.args[0] == attribute_name
    ]
    assert (
        len(calls) == 1
    ), f"Expected '{attribute_name}' to be set exactly once"
    assert calls[0].args[1] == '{}', (
        f"Expected JSON string '{{}}' for {attribute_name} when content capture is"
        f' disabled, got {calls[0].args[1]!r}'
    )

# Then in your test:
_assert_span_attribute_set_to_empty_json(mock_span_fixture, 'gcp.vertex.agent.llm_request')
_assert_span_attribute_set_to_empty_json(mock_span_fixture, 'gcp.vertex.agent.llm_response')

This would make the tests more concise and easier to maintain.



Expand Down Expand Up @@ -536,27 +543,31 @@ def test_trace_tool_call_disabling_request_response_content(
function_response_event=mock_event_fixture,
)

# Assert
assert not any(
arg_name == 'gcp.vertex.agent.tool_call_args' and arg_value != {}
for arg_name, arg_value in (
call_obj.args
for call_obj in mock_span_fixture.set_attribute.call_args_list
)
), (
"Attribute 'gcp.vertex.agent.tool_call_args' was incorrectly set on the"
' span.'
# Assert - Check attributes are set to JSON string '{}' not dict {}
tool_args_calls = [
call
for call in mock_span_fixture.set_attribute.call_args_list
if call.args[0] == 'gcp.vertex.agent.tool_call_args'
]
assert (
len(tool_args_calls) == 1
), "Expected 'gcp.vertex.agent.tool_call_args' to be set exactly once"
assert tool_args_calls[0].args[1] == '{}', (
"Expected JSON string '{}' for tool_call_args when content capture is"
f' disabled, got {tool_args_calls[0].args[1]!r}'
)

assert not any(
arg_name == 'gcp.vertex.agent.tool_response' and arg_value != {}
for arg_name, arg_value in (
call_obj.args
for call_obj in mock_span_fixture.set_attribute.call_args_list
)
), (
"Attribute 'gcp.vertex.agent.tool_response' was incorrectly set on the"
' span.'
tool_response_calls = [
call
for call in mock_span_fixture.set_attribute.call_args_list
if call.args[0] == 'gcp.vertex.agent.tool_response'
]
assert (
len(tool_response_calls) == 1
), "Expected 'gcp.vertex.agent.tool_response' to be set exactly once"
assert tool_response_calls[0].args[1] == '{}', (
"Expected JSON string '{}' for tool_response when content capture is"
f' disabled, got {tool_response_calls[0].args[1]!r}'
)


Expand Down Expand Up @@ -584,14 +595,16 @@ def test_trace_merged_tool_disabling_request_response_content(
function_response_event=mock_event_fixture,
)

# Assert
assert not any(
arg_name == 'gcp.vertex.agent.tool_response' and arg_value != {}
for arg_name, arg_value in (
call_obj.args
for call_obj in mock_span_fixture.set_attribute.call_args_list
)
), (
"Attribute 'gcp.vertex.agent.tool_response' was incorrectly set on the"
' span.'
# Assert - Check attribute is set to JSON string '{}' not dict {}
tool_response_calls = [
call
for call in mock_span_fixture.set_attribute.call_args_list
if call.args[0] == 'gcp.vertex.agent.tool_response'
]
assert (
len(tool_response_calls) == 1
), "Expected 'gcp.vertex.agent.tool_response' to be set exactly once"
assert tool_response_calls[0].args[1] == '{}', (
"Expected JSON string '{}' for tool_response when content capture is"
f' disabled, got {tool_response_calls[0].args[1]!r}'
)