Skip to content

fix(openai): mark traced openai spans as llm#85

Open
Abhijeet Prasad (AbhiPrasad) wants to merge 2 commits into
mainfrom
abhi-fix-81-openai-span-type
Open

fix(openai): mark traced openai spans as llm#85
Abhijeet Prasad (AbhiPrasad) wants to merge 2 commits into
mainfrom
abhi-fix-81-openai-span-type

Conversation

@AbhiPrasad
Copy link
Copy Markdown
Member

Set braintrust.span_attributes.type to "llm" for the chat completions and responses tracers so Braintrust classifies emitted spans correctly in the UI.

Extend the VCR-backed OpenAI assertions to verify the attribute is present on exported spans.

Fixes #81

Set braintrust.span_attributes.type to "llm" for the chat completions
and responses tracers so Braintrust classifies emitted spans correctly
in the UI.

Extend the VCR-backed OpenAI assertions to verify the attribute is
present on exported spans.

Fixes #81
Comment thread trace/contrib/openai/chatcompletions.go Outdated
}
}

if err := internal.SetJSONAttr(span, "braintrust.span_attributes", map[string]string{"type": "llm"}); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is a bit long i'd split the map into a constant

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

a map can't be constant, you can only declare it as a var, and I'd like to avoid that because I don't want it to be mutable. I guess I could use a helper function that returns a map. but that feels too much for this case.

I could extract braintrust.span_attributes and llm into constants, so you'd get something like:

if err := internal.SetJSONAttr(span, internal.AttrSpanAttributes, map[string]string{"type": internal.SpanTypeLLM}); err != nil {

but the length is still long. OTEL semantic conventions follows this above pattern.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'd still do attrs := map[]; setJsonAttr(span, attrs)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done with bc4e400

also going to follow up and clean up our integrations to use constants after this PR

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.

[bot] OpenAI tracers do not set braintrust.span_attributes with type: "llm"

2 participants