[sdk-node] automatically configure metrics exporter based on enviornment variables

[pichlermarc]

Currently, we don't auto-configure a MetricReader/exporter combination when using the @opentelemetry/sdk-node package. Goal of this issue is to implement exporter selection for metrics based on this specification:

• https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#exporter-selection

For this issue to be considered done we need to implement the following behavior:

If no metric reader is configured by the user:

• [ ] use OTEL_METRICS_EXPORTER environment variable to determine an exporter and add it to the MeterProvider that's created by NodeSDK
  • pair it up with a PeriodicExportingMetricReader if it is a PushMetricExporter
  • see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#exporter-selection
  • for now, do NOT set up a MeterProvider if OTEL_METRICS_EXPORTER is unset, any current code-only configuration should continue to work as it does now. If a metricReader is already provided by the user, do NOT override or add an additional reader based on the contents of OTEL_METRICS_EXPORTER
• [ ] use the OTEL_EXPORTER_OTLP_METRICS_PROTOCOL to determine the OTLP exporter to use (http/json, http/protobuf, grpc)
  • see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specify-protocol
• [ ] use the OTEL_EXPORTER_OTLP_PROTOCOL env var as a fallback to the above to determine the OTLP exporter to use (http/json, http/protobuf, grpc)
• [ ] fallback to using http/protobuf if none of the two protocol env vars are set

[Prashansa-K]

@pichlermarc I would like to work on this. I have used this package in the past.

[pichlermarc]

@Prashansa-K thanks for picking this up. It's yours :slightly_smiling_face:

[Prashansa-K]

@pichlermarc I have some doubts here.

1. In first action item from the description mentions "use OTEL_METRICS_EXPORTER environment variable to determine an exporter and add it to the MeterProvider that's created by NodeSDK", However, as I see the code, addMetricReader() method is deprecated and would be removed in v2. It is suggests to use the MeterProvider constructor here. So, should we keep creating new MeterProvider objects here based on the exporters provided via env variable?

2. I am assuming that we can provide >1 metric exporters via env configurations, just like trace exporters. Is that correct? I couldn't find a clarification for this in the specification document.

[pichlermarc]

So, should we keep creating new MeterProvider objects here based on the exporters provided via env variable?

