Track metrics in OpenAI functions #16

nandorsoma · 2024-11-07T12:43:54Z

This PR adds metrics to the openai functions.

To test it call this api few times:

curl -X POST "http://localhost:8888/graphql" \
  -H "Content-Type: application/json" \
  -d '{"query": "mutation { data(input: { prompt: \"Hellobello\" }) { prompt } }"}'

Then query for the metrics on these urls. Dont forget to replace the job and vertice id.

curl -X GET "http://localhost:8081/jobs/a20788b06357ca57e6de5d0d8e56fcbe/vertices/d3f21cabc6fe0fdf76c8be915bdb22a2/metrics?get=0.Calc%5B2%5D.com_datasqrl_openai_completions_callCount"

curl -X GET "http://localhost:8081/jobs/a20788b06357ca57e6de5d0d8e56fcbe/vertices/d3f21cabc6fe0fdf76c8be915bdb22a2/metrics?get=0.Calc%5B2%5D.com_datasqrl_openai_completions_errorCount"

curl -X GET "http://localhost:8081/jobs/a20788b06357ca57e6de5d0d8e56fcbe/vertices/d3f21cabc6fe0fdf76c8be915bdb22a2/metrics?get=0.Calc%5B2%5D.com_datasqrl_openai_completions_retryCount"

curl -X GET "http://localhost:8081/jobs/a20788b06357ca57e6de5d0d8e56fcbe/vertices/d3f21cabc6fe0fdf76c8be915bdb22a2/metrics?get=0.Calc%5B2%5D.com_datasqrl_openai_completions_p99"

nandorsoma · 2024-11-07T12:47:30Z

sqrl-openai/src/main/java/com/datasqrl/openai/util/P99LatencyTracker.java

+import java.util.Collections;
+import java.util.List;
+
+public class P99LatencyTracker {


I’m not completely satisfied that I had to create this class manually. It would have been much better to use the existing Histogram class here. Since this is just a temporary solution until we can integrate the org.apache.flink:flink-metrics-dropwizard dependency, I opted not to implement a more performant solution for now.

I see your point. I think this is good enough for now and we can adjust once we get customer feedback.

mbroecheler

This is a good first stab, thank you. Please DRY a bit.

mbroecheler · 2024-11-07T16:00:00Z

sqrl-openai/src/main/java/com/datasqrl/openai/completions.java

 @AutoService(ScalarFunction.class)
 public class completions extends ScalarFunction {

+    public static final String P99_METRIC = "com.datasqrl.openai.completions.p99";


You have the same code here 3 times. Move this into a utility class that takes the function name as the argument and DRYies the code.

mbroecheler · 2024-11-07T16:18:45Z

sqrl-openai/src/main/java/com/datasqrl/openai/util/P99LatencyTracker.java

+import java.util.Collections;
+import java.util.List;
+
+public class P99LatencyTracker {


I see your point. I think this is good enough for now and we can adjust once we get customer feedback.

mbroecheler · 2024-11-07T16:19:11Z

sqrl-openai/src/main/java/com/datasqrl/openai/completions.java

    @Override
    public void open(FunctionContext context) throws Exception {
        this.openAICompletions = createOpenAICompletions();
+        this.latencyTracker = new P99LatencyTracker(100);


Make 100 a static variable.

… to difference in formatting

… env

mbroecheler

It looks good. I made a suggestion to combine the helper classes. This is not a must-have but I recommend you implement it because we are trying to find the patterns that we can use across functions. Hence, investing some more time into cleaning that up will pay dividends.

mbroecheler · 2024-11-12T20:51:40Z

sqrl-openai/src/main/java/com/datasqrl/openai/extract_json.java

    public String eval(String prompt, String modelName, Double temperature, Double topP) {
-        return executeWithRetry(
-                () -> openAICompletions.callCompletions(prompt, modelName, true, null, temperature, topP)
+        if (prompt == null || modelName == null) return null;


My suggestion is to move the executeWithRetry method into the FunctionMetricTracker so that you can make the code DRYer. In fact, I think we can combine those two helper classes. Whenever you execute some function with retry, you also want to count how many times you executed it and how many failures you have. Keeping these two helper clases orthogonal, makes it awkward to use and leads to more code.

Thanks @mbroecheler! I was thinking about improving it further, but my understanding was that the retry utility is a temporary thing until we start using asynchronous functions. Does it make sense to improve it further if we are going to replace it anyway?

@nandorsoma, yes, we will move to async eventually. That's why it would be nice to have all code related to retries and metrics on function invocations encapsulated in one class. Then we would only have to update the helper class and not the body of every function implementation.
For now, it mostly gives us better architectural encapsulation as we are learning how to implement functions.

nandorsoma · 2024-12-09T16:21:27Z

@mbroecheler I've introduced another helper class because it felt weird to execute through the MetricsTracker. Especially that in good circumstances we could swap out the MetricsTracker to a more robust one. Let me know if it looks good.

Based on our recent discussion, later I can add the vector similarity check to this PR, but if you want to merge as is, I can open a separate PR for that.

mbroecheler

Looks good. Thank you

nandorsoma added 2 commits November 7, 2024 12:18

Track metrics in OpenAI functions

ad79e83

Fix tests failing due to inconsistent indentation in the response json.

5ca37e3

nandorsoma linked an issue Nov 7, 2024 that may be closed by this pull request

Track metrics in OpenAI functions #15

Closed

nandorsoma commented Nov 7, 2024

View reviewed changes

mbroecheler requested changes Nov 7, 2024

View reviewed changes

nandorsoma added 9 commits November 11, 2024 18:16

Extracting metrics to FunctionMetricTracker

e22f060

fixing doc

c9a66f3

extracting json field to fix sqrl test which intermittently fails due…

a2fe0b4

… to difference in formatting

return null when the inputs are null

4ff2485

specify java version in github workflow

1a9a31b

debug

ca6a16a

removing test dependencies from global dependencies

1a65ba1

fix junit dep

97546aa

fixing the use of transient keyword to optimize behavior in clustered…

00b868d

… env

mbroecheler requested changes Nov 12, 2024

View reviewed changes

nandorsoma added 2 commits December 9, 2024 16:26

Introducing FunctionExecutor to wrap metrics/retry

9717422

Updating snapshot to accomodate new vector_embedd results

8b412cc

mbroecheler approved these changes Dec 9, 2024

View reviewed changes

nandorsoma merged commit 8b62e4b into main Dec 10, 2024
1 check passed

nandorsoma deleted the 15-track-metrics-in-openai-functions branch December 10, 2024 13:42

Track metrics in OpenAI functions #16

Track metrics in OpenAI functions #16

Uh oh!

Conversation

nandorsoma commented Nov 7, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbroecheler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbroecheler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nandorsoma commented Dec 9, 2024

Uh oh!

mbroecheler left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants