open-telemetry / opentelemetry-js-contrib

OpenTelemetry instrumentation for JavaScript modules
https://opentelemetry.io
Apache License 2.0
691 stars 507 forks source link

Pika build monorepo crush because requiere install unused dependencies #878

Closed ccalderon9411 closed 2 years ago

ccalderon9411 commented 2 years ago

Please answer these questions before submitting a bug report. When I try to complie my monorepo it crush because need some dependecies that I not use. image

What version of OpenTelemetry are you using?

0.27.0

"dependencies": {
    "@opentelemetry/api": "^1.0.4",
    "@opentelemetry/auto-instrumentations-node": "^0.27.3",
    "@opentelemetry/core": "^1.0.1",
    "@opentelemetry/exporter-collector": "^0.25.0",
    "@opentelemetry/exporter-collector-grpc": "^0.25.0",
    "@opentelemetry/exporter-jaeger": "^1.0.1",
    "@opentelemetry/exporter-prometheus": "^0.27.0",
    "@opentelemetry/host-metrics": "^0.27.1",
    "@opentelemetry/propagator-b3": "^1.0.1",
    "@opentelemetry/propagator-jaeger": "^1.0.1",
    "@opentelemetry/sdk-metrics-base": "^0.27.0",
    "@opentelemetry/sdk-node": "^0.27.0",
    "@opentelemetry/sdk-trace-base": "^1.0.1",
    "@opentelemetry/semantic-conventions": "^1.0.1",   
  },

What version of Node are you using?

14.17.0

Please provide the code you used to setup the OpenTelemetry SDK

import {
  CompositePropagator,
  W3CBaggagePropagator,
  W3CTraceContextPropagator,
} from '@opentelemetry/core';
import { BatchSpanProcessor } from '@opentelemetry/sdk-trace-base';
import { JaegerExporter } from '@opentelemetry/exporter-jaeger';
import { getNodeAutoInstrumentations } from '@opentelemetry/auto-instrumentations-node';
import { JaegerPropagator } from '@opentelemetry/propagator-jaeger';
import { B3InjectEncoding, B3Propagator } from '@opentelemetry/propagator-b3';
import { PrometheusExporter } from '@opentelemetry/exporter-prometheus';
import { NodeSDK } from '@opentelemetry/sdk-node';
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
import { Resource } from '@opentelemetry/resources';
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { OpenTelemetryConfiguration } from '../interfaces/tracing.interface';
import * as _ from 'lodash';
import { defaultTraceExcludePaths } from './tracing.constants';

export const otelSDK = (options: OpenTelemetryConfiguration) => {
  return new NodeSDK({
    metricExporter: new PrometheusExporter({
      ...options.prometheusExporterConfig,
    }),
    metricInterval: options.metricInterval || 1000,
    spanProcessor: new BatchSpanProcessor(
      new JaegerExporter({
        ...options.jaegerExporterConfig,
      }),
    ),
    contextManager: new AsyncLocalStorageContextManager(),
    textMapPropagator: new CompositePropagator({
      propagators: [
        new JaegerPropagator(),
        new W3CTraceContextPropagator(),
        new W3CBaggagePropagator(),
        new B3Propagator(),
        new B3Propagator({ injectEncoding: B3InjectEncoding.MULTI_HEADER }),
      ],
    }),
    resource: new Resource({
      ...options.resourceAttributes,
      [SemanticResourceAttributes.SERVICE_NAME]: options.serviceName,
    }),
    instrumentations: [
      getNodeAutoInstrumentations(),
      new HttpInstrumentation({
        ignoreIncomingPaths: _.union(defaultTraceExcludePaths, options.excludeTracerPaths),
      }),
    ],
  });
};

What did you do?

  1. git clone https://github.com/tresdoce/nestjs-tracing.git
  2. git checkout feat/opentelemetry
  3. npm install
  4. npm run build
  5. see the error

What did you expect to see?

Complete building of the project

What did you see instead?

The error says that I have to install this dependencies: OS: Windows, MacOS

Additional context

I think that problem is in @opentelemetry/auto-instrumentations-node dependency

Flarna commented 2 years ago

I guess that's a similar problem then https://github.com/open-telemetry/opentelemetry-js-contrib/issues/802

As workaround you can set skipLibCheck: true in your tsconfig.json file.

dyladan commented 2 years ago

Another workaround is to use only the instrumentation packages for the modules you are using rather than the auto-instrumentations-node dependency.

I think this is because the types are a devDependency https://github.com/open-telemetry/opentelemetry-js-contrib/blob/main/plugins/node/opentelemetry-instrumentation-aws-sdk/package.json#L59

It seems like this is happening more for some reason. We should probably try to address this in a more systemic way. I can think of some possible solutions:

  1. add the instrumented module to the dependencies of the instrumentation module
  2. add the instrumented module to the peerDependencies of the instrumentation module
    • NPM 7.x+ has peerDependenciesMeta which allows us to define these as optional so they are not automatically installed. NPM <7.x will not automatically install peer dependencies, but will warn in the console at install time.
    • this actually doesn't work because the compilation will still fail if the peer dependency is not installed. Real dependency is better in this case.
  3. Vendor all types in the instrumentation module
    • This obviously increases the maintenance burden and chance for failures, but removes all external dependencies
dyladan commented 2 years ago

Transferred to contrib because it is actually an issue with the @opentelemetry/auto-instrumentations-node package

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

github-actions[bot] commented 2 years ago

This issue was closed because it has been stale for 14 days with no activity.