Skip to content

Commit 54dd058

Browse files
authored
Revert "Add logSource and taskQueue metadata on log messages (#1399)
1 parent 1708529 commit 54dd058

24 files changed

+139
-296
lines changed

packages/activity/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,8 @@ export class Context {
254254
* The logger for this Activity.
255255
*
256256
* This defaults to the `Runtime`'s Logger (see {@link Runtime.logger}). Attributes from the current Activity context
257-
* will automatically be included as metadata on every log entries. An extra `logSource` metadata attribute will also
258-
* be added, with value `activity`; this can be used for fine-grained filtering of log entries further downstream.
257+
* will automatically be included as metadata on every log entries, and some key events of the Activity's lifecycle
258+
* will automatically be logged (at 'DEBUG' level for most messages; 'WARN' for failures).
259259
*
260260
* To customize log attributes, register a {@link ActivityOutboundCallsInterceptor} that intercepts the
261261
* `getLogAttributes()` method.

packages/common/src/logger.ts

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,44 +13,3 @@ export interface Logger {
1313
warn(message: string, meta?: LogMetadata): any;
1414
error(message: string, meta?: LogMetadata): any;
1515
}
16-
17-
/**
18-
* Possible values of the `logSource` meta attributes on log messages. This
19-
* attribute indicates which subsystem emitted the log message; this may for
20-
* example be used to implement fine-grained filtering of log messages.
21-
*
22-
* Note that there is no guarantee that this list will remain stable in the
23-
* future; values may be added or removed, and messages that are currently
24-
* emitted with some `logSource` value may use a different value in the future.
25-
*/
26-
export enum LogSource {
27-
/**
28-
* Log Source value for messages emited from Workflow code, using the {@link Workflow
29-
* context logger|workflow.log}. The SDK itself never publishes messages with this source.
30-
*/
31-
workflow = 'workflow',
32-
33-
/**
34-
* Log Source value for messages emited from an activity, using the {@link activity
35-
* context logger|Context.log}. The SDK itself never publishes messages with this source.
36-
*/
37-
activity = 'activity',
38-
39-
/**
40-
* Log Source value for messages emited from a Temporal Worker instance.
41-
*
42-
* This notably includes:
43-
* - Issues with worker or runtime options or the JS execution environment;
44-
* - Worker's, Activity's, and Workflow's lifecycle events;
45-
* - Workflow Activation and Activity Task processing events;
46-
* - Workflow Bundling messages;
47-
* - Sink processing issues;
48-
* - Issues in interracting with the Core SDK library.
49-
*/
50-
worker = 'worker',
51-
52-
/**
53-
* Log Source value for all messages emited by the Core SDK library.
54-
*/
55-
core = 'core',
56-
}

packages/core-bridge/scripts/build.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,11 @@ function compile(requestedTarget) {
8282
const cmd = which.sync('cargo-cp-artifact');
8383

8484
console.log('Running', cmd, argv);
85-
const { status, error } = spawnSync(cmd, argv, {
85+
const { status } = spawnSync(cmd, argv, {
8686
stdio: 'inherit',
87-
shell: process.platform === 'win32',
8887
});
89-
if (status !== 0 || error) {
90-
throw new Error(`Failed to build${target ? ' for ' + target : ''}: status code ${status}`, error);
88+
if (status !== 0) {
89+
throw new Error(`Failed to build${target ? ' for ' + target : ''}`);
9190
}
9291
}
9392

packages/create-project/src/helpers/install.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,13 @@ interface InstallArgs {
2020
* @returns A Promise that resolves once the installation is finished.
2121
*/
2222
export function install({ root, useYarn }: InstallArgs): Promise<void> {
23-
const isWindows = process.platform === 'win32';
24-
const npm = isWindows ? 'npm.cmd' : 'npm';
23+
const npm = /^win/.test(process.platform) ? 'npm.cmd' : 'npm';
2524
const command: string = useYarn ? 'yarn' : npm;
2625

2726
return spawn(command, ['install'], {
2827
cwd: root,
2928
stdio: 'inherit',
3029
env: { ...process.env, ADBLOCK: '1', DISABLE_OPENCOLLECTIVE: '1' },
31-
shell: isWindows,
3230
});
3331
}
3432

packages/test/src/helpers-integration.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
} from '@temporalio/worker';
2222
import * as workflow from '@temporalio/workflow';
2323
import { ConnectionInjectorInterceptor } from './activities/interceptors';
24-
import { Worker, test as anyTest, bundlerOptions, registerDefaultCustomSearchAttributes } from './helpers';
24+
import { Worker, test as anyTest, bundlerOptions } from './helpers';
2525

2626
export interface Context {
2727
env: TestWorkflowEnvironment;
@@ -59,7 +59,6 @@ export function makeTestFunction(opts: {
5959
],
6060
},
6161
});
62-
await registerDefaultCustomSearchAttributes(env.connection);
6362
const workflowBundle = await bundleWorkflowCode({
6463
...bundlerOptions,
6564
workflowInterceptorModules: [...defaultWorkflowInterceptorModules, ...(opts.workflowInterceptorModules ?? [])],

packages/test/src/mock-native-worker.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
/* eslint-disable @typescript-eslint/no-non-null-assertion */
22
import { lastValueFrom } from 'rxjs';
3-
import { LogSource, defaultPayloadConverter, fromPayloadsAtIndex } from '@temporalio/common';
3+
import { defaultPayloadConverter, fromPayloadsAtIndex } from '@temporalio/common';
44
import { msToTs } from '@temporalio/common/lib/time';
55
import { coresdk } from '@temporalio/proto';
66
import { DefaultLogger, Runtime, ShutdownError } from '@temporalio/worker';
77
import { byteArrayToBuffer } from '@temporalio/worker/lib/utils';
88
import { NativeReplayHandle, NativeWorkerLike, Worker as RealWorker } from '@temporalio/worker/lib/worker';
9-
import { withMetadata } from '@temporalio/worker/lib/logger';
10-
import { CompiledWorkerOptions, compileWorkerOptions, WorkerOptions } from '@temporalio/worker/lib/worker-options';
9+
import {
10+
addDefaultWorkerOptions,
11+
CompiledWorkerOptions,
12+
compileWorkerOptions,
13+
WorkerOptions,
14+
} from '@temporalio/worker/lib/worker-options';
1115
import type { WorkflowCreator } from '@temporalio/worker/lib/workflow/interface';
1216
import * as activities from './activities';
1317

@@ -159,12 +163,11 @@ export class Worker extends RealWorker {
159163
}
160164

161165
public constructor(workflowCreator: WorkflowCreator, opts: CompiledWorkerOptions) {
162-
const logger = withMetadata(Runtime.instance().logger, {
163-
logSource: LogSource.worker,
164-
taskQueue: opts.taskQueue ?? 'default',
165-
});
166+
// Worker.create() accesses Runtime.instance(), which has some side effects that would not happen (or that would
167+
// happen too late) when creating a MockWorker. Force the singleton to be created now, if it doesn't already exist.
168+
Runtime.instance();
166169
const nativeWorker = new MockNativeWorker();
167-
super(nativeWorker, workflowCreator, opts, logger);
170+
super(nativeWorker, workflowCreator, opts);
168171
}
169172

170173
public runWorkflows(...args: Parameters<Worker['workflow$']>): Promise<void> {
@@ -180,10 +183,6 @@ export const defaultOptions: WorkerOptions = {
180183
};
181184

182185
export function isolateFreeWorker(options: WorkerOptions = defaultOptions): Worker {
183-
const logger = withMetadata(Runtime.instance().logger, {
184-
logSource: LogSource.worker,
185-
taskQueue: options.taskQueue ?? 'default',
186-
});
187186
return new Worker(
188187
{
189188
async createWorkflow() {
@@ -193,6 +192,6 @@ export function isolateFreeWorker(options: WorkerOptions = defaultOptions): Work
193192
/* Nothing to destroy */
194193
},
195194
},
196-
compileWorkerOptions(options, logger)
195+
compileWorkerOptions(addDefaultWorkerOptions(options))
197196
);
198197
}

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

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,52 +5,47 @@ import { MockActivityEnvironment, defaultActivityInfo } from '@temporalio/testin
55
import { isCancellation } from '@temporalio/workflow';
66
import { isAbortError } from '@temporalio/common/lib/type-helpers';
77
import * as activity from '@temporalio/activity';
8-
import { LogSource } from '@temporalio/common';
98
import { withZeroesHTTPServer } from './zeroes-http-server';
109
import { cancellableFetch } from './activities';
1110

1211
interface MyTestActivityContext extends activity.Context {
1312
logs: Array<LogEntry>;
1413
}
1514

16-
const mockLogger = new DefaultLogger('DEBUG', (entry) => {
17-
try {
15+
test.before(() => {
16+
const mockLogger = new DefaultLogger('DEBUG', (entry) => {
1817
(activity.Context.current() as MyTestActivityContext).logs ??= [];
1918
(activity.Context.current() as MyTestActivityContext).logs.push(entry);
20-
} catch (e) {
21-
// Ignore messages produced from non activity context
22-
if ((e as Error).message !== 'Activity context not initialized') throw e;
23-
}
24-
});
25-
Runtime.install({
26-
logger: mockLogger,
19+
});
20+
Runtime.install({
21+
logger: mockLogger,
22+
});
2723
});
2824

29-
test('Activity Context logger funnel through the parent Logger', async (t) => {
30-
const env = new MockActivityEnvironment({}, { logger: mockLogger });
25+
test("Activity Context logger defaults to Runtime's Logger", async (t) => {
26+
const env = new MockActivityEnvironment({});
3127
await env.run(async () => {
3228
activity.log.debug('log message from activity');
3329
});
3430
const logs = (env.context as MyTestActivityContext).logs;
3531
const entry = logs.find((x) => x.level === 'DEBUG' && x.message === 'log message from activity');
3632
t.not(entry, undefined);
37-
t.deepEqual(entry?.meta, { ...activityLogAttributes(defaultActivityInfo), logSource: LogSource.activity });
3833
});
3934

4035
test('Activity Worker logs when activity starts', async (t) => {
41-
const env = new MockActivityEnvironment({}, { logger: mockLogger });
36+
const env = new MockActivityEnvironment({});
4237
await env.run(async () => {
4338
activity.log.debug('log message from activity');
4439
});
4540
const logs = (env.context as MyTestActivityContext).logs;
4641
const entry = logs.find((x) => x.level === 'DEBUG' && x.message === 'Activity started');
4742
t.not(entry, undefined);
48-
t.deepEqual(entry?.meta, { ...activityLogAttributes(defaultActivityInfo), logSource: LogSource.worker });
43+
t.deepEqual(entry?.meta, activityLogAttributes(defaultActivityInfo));
4944
});
5045

5146
test('Activity Worker logs warning when activity fails', async (t) => {
5247
const err = new Error('Failed for test');
53-
const env = new MockActivityEnvironment({}, { logger: mockLogger });
48+
const env = new MockActivityEnvironment({});
5449
try {
5550
await env.run(async () => {
5651
throw err;
@@ -65,11 +60,11 @@ test('Activity Worker logs warning when activity fails', async (t) => {
6560
const { durationMs, error, ...rest } = entry?.meta ?? {};
6661
t.true(Number.isInteger(durationMs));
6762
t.is(err, error);
68-
t.deepEqual(rest, { ...activityLogAttributes(defaultActivityInfo), logSource: LogSource.worker });
63+
t.deepEqual(rest, activityLogAttributes(defaultActivityInfo));
6964
});
7065

7166
test('Activity Worker logs when activity completes async', async (t) => {
72-
const env = new MockActivityEnvironment({}, { logger: mockLogger });
67+
const env = new MockActivityEnvironment({});
7368
try {
7469
await env.run(async () => {
7570
throw new activity.CompleteAsyncError();
@@ -82,11 +77,11 @@ test('Activity Worker logs when activity completes async', async (t) => {
8277
t.not(entry, undefined);
8378
const { durationMs, ...rest } = entry?.meta ?? {};
8479
t.true(Number.isInteger(durationMs));
85-
t.deepEqual(rest, { ...activityLogAttributes(defaultActivityInfo), logSource: LogSource.worker });
80+
t.deepEqual(rest, activityLogAttributes(defaultActivityInfo));
8681
});
8782

8883
test('Activity Worker logs when activity is cancelled with promise', async (t) => {
89-
const env = new MockActivityEnvironment({}, { logger: mockLogger });
84+
const env = new MockActivityEnvironment({});
9085
env.on('heartbeat', () => env.cancel());
9186
try {
9287
await env.run(async () => {
@@ -101,11 +96,11 @@ test('Activity Worker logs when activity is cancelled with promise', async (t) =
10196
t.not(entry, undefined);
10297
const { durationMs, ...rest } = entry?.meta ?? {};
10398
t.true(Number.isInteger(durationMs));
104-
t.deepEqual(rest, { ...activityLogAttributes(defaultActivityInfo), logSource: LogSource.worker });
99+
t.deepEqual(rest, activityLogAttributes(defaultActivityInfo));
105100
});
106101

107102
test('Activity Worker logs when activity is cancelled with signal', async (t) => {
108-
const env = new MockActivityEnvironment({}, { logger: mockLogger });
103+
const env = new MockActivityEnvironment({});
109104
env.on('heartbeat', () => env.cancel());
110105
try {
111106
await env.run(async () => {
@@ -121,7 +116,7 @@ test('Activity Worker logs when activity is cancelled with signal', async (t) =>
121116
t.not(entry, undefined);
122117
const { durationMs, ...rest } = entry?.meta ?? {};
123118
t.true(Number.isInteger(durationMs));
124-
t.deepEqual(rest, { ...activityLogAttributes(defaultActivityInfo), logSource: LogSource.worker });
119+
t.deepEqual(rest, activityLogAttributes(defaultActivityInfo));
125120
});
126121

127122
test('(Legacy) ActivityInboundLogInterceptor does not override Context.log by default', async (t) => {
@@ -130,7 +125,6 @@ test('(Legacy) ActivityInboundLogInterceptor does not override Context.log by de
130125
{
131126
// eslint-disable-next-line deprecation/deprecation
132127
interceptors: [(ctx) => ({ inbound: new ActivityInboundLogInterceptor(ctx) })],
133-
logger: mockLogger,
134128
}
135129
);
136130
await env.run(async () => {
@@ -152,14 +146,15 @@ test('(Legacy) ActivityInboundLogInterceptor overrides Context.log if a logger i
152146
{
153147
// eslint-disable-next-line deprecation/deprecation
154148
interceptors: [(ctx) => ({ inbound: new ActivityInboundLogInterceptor(ctx, logger) })],
155-
logger: mockLogger,
156149
}
157150
);
158151
await env.run(async () => {
159152
activity.log.debug('log message from activity');
160153
});
161-
const entry = logs.find((x) => x.level === 'DEBUG' && x.message === 'log message from activity');
162-
t.not(entry, undefined);
154+
const entry1 = logs.find((x) => x.level === 'DEBUG' && x.message === 'Activity started');
155+
t.not(entry1, undefined);
156+
const entry2 = logs.find((x) => x.level === 'DEBUG' && x.message === 'log message from activity');
157+
t.not(entry2, undefined);
163158
});
164159

165160
test('(Legacy) ActivityInboundLogInterceptor overrides Context.log if class is extended', async (t) => {
@@ -177,7 +172,6 @@ test('(Legacy) ActivityInboundLogInterceptor overrides Context.log if class is e
177172
{},
178173
{
179174
interceptors: [(ctx) => ({ inbound: new CustomActivityInboundLogInterceptor(ctx) })],
180-
logger: mockLogger,
181175
}
182176
);
183177
await env.run(async () => {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,12 @@ test('HistorySize is visible in WorkflowExecutionInfo', async (t) => {
153153

154154
export async function suggestedCAN(): Promise<boolean> {
155155
const maxEvents = 40_000;
156-
const batchSize = 1000;
156+
const batchSize = 100;
157157
if (workflow.workflowInfo().continueAsNewSuggested) {
158158
return false;
159159
}
160160
while (workflow.workflowInfo().historyLength < maxEvents) {
161-
await Promise.all(Array.from({ length: batchSize }, (_) => workflow.sleep(1)));
161+
await Promise.all(new Array(batchSize).fill(undefined).map((_) => workflow.sleep(1)));
162162
if (workflow.workflowInfo().continueAsNewSuggested) {
163163
return true;
164164
}
Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,6 @@
11
import test from 'ava';
22
import { MockActivityEnvironment } from '@temporalio/testing';
3-
import * as activity from '@temporalio/activity';
4-
import { Runtime } from '@temporalio/worker';
5-
6-
test("MockActivityEnvironment doesn't implicitly instanciate Runtime", async (t) => {
7-
t.is(Runtime._instance, undefined);
8-
const env = new MockActivityEnvironment();
9-
await env.run(async (): Promise<void> => {
10-
activity.log.info('log message from activity');
11-
});
12-
t.is(Runtime._instance, undefined);
13-
});
3+
import { CancelledFailure, Context } from '@temporalio/activity';
144

155
test('MockActivityEnvironment can run a single activity', async (t) => {
166
const env = new MockActivityEnvironment();
@@ -29,12 +19,12 @@ test('MockActivityEnvironment emits heartbeat events and can be cancelled', asyn
2919
});
3020
await t.throwsAsync(
3121
env.run(async (x: number): Promise<number> => {
32-
activity.heartbeat(6);
33-
await activity.sleep(100);
22+
Context.current().heartbeat(6);
23+
await Context.current().sleep(100);
3424
return x + 1;
3525
}, 3),
3626
{
37-
instanceOf: activity.CancelledFailure,
27+
instanceOf: CancelledFailure,
3828
message: 'test',
3929
}
4030
);
@@ -43,7 +33,7 @@ test('MockActivityEnvironment emits heartbeat events and can be cancelled', asyn
4333
test('MockActivityEnvironment injects provided info', async (t) => {
4434
const env = new MockActivityEnvironment({ attempt: 3 });
4535
const res = await env.run(async (x: number): Promise<number> => {
46-
return x + activity.activityInfo().attempt;
36+
return x + Context.current().info.attempt;
4737
}, 1);
4838
t.is(res, 4);
4939
});

packages/test/src/test-runtime.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ if (RUN_INTEGRATION_TESTS) {
112112
t.is(typeof failingWftEntry.meta?.['failure'], 'string');
113113
t.is(typeof failingWftEntry.meta?.['runId'], 'string');
114114
t.is(typeof failingWftEntry.meta?.['workflowId'], 'string');
115-
t.is(typeof failingWftEntry.meta?.['logSource'], 'string');
115+
t.is(typeof failingWftEntry.meta?.['subsystem'], 'string');
116116
} finally {
117117
await Runtime.instance().shutdown();
118118
}

0 commit comments

Comments
 (0)