feat: OpenTelemetry Tracing API and Exporting #26710

devsnek · 2024-11-04T14:11:56Z

Initial import of OTEL code supporting tracing. Metrics soon to come. Implements APIs for https://jsr.io/@deno/otel so that code using OpenTelemetry.js just works tm.

There is still a lot of work to do with configuration and adding built-in tracing to core APIs, which will come in followup PRs.

bartlomieju

First pass from me. This looks really great 🚀

bartlomieju · 2024-11-07T23:33:24Z

Cargo.lock

@@ -1619,7 +1666,7 @@ dependencies = [
 "encoding_rs",
 "futures",
 "import_map",
- "indexmap",
+ "indexmap 2.3.0",


@devsnek could you try to remove so many duplicate versions of various dependencies added in this PR? Maybe we could bump some of the version we use in Deno?

This is because opentelemetry-otel uses tonic, which still relies on tonic 0.4 rather than tonic 0.5. This will be fixed in the next tonic release (see hyperium/tonic@58345a6). There is not much we can do for now.

cli/ops/bench.rs

bartlomieju · 2024-11-07T23:35:29Z

cli/tsc/dts/lib.deno.unstable.d.ts

+      exit(): void;
+
+      /**
+       * End this span, and exit it as the "current" span.


What's the difference between exit() and end(). Does end() not restore the previous one?

enter and exit control the span scope. end marks the end time of the span and submits it to be collected.

bartlomieju · 2024-11-07T23:35:44Z

cli/tsc/dts/lib.deno.unstable.d.ts

+
+      [Symbol.dispose](): void;
+
+      static current(): Span | undefined | null;


Is this on purpose that it's undefined | null?

bartlomieju · 2024-11-07T23:36:14Z

cli/tsc/dts/lib.deno.unstable.d.ts

+    }
+
+    /**
+     * A SpanExporter compatible with OpenTelemetry.js


Could you link here and in the next one to relevant package or repo?

ext/web/15_performance.js

ext/web/benches/timers_ops.rs

runtime/lib.rs

runtime/ops/os/mod.rs

runtime/ops/otel.rs

runtime/Cargo.toml

piscisaureus · 2024-11-08T00:32:53Z

runtime/js/telemetry.js

+  let out = "";
+  for (let i = 0; i < bytes / 4; i += 1) {
+    const r32 = (MathRandom() * 2 ** 32) >>> 0;
+    out += hexSliceLookupTable[(r32 >> 24) & 0xff];


Was this somehow faster than r32.toString(16).padStart(8, '0') ?

piscisaureus · 2024-11-08T00:40:57Z

runtime/ops/os/mod.rs

@@ -186,6 +186,8 @@ fn op_get_exit_code(state: &mut OpState) -> i32 {

 #[op2(fast)]
 fn op_exit(state: &mut OpState) {
+  crate::ops::otel::otel_drop_state(state);


We should have some tests that ensure the OTEL pipeline is properly flushed before the process exits. Some scenarios:

console.log("foo"); // Program naturally exits here

console.log("foo"); Deno.exit(0);

console.log("foo"); throw new Error("uncaught");

Web worker logs something and immediate calls self.close()

Web worker logs something and the main worker immediately calls worker.terminate()

Web worker logs something and calls Deno.exit()

Web worker logs something and throws an exception.

runtime/ops/otel.rs

runtime/js/telemetry.js

cli/args/mod.rs

runtime/ops/otel.rs

KnorpelSenf · 2024-11-08T15:41:19Z

Does this in any way go towards #22102?

lucacasonato · 2024-11-08T16:03:58Z

@KnorpelSenf No, not really.

lucacasonato · 2024-11-12T14:24:36Z

Cargo.lock

@@ -1619,7 +1666,7 @@ dependencies = [
 "encoding_rs",
 "futures",
 "import_map",
- "indexmap",
+ "indexmap 2.3.0",


This is because opentelemetry-otel uses tonic, which still relies on tonic 0.4 rather than tonic 0.5. This will be fixed in the next tonic release (see hyperium/tonic@58345a6). There is not much we can do for now.

lucacasonato · 2024-11-12T15:36:40Z

cli/tsc/dts/lib.deno.unstable.d.ts

+      /**
+       * Enter this span as the "current" span.
+       */
+      enter(): void;
+
+      /**
+       * Exit this span as the "current" span and restore the previous one.
+       */
+      exit(): void;


This is not something that exists in otel-js, right?

otel-js has a pretty confusing api but at a high level it does not have the ability to enter/exit spans without calling a callback, like this: const span = tracer.startActiveSpan(() => { doSomething(); span.end(); });. On our side we have enter/exit, but we also have the nicer choice of doing using span = new Deno.tracing.Span(); which will automatically exit/end when disposed.

Ok - because we have that, do we need enter and exit? I think we should just force people to use using?

lucacasonato · 2024-11-12T15:58:52Z

runtime/js/telemetry.js

+  startTime;
+  endTime;
+  status = null;
+  attributes = {};


Suggested change

attributes = {};

attributes = { __proto__: null };

lucacasonato · 2024-11-12T16:00:20Z

runtime/js/telemetry.js

+    if (span.spanContext) {
+      const context = span.spanContext();
+      traceId = context.traceId;
+      spanId = context.spanId;
+      traceFlags = context.traceFlags;
+    } else {
+      traceId = span.traceId;
+      spanId = span.spanId;
+      traceFlags = span.traceFlags;
+    }


I don't get it - when is spanContext empty?

originally when i wrote this our spans did not have spanContext for compat. i can reverse this to be if (span.spanId) to avoid calling spanContext unless needed.

yes that'd be good

runtime/js/telemetry.js

lucacasonato

LGTM once you resolve the two open threads

lucacasonato · 2024-11-12T17:03:03Z

runtime/js/telemetry.js

+        this.traceId = context.traceId ?? generateId(TRACE_ID_BYTES);
+      } else {
+        this.parentSpanId = parent.spanId;
+        this.traceId = parent.traceId ?? generateId(TRACE_ID_BYTES);


why does this need to default to generateId?

devsnek changed the title ~~feat: OpenTelemetry API and Exporting~~ feat: OpenTelemetry Tracing API and Exporting Nov 4, 2024

bartlomieju added this to the 2.1.0 milestone Nov 4, 2024

devsnek force-pushed the otel branch from ea17790 to b5ae5f8 Compare November 5, 2024 14:08

devsnek added the ci-draft Run the CI on draft PRs. label Nov 5, 2024

devsnek marked this pull request as ready for review November 6, 2024 10:15

devsnek force-pushed the otel branch from bf41fa9 to f9bfd5e Compare November 6, 2024 10:26

devsnek requested review from bartlomieju and lucacasonato November 6, 2024 10:35

devsnek force-pushed the otel branch from f9bfd5e to d13577f Compare November 6, 2024 16:58

bartlomieju reviewed Nov 7, 2024

View reviewed changes

piscisaureus reviewed Nov 8, 2024

View reviewed changes

runtime/Cargo.toml Outdated Show resolved Hide resolved

piscisaureus reviewed Nov 8, 2024

View reviewed changes

runtime/ops/otel.rs Show resolved Hide resolved

piscisaureus reviewed Nov 8, 2024

View reviewed changes

runtime/js/telemetry.js Show resolved Hide resolved

lucacasonato reviewed Nov 8, 2024

View reviewed changes

cli/args/mod.rs Outdated Show resolved Hide resolved

lucacasonato reviewed Nov 8, 2024

View reviewed changes

runtime/ops/otel.rs Show resolved Hide resolved

runtime/ops/otel.rs Show resolved Hide resolved

runtime/ops/otel.rs Outdated Show resolved Hide resolved

feat: otel

82470d5

devsnek force-pushed the otel branch from 86a99db to 82470d5 Compare November 11, 2024 10:24

upgrade otel and use hyper

0471c25

devsnek force-pushed the otel branch from 53f5b5e to 0471c25 Compare November 12, 2024 12:03

lucacasonato and others added 2 commits November 12, 2024 13:07

Update default attributes

048057c

hopefully fix windows?

75bb216

devsnek force-pushed the otel branch from 296547e to 75bb216 Compare November 12, 2024 14:59

lucacasonato reviewed Nov 12, 2024

View reviewed changes

lucacasonato approved these changes Nov 12, 2024

View reviewed changes

lucacasonato reviewed Nov 12, 2024

View reviewed changes

feedback

9b36a8d

coherency

01100c6

devsnek enabled auto-merge (squash) November 13, 2024 10:05

devsnek merged commit aa54618 into main Nov 13, 2024
17 checks passed

devsnek deleted the otel branch November 13, 2024 10:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: OpenTelemetry Tracing API and Exporting #26710

feat: OpenTelemetry Tracing API and Exporting #26710

devsnek commented Nov 4, 2024 •

edited

Loading

bartlomieju left a comment

bartlomieju Nov 7, 2024

lucacasonato Nov 12, 2024

bartlomieju Nov 7, 2024

devsnek Nov 8, 2024

bartlomieju Nov 7, 2024

bartlomieju Nov 7, 2024

piscisaureus Nov 8, 2024

piscisaureus Nov 8, 2024

KnorpelSenf commented Nov 8, 2024

lucacasonato commented Nov 8, 2024

lucacasonato Nov 12, 2024

lucacasonato Nov 12, 2024

devsnek Nov 12, 2024 •

edited

Loading

lucacasonato Nov 12, 2024

lucacasonato Nov 12, 2024

lucacasonato Nov 12, 2024

devsnek Nov 12, 2024

lucacasonato Nov 12, 2024

lucacasonato left a comment

lucacasonato Nov 12, 2024


		[Symbol.dispose](): void;

		static current(): Span \| undefined \| null;

feat: OpenTelemetry Tracing API and Exporting #26710

feat: OpenTelemetry Tracing API and Exporting #26710

Conversation

devsnek commented Nov 4, 2024 • edited Loading

bartlomieju 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KnorpelSenf commented Nov 8, 2024

lucacasonato commented Nov 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devsnek Nov 12, 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

lucacasonato left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devsnek commented Nov 4, 2024 •

edited

Loading

devsnek Nov 12, 2024 •

edited

Loading