feat: tracing framework #275
Conversation
| @@ -1168,9 +1171,15 @@ const startAgents = async () => { | |||
| ); | |||
| }; | |||
|
|
|||
| startAgents().catch((error) => { | |||
There was a problem hiding this comment.
Is there a reason we are removing this?
There was a problem hiding this comment.
We're not removing it, just wrapping, this is how the context functions are designed.
| CREATE TABLE IF NOT EXISTS traces ( | ||
| "id" BIGSERIAL NOT NULL, | ||
| "run" UUID, | ||
| "time" TIMESTAMP NOT NULL, | ||
| "name" VARCHAR(80) NOT NULL, | ||
| "data" JSON, | ||
| PRIMARY KEY ("id") | ||
| ); |
There was a problem hiding this comment.
We also add a category field like we discussed for the few types of logging we discussed: before llm call, interpolated prompt, prompt output, action result.
There was a problem hiding this comment.
The idea here is that name represents the type of event, and data represents the event-specific attributes. This makes it universal. For LLM prompts, currently we're tracing what is being sent to LLM and what is being received; as I said, from here on it's about choosing the rabbit hole we want to go down first.
| } catch (error) { | ||
| name = `ERROR: ${name}`; | ||
| jsonPayload = {ok: false, name: name, error: `${error}, inspect: ${inspect(data)}`}; | ||
| console.log(`Could not convert object to JSON for ${name}. Please select individual fields that are serializable.`); |
There was a problem hiding this comment.
This is intentional because (1) I don't want to see the colors, (2) I need the raw printout, and (3) this simply does not belong in the same category as logging.
The purpose for this is to catch the objects that fail to JSONify, and then selectively pick the properties we need.
When we are done, we should not need this anymore anyway.
| @@ -106,6 +107,8 @@ export class MemoryManager implements IMemoryManager { | |||
| start, | |||
| end, | |||
| }); | |||
| Instrumentation.trace("MemoryManager.getMemories", {roomId: roomId, count: count, result: result}); | |||
There was a problem hiding this comment.
Just to be clear for now you don't need to store all these values. Please goo into a PLUGIN /plugin-coinbase/plugins/trade.ts and please add 4 calls to Instrumentation as we discussed the value of composeState, the interpolated prompt so value from composeContext, the output of the LLM, the action and output of the action so 4 calls (this is an example we need to do the same thing for each of these)
There was a problem hiding this comment.
So we're testing coinbase plugin first? Would be nice if somebody answered my questions first.
There was a problem hiding this comment.
Yeah that would be a great start! Sorry I think I mentioned in on our call that day - sorry if I wasn't clear
|
We could do that, if it adds any value, which at this point I'm having a hard time to see how it would. What do you see as benefit? Consider also that the amount of data would be potentially in gigabytes, and piping that through REST would be a bummer. |
I think you are missing the purpose of this is the Prompt input, output, and action not storing everything so it will not be a crazy amount of data see my earlier comment |
|
You should only be calling Instrumentation.trace inside of a plugin for now |
I understood this, but what I'm missing is how to set up the scenario that we want to trace. Can you please explain that on the ticket? |
We discussed this async so @jzvikart is aligned btw |
This PR introduces a simple and versatile tracing framework that can be used for instrumentation, telemetry, profiling, debugging or runtime analysis. Traces are stored in PostgreSQL table
traces.