fix(otlp-exporter-base)!: split node and browser config types in two (#5917)

Author: pichlermarc

experimental/CHANGELOG.md

@@ -8,6 +8,13 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 
 ### :boom: Breaking Changes
 
+* fix(otlp-exporter-base)!: split node and browser config types in two [#5917](https://github.com/open-telemetry/opentelemetry-js/pull/5917) @pichlermarc
+  * Fixes a bug where Node.js modules would be incorrectly used in the instantiation of a web-targeted exporter
+  * Breaking changes:
+    * (user-facing) `createOtlpHttpExportDelegate(OtlpHttpConfiguration)` has been changed to take a different, but identical type `OtlpNodeHttpConfiguration` to differentiate it from the web-targeted exporters
+    * (user-facing) `convertLegacyHttpOptions(...)` now returns `OtlpNodeHttpConfiguration`, the returned object's contents remain identical.
+    * (user-facing) `agentFactory` has been dropped from `OtlpHttpConfiguration` as it is node-specific and is now part of `OtlpNodeHttpConfiguration` instead
+
 ### :rocket: Features
 
 ### :bug: Bug Fixes

experimental/packages/otlp-exporter-base/src/configuration/convert-legacy-node-http-options.ts

@@ -14,16 +14,16 @@
  * limitations under the License.
  */
 import { OTLPExporterNodeConfigBase } from './legacy-node-configuration';
-import {
-  getHttpConfigurationDefaults,
-  HttpAgentFactory,
-  httpAgentFactoryFromOptions,
-  mergeOtlpHttpConfigurationWithDefaults,
-  OtlpHttpConfiguration,
-} from './otlp-http-configuration';
-import { getHttpConfigurationFromEnvironment } from './otlp-http-env-configuration';
 import { diag } from '@opentelemetry/api';
 import { wrapStaticHeadersInFunction } from './shared-configuration';
+import {
+  getNodeHttpConfigurationDefaults,
+  HttpAgentFactory,
+  mergeOtlpNodeHttpConfigurationWithDefaults,
+  OtlpNodeHttpConfiguration,
+} from './otlp-node-http-configuration';
+import { httpAgentFactoryFromOptions } from '../index-node-http';
+import { getNodeHttpConfigurationFromEnvironment } from './otlp-node-http-env-configuration';
 
 function convertLegacyAgentOptions(
   config: OTLPExporterNodeConfigBase
@@ -56,13 +56,13 @@ export function convertLegacyHttpOptions(
   signalIdentifier: string,
   signalResourcePath: string,
   requiredHeaders: Record<string, string>
-): OtlpHttpConfiguration {
+): OtlpNodeHttpConfiguration {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   if ((config as any).metadata) {
     diag.warn('Metadata cannot be set when using http');
   }
 
-  return mergeOtlpHttpConfigurationWithDefaults(
+  return mergeOtlpNodeHttpConfigurationWithDefaults(
     {
       url: config.url,
       headers: wrapStaticHeadersInFunction(config.headers),
@@ -71,7 +71,10 @@ export function convertLegacyHttpOptions(
       compression: config.compression,
       agentFactory: convertLegacyAgentOptions(config),
     },
-    getHttpConfigurationFromEnvironment(signalIdentifier, signalResourcePath),
-    getHttpConfigurationDefaults(requiredHeaders, signalResourcePath)
+    getNodeHttpConfigurationFromEnvironment(
+      signalIdentifier,
+      signalResourcePath
+    ),
+    getNodeHttpConfigurationDefaults(requiredHeaders, signalResourcePath)
   );
 }

experimental/packages/otlp-exporter-base/src/configuration/legacy-node-configuration.ts

@@ -19,7 +19,7 @@ import type * as http from 'http';
 import type * as https from 'https';
 
 import type { OTLPExporterConfigBase } from './legacy-base-configuration';
-import type { HttpAgentFactory } from './otlp-http-configuration';
+import type { HttpAgentFactory } from './otlp-node-http-configuration';
 
 /**
  * Collector Exporter node base config

experimental/packages/otlp-exporter-base/src/configuration/otlp-http-configuration.ts

@@ -21,28 +21,9 @@ import {
 } from './shared-configuration';
 import { validateAndNormalizeHeaders } from '../util';
 
-// NOTE: do not change these imports to be actual imports, otherwise they WILL break `@opentelemetry/instrumentation-http`
-import type * as http from 'http';
-import type * as https from 'https';
-
-export type HttpAgentFactory = (
-  protocol: string
-) => http.Agent | https.Agent | Promise<http.Agent> | Promise<https.Agent>;
-
 export interface OtlpHttpConfiguration extends OtlpSharedConfiguration {
   url: string;
   headers: () => Record<string, string>;
-  /**
-   * Factory function for creating agents.
-   *
-   * @remarks
-   * Prefer using {@link httpAgentFactoryFromOptions} over manually writing a factory function wherever possible.
-   * If using a factory function (`HttpAgentFactory`), **do not import `http.Agent` or `https.Agent`
-   * statically at the top of the file**.
-   * Instead, use dynamic `import()` or `require()` to load the module. This ensures that the `http` or `https`
-   * module is not loaded before `@opentelemetry/instrumentation-http` can instrument it.
-   */
-  agentFactory: HttpAgentFactory;
 }
 
 function mergeHeaders(
@@ -86,16 +67,6 @@ function validateUserProvidedUrl(url: string | undefined): string | undefined {
   }
 }
 
-export function httpAgentFactoryFromOptions(
-  options: http.AgentOptions | https.AgentOptions
-): HttpAgentFactory {
-  return async protocol => {
-    const module = protocol === 'http:' ? import('http') : import('https');
-    const { Agent } = await module;
-    return new Agent(options);
-  };
-}
-
 /**
  * @param userProvidedConfiguration  Configuration options provided by the user in code.
  * @param fallbackConfiguration Fallback to use when the {@link userProvidedConfiguration} does not specify an option.
@@ -121,10 +92,6 @@ export function mergeOtlpHttpConfigurationWithDefaults(
       validateUserProvidedUrl(userProvidedConfiguration.url) ??
       fallbackConfiguration.url ??
       defaultConfiguration.url,
-    agentFactory:
-      userProvidedConfiguration.agentFactory ??
-      fallbackConfiguration.agentFactory ??
-      defaultConfiguration.agentFactory,
   };
 }
 
@@ -136,6 +103,5 @@ export function getHttpConfigurationDefaults(
     ...getSharedConfigurationDefaults(),
     headers: () => requiredHeaders,
     url: 'http://localhost:4318/' + signalResourcePath,
-    agentFactory: httpAgentFactoryFromOptions({ keepAlive: true }),
   };
 }

experimental/packages/otlp-exporter-base/src/configuration/otlp-node-http-configuration.ts

@@ -0,0 +1,85 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {
+  getHttpConfigurationDefaults,
+  mergeOtlpHttpConfigurationWithDefaults,
+  OtlpHttpConfiguration,
+} from './otlp-http-configuration';
+
+// NOTE: do not change these imports to be actual imports, otherwise they WILL break `@opentelemetry/instrumentation-http`
+import type * as http from 'http';
+import type * as https from 'https';
+
+export type HttpAgentFactory = (
+  protocol: string
+) => http.Agent | https.Agent | Promise<http.Agent> | Promise<https.Agent>;
+
+export interface OtlpNodeHttpConfiguration extends OtlpHttpConfiguration {
+  /**
+   * Factory function for creating agents.
+   *
+   * @remarks
+   * Prefer using {@link httpAgentFactoryFromOptions} over manually writing a factory function wherever possible.
+   * If using a factory function (`HttpAgentFactory`), **do not import `http.Agent` or `https.Agent`
+   * statically at the top of the file**.
+   * Instead, use dynamic `import()` or `require()` to load the module. This ensures that the `http` or `https`
+   * module is not loaded before `@opentelemetry/instrumentation-http` can instrument it.
+   */
+  agentFactory: HttpAgentFactory;
+}
+
+export function httpAgentFactoryFromOptions(
+  options: http.AgentOptions | https.AgentOptions
+): HttpAgentFactory {
+  return async protocol => {
+    const module = protocol === 'http:' ? import('http') : import('https');
+    const { Agent } = await module;
+    return new Agent(options);
+  };
+}
+
+/**
+ * @param userProvidedConfiguration  Configuration options provided by the user in code.
+ * @param fallbackConfiguration Fallback to use when the {@link userProvidedConfiguration} does not specify an option.
+ * @param defaultConfiguration The defaults as defined by the exporter specification
+ */
+export function mergeOtlpNodeHttpConfigurationWithDefaults(
+  userProvidedConfiguration: Partial<OtlpNodeHttpConfiguration>,
+  fallbackConfiguration: Partial<OtlpNodeHttpConfiguration>,
+  defaultConfiguration: OtlpNodeHttpConfiguration
+): OtlpNodeHttpConfiguration {
+  return {
+    ...mergeOtlpHttpConfigurationWithDefaults(
+      userProvidedConfiguration,
+      fallbackConfiguration,
+      defaultConfiguration
+    ),
+    agentFactory:
+      userProvidedConfiguration.agentFactory ??
+      fallbackConfiguration.agentFactory ??
+      defaultConfiguration.agentFactory,
+  };
+}
+
+export function getNodeHttpConfigurationDefaults(
+  requiredHeaders: Record<string, string>,
+  signalResourcePath: string
+): OtlpNodeHttpConfiguration {
+  return {
+    ...getHttpConfigurationDefaults(requiredHeaders, signalResourcePath),
+    agentFactory: httpAgentFactoryFromOptions({ keepAlive: true }),
+  };
+}

experimental/packages/otlp-exporter-base/src/configuration/otlp-http-env-configuration.ts renamed to experimental/packages/otlp-exporter-base/src/configuration/otlp-node-http-env-configuration.ts

@@ -16,8 +16,8 @@
 import { getStringFromEnv, parseKeyPairsIntoRecord } from '@opentelemetry/core';
 import { diag } from '@opentelemetry/api';
 import { getSharedConfigurationFromEnvironment } from './shared-env-configuration';
-import { OtlpHttpConfiguration } from './otlp-http-configuration';
 import { wrapStaticHeadersInFunction } from './shared-configuration';
+import { OtlpNodeHttpConfiguration } from './otlp-node-http-configuration';
 
 function getStaticHeadersFromEnv(
   signalIdentifier: string
@@ -123,10 +123,10 @@ function getSpecificUrlFromEnv(signalIdentifier: string): string | undefined {
  * @param signalIdentifier all caps part in environment variables that identifies the signal (e.g.: METRICS, TRACES, LOGS)
  * @param signalResourcePath signal resource path to append if necessary (e.g.: v1/metrics, v1/traces, v1/logs)
  */
-export function getHttpConfigurationFromEnvironment(
+export function getNodeHttpConfigurationFromEnvironment(
   signalIdentifier: string,
   signalResourcePath: string
-): Partial<OtlpHttpConfiguration> {
+): Partial<OtlpNodeHttpConfiguration> {
   return {
     ...getSharedConfigurationFromEnvironment(signalIdentifier),
     url:

experimental/packages/otlp-exporter-base/src/index-node-http.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-export { httpAgentFactoryFromOptions } from './configuration/otlp-http-configuration';
+export { httpAgentFactoryFromOptions } from './configuration/otlp-node-http-configuration';
 export { createOtlpHttpExportDelegate } from './otlp-http-export-delegate';
 export { getSharedConfigurationFromEnvironment } from './configuration/shared-env-configuration';
 export { convertLegacyHttpOptions } from './configuration/convert-legacy-node-http-options';

experimental/packages/otlp-exporter-base/src/otlp-http-export-delegate.ts

@@ -17,14 +17,14 @@ import {
   createOtlpExportDelegate,
   IOtlpExportDelegate,
 } from './otlp-export-delegate';
-import { OtlpHttpConfiguration } from './configuration/otlp-http-configuration';
 import { ISerializer } from '@opentelemetry/otlp-transformer';
 import { createHttpExporterTransport } from './transport/http-exporter-transport';
 import { createBoundedQueueExportPromiseHandler } from './bounded-queue-export-promise-handler';
 import { createRetryingTransport } from './retrying-transport';
+import { OtlpNodeHttpConfiguration } from './configuration/otlp-node-http-configuration';
 
 export function createOtlpHttpExportDelegate<Internal, Response>(
-  options: OtlpHttpConfiguration,
+  options: OtlpNodeHttpConfiguration,
   serializer: ISerializer<Internal, Response>
 ): IOtlpExportDelegate<Internal> {
   return createOtlpExportDelegate(

experimental/packages/otlp-exporter-base/src/transport/http-exporter-transport.ts

@@ -14,15 +14,14 @@
  * limitations under the License.
  */
 
-import type { HttpRequestParameters } from './http-transport-types';
-
 // NOTE: do not change these type imports to actual imports. Doing so WILL break `@opentelemetry/instrumentation-http`,
 // as they'd be imported before the http/https modules can be wrapped.
 import type * as https from 'https';
 import type * as http from 'http';
 import { ExportResponse } from '../export-response';
 import { IExporterTransport } from '../exporter-transport';
 import { sendWithHttp } from './http-transport-utils';
+import { NodeHttpRequestParameters } from './node-http-transport-types';
 
 interface Utils {
   agent: http.Agent | https.Agent;
@@ -32,7 +31,7 @@ interface Utils {
 class HttpExporterTransport implements IExporterTransport {
   private _utils: Utils | null = null;
 
-  constructor(private _parameters: HttpRequestParameters) {}
+  constructor(private _parameters: NodeHttpRequestParameters) {}
 
   async send(data: Uint8Array, timeoutMillis: number): Promise<ExportResponse> {
     const { agent, request } = await this._loadUtils();
@@ -80,7 +79,7 @@ async function requestFunctionFactory(
 }
 
 export function createHttpExporterTransport(
-  parameters: HttpRequestParameters
+  parameters: NodeHttpRequestParameters
 ): IExporterTransport {
   return new HttpExporterTransport(parameters);
 }

experimental/packages/otlp-exporter-base/src/transport/http-transport-types.ts

@@ -14,11 +14,8 @@
  * limitations under the License.
  */
 
-import { HttpAgentFactory } from '../configuration/otlp-http-configuration';
-
 export interface HttpRequestParameters {
   url: string;
   headers: () => Record<string, string>;
   compression: 'gzip' | 'none';
-  agentFactory: HttpAgentFactory;
 }