dexaai / dexter

LLM tools used in production at Dexa
https://dexter.dexa.ai
MIT License
69 stars 4 forks source link

Add Telemetry abstraction and remove Sentry as a peer dep #46

Closed transitive-bullshit closed 1 month ago

transitive-bullshit commented 1 month ago

Desc

Extends https://github.com/dexaai/dexter/pull/38 to no longer depend on @sentry/node, but rather a flexible, base, sentry-like telemetry provider.

By default, models will use a DefaultTelemetry if not given a telemetry provider like @sentry/node.

I added a unit test to ensure that at least the types of @sentry/node match the expected Telemetry.Base provider type.

@rileytomasek I'm sure there may be a cleaner way to do this, but I felt like the telemetry provider belonged outside of model since it could theoretically be used for datastore in the future. At the very least, this change lets us continue iterating on Dexter without it being dependent on Sentry, which I think is a net win.

Example

import { ChatModel } from '@dexaai/dexter'
import * as Sentry from '@sentry/node'

const model = new ChatModel({ telemetry: Sentry })
vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dexter ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2024 7:54am
transitive-bullshit commented 1 month ago

Note: the pnpm lockfile was also updated to the latest v9.

transitive-bullshit commented 1 month ago

@rileytomasek maybe we don't even need all of the Telemetry generics, and we just KISS? I was just trying to follow the existing pattern.

rileytomasek commented 1 month ago

@transitive-bullshit I tested locally against Dexa source and there were no issues 👌🏻.

I can't think of a scenario where we need the generics for Telemetry and it makes a pretty big mess of the code. It was easier to start from scratch so I just opened #47 with the simplified version.