open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.74k stars 801 forks source link

Missing "http.route" in custom OpenTelemetry Next.js configuration #5110

Open jhgeluk opened 3 days ago

jhgeluk commented 3 days ago

What happened?

Steps to Reproduce

Use a custom OpenTelemetry configuration in a NextJS project with the HttpInstrumentation and Prometheus exporter.

Expected Result

http_server_duration_bucket{http_route="ROUTE_HERE" http_scheme="http",http_method="GET",net_host_name="localhost",http_flavor="1.1",http_status_code="304",net_host_port="3000",le="250"} 2

Actual Result

http_server_duration_bucket{http_scheme="http",http_method="GET",net_host_name="localhost",http_flavor="1.1",http_status_code="304",net_host_port="3000",le="250"} 2

No routes show up in the metrics.

Additional Details

How can we add the http.route or next.route to the metrics returned by HttpInstrumentation?

I also tried to add it manually using span.setAttribute("http.route", 'ROUTE HERE'); but to no avail.

Is there any way we can add this manually, without using express as the server for my NextJS project?

OpenTelemetry Setup Code

// instrumentation-node.ts

import { PrometheusExporter } from "@opentelemetry/exporter-prometheus";
import { HostMetrics } from "@opentelemetry/host-metrics";
import { registerInstrumentations } from "@opentelemetry/instrumentation";
import { HttpInstrumentation } from "@opentelemetry/instrumentation-http";
import { RuntimeNodeInstrumentation } from "@opentelemetry/instrumentation-runtime-node";
import {
  Resource,
  detectResourcesSync,
  envDetector,
  hostDetector,
  processDetector,
} from "@opentelemetry/resources";
import { MeterProvider } from "@opentelemetry/sdk-metrics";
import {
  ATTR_SERVICE_NAME,
  ATTR_SERVICE_VERSION,
} from "@opentelemetry/semantic-conventions";

const exporter = new PrometheusExporter({
  port: 9464,
});

const detectedResources = detectResourcesSync({
  detectors: [envDetector, processDetector, hostDetector],
});

const customResources = new Resource({
  [ATTR_SERVICE_NAME]: "nrc-product-dashboard",
  [ATTR_SERVICE_VERSION]: "0.1.0",
});

const resources = detectedResources.merge(customResources);

const meterProvider = new MeterProvider({
  readers: [exporter],
  resource: resources,
});

// Custom counter to count the number of times request is made to the server
const meter = meterProvider.getMeter("default");

// Create a counter for total requests
const requestCounter = meter.createCounter("node_requests_total", {
  description: "Total number of requests",
});

const hostMetrics = new HostMetrics({
  name: `nrc-product-dashboard-metrics`,
  meterProvider,
});

registerInstrumentations({
  meterProvider,
  instrumentations: [
    new HttpInstrumentation({
      requestHook: (span, request) => {
        requestCounter.add(1);
        // span.setAttribute("custom_attribute", request.method);
      },
    }),
    new RuntimeNodeInstrumentation(),
  ],
});

hostMetrics.start();

package.json

{
  "name": "next-app",
  "version": "0.1.0",
  "private": true,
  "scripts": {
    "dev": "next dev",
    "dev:ssl": "next dev --experimental-https -H 0.0.0.0",
    "build": "next build",
    "start": "next start",
    "lint": "next lint",
    "codegen:compile": "graphql-codegen --require dotenv/config --config codegen.ts",
    "codegen:watch": "graphql-codegen -w"
  },
  "dependencies": {
    "@opentelemetry/exporter-prometheus": "^0.53.0",
    "@opentelemetry/host-metrics": "^0.35.4",
    "@opentelemetry/instrumentation": "^0.53.0",
    "@opentelemetry/instrumentation-http": "^0.53.0",
    "@opentelemetry/instrumentation-runtime-node": "^0.7.0",
  }
}

Relevant log output

No response

Netail commented 2 days ago

Related; https://github.com/open-telemetry/opentelemetry-js/issues/4890

Seems to be missing from the attributes indeed, as NextJS needs to set the context route? If I read https://github.com/open-telemetry/opentelemetry-js/issues/4890#issuecomment-2377348854 correctly

Screenshot 2024-11-05 at 14 23 54

But as nextjs uses node's http client (node:http), shouldn't we be able to get the route from the regular request? @pichlermarc, just like the other attributes

ciandt-crodrigues commented 2 days ago

I've tried both adding the span attibute and adding the RPCMetadata but neither seems to work

registerInstrumentations({
  meterProvider,
  instrumentations: [
    new HttpInstrumentation({
      requestHook: (span, request) => {
        const route = (request as IncomingMessage)?.url;
        if (route) {
          if (route && (route.endsWith(".json") || !route.includes("."))) {
            // Try to apply the route only for pages and client side fetches
            const rpcMetadata = getRPCMetadata(context.active()); // retrieve rpc metadata from the active context
            if (rpcMetadata) {
              if (rpcMetadata?.type === RPCType.HTTP) {
                rpcMetadata.route = route;
              }
            } else {
              setRPCMetadata(context.active(), {
                type: RPCType.HTTP,
                route,
                span,
              });
            }
            span.setAttribute(ATTR_HTTP_ROUTE, route);
          }
        }
      },
    }),
    new RuntimeNodeInstrumentation(),
  ],
});
pichlermarc commented 15 hours ago

