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

Missing http_route attribute on the http_server_duration metric when traces export is disabled #3862

Open
mivanilov opened this issue Jun 5, 2023 · 1 comment
Labels
bug Something isn't working has:reproducer This bug/feature has a minimal reproducer provided pkg:sdk-node priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect up-for-grabs Good for taking. Extra help will be provided by maintainers

Comments

@mivanilov
Copy link

What happened?

Steps to Reproduce

Set up NodeSDK with HttpInstrumentation, ExpressInstrumentation and metricReader. Run app with OTEL_TRACES_EXPORTER: none.

Expected Result

http_route attribute is present on the http_server_duration metric.

Actual Result

http_route attribute is missing on the http_server_duration metric.

Additional Details

If NodeSDK is set up with some traceExporter e.g. ConsoleSpanExporter then http_route attribute is present on the http_server_duration metric.
Sharing my investigation results - the http_route attribute is added to the metric attributes by HttpInstrumentation from the rpc metadata which in turn is added to the propagation context by ExpressInstrumentation only when tracing is enabled. Apparently at the moment there is no way to workaround it using instrumentation hooks as inside the hooks you have no means to manipulate the propagation context.
Also e.g. OTel Java instrumentation adds http_route to the http_server_duration metric even if OTEL_TRACES_EXPORTER: none.
There is a feature request that could help to workaround this #3694 but ideally this bug should be fixed.

OpenTelemetry Setup Code

import { OTLPMetricExporter } from '@opentelemetry/exporter-metrics-otlp-grpc';
import { PeriodicExportingMetricReader } from '@opentelemetry/sdk-metrics';
import { NodeSDK } from '@opentelemetry/sdk-node';
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express';
import * as process from 'process';

function startOTelNodeSDK() {
  const otelSDK = new NodeSDK({
    instrumentations: [
      new HttpInstrumentation(),
      new ExpressInstrumentation(),
    ],
    metricReader: new PeriodicExportingMetricReader({
      exporter: new OTLPMetricExporter(),
    }),
  });

  process.on('SIGTERM', () => {
    otelSDK
        .shutdown()
        .then(
            () => console.log('SDK shut down successfully'),
            (err) => console.log('Error shutting down SDK', err),
        )
        .finally(() => process.exit(0));
  });

  console.log('Starting OTel NodeSDK...');
  otelSDK.start();
}

startOTelNodeSDK();

package.json

{
  "name": "otel-instrumentation",
  "version": "0.0.1",
  "private": true,
  "scripts": {
    "clean": "rimraf build/*",
    "prepare": "npm run compile",
    "compile": "tsc -p .",
    "postcompile": "copyfiles -f 'build/src/**' build/workspace/ && copyfiles 'node_modules/**' build/workspace/"
  },
  "devDependencies": {
    "copyfiles": "^2.4.1",
    "rimraf": "^5.0.0",
    "typescript": "^5.0.4"
  },
  "dependencies": {
    "@opentelemetry/api": "^1.4.1",
    "@opentelemetry/auto-instrumentations-node": "^0.37.0",
    "@opentelemetry/core": "^1.13.0",
    "@opentelemetry/exporter-metrics-otlp-grpc": "^0.39.1",
    "@opentelemetry/instrumentation-express": "^0.32.3",
    "@opentelemetry/instrumentation-http": "^0.39.1",
    "@opentelemetry/sdk-metrics": "^1.13.0",
    "@opentelemetry/sdk-node": "^0.39.1",
  }
}

Relevant log output

No response

@mivanilov mivanilov added bug Something isn't working triage labels Jun 5, 2023
@pichlermarc pichlermarc added priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect and removed triage labels Jun 7, 2023
@pichlermarc
Copy link
Member

This is likely because there's no ContextManager registered, which the @opentelemetry/sdk-node package only does when there's a trace SDK to set up. This attribute information is added to the context and then passed to the @opentelemetry/instrumentation-http package.

So, to fix this issue we have to register a ContextManager even when there's no trace SDK set up.

In the meantime, you can work around this by doing this before setting up your SDK:

import {context} from "@opentelemetry/api";
import {AsyncLocalStorageContextManager} from "@opentelemetry/context-async-hooks";

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

I have created a reproducer for this issue: https://github.com/pichlermarc/repro-3862

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working has:reproducer This bug/feature has a minimal reproducer provided pkg:sdk-node priority:p2 Bugs and spec inconsistencies which cause telemetry to be incomplete or incorrect up-for-grabs Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

No branches or pull requests

2 participants