feat(opentelemetry-node): add @elastic/opentelemetry-instrumentation-openai #480

trentm · 2024-12-13T20:07:57Z

Adds the new instr to the default set.
Adds a simple test case for it. This adds the Ollama test-service and does a minimal test of openai embeddings, because we can then depend on a small model (~45MB). We leave the complex tests to the instr-openai package itself.
Adds a top-level examples/openai-chat.js.
Makes a couple tweaks to EDOT to avoid log.warn's from the underlying components. (I'll add reviewer notes for those.)

This does NOT include:

Support for this instrumentation in OTEL_NODE_ENABLED_INSTRUMENTATIONS and OTEL_NODE_DISABLED_INSTRUMENTATIONS. I think that is a longer discussion. I'll open a separate issue for that. -> supporting non-otel instrs in OTEL_NODE_{DIS,EN}ABLED_INSTRUMENTATIONS #481
A test case for instrumenting openai with ESM code. This requires some work on our module.register() usage in bootstrapping. -> use IITM wrap-only-hooked support for ESM #288

…openai

trentm · 2024-12-13T20:10:33Z

Running the top-level example looks like this:

% cd examples
% OPENAI_API_KEY=sk-... node -r @elastic/opentelemetry-node openai-chat.js | npx pino-pretty
[12:09:26.258] INFO (elastic-otel-node): start Elastic Distribution of OpenTelemetry Node.js
    preamble: true
    distroVersion: "0.5.0"
    env: {
      "os": "darwin 24.1.0",
      "arch": "arm64",
      "runtime": "Node.js v20.18.1"
    }
The sky appears blue because of Rayleigh scattering. When sunlight passes through the atmosphere, shorter blue wavelengths scatter more than longer red wavelengths, making the sky look predominantly blue to our eyes.

And telemetry (in mockotlpserver) looks like:

------ logs (1 records) ------
[2024-12-13T20:09:27.321999756Z] SEV-9 (unknown_service:node, traceId=be54f3, spanId=3a8a92): event "gen_ai.choice"
    {
      finish_reason: 'stop',
      index: 0,
      message: {}
    }
    --
    'event.name': 'gen_ai.choice',
    'gen_ai.system': 'openai'
