feat(instrumentation): add fw metrics #187

GALLLASMILAN · 2024-11-20T10:19:07Z

Description

I added the new custom middleware, that collects module_usage statistics with data about os, version etc. This middleware is enabled by default and sends data about the (agent, llm, tool) entities.
Data are automatically sent via the open telemetry SDK that is part of the framework right now, but it's configured only for metrics. I use PeriodicMetric reader, but I need to push the metrics when each run ends. Otherwise, I would drop a lot of metrics because the runtime does not wait for the SDK pushing.
This default telemetry can be disabled via the BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED environment variable. See the new ./docs/docs/native-telemetry.md file for more info.

Checklist

I have read the contributor guide
Linting passes: yarn lint or yarn lint:fix
Formatting is applied: yarn format or yarn format:fix
Unit tests pass: yarn test:unit
E2E tests pass: yarn test:e2e
Tests are included
Documentation is changed or added
Commit messages and PR title follow conventional commits

pilartomas · 2024-11-20T11:43:07Z

src/instrumentation/sdk.ts

+const metricExporter = new OTLPMetricExporter({
+  url: "https://bee-collector.apps.fmaas-backend.fmaas.res.ibm.com/v1/metrics",
+});


We will likely need some better domain that is stable. @matoushavlena

Discussed with @GALLLASMILAN. He is going to lead the next actions required for this.

I will switch the URL to https://bee-telemetry.res.ibm.com/v1/metrics when it's prepared.

pilartomas · 2024-11-20T11:46:36Z

package.json

+    "@opentelemetry/exporter-metrics-otlp-http": "^0.54.2",
+    "@opentelemetry/resources": "^1.27.0",
+    "@opentelemetry/sdk-metrics": "^1.27.0",
+    "@opentelemetry/sdk-node": "^0.54.2",
+    "@opentelemetry/semantic-conventions": "^1.27.0",


Do you need to specify the metrics deps directly? Following should work

import { metrics } from "@opentelemetry/sdk-node"

Tomas2D · 2024-11-20T13:15:40Z

README.md

