Skip to content

fix: Incorrect token count mapping in telemetry #2109

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

Jump to bottom
Closed
tl-nguyen wants to merge 4 commits into from
Closed

fix: Incorrect token count mapping in telemetry #2109

tl-nguyen wants to merge 4 commits into from

Conversation

@tl-nguyen
Copy link
Contributor

@tl-nguyen tl-nguyen commented Jul 22, 2025 •
edited
Loading

Fixes #2105

Problem

When integrating Google ADK with Langfuse using the @observe
decorator, the usage details displayed in Langfuse web UI were
incorrect.

The root cause was in the telemetry implementation where
total_token_count was being mapped to gen_ai.usage.output_tokens
instead of candidates_token_count.

  • Expected mapping:

    • candidates_token_count → completion_tokens (output tokens)
    • prompt_token_count → prompt_tokens (input tokens)

  • Previous incorrect mapping:

    • total_token_count → completion_tokens (wrong!)
    • prompt_token_count → prompt_tokens (correct)

Solution

Updated trace_call_llm function in telemetry.py to use
candidates_token_count for output token tracking instead of
total_token_count, ensuring proper token count reporting to
observability tools like Langfuse.

Testing plan

  • Updated test expectations in test_telemetry.py
  • Verified telemetry tests pass
  • Manual verification with Langfuse integration

Screenshots

Before

Screenshot from 2025-07-22 20-20-33

After

Screenshot from 2025-07-22 20-21-40

Notes: From the screenshot, there's another problem: thoughts_token_count field is not mapped, but this should be another issue imo

@lamn-ytree
Copy link

lamn-ytree commented Jul 22, 2025

Hi @hangfei @Jacksunwei , could you guys be able to have a look, many thanks! 🙏

boyangsvl
boyangsvl approved these changes Jul 22, 2025
View reviewed changes
Copy link
Collaborator

@boyangsvl boyangsvl left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@boyangsvl boyangsvl added the ready to pull [Status] This PR is ready to be importing back to Google label Jul 22, 2025
@hangfei hangfei removed the ready to pull [Status] This PR is ready to be importing back to Google label Jul 22, 2025
@hangfei
Copy link
Collaborator

hangfei commented Jul 22, 2025

@tl-nguyen could you fix the two failed checks?

@tl-nguyen tl-nguyen force-pushed the fix-telemetry-token-count-mapping branch from 9ae6dc6 to 895e3c8 Compare July 23, 2025 00:16
@tl-nguyen
Copy link
Contributor Author

tl-nguyen commented Jul 23, 2025 •
edited
Loading

@tl-nguyen could you fix the two failed checks?

@hangfei, I fixed them

many thanks!

@tl-nguyen tl-nguyen force-pushed the fix-telemetry-token-count-mapping branch from 895e3c8 to 7774e4e Compare July 23, 2025 00:27
@tl-nguyen tl-nguyen changed the title Fix incorrect token count mapping in telemetry fix: incorrect token count mapping in telemetry Jul 23, 2025
@tl-nguyen tl-nguyen changed the title fix: incorrect token count mapping in telemetry fix: Incorrect token count mapping in telemetry Jul 23, 2025
@tl-nguyen tl-nguyen force-pushed the fix-telemetry-token-count-mapping branch from 7774e4e to 4590843 Compare July 24, 2025 14:54
@tl-nguyen
Copy link
Contributor Author

tl-nguyen commented Jul 24, 2025

Hi @hangfei, can we have a look on this one again? 🙏 , many thanks!

@ankursharmas ankursharmas added ready to pull [Status] This PR is ready to be importing back to Google and removed ready to pull [Status] This PR is ready to be importing back to Google labels Jul 24, 2025
@ankursharmas ankursharmas added ready to pull [Status] This PR is ready to be importing back to Google and removed ready to pull [Status] This PR is ready to be importing back to Google labels Jul 24, 2025
@ankursharmas ankursharmas self-requested a review July 24, 2025 20:28
@ankursharmas ankursharmas added the ready to pull [Status] This PR is ready to be importing back to Google label Jul 24, 2025
copybara-service bot pushed a commit that referenced this pull request Jul 24, 2025
@tl-nguyen @copybara-github
fix: Fix incorrect token count mapping in telemetry
c8f8b4a 
Merge #2109

Fixes #2105

## Problem
  When integrating Google ADK with Langfuse using the @observe
  decorator, the usage details displayed in Langfuse web UI were
  incorrect.

  The root cause was in the telemetry implementation where
  total_token_count was being mapped to gen_ai.usage.output_tokens
  instead of candidates_token_count.

  - Expected mapping:
    - candidates_token_count → completion_tokens (output tokens)
    - prompt_token_count → prompt_tokens (input tokens)

  - Previous incorrect mapping:
    - total_token_count → completion_tokens (wrong!)
    - prompt_token_count → prompt_tokens (correct)

  ## Solution
  Updated trace_call_llm function in telemetry.py to use
  candidates_token_count for output token tracking instead of
  total_token_count, ensuring proper token count reporting to
  observability tools like Langfuse.

  ## Testing plan
  - Updated test expectations in test_telemetry.py
  - Verified telemetry tests pass
  - Manual verification with Langfuse integration

  ## Screenshots
  **Before**

