chore: Deprecate WorkflowLogInterceptor (#1290)

Author: mjameswh

packages/test/src/integration-tests.ts

@@ -36,7 +36,7 @@ import {
 import { msToNumber, tsToMs } from '@temporalio/common/lib/time';
 import { decode, decodeFromPayloadsAtIndex } from '@temporalio/common/lib/internal-non-workflow';
 import * as iface from '@temporalio/proto';
-import { appendDefaultInterceptors, DefaultLogger, makeTelemetryFilterString, Runtime } from '@temporalio/worker';
+import { DefaultLogger, makeTelemetryFilterString, Runtime } from '@temporalio/worker';
 import pkg from '@temporalio/worker/lib/pkg';
 import { UnsafeWorkflowInfo } from '@temporalio/workflow/src/interfaces';
 import * as activities from './activities';
@@ -95,9 +95,9 @@ export function runIntegrationTests(codec?: PayloadCodec): void {
       activities,
       taskQueue: 'test',
       dataConverter,
-      interceptors: appendDefaultInterceptors({
+      interceptors: {
         activity: [() => ({ inbound: new ConnectionInjectorInterceptor(connection, loadedDataConverter) })],
-      }),
+      },
       showStackTraceSources: true,
     });
 

packages/test/src/test-integration-workflows.ts

@@ -4,7 +4,6 @@ import { firstValueFrom, Subject } from 'rxjs';
 import { WorkflowFailedError, WorkflowHandle, WorkflowStartOptions } from '@temporalio/client';
 import { TestWorkflowEnvironment, workflowInterceptorModules } from '@temporalio/testing';
 import {
-  appendDefaultInterceptors,
   bundleWorkflowCode,
   DefaultLogger,
   LogLevel,
@@ -53,9 +52,9 @@ function helpers(t: ExecutionContext<Context>): Helpers {
         connection: t.context.env.nativeConnection,
         workflowBundle: t.context.workflowBundle,
         taskQueue,
-        interceptors: appendDefaultInterceptors({
+        interceptors: {
           activity: [() => ({ inbound: new ConnectionInjectorInterceptor(t.context.env.connection) })],
-        }),
+        },
         showStackTraceSources: true,
         ...opts,
       });

packages/test/src/test-local-activities.ts

@@ -14,7 +14,6 @@ import { msToNumber } from '@temporalio/common/lib/time';
 import { temporal } from '@temporalio/proto';
 import { TestWorkflowEnvironment, workflowInterceptorModules } from '@temporalio/testing';
 import {
-  appendDefaultInterceptors,
   bundleWorkflowCode,
   DefaultLogger,
   LogLevel,
@@ -61,11 +60,11 @@ function helpers(t: ExecutionContext<Context>): Helpers {
         connection: t.context.env.nativeConnection,
         workflowBundle: t.context.workflowBundle,
         taskQueue,
-        interceptors: appendDefaultInterceptors({
+        interceptors: {
           activity: interceptors?.activity ?? [
             () => ({ inbound: new ConnectionInjectorInterceptor(t.context.env.connection) }),
           ],
-        }),
+        },
         showStackTraceSources: true,
         ...rest,
       });

packages/test/src/test-workflow-log-interceptor.ts

@@ -62,7 +62,7 @@ async function withWorker(
   return logs as [LogEntry, LogEntry];
 }
 
-test('WorkflowInboundLogInterceptor logs when workflow completes', async (t) => {
+test('Workflow Worker logs when workflow completes', async (t) => {
   const { client } = t.context.testEnv;
   const workflowId = uuid4();
   const [startLog, endLog] = await withWorker(
@@ -81,7 +81,7 @@ test('WorkflowInboundLogInterceptor logs when workflow completes', async (t) =>
   t.is(endLog.message, 'Workflow completed');
 });
 
-test('WorkflowInboundLogInterceptor logs when workflow continues as new', async (t) => {
+test('Workflow Worker logs when workflow continues as new', async (t) => {
   const { client } = t.context.testEnv;
   const workflowId = uuid4();
   const [_, endLog] = await withWorker(
@@ -100,7 +100,7 @@ test('WorkflowInboundLogInterceptor logs when workflow continues as new', async
   t.is(endLog.message, 'Workflow continued as new');
 });
 
-test('WorkflowInboundLogInterceptor logs warning when workflow fails', async (t) => {
+test('Workflow Worker logs warning when workflow fails', async (t) => {
   const { client } = t.context.testEnv;
   const workflowId = uuid4();
   const [_, endLog] = await withWorker(

packages/worker/src/index.ts

@@ -37,7 +37,6 @@ export {
   WorkerStatus,
 } from './worker';
 export {
-  appendDefaultInterceptors,
   CompiledWorkerOptions,
   ReplayWorkerOptions,
   WorkerOptions,
@@ -46,17 +45,7 @@ export {
   WorkflowBundlePath,
 } from './worker-options';
 export { ReplayError, ReplayHistoriesIterable, ReplayResult } from './replay';
-export {
-  WorkflowInboundLogInterceptor, // eslint-disable-line deprecation/deprecation
-  WorkflowLogInterceptor,
-  workflowLogAttributes,
-} from './workflow-log-interceptor';
-export {
-  BundleOptions,
-  bundleWorkflowCode,
-  defaultWorkflowInterceptorModules,
-  WorkflowBundleWithSourceMap,
-} from './workflow/bundler';
+export { BundleOptions, bundleWorkflowCode, WorkflowBundleWithSourceMap } from './workflow/bundler';
 
 // Anything below this line is deprecated
 
@@ -76,6 +65,11 @@ export {
   activityLogAttributes,
 } from './activity';
 export {
+  /**
+   * @deprecated Including `appendDefaultInterceptors()` in the worker options is no longer required. To configure a
+   *             custom logger, set the {@see Runtime.logger} property instead.
+   */
+  appendDefaultInterceptors, // eslint-disable-line deprecation/deprecation
   /**
    * @deprecated Including `defaultSinks()` in the worker options is no longer required. To configure
    *             a custom logger, set the {@link Runtime.logger} property instead.
@@ -93,3 +87,27 @@ export {
    */
   LoggerSinks, // eslint-disable-line deprecation/deprecation
 } from '@temporalio/workflow';
+export {
+  /**
+   * @deprecated `WorkflowInboundLogInterceptor` is deprecated. Workflow lifecycle events are now automatically logged
+   *             by the SDK. To customize workflow log attributes, simply register a custom `WorkflowInterceptors` that
+   *             intercepts the `outbound.getLogAttributes()` method.
+   */
+  WorkflowInboundLogInterceptor, // eslint-disable-line deprecation/deprecation
+  /**
+   * @deprecated `WorkflowLogInterceptor` is deprecated. Workflow lifecycle events are now automatically logged
+   *             by the SDK. To customize workflow log attributes, simply register a custom `WorkflowInterceptors` that
+   *             intercepts the `outbound.getLogAttributes()` method.
+   */
+  WorkflowLogInterceptor, // eslint-disable-line deprecation/deprecation
+} from './workflow-log-interceptor';
+export {
+  /**
+   * @deprecated This function is meant for internal usage. Don't use it.
+   */
+  workflowLogAttributes,
+} from '@temporalio/workflow/lib/logs';
+/**
+ * @deprecated Including `defaultWorkflowInterceptorModules` in BundlerOptions.workflowInterceptorModules is no longer required.
+ */
+export const defaultWorkflowInterceptorModules = [];

packages/worker/src/worker-options.ts

@@ -14,7 +14,7 @@ import { initLoggerSink } from './workflow/logger';
 import { Runtime } from './runtime';
 import { InjectedSinks } from './sinks';
 import { MiB } from './utils';
-import { defaultWorkflowInterceptorModules, WorkflowBundleWithSourceMap } from './workflow/bundler';
+import { WorkflowBundleWithSourceMap } from './workflow/bundler';
 
 export type { WebpackConfiguration };
 
@@ -417,6 +417,9 @@ export interface WorkerOptions {
 
    * When using {@link workflowBundle}, these Workflow interceptors (`WorkerInterceptors.workflowModules`) are not used.
    * Instead, provide them via {@link BundleOptions.workflowInterceptorModules} when calling {@link bundleWorkflowCode}.
+   *
+   * Before v1.9.0, calling `appendDefaultInterceptors()` was required when registering custom interceptors in order to
+   * preserve SDK's logging interceptors. This is no longer the case.
    */
   interceptors?: WorkerInterceptors;
 
@@ -622,27 +625,25 @@ export function defaultSinks(logger?: Logger): InjectedSinks<LoggerSinks> {
  * Appends the default Worker logging interceptors to given interceptor arrays.
  *
  * @param logger a {@link Logger} - defaults to the {@link Runtime} singleton logger.
+ *
+ * @deprecated Calling `appendDefaultInterceptors()` is no longer required. To configure a custom logger, set the
+ *             {@see Runtime.logger} property instead.
  */
 export function appendDefaultInterceptors(
   interceptors: WorkerInterceptors,
   logger?: Logger | undefined
-): Required<WorkerInterceptors> {
-  // FIXME: Don't worry for this function being unclean, this code will be optimized in the next PR.
+): WorkerInterceptors {
+  if (!logger || logger === Runtime.instance().logger) return interceptors;
 
-  // eslint-disable-next-line deprecation/deprecation
-  let activityInbound = interceptors.activityInbound ?? [];
-  if (logger && logger !== Runtime.instance().logger) {
-    activityInbound = [
+  return {
+    activityInbound: [
       // eslint-disable-next-line deprecation/deprecation
       (ctx) => new ActivityInboundLogInterceptor(ctx, logger),
-      ...activityInbound,
-    ];
-  }
-
-  return {
-    activityInbound,
-    activity: interceptors.activity ?? [],
-    workflowModules: [...(interceptors.workflowModules ?? []), ...defaultWorkflowInterceptorModules],
+      // eslint-disable-next-line deprecation/deprecation
+      ...(interceptors.activityInbound ?? []),
+    ],
+    activity: interceptors.activity,
+    workflowModules: interceptors.workflowModules,
   };
 }
 
@@ -703,7 +704,12 @@ export function addDefaultWorkerOptions(options: WorkerOptions): WorkerOptionsWi
     showStackTraceSources: showStackTraceSources ?? false,
     reuseV8Context: reuseV8Context ?? false,
     debugMode: debugMode ?? false,
-    interceptors: appendDefaultInterceptors(interceptors ?? {}),
+    interceptors: {
+      activity: interceptors?.activity ?? [],
+      // eslint-disable-next-line deprecation/deprecation
+      activityInbound: interceptors?.activityInbound ?? [],
+      workflowModules: interceptors?.workflowModules ?? [],
+    },
     nonStickyToStickyPollRatio: nonStickyToStickyPollRatio ?? 0.2,
     sinks: {
       ...initLoggerSink(Runtime.instance().logger),

packages/worker/src/worker.ts

@@ -42,6 +42,7 @@ import {
 import { historyFromJSON } from '@temporalio/common/lib/proto-utils';
 import { optionalTsToDate, optionalTsToMs, tsToDate, tsToMs } from '@temporalio/common/lib/time';
 import { errorMessage, isError, SymbolBasedInstanceOfError } from '@temporalio/common/lib/type-helpers';
+import { workflowLogAttributes } from '@temporalio/workflow/lib/logs';
 import * as native from '@temporalio/core-bridge';
 import { ShutdownError, UnexpectedError } from '@temporalio/core-bridge';
 import { coresdk, temporal } from '@temporalio/proto';
@@ -72,7 +73,6 @@ import {
   WorkflowBundle,
 } from './worker-options';
 import { WorkflowCodecRunner } from './workflow-codec-runner';
-import { workflowLogAttributes } from './workflow-log-interceptor';
 import { defaultWorkflowInterceptorModules, WorkflowCodeBundler } from './workflow/bundler';
 import { Workflow, WorkflowCreator } from './workflow/interface';
 import { ReusableVMWorkflowCreator } from './workflow/reusable-vm';

packages/worker/src/workflow-log-interceptor.ts

@@ -1,74 +1,46 @@
 import {
-  isCancellation,
   Next,
   WorkflowExecuteInput,
   WorkflowInboundCallsInterceptor,
   WorkflowOutboundCallsInterceptor,
-  workflowInfo,
-  WorkflowInfo,
   WorkflowInterceptorsFactory,
-  log,
-  ContinueAsNew,
   GetLogAttributesInput,
 } from '@temporalio/workflow';
-import { untrackPromise } from '@temporalio/workflow/lib/stack-helpers';
 
 /**
- * Returns a map of attributes to be set on log messages for a given Workflow
+ * This interceptor used to be meant to log Workflow execution starts and completions, and attaches log attributes to
+ * `workflow.log` calls. It is now deprecated and behaves as a noop in all cases. It is only kept arround to avoid
+ * breaking code out there that was previously refering to it.
+ *
+ * @deprecated `WorkflowLogInterceptor` is deprecated. Workflow lifecycle events are now automatically logged
+ *             by the SDK. To customize workflow log attributes, simply register a custom `WorkflowInterceptors` that
+ *             intercepts the `outbound.getLogAttributes()` method.
  */
-export function workflowLogAttributes(info: WorkflowInfo): Record<string, unknown> {
-  return {
-    namespace: info.namespace,
-    taskQueue: info.taskQueue,
-    workflowId: info.workflowId,
-    runId: info.runId,
-    workflowType: info.workflowType,
-  };
-}
-
-/** Logs workflow execution starts and completions, attaches log attributes to `workflow.log` calls  */
 export class WorkflowLogInterceptor implements WorkflowInboundCallsInterceptor, WorkflowOutboundCallsInterceptor {
   getLogAttributes(
     input: GetLogAttributesInput,
     next: Next<WorkflowOutboundCallsInterceptor, 'getLogAttributes'>
   ): Record<string, unknown> {
-    return next({ ...input, ...workflowLogAttributes(workflowInfo()) });
+    return next(input);
   }
 
   execute(input: WorkflowExecuteInput, next: Next<WorkflowInboundCallsInterceptor, 'execute'>): Promise<unknown> {
-    log.debug('Workflow started');
-    const p = next(input).then(
-      (res) => {
-        log.debug('Workflow completed');
-        return res;
-      },
-      (error) => {
-        // Avoid using instanceof checks in case the modules they're defined in loaded more than once,
-        // e.g. by jest or when multiple versions are installed.
-        if (typeof error === 'object' && error != null) {
-          if (isCancellation(error)) {
-            log.debug('Workflow completed as cancelled');
-            throw error;
-          } else if (error instanceof ContinueAsNew) {
-            log.debug('Workflow continued as new');
-            throw error;
-          }
-        }
-        log.warn('Workflow failed', { error });
-        throw error;
-      }
-    );
-    // Avoid showing this interceptor in stack trace query
-    untrackPromise(p);
-    return p;
+    // Logging of workflow's lifecycle events is now handled in `workflow/src/logs.ts`
+    return next(input);
   }
 }
 
-/** @deprecated use {@link WorkflowLogInterceptor} instead */
+/**
+ * @deprecated `WorkflowInboundLogInterceptor` is deprecated. Workflow lifecycle events are now automatically logged
+ *             by the SDK. To customize workflow log attributes, simply register a custom `WorkflowInterceptors` that
+ *             intercepts the `outbound.getLogAttributes()` method.
+ */
+// eslint-disable-next-line deprecation/deprecation
 export const WorkflowInboundLogInterceptor = WorkflowLogInterceptor;
 
 // ts-prune-ignore-next
 export const interceptors: WorkflowInterceptorsFactory = () => {
+  // eslint-disable-next-line deprecation/deprecation
   const interceptor = new WorkflowLogInterceptor();
   return { inbound: [interceptor], outbound: [interceptor] };
 };

packages/worker/src/workflow/bundler.ts

@@ -67,7 +67,7 @@ export class WorkflowCodeBundler {
     this.workflowsPath = workflowsPath;
     this.payloadConverterPath = payloadConverterPath;
     this.failureConverterPath = failureConverterPath;
-    this.workflowInterceptorModules = workflowInterceptorModules ?? defaultWorkflowInterceptorModules;
+    this.workflowInterceptorModules = workflowInterceptorModules ?? [];
     this.ignoreModules = ignoreModules ?? [];
     this.webpackConfigHook = webpackConfigHook ?? ((config) => config);
   }
@@ -317,9 +317,6 @@ export interface BundleOptions {
    * List of modules to import Workflow interceptors from.
    *
    * Modules should export an `interceptors` variable of type {@link WorkflowInterceptorsFactory}.
-   *
-   * By default, {@link WorkflowInboundLogInterceptor} is installed. If you wish to customize the interceptors while
-   * keeping the defaults, add {@link defaultWorkflowInterceptorModules} to the provided array.
    */
   workflowInterceptorModules?: string[];
   /**

packages/workflow/src/errors.ts

@@ -1,5 +1,6 @@
 import { ActivityFailure, CancelledFailure, ChildWorkflowFailure } from '@temporalio/common';
 import { SymbolBasedInstanceOfError } from '@temporalio/common/lib/type-helpers';
+import { coresdk } from '@temporalio/proto';
 
 /**
  * Base class for all workflow errors
@@ -13,6 +14,16 @@ export class WorkflowError extends Error {}
 @SymbolBasedInstanceOfError('DeterminismViolationError')
 export class DeterminismViolationError extends WorkflowError {}
 
+/**
+ * A class that acts as a marker for this special result type
+ */
+@SymbolBasedInstanceOfError('LocalActivityDoBackoff')
+export class LocalActivityDoBackoff extends Error {
+  constructor(public readonly backoff: coresdk.activity_result.IDoBackoff) {
+    super();
+  }
+}
+
 /**
  * Returns whether provided `err` is caused by cancellation
  */