@@ -152,6 +152,10 @@ This project and everyone participating in it are governed by the [Code of Condu

 All content in these repositories including code has been provided by IBM under the associated open source software license and IBM is under no obligation to provide enhancements, updates, or support. IBM developers produced this code as an open source project (not as an IBM product), and IBM makes no assertions as to the level of quality nor security, and will not be maintaining this code going forward.

+## Telemetry
+
+Some metrics are collected by default. See the [Native Telemetry](./docs/native-telemetry.md) for more detail.


Use absolute path + run yarn docs:build afterward.

Tomas2D · 2024-11-20T13:17:13Z

package.json

+    "test:unit": "BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED=true vitest run src",
+    "test:unit:watch": "BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED=true vitest run src",
+    "test:e2e": "BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED=true vitest run tests/e2e",
+    "test:e2e:watch": "BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED=true vitest watch tests/e2e",
    "test:all": "yarn test:unit && yarn test:e2e && yarn test:examples",
-    "test:examples": "vitest --config ./examples/vitest.examples.config.ts run tests/examples",
-    "test:examples:watch": "vitest --config ./examples/vitest.examples.config.ts watch tests/examples",
+    "test:examples": "BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED=true vitest --config ./examples/vitest.examples.config.ts run tests/examples",
+    "test:examples:watch": "BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED=true vitest --config ./examples/vitest.examples.config.ts watch tests/examples",


I would add such variable to .env.test (just create it, it is going to be automatically used: see tests/setup.ts)

Tomas2D · 2024-11-20T13:18:19Z

src/instrumentation/config.ts

@@ -18,6 +18,9 @@ import { parseEnv } from "@/internals/env.js";
 import { z } from "zod";

 export const INSTRUMENTATION_ENABLED = parseEnv.asBoolean("BEE_FRAMEWORK_INSTRUMENTATION_ENABLED");
+export const INSTRUMENTATION_METRICS_ENABLED = !parseEnv.asBoolean(
+  "BEE_FRAMEWORK_INSTRUMENTATION_METRICS_DISABLED",


Could we not switch the flag to _ENABLED and set the default to true?

src/instrumentation/create-telemetry-metrics-middleware.ts

Tomas2D · 2024-11-20T13:20:04Z

src/instrumentation/create-telemetry-metrics-middleware.ts

+    // send metrics to the public collector
+    emitter.match(
+      (event) => event.path === `${basePath}.run.${finishEventName}`,
+      async () => await metricReader.forceFlush(),


TIP: metricReader.forceFlush.bind(metricReader)

Here, this inside forceFlush will always correctly refer to metricReader, so bind() is not required.

docs/native-telemetry.md

Tomas2D · 2024-11-22T11:28:01Z

docs/native-telemetry.md

+
+We understand that not all users want to send telemetry data. You can easily disable this feature by setting an environment variable:
+
+```bash


isn't there support for ```env ?

I haven't seen the ```env yet, I will remove the bash part and the markdown will highlight it correctly.

Signed-off-by: GALLLASMILAN <[email protected]>

pilartomas · 2025-01-23T09:35:35Z

package.json

@@ -277,6 +279,7 @@
    "@googleapis/customsearch": "^3.2.0",
    "@grpc/grpc-js": "^1.12.4",
    "@grpc/proto-loader": "^0.7.13",
+    "@ibm-generative-ai/node-sdk": "~3.2.3",


BAM is no longer supported

pilartomas · 2025-01-23T09:40:31Z

src/instrumentation/opentelemetry.ts

+const name = "bee-agent-framework";

-export const activeTracesMap = new Map<string, string>();
+export const meter = api.metrics.getMeter(name, Version);


I'd assume name and version are taken from the OTEL SDK instance. Is that not the case?

pilartomas · 2025-01-23T09:49:28Z

src/instrumentation/opentelemetry.ts

+  moduleUsageGauge.record(1, {
+    source: instance.constructor.name,
+    type: instance instanceof BaseAgent ? "agent" : instance instanceof Tool ? "tool" : "llm",
+    framework_version: Version,
+    os_type: os.type(),
+    os_release: os.release(),
+    os_arch: os.arch(),
+    trace_id: traceId,
+    event_id: eventId,
+  });


It is weird to see most of these labels,

os, fw version, trace are likely supplied by the SDK (not necessarily as a label but as OTEL resource or something like that)

trace_id and event_id are not a good fit for a metric

source and type are good but it signals this should be an incrementing counter not a gauge, it will also likely blow up our collector/backend.

Lastly, and this is likely the most important. I'd say we'd want a label that establishes an anonymous identity once framework is cloned (or first used). A random ID that's stable.

Tomas2D · 2025-01-23T09:47:46Z

docs/integrations.md

+    const answer = response.messages.at(-1)?.content;
+    return { next: Workflow.END, update: { answer: answer?.toString() } };


In the main branch this is not needed.

Tomas2D · 2025-01-23T09:48:01Z

examples/integrations/langgraph.ts

+    const answer = response.messages.at(-1)?.content;
+    return { next: Workflow.END, update: { answer: answer?.toString() } };


package.json

Tomas2D · 2025-01-23T09:49:33Z

src/instrumentation/opentelemetry.spec.ts

+    throw new Error("Method not implemented.");
+  }
+  public readonly emitter = Emitter.root.child<BaseLLMEvents>({
+    namespace: ["bam", "llm"],


update namespace please

Tomas2D · 2025-01-23T09:51:16Z

src/instrumentation/opentelemetry.ts

@@ -116,3 +127,26 @@ export function buildTraceTree({
    },
  );
 }
+
+export const isMeasurementedInstance = (instance: any) =>


use named function instead

Tomas2D · 2025-01-23T09:52:07Z

src/instrumentation/types.ts

@@ -14,22 +14,22 @@
 * limitations under the License.
 */

-import { Attributes, SpanStatus, TimeInput } from "@opentelemetry/api";
+import { api } from "@opentelemetry/sdk-node";


import type { api } ...

Tomas2D · 2025-01-23T09:52:38Z

src/internals/serializable.ts

@@ -20,6 +20,9 @@ import * as R from "remeda";
 import { extractClassName } from "@/serializer/utils.js";
 import { SerializerError } from "@/serializer/error.js";
 import { Cache } from "@/cache/decoratorCache.js";
+import { startMetricNodeSdkReader } from "@/instrumentation/sdk.js";
+
+startMetricNodeSdkReader();


Why the start function is here in serializable module?

Each exported measurable module inherits from this class.

Signed-off-by: Milan Gallas <[email protected]>

GALLLASMILAN requested a review from Tomas2D November 20, 2024 10:19

GALLLASMILAN force-pushed the telemetry-framework-metrics branch from 89064aa to 5de3232 Compare November 20, 2024 10:26

GALLLASMILAN requested a review from pilartomas November 20, 2024 10:26

pilartomas reviewed Nov 20, 2024

View reviewed changes

Tomas2D requested changes Nov 20, 2024

View reviewed changes

GALLLASMILAN force-pushed the telemetry-framework-metrics branch from 5de3232 to cbfaa9b Compare November 22, 2024 08:15

GALLLASMILAN requested review from Tomas2D and pilartomas November 22, 2024 08:16

Tomas2D reviewed Nov 22, 2024

View reviewed changes

GALLLASMILAN requested a review from Tomas2D November 22, 2024 12:13

mmurad2 added the blocked Waiting for an external factor to be completed. label Dec 16, 2024

GALLLASMILAN mentioned this pull request Jan 9, 2025

New UI for Observe i-am-bee/bee-observe#22

Open

GALLLASMILAN added 2 commits January 23, 2025 09:59

feat(instrumentation): add fw metrics

11826aa

Signed-off-by: GALLLASMILAN <[email protected]>

feat: pr review

d20938b

Signed-off-by: GALLLASMILAN <[email protected]>

GALLLASMILAN force-pushed the telemetry-framework-metrics branch from c8e45fb to af722fe Compare January 23, 2025 09:00

fix: review 2

969a12a

Signed-off-by: GALLLASMILAN <[email protected]>

GALLLASMILAN force-pushed the telemetry-framework-metrics branch from af722fe to 969a12a Compare January 23, 2025 09:06

GALLLASMILAN marked this pull request as ready for review January 23, 2025 09:31

GALLLASMILAN requested a review from a team as a code owner January 23, 2025 09:31

pilartomas requested changes Jan 23, 2025

View reviewed changes

Tomas2D requested changes Jan 23, 2025

View reviewed changes

fix: pr 1

d722a54

Signed-off-by: Milan Gallas <[email protected]>

GALLLASMILAN requested a review from pilartomas January 23, 2025 10:16

pilartomas approved these changes Jan 23, 2025

View reviewed changes

fix: pr 2

f137a0e

Signed-off-by: Milan Gallas <[email protected]>

GALLLASMILAN requested a review from Tomas2D January 23, 2025 11:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(instrumentation): add fw metrics #187

feat(instrumentation): add fw metrics #187

GALLLASMILAN commented Nov 20, 2024

pilartomas Nov 20, 2024 •

edited

Loading

matoushavlena Nov 21, 2024

GALLLASMILAN Nov 22, 2024 •

edited

Loading

pilartomas Nov 20, 2024

Tomas2D Nov 20, 2024

Tomas2D Nov 20, 2024

Tomas2D Nov 20, 2024

Tomas2D Nov 20, 2024

GALLLASMILAN Nov 21, 2024

Tomas2D Nov 22, 2024

GALLLASMILAN Nov 22, 2024

pilartomas Jan 23, 2025

pilartomas Jan 23, 2025

pilartomas Jan 23, 2025 •

edited

Loading

Tomas2D Jan 23, 2025

Tomas2D Jan 23, 2025

Tomas2D Jan 23, 2025

Tomas2D Jan 23, 2025

Tomas2D Jan 23, 2025

Tomas2D Jan 23, 2025

GALLLASMILAN Jan 23, 2025 •

edited

Loading


		We understand that not all users want to send telemetry data. You can easily disable this feature by setting an environment variable:

		```bash

		const answer = response.messages.at(-1)?.content;
		return { next: Workflow.END, update: { answer: answer?.toString() } };

feat(instrumentation): add fw metrics #187

Are you sure you want to change the base?

feat(instrumentation): add fw metrics #187

Conversation

GALLLASMILAN commented Nov 20, 2024

Description

Checklist

pilartomas Nov 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GALLLASMILAN Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pilartomas Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GALLLASMILAN Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

pilartomas Nov 20, 2024 •

edited

Loading

GALLLASMILAN Nov 22, 2024 •

edited

Loading

pilartomas Jan 23, 2025 •

edited

Loading

GALLLASMILAN Jan 23, 2025 •

edited

Loading