Merge branch 'main' into metrics/collector-registry

open-telemetry · Jan 3, 2023 · c54ba3b · c54ba3b
2 parents fc98cc4 + 93222fc
commit c54ba3b
Show file tree

Hide file tree

Showing 14 changed files with 354 additions and 73 deletions.
diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md
@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
 * fix(api): deprecate MetricAttributes and MetricAttributeValue [#3406](https://github.com/open-telemetry/opentelemetry-js/pull/3406) @blumamir
 * test(api): disable module concatenation in tree-shaking test [#3409](https://github.com/open-telemetry/opentelemetry-js/pull/3409) @legendecas
 * fix(api): use active context as default in NoopTracer [#3476](https://github.com/open-telemetry/opentelemetry-js/pull/3476) @flarna
+* fix(api): declare this parameter type in observable callbacks [#3497](https://github.com/open-telemetry/opentelemetry-js/pull/3497) @legendecas
 
 ## [1.3.0](https://www.github.com/open-telemetry/opentelemetry-js-api/compare/v1.2.0...v1.3.0)
 

diff --git a/api/src/metrics/ObservableResult.ts b/api/src/metrics/ObservableResult.ts
@@ -30,7 +30,11 @@ export interface ObservableResult<
    * one values associated with the same attributes values, SDK may pick the
    * last one or simply drop the entire observable result.
    */
-  observe(value: number, attributes?: AttributesTypes): void;
+  observe(
+    this: ObservableResult<AttributesTypes>,
+    value: number,
+    attributes?: AttributesTypes
+  ): void;
 }
 
 /**
@@ -49,6 +53,7 @@ export interface BatchObservableResult<
    * last one or simply drop the entire observable result.
    */
   observe(
+    this: BatchObservableResult<AttributesTypes>,
     metric: Observable<AttributesTypes>,
     value: number,
     attributes?: AttributesTypes

diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md
@@ -11,6 +11,8 @@ All notable changes to experimental packages in this project will be documented
 * feat(instrumentation-http): monitor error events with events.errorMonitor [#3402](https://github.com/open-telemetry/opentelemetry-js/pull/3402) @legendecas
 * feat(instrumentation-grpc): added grpc metadata client side attributes in instrumentation [#3386](https://github.com/open-telemetry/opentelemetry-js/pull/3386)
 * feat(instrumentation): add new `_setMeterInstruments` protected method that update the meter instruments every meter provider update.
+* feat(api-logs): add the `SeverityNumber` enumeration. [#3443](https://github.com/open-telemetry/opentelemetry-js/pull/3443/) @fuaiyi
+* feat(sdk-node): configure no-op sdk with `OTEL_SDK_DISABLED` environment variable [#3485](https://github.com/open-telemetry/opentelemetry-js/pull/3485/files/2211c78aec39aeb6b4b3dae71844edf8ce234d20)  @RazGvili
 
 ### :bug: (Bug Fix)
 
@@ -20,6 +22,7 @@ All notable changes to experimental packages in this project will be documented
 * fix(instrumentation): add back support for absolute paths via `require-in-the-middle` [#3457](https://github.com/open-telemetry/opentelemetry-js/pull/3457) @mhassan1
 * fix(prometheus-sanitization): replace repeated `_` with a single `_` [3470](https://github.com/open-telemetry/opentelemetry-js/pull/3470) @samimusallam
 * fix(prometheus-serializer): correct string used for NaN [#3477](https://github.com/open-telemetry/opentelemetry-js/pull/3477) @JacksonWeber
+* fix(instrumentation-http): close server span when response finishes [#3407](https://github.com/open-telemetry/opentelemetry-js/pull/3407) @legendecas
 
 ### :books: (Refine Doc)
 

diff --git a/experimental/packages/api-logs/README.md b/experimental/packages/api-logs/README.md
@@ -42,7 +42,7 @@ const logger = api.logs.getLogger(name, version);
 logger.emitEvent({ name: 'event-name', domain: 'event-domain' });
 
 // logging an event in a log appender
-logger.emitLogRecord({ severityNumber: 1, body: 'log data' });
+logger.emitLogRecord({ severityNumber: SeverityNumber.TRACE, body: 'log data' });
 ```
 
 ## Useful links

diff --git a/experimental/packages/api-logs/src/types/LogRecord.ts b/experimental/packages/api-logs/src/types/LogRecord.ts
@@ -16,6 +16,34 @@
 
 import { Attributes } from '@opentelemetry/api';
 
+export enum SeverityNumber {
+  UNSPECIFIED = 0,
+  TRACE = 1,
+  TRACE2 = 2,
+  TRACE3 = 3,
+  TRACE4 = 4,
+  DEBUG = 5,
+  DEBUG2 = 6,
+  DEBUG3 = 7,
+  DEBUG4 = 8,
+  INFO = 9,
+  INFO2 = 10,
+  INFO3 = 11,
+  INFO4 = 12,
+  WARN = 13,
+  WARN2 = 14,
+  WARN3 = 15,
+  WARN4 = 16,
+  ERROR = 17,
+  ERROR2 = 18,
+  ERROR3 = 19,
+  ERROR4 = 20,
+  FATAL = 21,
+  FATAL2 = 22,
+  FATAL3 = 23,
+  FATAL4 = 24,
+}
+
 export interface LogRecord {
   /**
    * The time when the log record occurred as UNIX Epoch time in nanoseconds.
@@ -25,7 +53,7 @@ export interface LogRecord {
   /**
    * Numerical value of the severity.
    */
-  severityNumber?: number;
+  severityNumber?: SeverityNumber;
 
   /**
    * The severity text.

diff --git a/experimental/packages/api-logs/test/noop-implementations/noop-logger.test.ts b/experimental/packages/api-logs/test/noop-implementations/noop-logger.test.ts
@@ -15,6 +15,7 @@
  */
 
 import * as assert from 'assert';
+import { SeverityNumber } from '../../src';
 import { NoopLogger } from '../../src/NoopLogger';
 import { NoopLoggerProvider } from '../../src/NoopLoggerProvider';
 
@@ -31,6 +32,9 @@ describe('NoopLogger', () => {
 
   it('calling emitLogRecord should not crash', () => {
     const logger = new NoopLoggerProvider().getLogger('test-noop');
-    logger.emitLogRecord({ severityNumber: 1, body: 'log body' });
+    logger.emitLogRecord({
+      severityNumber: SeverityNumber.TRACE,
+      body: 'log body',
+    });
   });
 });
diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts
@@ -47,7 +47,6 @@ import {
   HttpInstrumentationConfig,
   HttpRequestArgs,
   Https,
-  ResponseEndArgs,
 } from './types';
 import * as utils from './utils';
 import { VERSION } from './version';
@@ -488,7 +487,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
       };
 
       const startTime = hrTime();
-      let metricAttributes: MetricAttributes =
+      const metricAttributes =
         utils.getIncomingRequestMetricAttributes(spanAttributes);
 
       const ctx = propagation.extract(ROOT_CONTEXT, headers);
@@ -520,73 +519,29 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
             header => request.headers[header]
           );
 
-          // Wraps end (inspired by:
-          // https://github.com/GoogleCloudPlatform/cloud-trace-nodejs/blob/master/src/instrumentations/instrumentation-connect.ts#L75)
-          const originalEnd = response.end;
-          response.end = function (
-            this: http.ServerResponse,
-            ..._args: ResponseEndArgs
-          ) {
-            response.end = originalEnd;
-            // Cannot pass args of type ResponseEndArgs,
-            const returned = safeExecuteInTheMiddle(
-              () => response.end.apply(this, arguments as never),
-              error => {
-                if (error) {
-                  utils.setSpanWithError(span, error);
-                  instrumentation._closeHttpSpan(
-                    span,
-                    SpanKind.SERVER,
-                    startTime,
-                    metricAttributes
-                  );
-                  throw error;
-                }
-              }
-            );
-
-            const attributes = utils.getIncomingRequestAttributesOnResponse(
+          // After 'error', no further events other than 'close' should be emitted.
+          let hasError = false;
+          response.on('close', () => {
+            if (hasError) {
+              return;
+            }
+            instrumentation._onServerResponseFinish(
               request,
-              response
-            );
-            metricAttributes = Object.assign(
-              metricAttributes,
-              utils.getIncomingRequestMetricAttributesOnResponse(attributes)
-            );
-
-            instrumentation._headerCapture.server.captureResponseHeaders(
+              response,
               span,
-              header => response.getHeader(header)
+              metricAttributes,
+              startTime
             );
-
-            span.setAttributes(attributes).setStatus({
-              code: utils.parseResponseStatus(
-                SpanKind.SERVER,
-                response.statusCode
-              ),
-            });
-
-            if (instrumentation._getConfig().applyCustomAttributesOnSpan) {
-              safeExecuteInTheMiddle(
-                () =>
-                  instrumentation._getConfig().applyCustomAttributesOnSpan!(
-                    span,
-                    request,
-                    response
-                  ),
-                () => {},
-                true
-              );
-            }
-
-            instrumentation._closeHttpSpan(
+          });
+          response.on(errorMonitor, (err: Err) => {
+            hasError = true;
+            instrumentation._onServerResponseError(
               span,
-              SpanKind.SERVER,
+              metricAttributes,
               startTime,
-              metricAttributes
+              err
             );
-            return returned;
-          };
+          });
 
           return safeExecuteInTheMiddle(
             () => original.apply(this, [event, ...args]),
@@ -741,6 +696,56 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
     };
   }
 
+  private _onServerResponseFinish(
+    request: http.IncomingMessage,
+    response: http.ServerResponse,
+    span: Span,
+    metricAttributes: MetricAttributes,
+    startTime: HrTime
+  ) {
+    const attributes = utils.getIncomingRequestAttributesOnResponse(
+      request,
+      response
+    );
+    metricAttributes = Object.assign(
+      metricAttributes,
+      utils.getIncomingRequestMetricAttributesOnResponse(attributes)
+    );
+
+    this._headerCapture.server.captureResponseHeaders(span, header =>
+      response.getHeader(header)
+    );
+
+    span.setAttributes(attributes).setStatus({
+      code: utils.parseResponseStatus(SpanKind.SERVER, response.statusCode),
+    });
+
+    if (this._getConfig().applyCustomAttributesOnSpan) {
+      safeExecuteInTheMiddle(
+        () =>
+          this._getConfig().applyCustomAttributesOnSpan!(
+            span,
+            request,
+            response
+          ),
+        () => {},
+        true
+      );
+    }
+
+    this._closeHttpSpan(span, SpanKind.SERVER, startTime, metricAttributes);
+  }
+
+  private _onServerResponseError(
+    span: Span,
+    metricAttributes: MetricAttributes,
+    startTime: HrTime,
+    error: Err
+  ) {
+    utils.setSpanWithError(span, error);
+    this._closeHttpSpan(span, SpanKind.SERVER, startTime, metricAttributes);
+  }
+
   private _startHttpSpan(
     name: string,
     options: SpanOptions,

diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts
@@ -47,10 +47,6 @@ export type Http = typeof http;
 export type Https = typeof https;
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export type Func<T> = (...args: any[]) => T;
-export type ResponseEndArgs =
-  | [((() => void) | undefined)?]
-  | [unknown, ((() => void) | undefined)?]
-  | [unknown, string, ((() => void) | undefined)?];
 
 export interface HttpCustomAttributeFunction {
   (