@jhgeluk thanks for reaching out.

I also tried to add it manually using span.setAttribute("http.route", 'ROUTE HERE'); but to no avail.

Adding a route like this will set it on the Span. Since span is not readable (this is on purpose to avoid OpenTelemetry being used to influence business-logic by retrieving span information), we cannot retrieve any attributes from the span in such a manner. So this not working is intended behavior. We may add a hook that's specific to metrics in the future, see #5051, I'm currently still refining that one before it's ready for implementation.

@Netail touched on this a bit already - there is a workaround for this, which is using what I proposed at https://github.com/open-telemetry/opentelemetry-js/issues/4890#issuecomment-2377348854 We don't supply the route in the http instrumentation by default, because the http module has no concept of route, it's a higher level concept that is only introduced by systems that abstract and build on what the http does, like Express.

@jhgeluk would you mind trying this and reporting back if that works for you?

@ciandt-crodrigues, in your setup code, do you register a ContextManager with the API? Without a ContextManager, it'll not work (see https://github.com/open-telemetry/opentelemetry-js/issues/3862#issuecomment-2379464250 on how to do that). If there's any problems after having set up a context manager, please file another issue to avoid having two threads in one issue (it can be difficult to follow discussions when there's more than one thing going on).

ciandt-crodrigues commented 11 hours ago

@jhgeluk thanks for reaching out.

I also tried to add it manually using span.setAttribute("http.route", 'ROUTE HERE'); but to no avail.

Adding a route like this will set it on the Span. Since span is not readable (this is on purpose to avoid OpenTelemetry being used to influence business-logic by retrieving span information), we cannot retrieve any attributes from the span in such a manner. So this not working is intended behavior. We may add a hook that's specific to metrics in the future, see #5051, I'm currently still refining that one before it's ready for implementation.

@Netail touched on this a bit already - there is a workaround for this, which is using what I proposed at #4890 (comment) We don't supply the route in the http instrumentation by default, because the http module has no concept of route, it's a higher level concept that is only introduced by systems that abstract and build on what the http does, like Express.

@jhgeluk would you mind trying this and reporting back if that works for you?

@ciandt-crodrigues, in your setup code, do you register a ContextManager with the API? Without a ContextManager, it'll not work (see #3862 (comment) on how to do that). If there's any problems after having set up a context manager, please file another issue to avoid having two threads in one issue (it can be difficult to follow discussions when there's more than one thing going on).

Thanks, setting the ContextManager fixed for me

ciandt-crodrigues commented 11 hours ago

Just sharing the complete workaround for nextjs

// instrumentation.ts
export async function register() {
  if (process.env.NEXT_RUNTIME === "nodejs") {
    await import("./otel-prometheus");
  }
}
// otel-phometheus.ts
import { context } from "@opentelemetry/api";
import { AsyncLocalStorageContextManager } from "@opentelemetry/context-async-hooks";
import { getRPCMetadata, RPCType, setRPCMetadata } from "@opentelemetry/core";
import { PrometheusExporter } from "@opentelemetry/exporter-prometheus";
import { HostMetrics } from "@opentelemetry/host-metrics";
import { registerInstrumentations } from "@opentelemetry/instrumentation";
import { HttpInstrumentation } from "@opentelemetry/instrumentation-http";
import { RuntimeNodeInstrumentation } from "@opentelemetry/instrumentation-runtime-node";
import {
  detectResourcesSync,
  envDetector,
  hostDetector,
  processDetector,
  Resource,
} from "@opentelemetry/resources";
import { MeterProvider } from "@opentelemetry/sdk-metrics";
import {
  ATTR_HTTP_ROUTE,
  ATTR_SERVICE_NAME,
  ATTR_SERVICE_VERSION,
} from "@opentelemetry/semantic-conventions";
import { IncomingMessage } from "http";

// manually setting a context manager to replace the no-op context manager
context.setGlobalContextManager(new AsyncLocalStorageContextManager());

const exporter = new PrometheusExporter({
  port: 9464,
});
const detectedResources = detectResourcesSync({
  detectors: [envDetector, processDetector, hostDetector],
});

const customResources = new Resource({
  [ATTR_SERVICE_NAME]: "MyApp",
  [ATTR_SERVICE_VERSION]: "0.1.0",
});

const resources = detectedResources.merge(customResources);

const meterProvider = new MeterProvider({
  readers: [exporter],
  resource: resources,
});
const hostMetrics = new HostMetrics({
  name: "olo-r:shell",
  meterProvider,
});

registerInstrumentations({
  meterProvider,
  instrumentations: [
    new HttpInstrumentation({
      requestHook: (span, request) => {
        const route = (request as IncomingMessage)?.url;
        if (route) {
          if (route && (route.endsWith(".json") || !route.includes("."))) {
            // Try to apply the route only for pages and client side fetches
            const rpcMetadata = getRPCMetadata(context.active()); // retrieve rpc metadata from the active context
            if (rpcMetadata) {
              if (rpcMetadata?.type === RPCType.HTTP) {
                rpcMetadata.route = route;
              }
            } else {
              setRPCMetadata(context.active(), {
                type: RPCType.HTTP,
                route,
                span,
              });
            }
          }
        }
      },
    }),
    new RuntimeNodeInstrumentation(),
  ],
});
hostMetrics.start();