oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
71.7k stars 2.54k forks source link

OpenTelemetry doesn't seem to work on Bun 0.7.0 #3775

Open lewisl9029 opened 11 months ago

lewisl9029 commented 11 months ago

What version of Bun is running?

0.7.0

What platform is your computer?

Darwin 22.4.0 arm64 arm

What steps can reproduce the bug?

I've set up a repo to repro here with instructions: https://github.com/lewisl9029/bun-opentelemetry-issue-repro

Pasting here for convenience:

  1. Run bun run test-server
  2. Run bun run bun in a separate terminal
  3. Go to http://localhost:58050 in a browser

What is the expected behavior?

What do you see instead?

Additional information

Also provided a node server to demonstrate expected behavior, just run npm run node in step 2.

If I had to guess, the fact that bun lacks support for node's --experimental-loader=@opentelemetry/instrumentation/hook.mjs (or the --require=./app/tracing.cjs equivalent for setting up OTel in node with CJS) might be part of the problem?

Or it could also be something else entirely. OTel's tries to fail silently without affecting program behavior as much as possible, which means we'd probably have to dig down a lot deeper to find the root cause of something like this.

birkskyum commented 10 months ago

I'm not able to get the terminal output with npm run node.

nicu-chiciuc commented 9 months ago

This is the only thing that stops us from migrating from Node to Bun. There aren't clear instruction for how to add telemetry to Bun. I'm not entirely sure if this is the responsibility of this repo, or, if it should be added to opentelemetry-js https://github.com/open-telemetry/opentelemetry-js

jonataswalker commented 9 months ago

This is indeed a no-go for some of us. They are struggling to provide support for Deno.

zx8086 commented 8 months ago

This is one of two things stopping us committing to Bun over NodeJS, Opentelemetry instrumentation and connecting to a Couchbase database

kytay commented 6 months ago

Hi there.

I am trying to following the example on opentelemetry.js on how to setup auto instrumentation using --require instrumentation.ts in the command line.

Is using --require a must? Is it the reason why there isn't any documentation or articles online that explains how can we use opentelemetry.js with bun?

If I have only an index.ts

import express, { Express } from 'express';
import { NodeSDK } from '@opentelemetry/sdk-node';
import { ConsoleSpanExporter } from '@opentelemetry/sdk-trace-node';
import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node';
import {
  PeriodicExportingMetricReader,
  ConsoleMetricExporter,
} from '@opentelemetry/sdk-metrics';

const sdk = new NodeSDK({
  traceExporter: new ConsoleSpanExporter(),
  metricReader: new PeriodicExportingMetricReader({
    exporter: new ConsoleMetricExporter(),
  }),
  instrumentations: [getNodeAutoInstrumentations()],
});

sdk.start();

const PORT: number = parseInt(process.env.PORT || '8080');
const app: Express = express();

function getRandomNumber(min: number, max: number) {
  return Math.floor(Math.random() * (max - min) + min);
}

app.get('/rolldice', (req, res) => {
  res.send(getRandomNumber(1, 6).toString());
});

app.listen(PORT, () => {
  console.log(`Listening for requests on http://localhost:${PORT}`);
});

why this code won't work? I did sdk.start() before my application code, as per described in opentelemetry

ImLunaHey commented 6 months ago

the code needs to run before any of the imports. that's why it needs the --require

AlexandrePh commented 5 months ago

does bun offer the require functionality? Any updates on this issue?

ImLunaHey commented 5 months ago

