refactor(otlp-exporter-base): promisify sendWithHttp

experimental/CHANGELOG.md

@@ -24,6 +24,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2
 
 ### :house: Internal
 
+* refactor(otlp-exporter-base): promisify sendWithHttp() [#????](https://github.com/open-telemetry/opentelemetry-js/pull/6412) @pichlermarc
+
 ## 0.212.0
 
 ### :boom: Breaking Changes

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

@@ -29,21 +29,16 @@ class HttpExporterTransport implements IExporterTransport {
     const { agent, request } = await this._loadUtils();
     const headers = await this._parameters.headers();
 
-    return new Promise<ExportResponse>(resolve => {
-      sendWithHttp(
-        request,
-        this._parameters.url,
-        headers,
-        this._parameters.compression,
-        this._parameters.userAgent,
-        agent,
-        data,
-        result => {
-          resolve(result);
-        },
-        timeoutMillis
-      );
-    });
+    return sendWithHttp(
+      request,
+      this._parameters.url,
+      headers,
+      this._parameters.compression,
+      this._parameters.userAgent,
+      agent,
+      data,
+      timeoutMillis
+    );
   }
 
   shutdown() {

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

@@ -25,7 +25,6 @@ const DEFAULT_USER_AGENT = `OTel-OTLP-Exporter-JavaScript/${VERSION}`;
  * @param userAgent
  * @param agent
  * @param data
- * @param onDone
  * @param timeoutMillis
  */
 export function sendWithHttp(
@@ -36,105 +35,111 @@ export function sendWithHttp(
   userAgent: string | undefined,
   agent: http.Agent | https.Agent,
   data: Uint8Array,
-  onDone: (response: ExportResponse) => void,
   timeoutMillis: number
-): void {
-  const parsedUrl = new URL(url);
+): Promise<ExportResponse> {
+  return new Promise<ExportResponse>(resolve => {
+    const parsedUrl = new URL(url);
 
-  if (userAgent) {
-    headers['User-Agent'] = `${userAgent} ${DEFAULT_USER_AGENT}`;
-  } else {
-    headers['User-Agent'] = DEFAULT_USER_AGENT;
-  }
+    if (userAgent) {
+      headers['User-Agent'] = `${userAgent} ${DEFAULT_USER_AGENT}`;
+    } else {
+      headers['User-Agent'] = DEFAULT_USER_AGENT;
+    }
 
-  const options: http.RequestOptions | https.RequestOptions = {
-    hostname: parsedUrl.hostname,
-    port: parsedUrl.port,
-    path: parsedUrl.pathname,
-    method: 'POST',
-    headers,
-    agent,
-  };
-
-  const req = request(options, (res: http.IncomingMessage) => {
-    const responseData: Buffer[] = [];
-    res.on('data', chunk => responseData.push(chunk));
-
-    res.on('end', () => {
-      if (res.statusCode && res.statusCode <= 299) {
-        onDone({
-          status: 'success',
-          data: Buffer.concat(responseData),
-        });
-      } else if (res.statusCode && isExportHTTPErrorRetryable(res.statusCode)) {
-        onDone({
-          status: 'retryable',
-          retryInMillis: parseRetryAfterToMills(res.headers['retry-after']),
-        });
-      } else {
-        const error = new OTLPExporterError(
-          res.statusMessage,
-          res.statusCode,
-          Buffer.concat(responseData).toString()
-        );
-        onDone({
-          status: 'failure',
-          error,
-        });
-      }
+    const options: http.RequestOptions | https.RequestOptions = {
+      hostname: parsedUrl.hostname,
+      port: parsedUrl.port,
+      path: parsedUrl.pathname,
+      method: 'POST',
+      headers,
+      agent,
+    };
+
+    const req = request(options, (res: http.IncomingMessage) => {
+      const responseData: Buffer[] = [];
+      res.on('data', chunk => responseData.push(chunk));
+
+      res.on('end', () => {
+        if (res.statusCode && res.statusCode <= 299) {
+          resolve({
+            status: 'success',
+            data: Buffer.concat(responseData),
+          });
+        } else if (
+          res.statusCode &&
+          isExportHTTPErrorRetryable(res.statusCode)
+        ) {
+          resolve({
+            status: 'retryable',
+            retryInMillis: parseRetryAfterToMills(res.headers['retry-after']),
+          });
+        } else {
+          const error = new OTLPExporterError(
+            res.statusMessage,
+            res.statusCode,
+            Buffer.concat(responseData).toString()
+          );
+          resolve({
+            status: 'failure',
+            error,
+          });
+        }
+      });
+
+      res.on('error', (error: Error) => {
+        // Note: 'end' may still be emitted after 'error' on the same response object, since we're resolving a promise,
+        // the first call to resolve() will determine the final state.
+        if (res.statusCode && res.statusCode <= 299) {
+          // If the response is successful but an error occurs while reading the response,
+          // we consider it a success since the data has been sent successfully.
+          resolve({
+            status: 'success',
+          });
+        } else if (
+          res.statusCode &&
+          isExportHTTPErrorRetryable(res.statusCode)
+        ) {
+          resolve({
+            status: 'retryable',
+            error: error,
+            retryInMillis: parseRetryAfterToMills(res.headers['retry-after']),
+          });
+        } else {
+          resolve({
+            status: 'failure',
+            error,
+          });
+        }
+      });
     });
 
-    res.on('error', (error: Error) => {
-      // Note: 'end' may still be emitted after 'error' on the same response object.
-      // However, since onDone maps to a Promise resolve/reject, only the first call takes effect.
-      // This will be addressed in https://github.com/open-telemetry/opentelemetry-js/issues/5990
-      if (res.statusCode && res.statusCode <= 299) {
-        // If the response is successful but an error occurs while reading the response,
-        // we consider it a success since the data has been sent successfully.
-        onDone({
-          status: 'success',
-        });
-      } else if (res.statusCode && isExportHTTPErrorRetryable(res.statusCode)) {
-        onDone({
+    req.setTimeout(timeoutMillis, () => {
+      req.destroy();
+      resolve({
+        status: 'retryable',
+        error: new Error('Request timed out'),
+      });
+    });
+
+    req.on('error', (error: Error) => {
+      if (isHttpTransportNetworkErrorRetryable(error)) {
+        resolve({
           status: 'retryable',
-          error: error,
-          retryInMillis: parseRetryAfterToMills(res.headers['retry-after']),
+          error,
         });
       } else {
-        onDone({
+        resolve({
           status: 'failure',
           error,
         });
       }
     });
-  });
-
-  req.setTimeout(timeoutMillis, () => {
-    req.destroy();
-    onDone({
-      status: 'retryable',
-      error: new Error('Request timed out'),
-    });
-  });
 
-  req.on('error', (error: Error) => {
-    if (isHttpTransportNetworkErrorRetryable(error)) {
-      onDone({
-        status: 'retryable',
-        error,
-      });
-    } else {
-      onDone({
+    compressAndSend(req, compression, data, (error: Error) => {
+      resolve({
         status: 'failure',
         error,
       });
-    }
-  });
-
-  compressAndSend(req, compression, data, (error: Error) => {
-    onDone({
-      status: 'failure',
-      error,
     });
   });
 }

experimental/packages/otlp-exporter-base/test/node/http-transport-utils.test.ts

@@ -39,57 +39,37 @@ describe('sendWithHttp', function () {
     sentUserAgent = '';
   });
 
-  it('sends a request setting the default user-agent header', function (done) {
-    let firstCallback = true;
-    sendWithHttp(
+  it('sends a request setting the default user-agent header', async function () {
+    await sendWithHttp(
       requestFn,
       'http://localhost:8080',
       {},
       'gzip',
       undefined,
       new http.Agent(),
       Buffer.from([1, 2, 3]),
-      // TODO: the `onDone` callback is called twice because there are two error handlers
-      // - first is attached on the request created in `sendWithHttp`
-      // - second is attached on the pipe within `compressAndSend`
-      () => {
-        if (firstCallback) {
-          firstCallback = false;
-          assert.strictEqual(
-            sentUserAgent,
-            `OTel-OTLP-Exporter-JavaScript/${VERSION}`
-          );
-          done();
-        }
-      },
       100
     );
+    assert.strictEqual(
+      sentUserAgent,
+      `OTel-OTLP-Exporter-JavaScript/${VERSION}`
+    );
   });
 
-  it('sends a request prepending the provided user-agent to the default one', function (done) {
-    let firstCallback = true;
-    sendWithHttp(
+  it('sends a request prepending the provided user-agent to the default one', async function () {
+    await sendWithHttp(
       requestFn,
       'http://localhost:8080',
       {},
       'gzip',
       'Transport-User-Agent/1.2.3',
       new http.Agent(),
       Buffer.from([1, 2, 3]),
-      // TODO: the `onDone` callback is called twice because there are two error handlers
-      // - first is attached on the request created in `sendWithHttp`
-      // - second is attached on the pipe within `compressAndSend`
-      () => {
-        if (firstCallback) {
-          firstCallback = false;
-          assert.strictEqual(
-            sentUserAgent,
-            `Transport-User-Agent/1.2.3 OTel-OTLP-Exporter-JavaScript/${VERSION}`
-          );
-          done();
-        }
-      },
       100
     );
+    assert.strictEqual(
+      sentUserAgent,
+      `Transport-User-Agent/1.2.3 OTel-OTLP-Exporter-JavaScript/${VERSION}`
+    );
   });
 });