Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
lewisl9029 opened this issue Jul 24, 2023 · 19 comments
Open

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

lewisl9029 opened this issue Jul 24, 2023 · 19 comments
Assignees
Labels
bug Something isn't working node.js Compatibility with Node.js APIs

Comments

@lewisl9029
Copy link

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?

  • Request to /v1/traces shows up in test-server terminal

What do you see instead?

  • Nothing shows up

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.

@lewisl9029 lewisl9029 added the bug Something isn't working label Jul 24, 2023
@robobun robobun added the node.js Compatibility with Node.js APIs label Jul 24, 2023
@birkskyum
Copy link
Collaborator

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

@nicu-chiciuc
Copy link

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
Copy link

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

@cirospaciari cirospaciari self-assigned this Sep 15, 2023
@zx8086
Copy link

zx8086 commented Oct 21, 2023

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

@kytay
Copy link

kytay commented Dec 10, 2023

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
Copy link
Contributor

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

@AlexandrePh
Copy link

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

@ImLunaHey
Copy link
Contributor

ImLunaHey commented Jan 15, 2024

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
Copy link

chlorophant commented Jan 27, 2024

Did this end up working as expected?

@ImLunaHey
Copy link
Contributor

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

novaluke added a commit to novaluke/otel-auto-bun-repro that referenced this issue Jan 30, 2024
novaluke added a commit to novaluke/otel-auto-bun-repro that referenced this issue Jan 30, 2024
@ricardo-devis-agullo
Copy link

ricardo-devis-agullo commented Mar 6, 2024

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
Copy link

ricardo-devis-agullo commented Mar 6, 2024

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
Copy link

zx8086 commented Mar 8, 2024

bun run --preload ./instrumentation.ts  ./app.ts

This solves it

@ricardo-devis-agullo
Copy link

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
Copy link

zx8086 commented Mar 9, 2024

Did this end up working as expected?

What didn't work for you ?

@dusty-phillips
Copy link

dusty-phillips commented May 18, 2024

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
Copy link
Contributor

@acuderman
Copy link

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?

@SlexAxton
Copy link

yea, i think I'm experiencing the same thing:

@opentelemetry/instrumentation-http Module http has been loaded before @opentelemetry/instrumentation-http so it might not work, please initialize it before requiring http
@opentelemetry/instrumentation-http Module https has been loaded before @opentelemetry/instrumentation-http so it might not work, please initialize it before requiring https

I tried explicitly importing this module into my telemetry.ts file, but to no effect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working node.js Compatibility with Node.js APIs
Projects
None yet
Development

No branches or pull requests