------ trace 946ab6 (1 span) ------
       span 469499 "GET" (17.1ms, STATUS_CODE_ERROR, SPAN_KIND_CLIENT, GET http://169.254.169.254/computeMetadata/v1/instance)
------ trace 9cdd2c (1 span) ------
       span 52dbb3 "GET" (17.4ms, STATUS_CODE_ERROR, SPAN_KIND_CLIENT, GET http://metadata.google.internal./computeMetadata/v1/instance)
------ trace be54f3 (2 spans) ------
       span 3a8a92 "chat gpt-4o-mini" (1011.0ms, SPAN_KIND_CLIENT, GenAI openai, finish_reasons=stop, tokens 16in/37out)
  +8ms `- span 337d7c "POST" (998.5ms, SPAN_KIND_CLIENT, POST https://api.openai.com/v1/chat/completions -> 200)

(Aside: The GCP resource detector spans are still something we want to deal with.)

…t to include the license file in the published package

david-luna · 2024-12-16T08:16:42Z

packages/opentelemetry-node/lib/instrumentations.js

@@ -59,6 +60,7 @@
 */

 /* eslint-disable prettier/prettier */
+const {OpenAIInstrumentation} = require('@elastic/opentelemetry-instrumentation-openai');


I guess the alphabetical order is per package scope & name? if this goes upstream we should move it down then?

I guess. That's why I put it at the top. I don't have a preference though.

david-luna · 2024-12-16T08:42:14Z

@trentm the openAI instrumentation has some warnings that now appear in all PRs. I'm okay withe the any types but the linter makes noise. Could you leverage this one to fix them or disable the linter?

trentm · 2024-12-17T03:06:26Z

@trentm the openAI instrumentation has some warnings that now appear in all PRs. I'm okay withe the any types but the linter makes noise. Could you leverage this one to fix them or disable the linter?

@david-luna
There had been some lint errors in packages/mockotlpserver that I have fixed.

Do you mean the "any" lint warnings?


/Users/trentm/el/elastic-otel-node6/packages/instrumentation-openai/src/instrumentation.ts
   71:11  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  160:47  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  162:43  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  169:22  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  186:35  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  242:13  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  285:35  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  331:51  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  391:31  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  404:45  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  523:13  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  531:32  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  545:39  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  556:69  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  643:23  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  645:43  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  667:26  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  685:32  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  721:13  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/Users/trentm/el/elastic-otel-node6/packages/instrumentation-openai/src/utils.ts
  96:43  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

Do you suggest I added eslint-disable comments for all of these?

At least in opentelemetry-js-contrib.git this isn't typically done. E.g. instrumentation-bunyan has a few warnings:

[19:04:56 trentm@peach:~/src/opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-bunyan (git:main)]
% npm run lint

> @opentelemetry/instrumentation-bunyan@0.44.0 lint
> eslint . --ext .ts


/Users/trentm/src/opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-bunyan/src/OpenTelemetryBunyanStream.ts
  133:29  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/Users/trentm/src/opentelemetry-js-contrib/plugins/node/opentelemetry-instrumentation-bunyan/src/instrumentation.ts
   46:18  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
   60:39  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  145:30  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

but those don't show up in other opentelemetry-js-contrib.git PRs, IIUC.

david-luna · 2024-12-17T15:35:17Z

but those don't show up in other opentelemetry-js-contrib.git PRs, IIUC.

Not showing up in the PR would be enough for me. Do you know how is not showing in opentelemetry-js-contrib.git?

trentm · 2024-12-17T16:16:53Z

Do you know how is not showing in opentelemetry-js-contrib.git?

No :/
Perhaps just the scale of so many packages in the workspace and lint warnings? Or not, I'm guessing.

These warnings are "annotations", see the top of https://github.com/elastic/elastic-otel-node/actions/runs/12365514795/job/34510641198?pr=480

I don't really know what the process is for those being set, especially on unrelated PRs.

I'll merge this and then I'm curious to see if they remain, then we can follow-up separately.

feat(opentelemetry-node): add @elastic/opentelemetry-instrumentation-…

0c53af4

…openai

trentm requested a review from david-luna December 13, 2024 20:07

trentm self-assigned this Dec 13, 2024

trentm added 6 commits December 13, 2024 12:11

fix lint

d49c0cc

regenerate types

6dfeb3e

skip openai tests with node<18; openai itself requires node 18 or later

c39c767

Merge branch 'main' into trentm/edot-add-instr-openai

bebec32

instr-openai: add the missing license file

4287755

add a pointer to the instr-openai license text for v0.4.0 which forgo…

33bc2a5

…t to include the license file in the published package

david-luna approved these changes Dec 16, 2024

View reviewed changes

trentm added 2 commits December 16, 2024 19:01

Merge branch 'main' into trentm/edot-add-instr-openai

24249a0

fix some lint (style) errors

5f05ddd

trentm merged commit 30b1121 into main Dec 17, 2024
17 checks passed

trentm deleted the trentm/edot-add-instr-openai branch December 17, 2024 16:17

trentm mentioned this pull request Dec 19, 2024

chore(instr-openai): avoid '@typescript-eslint/no-explicit-any' warnings #498

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(opentelemetry-node): add @elastic/opentelemetry-instrumentation-openai #480

feat(opentelemetry-node): add @elastic/opentelemetry-instrumentation-openai #480

trentm commented Dec 13, 2024 •

edited

Loading

trentm commented Dec 13, 2024

david-luna Dec 16, 2024

trentm Dec 17, 2024

david-luna commented Dec 16, 2024

trentm commented Dec 17, 2024 •

edited

Loading

david-luna commented Dec 17, 2024

trentm commented Dec 17, 2024

feat(opentelemetry-node): add @elastic/opentelemetry-instrumentation-openai #480

feat(opentelemetry-node): add @elastic/opentelemetry-instrumentation-openai #480

Conversation

trentm commented Dec 13, 2024 • edited Loading

This does NOT include:

trentm commented Dec 13, 2024

david-luna Dec 16, 2024

Choose a reason for hiding this comment

trentm Dec 17, 2024

Choose a reason for hiding this comment

david-luna commented Dec 16, 2024

trentm commented Dec 17, 2024 • edited Loading

david-luna commented Dec 17, 2024

trentm commented Dec 17, 2024

trentm commented Dec 13, 2024 •

edited

Loading

trentm commented Dec 17, 2024 •

edited

Loading