Here is a working example for Bun. Thank you @Jarred-Sumner (https://discord.com/channels/876711213126520882/876711213126520885/1196585812486279208)

/*instrumentation.ts*/
// Require dependencies
import { NodeSDK } from '@opentelemetry/sdk-node';
import { ConsoleSpanExporter } from '@opentelemetry/sdk-trace-node';
import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node';
import { PeriodicExportingMetricReader, ConsoleMetricExporter } from '@opentelemetry/sdk-metrics';

const sdk = new NodeSDK({
  traceExporter: new ConsoleSpanExporter(),
  metricReader: new PeriodicExportingMetricReader({
    exporter: new ConsoleMetricExporter(),
  }),
  instrumentations: [getNodeAutoInstrumentations()],
});

sdk.start();
/*app.ts*/
import express from 'express';

const PORT = parseInt(process.env.PORT || '8080');
const app = express();

function getRandomNumber(min: number, max: number) {
  return Math.floor(Math.random() * (max - min) + min);
}

app.get('/rolldice', (req, res) => {
  res.send(getRandomNumber(1, 6).toString());
});

app.listen(PORT, () => {
  console.log(`Listening for requests on http://localhost:${PORT}`);
});

To run the example:

bun run --preload ./instrumentation.ts  ./app.ts
chlorophant commented 5 months ago

Did this end up working as expected?

ImLunaHey commented 5 months ago

Yes. The only thing you need to do differently is use the preload flag instead of require.

ricardo-devis-agullo commented 3 months ago

This solution kinda worked for me, but at least when using applicationinsights package (which uses opentelemetry internally), auto collection of http requests still didn't work (on Elysia server).

ricardo-devis-agullo commented 3 months ago

I got a minimal reproduction example that doesn't even need Elyisia, just Bun.serve

import appInsights from "applicationinsights";
import http from "http";

appInsights.setup("YOUR-CONNECTION-STRING").start();

appInsights.defaultClient.addTelemetryProcessor((envelope) => {
  envelope.data.baseType === "RequestData" && console.log("GOT REQUEST DATA");
});

const PORT = 3000;

function bunServer() {
  Bun.serve({
    port: PORT,
    fetch(req) {
      return new Response("Hello World\n");
    },
  });
}

function nodeServer() {
  const server = http.createServer((req, res) => {
    res.writeHead(200, { "Content-Type": "text/plain" });
    res.end("Hello World\n");
  });
  server.listen(PORT);
}

// This will output "GOT REQUEST DATA" after 5 seconds
const app = nodeServer;
// This will never output "GOT REQUEST DATA"
// const app = bunServer;

app();

setTimeout(() => {
  fetch(`http://localhost:${PORT}`);
}, 5000);
zx8086 commented 3 months ago
bun run --preload ./instrumentation.ts  ./app.ts

This solves it

ricardo-devis-agullo commented 3 months ago

The thing I mentioned it doesn't, because I think it boils down to appInsights trying to attach itself to http module to listen to requests, and Bun.serve just using something completely different.

zx8086 commented 3 months ago

Did this end up working as expected?

What didn't work for you ?

dusty-phillips commented 1 month ago

I experienced similar to ricardo-devis-agullo. I am able to send manual traces using a variation of ImLunaHey's code, but autoinstrumenting the http endpoints doesn't work.

I have a feeling that the autoinstrumentation for express works differently from the instrumentation for http, so ImLunaHey's code works with express but not Elysia or (in my case) Hono.

My workaround is to manually create spans in a tracing middleware. Because it took me a bloody long time to track down all the attributes, I'm sharing my (Hono-style, but easily adapted) middleware for other users:

import { trace, context, propagation } from "@opentelemetry/api"
import { createMiddleware } from "hono/factory"

const tracer = trace.getTracer("<your-app-name>")

export const tracingMiddleware = createMiddleware(async (c, next) => {
  const traceparent = c.req.header("traceparent")
  const carrier = traceparent ? { traceparent } : {}
  const extractedContext = propagation.extract(context.active(), carrier)

  await context.with(extractedContext, async () => {
    const span = tracer.startSpan(c.req.path)
    span.setAttribute("passage.id", c.get("passageId") ?? "")

    // TODO: wasn't able to find http.flavour (http version number)
    span.setAttribute("http.route", c.req.path)
    span.setAttribute("http.method", c.req.method)
    span.setAttribute("http.url", c.req.url)
    span.setAttribute("http.schema", new URL(c.req.raw.url).protocol)
    span.setAttribute("http.referrer", c.req.raw.referrer)
    span.setAttribute("http.client_ip", c.env.requestIP(c.req.raw))
    span.setAttribute("http.user_agent", c.req.header("User-Agent") ?? "")
    span.setAttribute(
      "http.request_content_length",
      c.req.header("Content-Length") ?? "",
    )
    span.setAttribute("span.kind", "server")
    span.setAttribute("type", "server")
    span.setAttribute("is_root", true)
    if (traceparent) {
      const parts = traceparent.split("-")
      if (parts.length === 4) {
        span.setAttribute("trace.parent_id", parts[2])
        span.setAttribute("trace.trace_id", parts[1])
      }
    }

    await context.with(trace.setSpan(extractedContext, span), async () => {
      await next()
      span.setAttribute("http.status_code", c.res.status)
      span.setAttribute("status_code", c.res.status)
      // TODO: would like to get response content length in here

      span.end()
    })
  })
})

In addition, I extended my tracing preloader to replace global.fetch with a version that traces outgoing http calls:

import { NodeSDK } from "@opentelemetry/sdk-node"
import { getNodeAutoInstrumentations } from "@opentelemetry/auto-instrumentations-node"
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-proto"
import opentelemetry from "@opentelemetry/api"

const sdk = new NodeSDK({
  traceExporter: new OTLPTraceExporter(),
  instrumentations: [getNodeAutoInstrumentations()],
})

sdk.start()

const globalFetch = global.fetch

export async function fetcher(
  input: URL | RequestInfo,
  init?: RequestInit,
): Promise<Response> {
  console.log("Attempting to trace fetch")
  const tracer = opentelemetry.trace.getTracer("fablehenge-hono")
  let url: URL
  if (typeof input === "string") {
    url = new URL(input)
  } else if (input instanceof URL) {
    url = input
  } else {
    url = new URL(input.url)
  }
  const method = init?.method ?? "GET"

  const parentSpan = opentelemetry.trace.getSpan(opentelemetry.context.active())
  console.log(parentSpan)

  return await tracer.startActiveSpan(`HTTP ${method}`, async (span) => {
    span.setAttribute("component", "fetch")
    span.setAttribute("http.url", url.toString())
    span.setAttribute("http.method", method)
    span.setAttribute("http.scheme", url.protocol)

    const response = await globalFetch(url, init)

    span.setAttribute("http.status_code", response.status)

    span.end()

    return response
  })
}

console.info("(Fetch is patched)")
global.fetch = fetcher
ImLunaHey commented 1 month ago

you could have used this for fetch calls https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/instrumentation-undici/

acuderman commented 1 week ago

I am also experiencing issues with auto-instrumentation for the pg, pino, and http modules (bun 1.1.13). Despite using the suggested --preload approach for setting up OpenTelemetry instrumentation, these modules do not seem to be instrumented correctly.

Has anyone managed to get these working?