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.

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

Tomas2D · 2024-11-22T11:27:35Z

docs/native-telemetry.md

Consider adding this document to the documentation sidebar. (docs/_sidebar.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]>

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 added 2 commits November 22, 2024 09:15

feat(instrumentation): add fw metrics

24425ee

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

feat: pr review

cbfaa9b

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

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

fix: review 2

c8e45fb

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

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

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

Tomas2D Nov 22, 2024

GALLLASMILAN Nov 22, 2024


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

		```bash

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

pilartomas Nov 20, 2024 •

edited

Loading

GALLLASMILAN Nov 22, 2024 •

edited

Loading