<img width="1187" height="329" alt="Screenshot from 2025-07-22 20-20-33" src="https://github.com/user-attachments/assets/ad5fc957-64a2-4524-bd31-0cebb15a5270" />

  **After**

<img width="1187" height="329" alt="Screenshot from 2025-07-22 20-21-40" src="https://github.com/user-attachments/assets/3920df2a-be75-47e0-9bd0-f961bb72c838" />

 _Notes_: From the screenshot, there's another problem: thoughts_token_count field is not mapped, but this should be another issue imo

COPYBARA_INTEGRATE_REVIEW=#2109 from tl-nguyen:fix-telemetry-token-count-mapping 3d043f5
PiperOrigin-RevId: 786827802
@ankursharmas
Copy link
Collaborator

ankursharmas commented Jul 24, 2025

This change has been accepted as a part of c8f8b4a

pandasanjay pushed a commit to pandasanjay/adk-python that referenced this pull request Jul 28, 2025
@tl-nguyen @copybara-github
fix: Fix incorrect token count mapping in telemetry
0300929 
Merge google#2109

Fixes google#2105

## Problem
  When integrating Google ADK with Langfuse using the @observe
  decorator, the usage details displayed in Langfuse web UI were
  incorrect.

  The root cause was in the telemetry implementation where
  total_token_count was being mapped to gen_ai.usage.output_tokens
  instead of candidates_token_count.

  - Expected mapping:
    - candidates_token_count → completion_tokens (output tokens)
    - prompt_token_count → prompt_tokens (input tokens)

  - Previous incorrect mapping:
    - total_token_count → completion_tokens (wrong!)
    - prompt_token_count → prompt_tokens (correct)

  ## Solution
  Updated trace_call_llm function in telemetry.py to use
  candidates_token_count for output token tracking instead of
  total_token_count, ensuring proper token count reporting to
  observability tools like Langfuse.

  ## Testing plan
  - Updated test expectations in test_telemetry.py
  - Verified telemetry tests pass
  - Manual verification with Langfuse integration

  ## Screenshots
  **Before**

<img width="1187" height="329" alt="Screenshot from 2025-07-22 20-20-33" src="https://github.com/user-attachments/assets/ad5fc957-64a2-4524-bd31-0cebb15a5270" />

  **After**

<img width="1187" height="329" alt="Screenshot from 2025-07-22 20-21-40" src="https://github.com/user-attachments/assets/3920df2a-be75-47e0-9bd0-f961bb72c838" />

 _Notes_: From the screenshot, there's another problem: thoughts_token_count field is not mapped, but this should be another issue imo

COPYBARA_INTEGRATE_REVIEW=google#2109 from tl-nguyen:fix-telemetry-token-count-mapping 3d043f5
PiperOrigin-RevId: 786827802
knaou pushed a commit to knaou/adk-python that referenced this pull request Aug 16, 2025
@tl-nguyen @knaou
fix: Fix incorrect token count mapping in telemetry
dd59232 
Merge google#2109

Fixes google#2105

## Problem
  When integrating Google ADK with Langfuse using the @observe
  decorator, the usage details displayed in Langfuse web UI were
  incorrect.

  The root cause was in the telemetry implementation where
  total_token_count was being mapped to gen_ai.usage.output_tokens
  instead of candidates_token_count.

  - Expected mapping:
    - candidates_token_count → completion_tokens (output tokens)
    - prompt_token_count → prompt_tokens (input tokens)

  - Previous incorrect mapping:
    - total_token_count → completion_tokens (wrong!)
    - prompt_token_count → prompt_tokens (correct)

  ## Solution
  Updated trace_call_llm function in telemetry.py to use
  candidates_token_count for output token tracking instead of
  total_token_count, ensuring proper token count reporting to
  observability tools like Langfuse.

  ## Testing plan
  - Updated test expectations in test_telemetry.py
  - Verified telemetry tests pass
  - Manual verification with Langfuse integration

  ## Screenshots
  **Before**

<img width="1187" height="329" alt="Screenshot from 2025-07-22 20-20-33" src="https://github.com/user-attachments/assets/ad5fc957-64a2-4524-bd31-0cebb15a5270" />

  **After**

<img width="1187" height="329" alt="Screenshot from 2025-07-22 20-21-40" src="https://github.com/user-attachments/assets/3920df2a-be75-47e0-9bd0-f961bb72c838" />

 _Notes_: From the screenshot, there's another problem: thoughts_token_count field is not mapped, but this should be another issue imo

COPYBARA_INTEGRATE_REVIEW=google#2109 from tl-nguyen:fix-telemetry-token-count-mapping 3d043f5
PiperOrigin-RevId: 786827802
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ankursharmas ankursharmas ankursharmas approved these changes

@boyangsvl boyangsvl boyangsvl approved these changes

Assignees

No one assigned

Labels

ready to pull [Status] This PR is ready to be importing back to Google

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect mapping in telemetry usage attributes

5 participants

@tl-nguyen @lamn-ytree @hangfei @ankursharmas @boyangsvl