If the env var is set, only create a single MeterProvider and add all MetricReaders to that MeterProvider via the constructor option (it's an array). That means that creation of the exporters with auto-paired MetricReaders has to happen before instantiating a MeterProvider.

I am assuming that we can provide >1 metric exporters via env configurations, just like trace exporters. Is that correct? I couldn't find a clarification for this in the specification document.

That's correct. :slightly_smiling_face: The specification reads:

The implementation MAY accept a comma-separated list to enable setting multiple exporters.

so that's applicable to all exporter selection env vars.

[bhaskarbanerjee]

@pichlermarc is this behavior consistent across python,java, go etc?

[bhaskarbanerjee]

@pichlermarc would it be possible to have some answers on the above question? I am trying to understand if this by design for all nodejs layer.sdk or all other languages too?

[pichlermarc]

@pichlermarc is this behavior consistent across python,java, go etc? would it be possible to have some answers on the above question? I am trying to understand if this by design for all nodejs layer.sdk or all other languages too?

I'm not sure what the question is here - the feature is not implemented yet. As with all specification features, the behavior aims to be aligned across all languages.

[bhaskarbanerjee]

@pichlermarc what I mean to understand is whether python, go, java export metrics by deafult or do they not just like in nodejs. same goes for logs.

[bhaskarbanerjee]

@pichlermarc if you could please help me get answer on my above question? @Prashansa-K how far is the PR that you are working on?

[silpamittapalli]

Hi there @pichlermarc Following up on Bhaskar's comment above. Is the MetricReader/exporter combination not auto-configured when using @opentelemetry/sdk-node package only? or Is this behavior expected with other language SDKs too? Please clarify.

If this choice was made by design, we would love to understand the reasoning behind this choice.

To expedite progress, we would be happy to claim and/or contribute to this feature if needed. Please let us know @Prashansa-K

[pichlermarc]

Following up on Bhaskar's comment above. Is the MetricReader/exporter combination not auto-configured when using @opentelemetry/sdk-node package only? or Is this behavior expected with other language SDKs too? Please clarify.

I don't know the exact state of all the other language SDKs autoconfigure packages. I know that the specification does not prescribe a default. Posting research here would be very welcome :slightly_smiling_face:

If this choice was made by design, we would love to understand the reasoning behind this choice.

It was a conscious decision. As there was no way to set up the metrics exporter, automatically setting up the MeterProvider for it would've been more annoying than helpful until this feature here and #4655 are implemented.

I updated the issue description to reflect that. When implementing this feature do NOT set up a MeterProvider if the OTEL_METRICS_EXPORTER env var is not set. We can revisit that decision when #4655 is implemented.

[bhaskarbanerjee]

Thanks @pichlermarc for the clarification. As we dig more, shall share the findings here.

[bhaskarbanerjee]

@Prashansa-K please help us understand how far the PR is. We have needs in this area and are happy to pair up, if that can expedite your contribution.

[bhaskarbanerjee]

@pichlermarc Is there anything we can do to expedite this? We are also very willing to cut out a PR for this, if other contributors are busy. Please advice.

[pichlermarc]

@bhaskarbanerjee I don't have any way to reach out to @Prashansa-K, unfortunately.

Looks like there's no response to the above comment and the issue was assigned a little over a month ago. @bhaskarbanerjee I'd appreciate you opening a PR to implement this feature.

[bhaskarbanerjee]

Thanks for prompt response @pichlermarc we will get on top of this. cc @silpamittapalli

[haythemtrabelsi117]

hi, what's the status on this?

[bhaskarbanerjee]

@haythemtrabelsi117 we got delayed but are now actively working on this. Please expect a PR by end of this month.

[bhaskarbanerjee]

@pichlermarc @dyladan I need to make updates to opentelemetry-js/opentelemetry-core (add another supported env var named OTEL_METRICS_EXPORTER) before I can make other updates to the opentelemetry-js/opentelemetry-sdk-node. should I raise 2 separate PRs?

[bhaskarbanerjee]

@pichlermarc can you please assign this ticket to me?

[pichlermarc]

@bhaskarbanerjee please don't access the core module anymore to get the environment config, we're moving away from that. You can directly use process.env.OTEL_METRICS_EXPORTER and parse that input, then you can do it in the same PR. :slightly_smiling_face:

[bhaskarbanerjee]

Thanks for that info @pichlermarc , will follow that.

[bhaskarbanerjee]

@pichlermarc I am in process of drafting this PR. I see there is a function for configuring the log provider from env.

The simplest way to solve this would be to add a similar function for configuring the metric provider and then write tests on top of this.

Are you good with approach or do you have another suggestion?

private configureMetricProviderFromEnv(): void {
    const metricsExporterList = process.env.OTEL_METRICS_EXPORTER ?? '';
    const enabledExporters = filterBlanksAndNulls(metricsExporterList.split(','));
    if (enabledExporters.length === 0) {
      diag.info('OTEL_METRICS_EXPORTER is empty. Using default otlp exporter.');
      enabledExporters.push('otlp');
    }

    if (enabledExporters.includes('none')) {
      diag.info(
        `OTEL_METRICS_EXPORTER contains "none". Metric provider will not be initialized.`
      );
      return;
    }

    enabledExporters.forEach(exporter => {
      if (exporter === 'otlp') {
        const protocol = (
          process.env.OTEL_EXPORTER_OTLP_METRIC_PROTOCOL ??
          process.env.OTEL_EXPORTER_OTLP_PROTOCOL
        )?.trim();

        switch (protocol) {
          case 'grpc':
            exporters.push(new OTLPGrpcMetricExporter());
            break;
          case 'http/json':
            exporters.push(new OTLPHttpMetricExporter());
            break;
          case 'http/protobuf':
            exporters.push(new OTLPProtoMetricExporter());
            break;
          case undefined:
          case '':
            exporters.push(new OTLPProtoMetricExporter());
            break;
          default:
            diag.warn(
              `Unsupported OTLP metrics protocol: "${protocol}". Using http/protobuf.`
            );
            exporters.push(new OTLPProtoMetricExporter());
        }
      } else if (exporter === 'console') {
        exporters.push(new ConsoleMetricExporter());
      } else if (exporter === 'prometheus') {
        exporters.push(new PrometheusMetricExporter());
      } else {
        diag.warn(
          `Unsupported OTEL_METRICS_EXPORTER value: "${exporter}". Supported values are: otlp, console, prometheus, none.`
        );
      }
    });

    if (exporters.length > 0) {
      exporters.map(exporter => {
          if (exporter instanceof PrometheusMetricExporter) {
            // return a PullBasedReader
            return;
          } else {
            const exportIntervalMillis = process.env.OTEL_METRICS_EXPORTER_INTERVAL ?? 500
            return new PeriodicExportingMetricReader({
              exporter: exporter,
              exportIntervalMillis: exportIntervalMillis
            });
          }
        })
    }
  }

And then add the below lines to end of https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-sdk-node/src/sdk.ts#L212

  else {
      this._meterProviderConfig.reader = this.configureMetricProviderFromEnv()
    }

[bhaskarbanerjee]

@pichlermarc one more testing question. Apart from Unit testing, what other testing is encouraged and is there a designated process to do so?

[bhaskarbanerjee]

@pichlermarc any thoughts on the above?

[pichlermarc]

@bhaskarbanerjee yes that sounds good - some suggestion for the code above: you may want to immediately pair the exporter with a PeriodiceExportingMetricReader where applicable - this way you can hold MetricReader instances and you can drop the instanceof check for the prometheus exporter. Other than that this looks good.

@pichlermarc one more testing question. Apart from Unit testing, what other testing is encouraged and is there a designated process to do so?

There's only "unit"-tests in @opentelemetry/sdk-node - if you add tests for the functionality you're adding then that's sufficient. You can check the existing tests - I think similar ones should suffice. FYI: the quality of tests in @opentelemetry/sdk-node is not the best at the moment (they're asserting on lots of internal state of other packages, etc) - if you can come up with higher-quality ones instead of following what's already there, I'd appreciate it. :slightly_smiling_face:

