[sdk-metrics] Metric Instruments are not available in MetricReaders added after creation

[hectorhdzg]

What happened?

Steps to Reproduce

Create some Metric instruments in a specific Meter, then create another Metric reader exporting the data somewhere else.

Expected Result

All Metric instruments are available in all Metric readers

Actual Result

Only Metrics instruments created while the readers were present are available

Additional Details

This is a simplistic code to demonstrate the issue, the actual problem we are having is that the Metric Instruments are created from OTel Instrumentation libraries in Azure distro package, and we will like to export those using a different exporter.

OpenTelemetry Setup Code

import { OTLPMetricExporter } from "@opentelemetry/exporter-metrics-otlp-http";
import {
  ConsoleMetricExporter,
  MeterProvider,
  PeriodicExportingMetricReader,
} from "@opentelemetry/sdk-metrics";

const meterProvider = new MeterProvider();
// Add OTLP Exporter
const otlpMetricsExporter = new OTLPMetricExporter();
const otlpMetricReader = new PeriodicExportingMetricReader({
  exporter: otlpMetricsExporter,
});
meterProvider.addMetricReader(otlpMetricReader);
// Create metric
const testCounter1 = meterProvider.getMeter("testMeter").createCounter("testCounter1");

// Add console Exporter
const metricReader = new PeriodicExportingMetricReader({
  exporter: new ConsoleMetricExporter()
});
meterProvider.addMetricReader(metricReader);
// Create another metric
const testCounter2 = meterProvider.getMeter("testMeter").createCounter("testCounter2");

setInterval(() => {
  // Increase counters
  testCounter1.add(1);
  testCounter2.add(2);
}, 10000);

package.json

No response

Relevant log output

No response

[hectorhdzg]

Forgot to add the output, only testCounter2 is available in console

{ descriptor: { name: 'testCounter2', type: 'COUNTER', description: '', unit: '', valueType: 1 }, dataPointType: 3, dataPoints: [ { attributes: {}, startTime: [Array], endTime: [Array], value: 10 } ] }

[pichlermarc]

Hmm, I think our metrics SDK is not built in a way to handle this case. IIRC none of the other language implementations support registering readers after creating a MeterProvider.

@hectorhdzg In your specific case, would it be possible to move the second call to addMetricReader() to before the instruments are created? :thinking:

I think offering this kind of after-the-fact registration was not optimal and something we should consider dropping in SDK 2.0. Maybe we should only allow to add readers via MeterProvider constructor options. (We did move view registration to the constructor for the very same reason #3066)

[arendjr]

I just ran into this same issue today and it creates for some really unintuitive and hard-to-debug behavior in the Otel library. Do note in our case we do create the MeterProvider first, it's merely the registration of the MetricReader that's postponed. For us, we cannot ensure the call to addMetricReader() happens before the creation of counters, because the meter provider and the readers live in separate packages. Even if we could, frankly I don't think we want to, since we want to support dynamic situations where a client may perform an async call before the reader is registered, since we're creating a reusable library and we cannot enforce the order in which consumers call certain functions.

For an example of the type of trickery people run into when trying to enforce calling order, see this issue filed against our project: https://github.com/autometrics-dev/autometrics-ts/issues/113 In short: code in decorators always gets invoked before any other code in a module, and since our decorators register metrics, things get called sooner than one may expect. Similar issues might occur when someone imports code from another module which happens to try to create metrics. Altogether, I don't think it's reasonable to expect the metric reader to always be the first thing to be instantiated in a project.

[david-luna]

@pichlermarc you pointed out that https://github.com/open-telemetry/opentelemetry-js/pull/4419 fixes this issue. I've tried with the latest version and indeed it works.

IIUC the deprecation of addMetricReader and future removal of it in next major will compel users to define metric readers upfront (in MeterProvider constructor) which will make the approach commented in last comment not viable.

I'm not opposing to that change but I guess we should highlight this and provide some guidance so users (like autometrics-dev) can adapt their codebase.

[pichlermarc]

@pichlermarc you pointed out that #4419 fixes this issue. I've tried with the latest version and indeed it works.

IIUC the deprecation of addMetricReader and future removal of it in next major will compel users to define metric readers upfront (in MeterProvider constructor) which will make the approach commented in last comment not viable.

I'm not opposing to that change but I guess we should highlight this and provide some guidance so users (like autometrics-dev) can adapt their codebase.

@david-luna IIUC this issue would really be fixed by implementing a delegating no-op for metrics as defined in #3622. :thinking:

As all data-points written to the instruments would be dropped anyway in absence of a MetricReader. By implementing a feature that allows for registering a MeterProvider + all MetricReaders at a later time (as is currently the case with TracerProviders), we would eliminate the need of a MetricReader that's registered in a delayed manner. WDYT? :thinking:

This would also mean that this issue here is a duplicate of #3622