feat(trace): implement otlp http backend #7

GALLLASMILAN · 2024-12-04T09:21:20Z

New OTLP backend

I implemented the custom OTLP backend that supports the HTTP-proto OpenTelemetry format a saves the Traces to the Observe (MongoDB) and MLflow.
See the docs/using-with-opentelemetry.md file for more info about integration with OpenTelemetry stack.

I changed the default port to 4318. This is the standard port for all OTLP backends for http-proto

The new routes are under the v1 prefix.

This is a breaking change with no backward compatibility. It's agreed with @matoushavlena

I added the new system route with the base data about versions and used environment variables. It's hidden in the Swagger

I use the bee-agent-framework directly in the integration tests.

Typescript files are automatically generated from the proto files copied from https://github.com/open-telemetry/opentelemetry-proto/tree/main/opentelemetry/proto

the traces/:id and spans routes return the trace by the frameworkTraceId provided from the framework context. It is the only one ID that we are able to save in both places (observe, bee-api) and then load the trace by this attribute.
We cannot process the POST v1/traces response right now the trace will be sent via the https://www.npmjs.com/package/@opentelemetry/sdk-node and the https://opentelemetry.io/docs/collector/

Release instructions

The ingress must be changed on the new port 4318.
The collector config must be updated.

add the new exporter

  otlphttp:
    endpoint: <observe-url>
    compression: none
    headers:
      x-bee-authorization: "<AUTH_KEY>"

add this new otlphttp exporter to the exporters array in the service.pipelines.traces/dev section

Merge and deploy API PR: feat(observe): implement opentelemetry bee-api#113
Merge and deploy UI PR: feat(observe): new OpenTelemetry observe backend implementation bee-ui#118

Signed-off-by: GALLLASMILAN <gallas.milan@gmail.com>

docs/using-with-opentelemetry.md

pilartomas · 2024-12-05T07:01:50Z

src/app.ts

+  // Custom parser for "application/x-protobuf"
+  app.addContentTypeParser(
+    'application/x-protobuf',
+    { parseAs: 'buffer' },
+    traceProtobufBufferParser
+  );


It's weird that traceProfobufBufferParser parses anything protobuf.

I need to define the custom contentTypeParser, otherwise I get following:

OTLPExporterError: Unsupported Media Type at IncomingMessage.

error.
The traceProtobufBufferParser filters the parser logic only for the v1/traces route.

can you utilize this somehow?

As with the other APIs, addContentTypeParser is encapsulated in the scope in which it is declared. This means that if you declare it in the root scope it will be available everywhere, while if you declare it inside a plugin it will be available only in that scope and its children.

src/protos/trace_service.proto

pilartomas · 2024-12-05T07:21:49Z

src/trace/trace.service.ts

