Skip to content

Conversation

@InTheCloudDan
Copy link
Contributor

Merge AI Tracking updates into overall model config branch.

  • Added new methods for durationOf trackOpenAI and trackBedrockConverse.
  • Fixed NPM scripts
  • Removed extraneous console logs

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Provide links to any issues in this repository or elsewhere relating to this pull request.

Describe the solution you've provided

Provide a clear and concise description of what you expect to happen.

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context about the pull request here.

Daniel OBrien added 3 commits October 25, 2024 11:44
Add Bedrock and OpenAI methods

fix npm scripts
@InTheCloudDan InTheCloudDan requested a review from a team as a code owner October 25, 2024 16:48
UnderscoreTokenUsage,
} from './api/metrics';

export class LDAIConfigTracker {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to check the base PR, but this should be implementing an interface, and the interface is what the customer is aware of.

outputTokens: number;
totalTokens: number;

constructor(data: any) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A type of some kind would be good here. Typescript is structural, so it just needs to have the stuff that we need to understand.

prompt_tokens: number;
completion_tokens: number;

constructor(data: any) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as others.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to stop tagging now. But generally we want to avoid any, unless we really need to. Here we know several fields, so we should be able to say we need those.

"name": "@launchdarkly/ai",
"version": "0.1.0",
"description": "LaunchDarkly AI SDK for Node.js",
"main": "dist/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the package name should follow our normal conventions, likely @launchdarkly/node-server-sdk-ai. I can see this might require some discussion.

@InTheCloudDan InTheCloudDan deleted the dob/aiTracking branch November 1, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants