Merge branch 'main' into renovate/all-patch

packages/opentelemetry-test-utils/src/test-fixtures.ts

@@ -110,14 +110,38 @@ export class TestCollector {
     return this._http.close();
   }
 
-  // Return the spans sorted by start time for testing convenience.
+  /**
+   * Return the spans sorted by which started first, for testing convenience.
+   *
+   * Note: This sorting is a *best effort*. `span.startTimeUnixNano` has
+   * millisecond accuracy, so if multiple spans start in the same millisecond
+   * then this cannot know the start ordering. If `startTimeUnixNano` are the
+   * same, this attempts to get the correct ordering using `parentSpanId` -- a
+   * parent span starts before any of its direct children. This isn't perfect.
+   */
   get sortedSpans(): Array<TestSpan> {
     return this.spans.slice().sort((a, b) => {
       assert(typeof a.startTimeUnixNano === 'string');
       assert(typeof b.startTimeUnixNano === 'string');
       const aStartInt = BigInt(a.startTimeUnixNano);
       const bStartInt = BigInt(b.startTimeUnixNano);
-      return aStartInt < bStartInt ? -1 : aStartInt > bStartInt ? 1 : 0;
+      if (aStartInt < bStartInt) {
+        return -1;
+      } else if (aStartInt > bStartInt) {
+        return 1;
+      } else {
+        // Same startTimeUnixNano, which has millisecond accuracy. This is
+        // common for Express middleware spans on a fast enough dev machine.
+        // Attempt to use spanId/parentSpanId to decide on span ordering.
+        if (a.traceId === b.traceId) {
+          if (a.spanId === b.parentSpanId) {
+            return -1;
+          } else if (a.parentSpanId === b.spanId) {
+            return 1;
+          }
+        }
+        return 0;
+      }
     });
   }
 

plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts

@@ -48,6 +48,7 @@ import {
   SEMRESATTRS_CLOUD_ACCOUNT_ID,
   SEMRESATTRS_FAAS_ID,
 } from '@opentelemetry/semantic-conventions';
+import { ATTR_FAAS_COLDSTART } from '@opentelemetry/semantic-conventions/incubating';
 
 import {
   APIGatewayProxyEventHeaders,
@@ -72,6 +73,7 @@ const headerGetter: TextMapGetter<APIGatewayProxyEventHeaders> = {
 };
 
 export const traceContextEnvironmentKey = '_X_AMZN_TRACE_ID';
+export const lambdaMaxInitInMilliseconds = 10_000;
 
 export class AwsLambdaInstrumentation extends InstrumentationBase<AwsLambdaInstrumentationConfig> {
   private _traceForceFlusher?: () => Promise<void>;
@@ -135,6 +137,10 @@ export class AwsLambdaInstrumentation extends InstrumentationBase<AwsLambdaInstr
       functionName,
     });
 
+    const lambdaStartTime =
+      this.getConfig().lambdaStartTime ||
+      Date.now() - Math.floor(1000 * process.uptime());
+
     return [
       new InstrumentationNodeModuleDefinition(
         // NB: The patching infrastructure seems to match names backwards, this must be the filename, while
@@ -151,7 +157,11 @@ export class AwsLambdaInstrumentation extends InstrumentationBase<AwsLambdaInstr
               if (isWrapped(moduleExports[functionName])) {
                 this._unwrap(moduleExports, functionName);
               }
-              this._wrap(moduleExports, functionName, this._getHandler());
+              this._wrap(
+                moduleExports,
+                functionName,
+                this._getHandler(lambdaStartTime)
+              );
               return moduleExports;
             },
             (moduleExports?: LambdaModule) => {
@@ -164,16 +174,47 @@ export class AwsLambdaInstrumentation extends InstrumentationBase<AwsLambdaInstr
     ];
   }
 
