pragmaticivan / nestjs-otel

OpenTelemetry (Tracing + Metrics) module for Nest framework (node.js) 🔭
Apache License 2.0
556 stars 49 forks source link

Custom metrics README example is generating TS errors #398

Open laurencefass opened 1 year ago

laurencefass commented 1 year ago

The example for metrics generates a typescript error

package configuration:

        "nestjs-otel": "^3.0.4",
        "@opentelemetry/api": "^1.0.4",
        "@opentelemetry/exporter-metrics-otlp-grpc": "^0.27.0",
        "@opentelemetry/exporter-trace-otlp-grpc": "^0.27.0",
        "@opentelemetry/instrumentation-express": "^0.27.0",
        "@opentelemetry/instrumentation-http": "^0.27.0",
        "@opentelemetry/instrumentation-winston": "^0.27.0",
        "@opentelemetry/sdk-node": "^0.27.0",

Referring to this example

https://github.com/pragmaticivan/nestjs-otel#metric-service

import { MetricService } from 'nestjs-otel';
import { Counter } from '@opentelemetry/api';

@Injectable()
export class BookService {
  private customMetricCounter: Counter;

  constructor(private readonly metricService: MetricService) {
    this.customMetricCounter = this.metricService.getCounter('custom_counter', {
      description: 'Description for counter',
    });
  }

  async getBooks() {
    this.customMetricCounter.add(1);
    return [`Harry Potter and the Philosopher's Stone`];
  }
}

import { Counter } from '@opentelemetry/api'; reports error

Module '"@opentelemetry/api"' has no exported member 'Counter'. Did you mean to use 'import Counter from "@opentelemetry/api"' instead?ts(2614)

Im guessing this is another mismatch caused by updates to the api not yet updated here?

Thanks

pragmaticivan commented 1 year ago

Hi @laurencefass have you tried with the version ^1.4.1 of the api package? Otel had some breaking changes in that version while the package was reaching stable status.

coler-j commented 1 year ago

@pragmaticivan would it be possible to declare the list of peer dependencies for this project and add that to the package.json? Both times i've tried to add this project to a nest project it is unclear when otel library versions to use.

If you install the most recent versions there are considerable type issues with the updated interfaces and your displayed examples.

See other errors as well:

pragmaticivan commented 1 year ago

@coler-j ,let me take a look, the reason why I didn't do that in the past was that OTEL metric SDK was not respecting version stability yet but that's not the case anymore.

pragmaticivan commented 1 year ago

Meanwhile, that's an updated working project https://github.com/pragmaticivan/nestjs-otel-prom-grafana-tempo as example

coler-j commented 11 months ago

I also think @opentelemetry/api should be moved to peer dependencies. For example, in my project nestjs-otel downloaded @opentelemetry/api@1.7.0 as a transitive dependency, but (since I also use the trace api directly for debugging) I also had @opentelemetry/api@1.6.0 install locally.

Since these dependencies were at different versions nestjs-otel was not able to see the tracer and metric providers established by the other lib causing unknown failures.

If you want @pragmaticivan I can attempt to clear this up in a PR and also add more explicit wording around dependency selection then we can close the multiple issues that exist (and might stear people away from using this library).