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 ollama/ollama#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: #587
elastic · Feb 5, 2025 · 6300fed · 6300fed
1 parent d5f8837
commit 6300fed
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 30 deletions.
diff --git 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
@@ -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();
 });