-
Notifications
You must be signed in to change notification settings - Fork 40
[Weave] Adds OTEL Typescript examples #2038
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
base: main
Are you sure you want to change the base?
Conversation
📚 Mintlify Preview Links📝 Changed (1 total)📄 Pages (1)
🤖 Generated automatically when Mintlify deployment succeeds |
zbirenbaum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lower examples look good, but I think the instrumented examples can be improved a bit.
I think you can do something like this to perform a lot of the work in one step (I haven't verified but google thinks so).
Also, soon we will use ResourceAttributes for handling project and entity, alongside any other wandb specific attributes like wb_run_id so the below code is even more applicable.
// instrumentation.ts
import { NodeSDK } from "@opentelemetry/sdk-trace-node";
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http";
import { OpenAIInstrumentation } from "@arizeai/openinference-instrumentation-openai";
import { Resource } from "@opentelemetry/resources";
import { SemanticResourceAttributes } from "@opentelemetry/semantic-conventions";
const sdk = new NodeSDK({
resource: new Resource({
[SemanticResourceAttributes.SERVICE_NAME]: "my-llm-app",
}),
traceExporter: new OTLPTraceExporter({
// Default to http://localhost:4318/v1/traces use weave url here
}),
instrumentations: [new OpenAIInstrumentation()],
});
sdk.start();
| const provider = new NodeTracerProvider({ | ||
| spanProcessors: [ | ||
| new BatchSpanProcessor(exporter), | ||
| new BatchSpanProcessor(new ConsoleSpanExporter()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe comment this line and put a note that says 'If you want to print to console as well'.
This isn't strictly required to export to weave and might mess up the user's existing logs.
| // Manually instrument OpenAI since we're using ESM | ||
| openAIInstrumentation.manuallyInstrument(OpenAI); | ||
|
|
||
| console.log("Instrumentation registered. Is patched?", isPatched()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't do this sort of logging in the python example I don't think
| console.log("OpenAI is patched?", isPatched()); | ||
|
|
||
| // Create a manual span to verify pipeline works | ||
| const tracer = trace.getTracer("test-tracer"); | ||
| const manualSpan = tracer.startSpan("manual-test-span"); | ||
| manualSpan.setAttribute("test", "manual span works"); | ||
| manualSpan.end(); | ||
| console.log("Manual span created"); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of this should be required if you are using Arize. The logs are also probably not necessary
|
|
||
| // Give spans time to flush | ||
| console.log("Waiting 2 seconds for spans to flush..."); | ||
| await new Promise(resolve => setTimeout(resolve, 2000)); | ||
|
|
||
| await provider.shutdown(); // flush all pending spans before exit | ||
| console.log("Shutdown complete"); | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't await outside an async function.
Description
This PR adds the equivalent TypeScript examples to the OTEL integration doc.