feat(js): OpenAI embeddings Instrumentation #34
Conversation
| @@ -2,4 +2,5 @@ | |||
| module.exports = { | |||
| preset: "ts-jest", | |||
| testEnvironment: "node", | |||
| prettierPath: null, | |||
There was a problem hiding this comment.
jest doesn't support prettier 3
| attributes: { | ||
| [SemanticConventions.OPENINFERENCE_SPAN_KIND]: | ||
| OpenInferenceSpanKind.EMBEDDING, | ||
| [SemanticConventions.EMBEDDING_MODEL_NAME]: body.model, |
There was a problem hiding this comment.
Probably want to capture model from the response instead, because the one in the request can be part of invocation_parameters. Also, for Azure this is the deployment name, although last I checked, Azure is actually returning the wrong model names in embeddings response...
There was a problem hiding this comment.
I need to double-check as a follow-up on Azure. It's a bit different in JS. They have a supplamentary package. I'll file a follow-up ticket for that.
| (error) => { | ||
| // Push the error to the span | ||
| if (error) { | ||
| span.recordException(error); |
There was a problem hiding this comment.
Recording exceptions doesn't actually update the status, because a span can record multiple exceptions such as retry errors and still turn out OK.
| span.recordException(error); | |
| span.recordException(error); | |
| span.setStatus({ code: SpanStatusCode.ERROR }); | |
| span.end(); |
There was a problem hiding this comment.
True, I was thinking of leaving it mimimal - I'll at least set the status code. Might leave off the span end for now. Not sure if that's necessary. Will look into it a bit more.
There was a problem hiding this comment.
Oh actually I can put it below. I'll do that.
js/packages/openinference-instrumentation-openai/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
resolves #25
Adds openAI embeddings instrumentaiton