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

Fix missing attributes in openai_v2 #3058

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alizenhom
Copy link
Contributor

@alizenhom alizenhom commented Dec 2, 2024

Description

While testing i found out that there were missing attributes not getting set for some reason which are

  • gen_ai.operation.name
  • gen_ai.system
  • gen_ai.request.model
  • server.address

Type of change

  • There is an existing util function called set_span_attributes which simply takes a dict of attributes and sets the attributes to the span, used this function -- i fear that this is redundant as we already set the span attributes when using start_as_current_span but for some reason which i failed to find out the attributes do not get set unless using set_span_attributes -- also the tests were not failing in both scenarios which confused me a bit as well.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • tox -e py312-test-instrumentation-openai-v2-1
  • tox -e py312-test-instrumentation-openai-v2-0

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

Kindly check before & after screenshots
Screenshot 2024-12-02 at 12 56 35 PM
Screenshot 2024-12-02 at 12 57 09 PM

Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

If the tests are passing it means they are not asserting these attributes right?

@aabmass
Copy link
Member

aabmass commented Dec 2, 2024

If the tests are passing it means they are not asserting these attributes right?

Agree with xrmx, please try to get the tests to reproduce the issue. It's possible something is wrong with the tox setup or your manual testing setup.

If the current code truly isn't working, it sounds like an OTel SDK bug we should fix.

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.

9 participants