feat(typesafe): add JavaScript SDK instrumentation - #3779
Merged
Merged
Conversation
@arizeai/openinference-core
@arizeai/openinference-genai
@arizeai/openinference-instrumentation-anthropic
@arizeai/openinference-instrumentation-bedrock
@arizeai/openinference-instrumentation-bedrock-agent-runtime
@arizeai/openinference-instrumentation-beeai
@arizeai/openinference-instrumentation-claude-agent-sdk
@arizeai/openinference-instrumentation-langchain
@arizeai/openinference-instrumentation-langchain-v0
@arizeai/openinference-instrumentation-mcp
@arizeai/openinference-instrumentation-openai
@arizeai/openinference-instrumentation-openai-agents
@arizeai/openinference-instrumentation-typesafe
@arizeai/openinference-semantic-conventions
@arizeai/openinference-tanstack-ai
@arizeai/openinference-vercel
commit: |
mikeldking
marked this pull request as ready for review
September 18, 2026 01:11
Resolve merge conflicts keeping structured JSON payloads, add TypeSafe to Go semconv, and ignore LiteLLM model-price fetches in dspy VCR so Python CI Required can pass. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Co-authored-by: Cursor <cursoragent@cursor.com>
mikeldking
commented
Sep 18, 2026
mikeldking
commented
Sep 18, 2026
mikeldking
commented
Sep 18, 2026
mikeldking
commented
Sep 18, 2026
mikeldking
commented
Sep 18, 2026
mikeldking
commented
Sep 18, 2026
Drop the SC alias for destructured SemanticConventions keys, set llm.request/response.model_name, and trim the README to publishable usage docs. Co-authored-by: Cursor <cursoragent@cursor.com>
Reuse core getLLMAttributes/withSafety, skip payload stringify when masked, and prefer the SDK parse for await paths so clone JSON is only used for asResponse-only observation. Co-authored-by: Cursor <cursoragent@cursor.com>
mikeldking
commented
Sep 18, 2026
mikeldking
commented
Sep 18, 2026
Record the actual systemOne request/response in input/output values without injecting the resolved model, let OITracer handle masking, and punt instrumentor-owned metadata for now. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Simplify response attribute extraction, replace masking asides with self-contained JSDoc, and accept @typesafe-ai/sdk >=0.6.0 without an upper bound. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
1 similar comment
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Keep verification images out of git; they belong on the PR via upload, not the repo. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the JS instrumentation PR scoped to JavaScript; cross-language semconv and the dspy VCR tweak belong elsewhere. Co-authored-by: Cursor <cursoragent@cursor.com>
Match the shared instrumentation README structure/footer and remove the low-signal CJS/ESM subprocess coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
The APIPromise parse callback had no error handling, so a rejection raised after a successful HTTP response (e.g. body parsing failing) was never recorded on the span. The fallback path had already declined to end it, and the raw-response rejection handler only fires when the request itself fails. Route both async failure paths through an `endError` helper gated on the same `ended` flag as `endOk`, so the span is ended exactly once and an HTTP failure is not recorded twice.
The Examples section still carried repo-local `cd` / `pnpm install` / `pnpm -r build` commands and a relative `./examples/` link. `files` is `["dist","src"]`, so `examples/` is not published and neither renders usefully on npm. Point at the examples directory on GitHub with an absolute URL, matching openinference-core.
| "api.minimax.io": LLMProvider.MINIMAX, | ||
| "api.minimaxi.com": LLMProvider.MINIMAX, | ||
| "api.minimax.chat": LLMProvider.MINIMAX, | ||
| "api.typesafe.ai": LLMProvider.TYPESAFE, |
Contributor
There was a problem hiding this comment.
what is this change for? Why in openai instr?
Contributor
Author
There was a problem hiding this comment.
this is if typesafe got used through openai, which it wouldn't. so I'll remove
Contributor
Author
There was a problem hiding this comment.
Removed in 61f3e0b — both the HOST_SUFFIX_TO_PROVIDER entry and its test case are gone, so openinference-instrumentation-openai/src/instrumentation.ts now matches main.
One knock-on change remains in that package: LLMProvider.TYPESAFE stays in the semantic conventions (the Typesafe instrumentor sets llm.provider/llm.system), and the existing every provider has at least one host entry test enumerates Object.values(LLMProvider), so dropping the mapping made it fail. The test now skips providers listed in a new PROVIDERS_WITHOUT_OPENAI_COMPATIBLE_HOST set, which holds TYPESAFE only. Happy to invert it (assert against an explicit expected list instead) if you would rather not carry an exemption set there.
pnpm run test and type:check pass for the openai package.
cephalization
approved these changes
Sep 18, 2026
…tion The `api.typesafe.ai` suffix was added to `HOST_SUFFIX_TO_PROVIDER` so an OpenAI client pointed at Typesafe would be attributed correctly, but the Typesafe SDK is not OpenAI-compatible, so that path does not exist. Remove the mapping and its test case. `LLMProvider.TYPESAFE` stays in the semantic conventions (the Typesafe instrumentor sets it), so the OpenAI package's "every provider has at least one host entry" test needs an explicit exemption for providers that are not reachable through an OpenAI-compatible endpoint.
Contributor
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
@arizeai/openinference-instrumentation-typesafe: one OpenInferenceLLMspan perTypeSafeClient.systemOnecall, with JSONinput.value/output.value, resolved model, token usage, and provider/systemtypesafe.Closes #3770.
Phoenix verification
Verified against local Phoenix with the package examples:
typesafe-basic-usage-20260917-2038— singleTypeSafeClient.systemOneLLM span with structured JSON input/outputtypesafe-guardrail-routing—support.routechain with TypeSafe + OpenAI LLM children, session, and workflow metadata