Meter metrics get override in instrumentations after setMeterProvider has called

[osherv]

Hey guys, I'm currently working on the MongoDB metrics collections and noticed some issue (not huge but can lead to many bugs in the future). In order to produce metrics, we need to create some metric instruments, and these metric instruments automatically get overridden when setting a new meter provider. Maybe it's ok and that's the best practice but it requires the instrumentations to implement something like:

export class MongoDBInstrumentation extends InstrumentationBase {
  private _connectionsUsage!: UpDownCounter;

  constructor(protected override _config: MongoDBInstrumentationConfig = {}) {
    super('@opentelemetry/instrumentation-mongodb', VERSION, _config);
    this._updateMetricInstruments();
  }

  override setMeterProvider(meterProvider: MeterProvider) {
    super.setMeterProvider(meterProvider);
    this._updateMetricInstruments();
  }

  private _updateMetricInstruments() {
    this._connectionsUsage = this.meter.createUpDownCounter(
      'active_connections',
      {
        description:
          'The number of connections that are currently in state described by the state attribute.',
      }
    );
  }

• [ ] This only affects the JavaScript OpenTelemetry library
• [x] This may affect other libraries, but I would like to get opinions here first

[pichlermarc]

Hmm, I wonder if we should add an optional updateMetricInstruments() to InstrumentationAbstract that is called when setMeterProvider() is called. I feel like I don't know enough about the instrumentations to give a definitive answer though. :thinking:

[legendecas]

I can see this common metrics problem diverging from the tracing part as the metric instruments need to be re-created after the meter provider has been updated. An abstract method called createMetricInstruments or updateMetricInstruments sounds good to me.

[osherv]

Yea sounds like a good solution. You can assign this to me :)

[weyert]

How will this work when you want to use histogram as a metric? You would need to define a view for a histogram to ensure the correct buckets are used and you can only define Views when instantiating the meter provider.

[legendecas]

@weyert: How will this work when you want to use histogram as a metric? You would need to define a view for a histogram to ensure the correct buckets are used and you can only define Views when instantiating the meter provider.

Instrumentations are supposed to create histogram metrics in the _updateMetricInstruments method. I don't think there is any difference between histograms with other types of metrics.

Yes, a MeterProvider must be constructed with views before setting it up with instrumentation. That is to say, metrics API didn't provide a means to instruct the SDK to configure the buckets for a histogram right now. It's being tracked at https://github.com/open-telemetry/opentelemetry-specification/issues/2229.