Merge branch 'main' into ot-shim

open-telemetry · Oct 23, 2024 · 2556bc8 · 2556bc8
2 parents 7695fbd + 006fe44
commit 2556bc8
Show file tree

Hide file tree

Showing 42 changed files with 367 additions and 2,026 deletions.
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -28,7 +28,6 @@ jobs:
         run: |
           npm run lint
           npm run lint:examples
-          npm run lint:markdown
 
       - name: Lint doc files
         run: |

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,10 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
 * fix(sdk-trace-base): avoid keeping non-string `status.message` on `Span#setStatus()` [#4999](https://github.com/open-telemetry/opentelemetry-js/pull/4999) @pichlermarc
 * fix(sdk-metrics): Add missing catch and handle error in promise of `PeriodicExportingMetricReader` [#5006](https://github.com/open-telemetry/opentelemetry-js/pull/5006) @jj22ee
 * fix(opentelemetry-core): confusing log extract of composite propagator [#5017](https://github.com/open-telemetry/opentelemetry-js/pull/5017) @rv2673
+* fix(propagator-aws-xray-*): move propagators back to contrib repository [#4966](https://github.com/open-telemetry/opentelemetry-js/pull/4966) @pichlermarc
+  * The [specification](https://github.com/open-telemetry/opentelemetry-specification/blob/6672dbc97ddeb34f36c020a0f0a30323c8bc4d95/specification/context/api-propagators.md?plain=1#L354-L356) prohibits hosting these packages in the core repository
+  * `@opentelemetry/propagator-aws-xray` is now located in [open-telemetry/opentelemetry-js-contrib](https://github.com/open-telemetry/opentelemetry-js-contrib)
+  * `@opentelemetry/propagator-aws-xray-lambda` is now located in [open-telemetry/opentelemetry-js-contrib](https://github.com/open-telemetry/opentelemetry-js-contrib)
 
 ### :books: (Refine Doc)
 

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -257,13 +257,21 @@ cd packages/opentelemetry-module-name
 npm run lint:fix
 ```
 
-Similarly, Markdown files (such as README.md files) can be linted:
+The default lint command will check majority of files, including Markdown files (such as README.md files), but you
+also have the option to check just the Markdown files with:
 
 ```sh
 npm run lint:markdown
 npm run lint:markdown:fix # can automatically fix some Markdown rules
 ```
 
+The default command doesn't check the examples folder. To lint just the examples, use the script:
+
+```sh
+npm run lint:examples
+npm run lint:examples:fix # can automatically fix some errors
+```
+
 ### Generating docs
 
 We use [typedoc](https://www.npmjs.com/package/typedoc) to generate the api documentation.

diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -49,6 +49,7 @@ All notable changes to experimental packages in this project will be documented
     * `OTEL_EXPORTER_OTLP_LOGS_INSECURE`
 * fix(sdk-node): use warn instead of error on unknown OTEL_NODE_RESOURCE_DETECTORS values [#5034](https://github.com/open-telemetry/opentelemetry-js/pull/5034)
 * fix(exporter-logs-otlp-proto): Use correct config type in Node constructor
+* fix(instrumentation-http): Fix instrumentation of `http.get`, `http.request`, `https.get`, and `https.request` when used from ESM code and imported via the `import defaultExport from 'http'` style. [#5024](https://github.com/open-telemetry/opentelemetry-js/issues/5024) @trentm
 
 ### :books: (Refine Doc)
 

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -65,7 +65,6 @@ import { errorMonitor } from 'events';
 import {
   ATTR_HTTP_REQUEST_METHOD,
   ATTR_HTTP_RESPONSE_STATUS_CODE,
-  ATTR_HTTP_ROUTE,
   ATTR_NETWORK_PROTOCOL_VERSION,
   ATTR_SERVER_ADDRESS,
   ATTR_SERVER_PORT,
@@ -80,6 +79,7 @@ import {
   getIncomingRequestAttributesOnResponse,
   getIncomingRequestMetricAttributes,
   getIncomingRequestMetricAttributesOnResponse,
+  getIncomingStableRequestMetricAttributesOnResponse,
   getOutgoingRequestAttributes,
   getOutgoingRequestAttributesOnResponse,
   getOutgoingRequestMetricAttributes,
@@ -93,7 +93,7 @@ import {
 } from './utils';
 
 /**
- * Http instrumentation instrumentation for Opentelemetry
+ * `node:http` and `node:https` instrumentation for OpenTelemetry
  */
 export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentationConfig> {
   /** keep track on spans not ended */
@@ -235,17 +235,24 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       'http',
       ['*'],
       (moduleExports: Http): Http => {
+        const isESM = (moduleExports as any)[Symbol.toStringTag] === 'Module';
         if (!this.getConfig().disableOutgoingRequestInstrumentation) {
           const patchedRequest = this._wrap(
             moduleExports,
             'request',
             this._getPatchOutgoingRequestFunction('http')
           ) as unknown as Func<http.ClientRequest>;
-          this._wrap(
+          const patchedGet = this._wrap(
             moduleExports,
             'get',
             this._getPatchOutgoingGetFunction(patchedRequest)
           );
+          if (isESM) {
+            // To handle `import http from 'http'`, which returns the default
+            // export, we need to set `module.default.*`.
+            (moduleExports as any).default.request = patchedRequest;
+            (moduleExports as any).default.get = patchedGet;
+          }
         }
         if (!this.getConfig().disableIncomingRequestInstrumentation) {
           this._wrap(
@@ -275,17 +282,24 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       'https',
       ['*'],
       (moduleExports: Https): Https => {
+        const isESM = (moduleExports as any)[Symbol.toStringTag] === 'Module';
         if (!this.getConfig().disableOutgoingRequestInstrumentation) {
           const patchedRequest = this._wrap(
             moduleExports,
             'request',
             this._getPatchHttpsOutgoingRequestFunction('https')
           ) as unknown as Func<http.ClientRequest>;
-          this._wrap(
+          const patchedGet = this._wrap(
             moduleExports,
             'get',
             this._getPatchHttpsOutgoingGetFunction(patchedRequest)
           );
+          if (isESM) {
+            // To handle `import https from 'https'`, which returns the default
+            // export, we need to set `module.default.*`.
+            (moduleExports as any).default.request = patchedRequest;
+            (moduleExports as any).default.get = patchedGet;
+          }
         }
         if (!this.getConfig().disableIncomingRequestInstrumentation) {
           this._wrap(
@@ -416,7 +430,8 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
    * @param request The original request object.
    * @param span representing the current operation
    * @param startTime representing the start time of the request to calculate duration in Metric
-   * @param oldMetricAttributes metric attributes
+   * @param oldMetricAttributes metric attributes for old semantic conventions
+   * @param stableMetricAttributes metric attributes for new semantic conventions
    */
   private _traceClientRequest(
     request: http.ClientRequest,
@@ -652,12 +667,6 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
         [ATTR_URL_SCHEME]: spanAttributes[ATTR_URL_SCHEME],
       };
 
-      // required if and only if one was sent, same as span requirement
-      if (spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE]) {
-        stableMetricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] =
-          spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
-      }
-
       // recommended if and only if one was sent, same as span recommendation
       if (spanAttributes[ATTR_NETWORK_PROTOCOL_VERSION]) {
         stableMetricAttributes[ATTR_NETWORK_PROTOCOL_VERSION] =
@@ -917,6 +926,10 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
       oldMetricAttributes,
       getIncomingRequestMetricAttributesOnResponse(attributes)
     );
+    stableMetricAttributes = Object.assign(
+      stableMetricAttributes,
+      getIncomingStableRequestMetricAttributesOnResponse(attributes)
+    );
 
     this._headerCapture.server.captureResponseHeaders(span, header =>
       response.getHeader(header)
@@ -929,7 +942,6 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
     const route = attributes[SEMATTRS_HTTP_ROUTE];
     if (route) {
       span.updateName(`${request.method || 'GET'} ${route}`);
-      stableMetricAttributes[ATTR_HTTP_ROUTE] = route;
     }
 
     if (this.getConfig().applyCustomAttributesOnSpan) {

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts b/experimental/packages/opentelemetry-instrumentation-http/src/utils.ts
@@ -26,6 +26,7 @@ import {
   ATTR_HTTP_REQUEST_METHOD,
   ATTR_HTTP_REQUEST_METHOD_ORIGINAL,
   ATTR_HTTP_RESPONSE_STATUS_CODE,
+  ATTR_HTTP_ROUTE,
   ATTR_NETWORK_PEER_ADDRESS,
   ATTR_NETWORK_PEER_PORT,
   ATTR_NETWORK_PROTOCOL_VERSION,
@@ -822,7 +823,7 @@ export const getIncomingRequestAttributesOnResponse = (
   const { socket } = request;
   const { statusCode, statusMessage } = response;
 
-  const newAttributes = {
+  const newAttributes: Attributes = {
     [ATTR_HTTP_RESPONSE_STATUS_CODE]: statusCode,
   };
 
@@ -842,6 +843,7 @@ export const getIncomingRequestAttributesOnResponse = (
 
   if (rpcMetadata?.type === RPCType.HTTP && rpcMetadata.route !== undefined) {
     oldAttributes[SEMATTRS_HTTP_ROUTE] = rpcMetadata.route;
+    newAttributes[ATTR_HTTP_ROUTE] = rpcMetadata.route;
   }
 
   switch (semconvStability) {
@@ -872,6 +874,22 @@ export const getIncomingRequestMetricAttributesOnResponse = (
   return metricAttributes;
 };
 
+export const getIncomingStableRequestMetricAttributesOnResponse = (
+  spanAttributes: Attributes
+): Attributes => {
+  const metricAttributes: Attributes = {};
+  if (spanAttributes[ATTR_HTTP_ROUTE] !== undefined) {
+    metricAttributes[ATTR_HTTP_ROUTE] = spanAttributes[SEMATTRS_HTTP_ROUTE];
+  }
+
+  // required if and only if one was sent, same as span requirement
+  if (spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE]) {
+    metricAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE] =
+      spanAttributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
+  }
+  return metricAttributes;
+};
+
 export function headerCapture(type: 'request' | 'response', headers: string[]) {
   const normalizedHeaders = new Map<string, string>();
   for (let i = 0, len = headers.length; i < len; i++) {

diff --git a/...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts b/...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
@@ -33,6 +33,7 @@ import {
   ATTR_CLIENT_ADDRESS,
   ATTR_HTTP_REQUEST_METHOD,
   ATTR_HTTP_RESPONSE_STATUS_CODE,
+  ATTR_HTTP_ROUTE,
   ATTR_NETWORK_PEER_ADDRESS,
   ATTR_NETWORK_PEER_PORT,
   ATTR_NETWORK_PROTOCOL_VERSION,
@@ -1134,6 +1135,32 @@ describe('HttpInstrumentation', () => {
           [ATTR_URL_SCHEME]: protocol,
         });
       });
+
+      it('should generate semconv 1.27 server spans with route when RPC metadata is available', async () => {
+        const response = await httpRequest.get(
+          `${protocol}://${hostname}:${serverPort}${pathname}/setroute`
+        );
+        const spans = memoryExporter.getFinishedSpans();
+        const [incomingSpan, _] = spans;
+        assert.strictEqual(spans.length, 2);
+
+        const body = JSON.parse(response.data);
+
+        // should have only required and recommended attributes for semconv 1.27
+        assert.deepStrictEqual(incomingSpan.attributes, {
+          [ATTR_CLIENT_ADDRESS]: body.address,
+          [ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET,
+          [ATTR_SERVER_ADDRESS]: hostname,
+          [ATTR_HTTP_ROUTE]: 'TheRoute',
+          [ATTR_SERVER_PORT]: serverPort,
+          [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
+          [ATTR_NETWORK_PEER_ADDRESS]: body.address,
+          [ATTR_NETWORK_PEER_PORT]: response.clientRemotePort,
+          [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
+          [ATTR_URL_PATH]: `${pathname}/setroute`,
+          [ATTR_URL_SCHEME]: protocol,
+        });
+      });
     });
 
     describe('with semconv stability set to http/dup', () => {
@@ -1146,6 +1173,13 @@ describe('HttpInstrumentation', () => {
         instrumentation['_semconvStability'] = SemconvStability.DUPLICATE;
         instrumentation.enable();
         server = http.createServer((request, response) => {
+          if (request.url?.includes('/setroute')) {
+            const rpcData = getRPCMetadata(context.active());
+            assert.ok(rpcData != null);
+            assert.strictEqual(rpcData.type, RPCType.HTTP);
+            assert.strictEqual(rpcData.route, undefined);
+            rpcData.route = 'TheRoute';
+          }
           response.setHeader('Content-Type', 'application/json');
           response.end(
             JSON.stringify({ address: getRemoteClientAddress(request) })
@@ -1241,6 +1275,50 @@ describe('HttpInstrumentation', () => {
           [AttributeNames.HTTP_STATUS_TEXT]: 'OK',
         });
       });
+
+      it('should create server spans with semconv 1.27 and old 1.7 including http.route if RPC metadata is available', async () => {
+        const response = await httpRequest.get(
+          `${protocol}://${hostname}:${serverPort}${pathname}/setroute`
+        );
+        const spans = memoryExporter.getFinishedSpans();
+        assert.strictEqual(spans.length, 2);
+        const incomingSpan = spans[0];
+        const body = JSON.parse(response.data);
+
+        // should have only required and recommended attributes for semconv 1.27
+        assert.deepStrictEqual(incomingSpan.attributes, {
+          // 1.27 attributes
+          [ATTR_CLIENT_ADDRESS]: body.address,
+          [ATTR_HTTP_REQUEST_METHOD]: HTTP_REQUEST_METHOD_VALUE_GET,
+          [ATTR_SERVER_ADDRESS]: hostname,
+          [ATTR_SERVER_PORT]: serverPort,
+          [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
+          [ATTR_NETWORK_PEER_ADDRESS]: body.address,
+          [ATTR_NETWORK_PEER_PORT]: response.clientRemotePort,
+          [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
+          [ATTR_URL_PATH]: `${pathname}/setroute`,
+          [ATTR_URL_SCHEME]: protocol,
+          [ATTR_HTTP_ROUTE]: 'TheRoute',
+
+          // 1.7 attributes
+          [SEMATTRS_HTTP_FLAVOR]: '1.1',
+          [SEMATTRS_HTTP_HOST]: `${hostname}:${serverPort}`,
+          [SEMATTRS_HTTP_METHOD]: 'GET',
+          [SEMATTRS_HTTP_SCHEME]: protocol,
+          [SEMATTRS_HTTP_STATUS_CODE]: 200,
+          [SEMATTRS_HTTP_TARGET]: `${pathname}/setroute`,
+          [SEMATTRS_HTTP_URL]: `http://${hostname}:${serverPort}${pathname}/setroute`,
+          [SEMATTRS_NET_TRANSPORT]: 'ip_tcp',
+          [SEMATTRS_NET_HOST_IP]: body.address,
+          [SEMATTRS_NET_HOST_NAME]: hostname,
+          [SEMATTRS_NET_HOST_PORT]: serverPort,
+          [SEMATTRS_NET_PEER_IP]: body.address,
+          [SEMATTRS_NET_PEER_PORT]: response.clientRemotePort,
+
+          // unspecified old names
+          [AttributeNames.HTTP_STATUS_TEXT]: 'OK',
+        });
+      });
     });
 
     describe('with require parent span', () => {

diff --git a/...imental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts b/...imental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts
@@ -22,6 +22,7 @@ import {
 import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
 import {
   ATTR_HTTP_REQUEST_METHOD,
+  ATTR_HTTP_RESPONSE_STATUS_CODE,
   ATTR_HTTP_ROUTE,
   ATTR_NETWORK_PROTOCOL_VERSION,
   ATTR_SERVER_ADDRESS,
@@ -181,7 +182,7 @@ describe('metrics', () => {
     });
   });
 
-  describe('with no semconv stability set to stable', () => {
+  describe('with semconv stability set to stable', () => {
     before(() => {
       instrumentation['_semconvStability'] = SemconvStability.STABLE;
     });
@@ -217,7 +218,9 @@ describe('metrics', () => {
       assert.deepStrictEqual(metrics[0].dataPoints[0].attributes, {
         [ATTR_HTTP_REQUEST_METHOD]: 'GET',
         [ATTR_URL_SCHEME]: 'http',
+        [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
         [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
+        [ATTR_HTTP_ROUTE]: 'TheRoute',
       });
 
       assert.strictEqual(metrics[1].dataPointType, DataPointType.HISTOGRAM);
@@ -244,7 +247,7 @@ describe('metrics', () => {
     });
   });
 
-  describe('with no semconv stability set to duplicate', () => {
+  describe('with semconv stability set to duplicate', () => {
     before(() => {
       instrumentation['_semconvStability'] = SemconvStability.DUPLICATE;
     });
@@ -353,6 +356,7 @@ describe('metrics', () => {
       assert.deepStrictEqual(metrics[2].dataPoints[0].attributes, {
         [ATTR_HTTP_REQUEST_METHOD]: 'GET',
         [ATTR_URL_SCHEME]: 'http',
+        [ATTR_HTTP_RESPONSE_STATUS_CODE]: 200,
         [ATTR_NETWORK_PROTOCOL_VERSION]: '1.1',
         [ATTR_HTTP_ROUTE]: 'TheRoute',
       });