+export async function createTrace(traceBody: ExportTraceServiceRequest__Output): Promise<TraceDto> {
+  const spans = [...traceBody.resourceSpans].flatMap((resourceSpan) => {


I think it would also be good to check the instrumentation scope as a guardrail for unwanted data?

done in f1485c7

Why the filter, wouldn't you rather fail if you get invalid data?

No, in case we send more data scopes on observe, it should be able to filter only the right one.

Signed-off-by: GALLLASMILAN <gallas.milan@gmail.com>

pilartomas

Overall structure looks good.

jezekra1 · 2024-12-11T07:44:27Z

.github/workflows/main.yml

+jobs:
+  main:
+    timeout-minutes: 20
+    name: Lint & Build & Test


nitpick: I don't see any test :D

jezekra1 · 2024-12-11T07:47:38Z

package.json

@@ -6,7 +6,8 @@
  "description": "Observability API server for bee-agent-framework",
  "type": "module",
  "scripts": {
-    "build": "rm -rf dist && tsc",
+    "build": "rm -rf dist && tsc && cp -R src/protos dist/protos",
+    "proto:generate": "proto-loader-gen-types --defaults --oneofs --longs=Number --enums=String --grpcLib=@grpc/grpc-js --outDir=./src/types/generated ./src/protos/*.proto && node scripts/add_js_extensions.js",


This is hacky, we already have a proper setup for generating proto types in the framework using tsup, please copy it from there

https://github.com/i-am-bee/bee-agent-framework/blob/main/scripts/ibm_vllm_generate_protos

jezekra1 · 2024-12-11T07:47:56Z

scripts/add_js_extensions.js

+
+const directory = path.resolve(__dirname, '../src/types/generated');
+
+async function addJsExtensions(dir) {


not necessary with a proper tsup setup (see framework)

jezekra1 · 2024-12-11T07:53:41Z

src/protos/common.proto

@@ -0,0 +1,81 @@
+// Copyright 2019, OpenTelemetry Authors


Why not use the http version? @opentelemetry/exporter-trace-otlp-http

We're getting rid of protobufs in as many places as possible

The observe uses the http proto right now. I generate the types from the proto files provided by the OpenTelemetry framework.
We will use the same proto files for the grpc server in the future.
The HTTP proto with the default port 4318 is only one part the the standard OTLP server. The second part is grpc server on port 4317 that should be implemented later

The second thing, is I need to parse protobuf to json.

jezekra1

Agreed offline that protofile type generation will be addressed later in a separate PR

feat(trace): implement otlp http backend

9cb14c6

Signed-off-by: GALLLASMILAN <gallas.milan@gmail.com>

GALLLASMILAN force-pushed the otlp-http-backend branch from b58793d to 9cb14c6 Compare December 4, 2024 09:24

GALLLASMILAN requested review from pilartomas and jezekra1 December 4, 2024 09:27

GALLLASMILAN commented Dec 4, 2024

View reviewed changes

docs/using-with-opentelemetry.md Outdated Show resolved Hide resolved

pilartomas reviewed Dec 5, 2024

View reviewed changes

fix: review 1

f1485c7

Signed-off-by: GALLLASMILAN <gallas.milan@gmail.com>

GALLLASMILAN requested a review from pilartomas December 5, 2024 08:38

GALLLASMILAN added 2 commits December 5, 2024 11:37

fix: review 2

f5ef0b3

Signed-off-by: GALLLASMILAN <gallas.milan@gmail.com>

fix: spans route params schema

2eef891

Signed-off-by: GALLLASMILAN <gallas.milan@gmail.com>

pilartomas approved these changes Dec 5, 2024

View reviewed changes

This was referenced Dec 6, 2024

feat(observe): implement opentelemetry i-am-bee/bee-api#113

Merged

feat(observe): new OpenTelemetry observe backend implementation i-am-bee/bee-ui#118

Merged

jezekra1 requested changes Dec 11, 2024

View reviewed changes

jezekra1 approved these changes Dec 11, 2024

View reviewed changes

matoushavlena merged commit 1296ef5 into main Dec 11, 2024
2 checks passed

matoushavlena deleted the otlp-http-backend branch December 11, 2024 11:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(trace): implement otlp http backend #7

feat(trace): implement otlp http backend #7

GALLLASMILAN commented Dec 4, 2024 •

edited

Loading

pilartomas Dec 5, 2024

GALLLASMILAN Dec 5, 2024

pilartomas Dec 5, 2024 •

edited

Loading

pilartomas Dec 5, 2024

GALLLASMILAN Dec 5, 2024

pilartomas Dec 5, 2024

GALLLASMILAN Dec 5, 2024

pilartomas left a comment

jezekra1 Dec 11, 2024

jezekra1 Dec 11, 2024

jezekra1 Dec 11, 2024

jezekra1 Dec 11, 2024

GALLLASMILAN Dec 11, 2024

GALLLASMILAN Dec 11, 2024

jezekra1 left a comment

		export async function createTrace(traceBody: ExportTraceServiceRequest__Output): Promise<TraceDto> {
		const spans = [...traceBody.resourceSpans].flatMap((resourceSpan) => {


		const directory = path.resolve(__dirname, '../src/types/generated');

		async function addJsExtensions(dir) {

feat(trace): implement otlp http backend #7

feat(trace): implement otlp http backend #7

Conversation

GALLLASMILAN commented Dec 4, 2024 • edited Loading

New OTLP backend

Release instructions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pilartomas Dec 5, 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

pilartomas left a 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

jezekra1 left a comment

Choose a reason for hiding this comment

GALLLASMILAN commented Dec 4, 2024 •

edited

Loading

pilartomas Dec 5, 2024 •

edited

Loading