-  private _getHandler() {
+  private _getHandler(handlerLoadStartTime: number) {
     return (original: Handler) => {
-      return this._getPatchHandler(original);
+      return this._getPatchHandler(original, handlerLoadStartTime);
     };
   }
 
-  private _getPatchHandler(original: Handler) {
+  private _getPatchHandler(original: Handler, lambdaStartTime: number) {
     diag.debug('patch handler function');
     const plugin = this;
 
+    let requestHandledBefore = false;
+    let requestIsColdStart = true;
+
+    function _onRequest(): void {
+      if (requestHandledBefore) {
+        // Non-first requests cannot be coldstart.
+        requestIsColdStart = false;
+      } else {
+        if (
+          process.env.AWS_LAMBDA_INITIALIZATION_TYPE ===
+          'provisioned-concurrency'
+        ) {
+          // If sandbox environment is initialized with provisioned concurrency,
+          // even the first requests should not be considered as coldstart.
+          requestIsColdStart = false;
+        } else {
+          // Check whether it is proactive initialization or not:
+          // https://aaronstuyvenberg.com/posts/understanding-proactive-initialization
+          const passedTimeSinceHandlerLoad: number =
+            Date.now() - lambdaStartTime;
+          const proactiveInitialization: boolean =
+            passedTimeSinceHandlerLoad > lambdaMaxInitInMilliseconds;
+
+          // If sandbox has been initialized proactively before the actual request,
+          // even the first requests should not be considered as coldstart.
+          requestIsColdStart = !proactiveInitialization;
+        }
+        requestHandledBefore = true;
+      }
+    }
+
     return function patchedHandler(
       this: never,
       // The event can be a user type, it truly is any.
@@ -182,6 +223,8 @@ export class AwsLambdaInstrumentation extends InstrumentationBase<AwsLambdaInstr
       context: Context,
       callback: Callback
     ) {
+      _onRequest();
+
       const config = plugin.getConfig();
       const parent = AwsLambdaInstrumentation._determineParent(
         event,
@@ -203,6 +246,7 @@ export class AwsLambdaInstrumentation extends InstrumentationBase<AwsLambdaInstr
               AwsLambdaInstrumentation._extractAccountId(
                 context.invokedFunctionArn
               ),
+            [ATTR_FAAS_COLDSTART]: requestIsColdStart,
           },
         },
         parent

plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts

@@ -41,4 +41,5 @@ export interface AwsLambdaInstrumentationConfig extends InstrumentationConfig {
   disableAwsContextPropagation?: boolean;
   eventContextExtractor?: EventContextExtractor;
   lambdaHandler?: string;
+  lambdaStartTime?: number;
 }

plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts

@@ -23,6 +23,7 @@ import {
   AwsLambdaInstrumentation,
   AwsLambdaInstrumentationConfig,
   traceContextEnvironmentKey,
+  lambdaMaxInitInMilliseconds,
 } from '../../src';
 import {
   BatchSpanProcessor,
@@ -34,6 +35,7 @@ import { Context } from 'aws-lambda';
 import * as assert from 'assert';
 import {
   SEMATTRS_EXCEPTION_MESSAGE,
+  SEMATTRS_FAAS_COLDSTART,
   SEMATTRS_FAAS_EXECUTION,
   SEMRESATTRS_FAAS_NAME,
 } from '@opentelemetry/semantic-conventions';
@@ -295,6 +297,100 @@ describe('lambda handler', () => {
       assert.strictEqual(span.parentSpanId, undefined);
     });
 
+    it('should record coldstart', async () => {
+      initializeHandler('lambda-test/sync.handler');
+
+      const handlerModule = lambdaRequire('lambda-test/sync');
+
+      const result1 = await new Promise((resolve, reject) => {
+        handlerModule.handler('arg', ctx, (err: Error, res: any) => {
+          if (err) {
+            reject(err);
+          } else {
+            resolve(res);
+          }
+        });
+      });
+
+      const result2 = await new Promise((resolve, reject) => {
+        handlerModule.handler('arg', ctx, (err: Error, res: any) => {
+          if (err) {
+            reject(err);
+          } else {
+            resolve(res);
+          }
+        });
+      });
+
+      const spans = memoryExporter.getFinishedSpans();
+      assert.strictEqual(spans.length, 2);
+      const [span1, span2] = spans;
+
+      assert.strictEqual(result1, 'ok');
+      assertSpanSuccess(span1);
+      assert.strictEqual(span1.parentSpanId, undefined);
+      assert.strictEqual(span1.attributes[SEMATTRS_FAAS_COLDSTART], true);
+
+      assert.strictEqual(result2, 'ok');
+      assertSpanSuccess(span2);
+      assert.strictEqual(span2.parentSpanId, undefined);
+      assert.strictEqual(span2.attributes[SEMATTRS_FAAS_COLDSTART], false);
+    });
+
+    it('should record coldstart with provisioned concurrency', async () => {
+      process.env.AWS_LAMBDA_INITIALIZATION_TYPE = 'provisioned-concurrency';
+
+      initializeHandler('lambda-test/sync.handler');
+
+      const result = await new Promise((resolve, reject) => {
+        lambdaRequire('lambda-test/sync').handler(
+          'arg',
+          ctx,
+          (err: Error, res: any) => {
+            if (err) {
+              reject(err);
+            } else {
+              resolve(res);
+            }
+          }
+        );
+      });
+      assert.strictEqual(result, 'ok');
+      const spans = memoryExporter.getFinishedSpans();
+      const [span] = spans;
+      assert.strictEqual(spans.length, 1);
+      assertSpanSuccess(span);
+      assert.strictEqual(span.parentSpanId, undefined);
+      assert.strictEqual(span.attributes[SEMATTRS_FAAS_COLDSTART], false);
+    });
+
+    it('should record coldstart with proactive initialization', async () => {
+      initializeHandler('lambda-test/sync.handler', {
+        lambdaStartTime: Date.now() - 2 * lambdaMaxInitInMilliseconds,
+      });
+
+      const result = await new Promise((resolve, reject) => {
+        lambdaRequire('lambda-test/sync').handler(
+          'arg',
+          ctx,
+          (err: Error, res: any) => {
+            if (err) {
+              reject(err);
+            } else {
+              resolve(res);
+            }
+          }
+        );
+      });
+      assert.strictEqual(result, 'ok');
+      const spans = memoryExporter.getFinishedSpans();
+      const [span] = spans;
+      assert.strictEqual(spans.length, 1);
+      assertSpanSuccess(span);
+      assert.strictEqual(span.parentSpanId, undefined);
+      assert.strictEqual(span.attributes[SEMATTRS_FAAS_COLDSTART], false);
+    });
+
     it('should record error', async () => {
       initializeHandler('lambda-test/sync.error');
 

plugins/node/opentelemetry-instrumentation-express/test/express.test.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { SpanStatusCode, context, SpanKind, trace } from '@opentelemetry/api';
+import { SpanStatusCode, context, trace } from '@opentelemetry/api';
 import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
 import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
 import {
@@ -675,7 +675,7 @@ describe('ExpressInstrumentation', () => {
           spans[5].name,
           'request handler - /\\/test\\/regex/'
         );
-        assert.strictEqual(spans[5].kind, SpanKind.SERVER);
+        assert.strictEqual(spans[5].kind, testUtils.OtlpSpanKind.INTERNAL);
         assert.strictEqual(spans[5].parentSpanId, spans[1].spanId);
       },
     });

plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts

@@ -46,7 +46,7 @@ function removeCredentialsFromDBConnectionStringAttribute(
   diag: DiagLogger,
   url?: unknown
 ): string | undefined {
-  if (typeof url !== 'string') {
+  if (typeof url !== 'string' || !url) {
     return;
   }
 

plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts

@@ -13,6 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+import { diag, DiagLogLevel } from '@opentelemetry/api';
 import {
   getTestSpans,
   registerInstrumentationTesting,
@@ -288,6 +290,36 @@ describe('redis@^4.0.0', () => {
         expectAttributeConnString
       );
     });
+
+    it('with empty string for client URL, there is no crash and no diag.error', async () => {
+      // Note: This messily leaves the diag logger set for other tests.
+      const diagErrors = [] as any;
+      diag.setLogger(
+        {
+          verbose() {},
+          debug() {},
+          info() {},
+          warn() {},
+          error(...args) {
+            diagErrors.push(args);
+          },
+        },
+        DiagLogLevel.WARN
+      );
+
+      const newClient = createClient({ url: '' });
+      try {
+        await newClient.connect();
+      } catch (_err) {
+        // Ignore. If the test Redis is not at the default port we expect this
+        // to error.
+      }
+      await newClient.disconnect();
+
+      const [span] = getTestSpans();
+      assert.strictEqual(span.name, 'redis-connect');
+      assert.strictEqual(diagErrors.length, 0, "no diag.error's");
+    });
   });
 
   describe('multi (transactions) commands', () => {