Skip to content
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.

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

Gen-AI python implementation #290

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

Jeel-mehta
Copy link

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Jeel-mehta Jeel-mehta requested a review from a team as a code owner November 8, 2024 23:42
@yiyuan-he yiyuan-he changed the title Gen-AI python implementation (Not Ready) Gen-AI python implementation Nov 14, 2024
@yiyuan-he
Copy link
Contributor

Workflow builds are failing. Could you take a look?

They are probably just linting failures which you can resolve by following the instructions under Code Style Check here

if "inputTextTokenCount" in response_body:
span.set_attribute(GEN_AI_USAGE_INPUT_TOKENS, response_body["inputTextTokenCount"])

result = response_body["results"][0]
Copy link
Contributor

Choose a reason for hiding this comment

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

Trying to access element with [0] will throw an error if the results array doesn't exist. I think we need another conditional check here.

For example:

if 'results' in response_body:
   result = response_body["results"][0]

or something more concise like:

result = response_body.get('results', [{}])

Copy link
Author

Choose a reason for hiding this comment

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

I just saw this, I will fix this and make another commit

Comment on lines 366 to 370
if "results" in response_body:
result = response_body["results"][0]
if "tokenCount" in result:
span.set_attribute(GEN_AI_USAGE_OUTPUT_TOKENS, result["tokenCount"])
if "completionReason" in result:
Copy link
Contributor

@yiyuan-he yiyuan-he Nov 15, 2024

Choose a reason for hiding this comment

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

I think this logic will still throw an error.

For example, if results does not exist in the response_body, the checks will still continue to the other conditionals like if "tokenCount" in result:, which will then give an error since result will be unbound.

You can do something like this instead:

if "results" in response_body and response_body["results"]:
      result = response_body["results"][0]
      if "tokenCount" in result:
          span.set_attribute(GEN_AI_USAGE_OUTPUT_TOKENS, result["tokenCount"])
      if "completionReason" in result:
          ...

Copy link
Author

Choose a reason for hiding this comment

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

I fixed this

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants