From 6300fedc98451279d469e794d28a291e7a0faaef Mon Sep 17 00:00:00 2001
From: Trent Mick <trent.mick@elastic.co>
Date: Wed, 5 Feb 2025 10:04:10 -0800
Subject: [PATCH] test(edot): temporarily disable instr-elastic-openai.test.js
 (#588)

This temporarily disables the flaky instr-elastic-openai.test.js
until the Ollama crash bug https://github.com/ollama/ollama/issues/8400
is resolved.

As well, this improves the test file's handling for pulling the test
model. Before this the setup code was errorneously assuming the
Ollama response statusCode would indicate success. It doesn't.

Refs: https://github.com/elastic/elastic-otel-node/issues/587
---
 .github/workflows/test-edot.yml               | 14 ++--
 .../test/instr-elastic-openai.test.js         | 80 +++++++++++++------
 2 files changed, 64 insertions(+), 30 deletions(-)

diff --git a/.github/workflows/test-edot.yml b/.github/workflows/test-edot.yml
index 560111a2..03fc5991 100644
--- a/.github/workflows/test-edot.yml
+++ b/.github/workflows/test-edot.yml
@@ -98,12 +98,14 @@ jobs:
         env:
           MYSQL_ALLOW_EMPTY_PASSWORD: 1
 
-      ollama:
-        # A light fork of Ollama to float some in-progress contributions related
-        # to more closely matching OpenAI behavior.
-        image: ghcr.io/elastic/ollama/ollama:testing
-        ports:
-          - 11434:11434
+      # Disabled until https://github.com/ollama/ollama/issues/8400 is resolved.
+      # ollama:
+      #   # A light fork of Ollama to float some in-progress contributions related
+      #   # to more closely matching OpenAI behavior.
+      #   image: ghcr.io/elastic/ollama/ollama:testing
+      #   # image: ollama/ollama:0.5.7
+      #   ports:
+      #     - 11434:11434
 
       postgres:
         image: postgres:16
diff --git a/packages/opentelemetry-node/test/instr-elastic-openai.test.js b/packages/opentelemetry-node/test/instr-elastic-openai.test.js
index 38a3d5b5..9cb7a726 100644
--- a/packages/opentelemetry-node/test/instr-elastic-openai.test.js
+++ b/packages/opentelemetry-node/test/instr-elastic-openai.test.js
@@ -18,21 +18,26 @@
  */
 
 const http = require('http');
+const {basename} = require('path');
 const test = require('tape');
-const semver = require('semver');
+// const semver = require('semver');
 const {runTestFixtures, assertDeepMatch} = require('./testutils');
 
-let skip = process.env.TEST_GENAI_MODEL === undefined;
-if (skip) {
-    console.log(
-        '# SKIP elastic openai tests: TEST_GENAI_MODEL is not set (load env from test/test-services.env)'
-    );
-} else {
-    skip = !semver.satisfies(process.version, '>=18');
-    if (skip) {
-        console.log('# SKIP elastic openai requires node >=18');
-    }
-}
+let skip = true;
+console.log(
+    '# SKIP elastic/instr-openai test until https://github.com/ollama/ollama/issues/8400 is resolved'
+);
+// let skip = process.env.TEST_GENAI_MODEL === undefined;
+// if (skip) {
+//     console.log(
+//         '# SKIP elastic openai tests: TEST_GENAI_MODEL is not set (load env from test/test-services.env)'
+//     );
+// } else {
+//     skip = !semver.satisfies(process.version, '>=18');
+//     if (skip) {
+//         console.log('# SKIP elastic openai requires node >=18');
+//     }
+// }
 
 /** @type {import('./testutils').TestFixture[]} */
 const testFixtures = [
@@ -99,18 +104,34 @@ async function testModelIsPulled() {
                 method: 'POST',
             },
             (res) => {
-                // This is lazy error handling. We should watch for the response
-                // data ending with `{"status":"success"}`.
-                res.resume();
+                if (res.statusCode !== 200) {
+                    reject(
+                        new Error(
+                            `unexpected status code from Ollama: ${res.statusCode}`
+                        )
+                    );
+                    res.resume();
+                    return;
+                }
+
+                // If the pull is successful, the last line of the body will
+                // be: `{"status":"success"}`.  Otherwise, typically the last
+                // line indicates the error.
+                const chunks = [];
+                res.on('data', (chunk) => {
+                    chunks.push(chunk);
+                });
                 res.on('end', () => {
-                    if (res.statusCode !== 200) {
+                    const body = Buffer.concat(chunks).toString('utf8');
+                    const lastLine = body.trim().split(/\n/g).slice(-1)[0];
+                    if (lastLine === '{"status":"success"}') {
+                        resolve();
+                    } else {
                         reject(
                             new Error(
-                                `unexpected status code from Ollama: ${res.statusCode}`
+                                `could not pull "${process.env.TEST_GENAI_MODEL}" model: lastLine=${lastLine}`
                             )
                         );
-                    } else {
-                        resolve();
                     }
                 });
             }
@@ -121,10 +142,21 @@ async function testModelIsPulled() {
     });
 }
 
-test('elastic openai instrumentation', {skip}, async (suite) => {
-    suite.comment(`pulling test GenAI model (${process.env.TEST_GENAI_MODEL})`);
-    await testModelIsPulled();
+test(basename(__filename), {skip}, async (t) => {
+    t.comment(`pulling test GenAI model (${process.env.TEST_GENAI_MODEL})`);
+    let isPulled = false;
+    const startTime = new Date();
+    try {
+        await testModelIsPulled();
+        isPulled = true;
+        const deltaS = Math.round((new Date() - startTime) / 1000);
+        t.comment(`successfully pulled model (${deltaS}s)`);
+    } catch (pullErr) {
+        t.error(pullErr);
+    }
 
-    runTestFixtures(suite, testFixtures);
-    suite.end();
+    if (isPulled) {
+        runTestFixtures(t, testFixtures);
+    }
+    t.end();
 });