feat: implementing openai wrapper #114

noble-varghese · 2024-04-03T10:37:09Z

Problem

Implementing the openAI wrapper for langfuse js

Changes

Added the openAI wrapper in the langfuse package which implements tracing out of the box for all the openai chat and completions calls (with/without streaming).
Added test cases to support and validate the changes that were made.
Added a sample implementation below.

import OpenAI from "openai";
import { OpenAIWrapper } from "langfuse";


const client = OpenAIWrapper(new OpenAI());

const main = async () => {
    const completion = await client.chat.completions.create({
        messages: [{ role: "system", content: "Say this is a test!" }],
        model: "gpt-3.5-turbo",
        stream: true,
        user: "[email protected]",
        max_tokens: 300
    }, {});

    for await (const chunk of completion) {
        console.log(chunk.choices[0]?.delta?.content || "")
    }
    
    await client.flushAsync()
}

main();

Release info Sub-libraries affected

Bump level

Major
Minor
Patch

Libraries affected

All of them
langfuse
langfuse-node

Changelog notes

Added support for X

- Added docs for the wrapper - Modified the model Params to fetch all the parameter of openai chat and completions api - Added the config params for the langfuse tracer like metadata, trace_name, session_id, release and version

Added the following tests - Chat completion streaming - Chat completion without streaming - Completion with streaming - Completion without streaming - Few short learning with streaming. - Few short learning without streaming

…the input structure for all the calls and output structure to send the entire message in chat calls

…se/langfuse-js into noble-varghese/openai-integration

marcklingen · 2024-04-04T13:45:59Z

langfuse/test/langfuse-openai.spec.ts

+ expect(generation.totalTokens).toBeDefined()
+ expect(generation.promptTokens).toBeDefined()
+ expect(generation.completionTokens).toBeDefined()
+ expect(generation.input).toBeDefined()


add message array to test here?

You mean statusMessage ?

I have added statusMessage. Did not find anything called message. Input and output are anyway being validated.

I meant checking whether input is correct

langfuse/test/langfuse-openai.spec.ts

marcklingen · 2024-04-04T13:47:42Z

langfuse/test/langfuse-openai.spec.ts

+ expect(generation.output).toMatch(content)
+ }, 10000);
+
+ it("Few Short learning with streaming", async () => {


what does this test add?

This tests the scenario where multiple inputs are sent in the messages of chat API. Like a conversational message. Tests if the integration captures it in the trace.,

langfuse/test/langfuse-openai.spec.ts

package-lock.json

marcklingen · 2024-04-04T14:33:24Z

langfuse/src/openai.ts

+ modelParams: Record<string, any>
+}
+
+const parseInputArgs = (args: Record<string, any>): InputArgsData => {


could map tools / toolchoice etc to metadata

functions, function_call, tools and tool_choice are being sent as part of the input right now. Should it remain the same and additionally send it in the metadata as well ?
Or should it be sent only in metadata. ?

The UI doesn't display tools and tool_choice is regular dislay, but in the json mode it is visible.

In input is correct, need to fix the front-end rendering when tools are on top-level, currently only rendered well when they are within a single message.

Makes sense to not make them to metadata, agree!

marcklingen · 2024-04-04T14:40:04Z

langfuse/src/openai.ts

+
+ createGeneration(
+ output?: string,
+ usage?: Record<string, any>,


does it make sense to type usage more strictly? promptTokens, completionTokens

It does make sense to have typing there. Realised that langfuse doesn't take the openai usage details directly. Have updated in code and added tests for the same :)

What kind of translation do you need to do here? generally we tried to take the openai object as it is

This is more like a UI test case, which checks if the UI is rendering the chats properly. Backend test doesn't make a lot of sense actually.
Additionally, we can always confirm that the wrapper is not just taking the first element in the array.

Removing this test from list.

langfuse/test/langfuse-openai.spec.ts

…dditionally, added tests for usage, input, output and total costs in all tests

…ed own type since open.completion usage may tend to be different across versions

…ghese/openai-integration

marcklingen · 2024-04-04T18:25:20Z

langfuse/src/openai.ts

+ }
+}
+
+interface WrapperConfig {


I think going for camelcase here would be a better fit to the rest of the JS SDK (even though the openai sdk uses snake case, what do you think?

I agree. This makes sense. We use camel casing on all the other places and since this is our wrapper, it would make sense to keep it as camel cased.

…yle to camel case in the wrapper

…se/langfuse-js into noble-varghese/openai-integration

…letion

…apped function to manually flush the data in code

…se/langfuse-js into noble-varghese/openai-integration

WIP: implementing openai wrapper

546ccb8

noble-varghese requested a review from a team as a code owner April 3, 2024 10:37

noble-varghese marked this pull request as draft April 3, 2024 10:37

noble-varghese requested review from marcklingen and maxdeichmann April 3, 2024 10:37

noble-varghese and others added 14 commits April 3, 2024 22:40

refactor: Code clean up, docs and other modifications

9cfa6a5

- Added docs for the wrapper - Modified the model Params to fetch all the parameter of openai chat and completions api - Added the config params for the langfuse tracer like metadata, trace_name, session_id, release and version

chore: todo comment

2975e58

feat: error handling in the wrapper

960277b

fix: adding missing details in trace

f3cf78a

tests: adding test cases for the integration

c6b9ff6

Added the following tests - Chat completion streaming - Chat completion without streaming - Completion with streaming - Completion without streaming - Few short learning with streaming. - Few short learning without streaming

fix: removed the unsed langfuse client

7d8c21a

revert: reverting the changes in tsconfig

702e68b

fix: removing the input parsing for multimessage scenario

84fcf7e

Merge branch 'main' into noble-varghese/openai-integration

617a075

feat: adding the function_call and tests for function calls. Changed …

6cac179

…the input structure for all the calls and output structure to send the entire message in chat calls

Merge branch 'noble-varghese/openai-integration' of github.com:langfu…

3a2eb5d

…se/langfuse-js into noble-varghese/openai-integration

chore: removing example file

837c61a

fix: updating the baseurl

2df5ab1

chore: lint error fixes

30758a8

marcklingen reviewed Apr 4, 2024

View reviewed changes

noble-varghese added 10 commits April 4, 2024 20:20

chore: removed package-lock file. repo uses yarn

37dbabb

fix: added types to usage and retrived the data for langfuse usage. A…

90cf13c

…dditionally, added tests for usage, input, output and total costs in all tests

chore: removed dependency with openai completion usage. Instead defin…

fe3abe1

…ed own type since open.completion usage may tend to be different across versions

fix: fixed output types in the trace generation part

135fd0a

chore: yarnlock file

1c52492

Merge branch 'main' of github.com:langfuse/langfuse-js into noble-var…

6af75f0

…ghese/openai-integration

test: test for common declared openai wrapper client

e5eeaca

test: more tests on single client and wrapper

2f0f59b

test: Added test cases for extra paras in the wrapper

7ac7afb

test: added test for error handling cases

a5f678f

noble-varghese and others added 2 commits April 4, 2024 23:22

chore: removed redundant test and removed a lint error

0f960dc

Merge branch 'main' into noble-varghese/openai-integration

7b6ce7d

marcklingen reviewed Apr 4, 2024

View reviewed changes

noble-varghese added 11 commits April 5, 2024 00:14

fix: prettier and linting issue. Also changed the parameter casing st…

6bf14af

…yle to camel case in the wrapper

Merge branch 'noble-varghese/openai-integration' of github.com:langfu…

875d005

…se/langfuse-js into noble-varghese/openai-integration

refactor: moved the tests to integration-tests folder

4cb8e92

fix: test case error

599ada0

fix: selectively adding the input params for completion and chat comp…

b3520de

…letion

Merge branch 'main' into noble-varghese/openai-integration

b9b5bb5

chore: lint fixes

d660380

fix: removed the flush from tracer and added flush property in the wr…

0f840a2

…apped function to manually flush the data in code

Merge branch 'main' into noble-varghese/openai-integration

818c2f6

revert: changes in the utils.spec

6df74ec

Merge branch 'noble-varghese/openai-integration' of github.com:langfu…

c6b84a9

…se/langfuse-js into noble-varghese/openai-integration

noble-varghese requested a review from marcklingen April 5, 2024 06:53

noble-varghese marked this pull request as ready for review April 5, 2024 06:53

noble-varghese changed the title ~~WIP: implementing openai wrapper~~ feat: implementing openai wrapper Apr 5, 2024

hassiebp changed the base branch from main to hassieb/lfe-784-feat-add-openai-integration April 12, 2024 07:09

hassiebp merged commit b88776b into hassieb/lfe-784-feat-add-openai-integration Apr 12, 2024
9 checks passed

hassiebp deleted the noble-varghese/openai-integration branch April 12, 2024 07:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implementing openai wrapper #114

feat: implementing openai wrapper #114

noble-varghese commented Apr 3, 2024 •

edited

Loading

marcklingen Apr 4, 2024

noble-varghese Apr 4, 2024

noble-varghese Apr 4, 2024

marcklingen Apr 4, 2024

marcklingen Apr 4, 2024

noble-varghese Apr 4, 2024

marcklingen Apr 4, 2024

noble-varghese Apr 4, 2024

noble-varghese Apr 4, 2024

marcklingen Apr 4, 2024

marcklingen Apr 4, 2024

noble-varghese Apr 4, 2024

marcklingen Apr 4, 2024

noble-varghese Apr 4, 2024

noble-varghese Apr 4, 2024

marcklingen Apr 4, 2024

noble-varghese Apr 4, 2024

feat: implementing openai wrapper #114

feat: implementing openai wrapper #114

Conversation

noble-varghese commented Apr 3, 2024 • edited Loading

Problem

Changes

Release info Sub-libraries affected

Bump level

Libraries affected

Changelog notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

noble-varghese commented Apr 3, 2024 •

edited

Loading