[bhaskarbanerjee]

🆒 Thanks @pichlermarc My PR should be therein a few days. I will try my best with the unit tests. The current unit tests are failing on other packages. Is there a way to run only the tests for sdk-node package?

 --------------------|---------|----------|---------|---------|--------------------------------------
       File                | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                    
       --------------------|---------|----------|---------|---------|--------------------------------------
       All files           |   89.66 |    82.43 |   98.01 |   89.68 |                                      
        src                |    89.6 |    82.36 |      98 |   89.61 |                                      
         http.ts           |   90.42 |    87.38 |   97.26 |   91.07 | ...,722-734,775,833,839,964-966,1011 
         utils.ts          |   88.92 |    78.59 |     100 |   88.32 | ...0,636-650,670-672,681,750,924,932 
        src/enums          |     100 |      100 |     100 |     100 |                                      
         AttributeNames.ts |     100 |      100 |     100 |     100 |                                      
       --------------------|---------|----------|---------|---------|--------------------------------------
       npm ERR! Lifecycle script `test:cjs` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http 
       npm ERR! Lifecycle script `test` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-http 
       

    ✖  @opentelemetry/exporter-logs-otlp-proto:test
       > @opentelemetry/[email protected] test
       > nyc mocha 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'
       
       
        Exception during run: src/platform/node/OTLPLogExporter.ts(27,25): error TS2307: Cannot find module '../../version' or its corresponding type declarations.
       
       ----------|---------|----------|---------|---------|-------------------
       File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
       ----------|---------|----------|---------|---------|-------------------
       All files |       0 |        0 |       0 |       0 |                   
       ----------|---------|----------|---------|---------|-------------------
       npm ERR! Lifecycle script `test` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/exporter-logs-otlp-proto 
       

    ✖  @opentelemetry/exporter-logs-otlp-http:test
       > @opentelemetry/[email protected] test
       > nyc mocha 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'
       
       
        Exception during run: test/node/OTLPLogExporter.test.ts(33,25): error TS2307: Cannot find module '../../src/version' or its corresponding type declarations.
       
       ----------|---------|----------|---------|---------|-------------------
       File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
       ----------|---------|----------|---------|---------|-------------------
       All files |       0 |        0 |       0 |       0 |                   
       ----------|---------|----------|---------|---------|-------------------
       npm ERR! Lifecycle script `test` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/exporter-logs-otlp-http 
       
    ✔  @opentelemetry/instrumentation-grpc:test (15s)

    ✖  @opentelemetry/exporter-metrics-otlp-http:test
       > @opentelemetry/[email protected] test
       > nyc mocha 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'
       
       
        Exception during run: src/platform/node/OTLPMetricExporter.ts(28,25): error TS2307: Cannot find module '../../version' or its corresponding type declarations.
       
       ----------|---------|----------|---------|---------|-------------------
       File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
       ----------|---------|----------|---------|---------|-------------------
       All files |       0 |        0 |       0 |       0 |                   
       ----------|---------|----------|---------|---------|-------------------
       npm ERR! Lifecycle script `test` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/opentelemetry-exporter-metrics-otlp-http 
       

    ✖  @opentelemetry/exporter-trace-otlp-http:test
       > @opentelemetry/[email protected] test
       > nyc mocha 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'
       
       
        Exception during run: src/platform/node/OTLPTraceExporter.ts(21,25): error TS2307: Cannot find module '../../version' or its corresponding type declarations.
       
       ----------|---------|----------|---------|---------|-------------------
       File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
       ----------|---------|----------|---------|---------|-------------------
       All files |       0 |        0 |       0 |       0 |                   
       ----------|---------|----------|---------|---------|-------------------
       npm ERR! Lifecycle script `test` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/exporter-trace-otlp-http 
       

    ✖  @opentelemetry/exporter-trace-otlp-proto:test
       > @opentelemetry/[email protected] test
       > nyc mocha 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'
       
       
        Exception during run: test/node/OTLPTraceExporter.test.ts(39,25): error TS2307: Cannot find module '../../src/version' or its corresponding type declarations.
       
       ----------|---------|----------|---------|---------|-------------------
       File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
       ----------|---------|----------|---------|---------|-------------------
       All files |       0 |        0 |       0 |       0 |                   
       ----------|---------|----------|---------|---------|-------------------
       npm ERR! Lifecycle script `test` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/exporter-trace-otlp-proto 
       

    ✖  @opentelemetry/otlp-grpc-exporter-base:test
       > @opentelemetry/[email protected] test
       > nyc mocha 'test/**/*.test.ts'
       
       
        Exception during run: test/configuration/otlp-grpc-configuration.test.ts(29,25): error TS2307: Cannot find module '../../src/version' or its corresponding type declarations.
       
       ----------|---------|----------|---------|---------|-------------------
       File      | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s 
       ----------|---------|----------|---------|---------|-------------------
       All files |       0 |        0 |       0 |       0 |                   
       ----------|---------|----------|---------|---------|-------------------
       npm ERR! Lifecycle script `test` failed with error: 
       npm ERR! Error: command failed 
       npm ERR!   in workspace: @opentelemetry/[email protected] 
       npm ERR!   at location: /Users/hkn097/workspace-git/OpenSource/opentelemetry-js/experimental/packages/otlp-grpc-exporter-base 
       

 ———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 >  Lerna (powered by Nx)   Ran target test for 40 projects (1m)
 
    ✔    28/35 succeeded [0 read from cache]
 
    ✖    7/35 targets failed, including the following:
         - @opentelemetry/instrumentation-http:test
         - @opentelemetry/exporter-logs-otlp-proto:test
         - @opentelemetry/exporter-logs-otlp-http:test
         - @opentelemetry/exporter-metrics-otlp-http:test
         - @opentelemetry/exporter-trace-otlp-http:test
         ...and 2 more...
 
   Hint: Try "nx view-logs" to get structured, searchable errors logs in your browser.
 
sh-3.2$ npm -v
10.5.0
sh-3.2$ node -v
v20.12.0
sh-3.2$ npx lerna run test

[pichlermarc]

@bhaskarbanerjee please read https://github.com/open-telemetry/opentelemetry-js/blob/be1737fc46c1c0fbe75f4d30ce5a196ebeee4c09/CONTRIBUTING.md#development-quick-start and follow the steps there - the steps that are listed there are necessary to setup auto-generated files that are required to make tests pass.

After doing that once you can just run npm run test in ./experimental/opentelemetry-sdk-node/ and it'll just run the tests for that package.

[bhaskarbanerjee]

Thanks @pichlermarc I will check this out