mirror of
https://github.com/n8n-io/n8n.git
synced 2026-07-28 03:24:59 +02:00
fix(core): Start a new trace for scheduled poll triggers (no-changelog) (#33421)
This commit is contained in:
parent
c3898fba89
commit
672ddbdddb
|
|
@ -41,6 +41,32 @@ describe('PollTriggerExecutor', () => {
|
|||
expect(logger.scoped).toHaveBeenCalledWith('poll-trigger');
|
||||
});
|
||||
|
||||
describe('tracing', () => {
|
||||
it('starts its own root trace for a scheduled poll', async () => {
|
||||
const startNewTraceSpan = vi.spyOn(tracing, 'startNewTraceSpan');
|
||||
const startSpan = vi.spyOn(tracing, 'startSpan');
|
||||
triggersAndPollers.runPollFunction.mockResolvedValueOnce(null);
|
||||
|
||||
const execute = executor.create(workflow, node, pollFunctions, () => true);
|
||||
await execute();
|
||||
|
||||
expect(startNewTraceSpan).toHaveBeenCalledTimes(1);
|
||||
expect(startSpan).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('nests the activation poll in the current trace', async () => {
|
||||
const startNewTraceSpan = vi.spyOn(tracing, 'startNewTraceSpan');
|
||||
const startSpan = vi.spyOn(tracing, 'startSpan');
|
||||
triggersAndPollers.runPollFunction.mockResolvedValueOnce(null);
|
||||
|
||||
const execute = executor.create(workflow, node, pollFunctions, () => true);
|
||||
await execute(true);
|
||||
|
||||
expect(startSpan).toHaveBeenCalledTimes(1);
|
||||
expect(startNewTraceSpan).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('initial activation poll (testingTrigger=true)', () => {
|
||||
it('emits the poll result without acquiring the isolate', async () => {
|
||||
const result: INodeExecutionData[][] = [[{ json: { ok: true } }]];
|
||||
|
|
|
|||
|
|
@ -40,7 +40,14 @@ export class PollTriggerExecutor {
|
|||
isCurrent: () => boolean,
|
||||
): PollTriggerExecuteFn {
|
||||
return async (testingTrigger = false) => {
|
||||
return await this.tracing.startSpan(
|
||||
// Scheduled polls start their own root trace so they don't attach to the
|
||||
// publication transaction whose async context the poller was created in.
|
||||
// The activation poll genuinely belongs to publication, so leave it nested.
|
||||
const startTraced = testingTrigger
|
||||
? this.tracing.startSpan.bind(this.tracing)
|
||||
: this.tracing.startNewTraceSpan.bind(this.tracing);
|
||||
|
||||
return await startTraced(
|
||||
{
|
||||
name: 'Workflow Trigger Poll',
|
||||
op: 'trigger.poll',
|
||||
|
|
|
|||
|
|
@ -10,10 +10,13 @@ describe('SentryTracing', () => {
|
|||
let sentryTracing: SentryTracing;
|
||||
const mockSentry = mock({
|
||||
startSpan: vi.fn(),
|
||||
startNewTrace: vi.fn(),
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
mockClear(mockSentry);
|
||||
// Pass through to the callback so the wrapped span still runs.
|
||||
mockSentry.startNewTrace.mockImplementation((cb) => cb());
|
||||
|
||||
sentryTracing = new SentryTracing(mockSentry);
|
||||
});
|
||||
|
|
@ -126,6 +129,23 @@ describe('SentryTracing', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('startNewTraceSpan', () => {
|
||||
it('wraps the span in a new trace and returns the callback result', async () => {
|
||||
const options: StartSpanOptions = { name: 'new-trace-span' };
|
||||
const mockSpan = { name: 'mock-span' } as unknown as Sentry.Span;
|
||||
const callback = vi.fn().mockResolvedValue('result');
|
||||
|
||||
mockSentry.startSpan.mockImplementation(async (_opts, cb) => await cb(mockSpan));
|
||||
|
||||
const result = await sentryTracing.startNewTraceSpan(options, callback);
|
||||
|
||||
expect(mockSentry.startNewTrace).toHaveBeenCalledWith(expect.any(Function));
|
||||
expect(mockSentry.startSpan).toHaveBeenCalledWith(options, expect.any(Function));
|
||||
expect(callback).toHaveBeenCalledWith(mockSpan);
|
||||
expect(result).toBe('result');
|
||||
});
|
||||
});
|
||||
|
||||
describe('error handling', () => {
|
||||
let span: Span;
|
||||
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ describe('tracing', () => {
|
|||
beforeEach(() => {
|
||||
mockTracingImplementation = {
|
||||
startSpan: vi.fn(),
|
||||
startNewTraceSpan: vi.fn(),
|
||||
} as unknown as Mocked<Tracer>;
|
||||
|
||||
tracing.setTracingImplementation(noopTracing);
|
||||
|
|
@ -32,6 +33,31 @@ describe('tracing', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('startNewTraceSpan', () => {
|
||||
it('should delegate to the current implementation', async () => {
|
||||
mockTracingImplementation.startNewTraceSpan.mockResolvedValue('test-result');
|
||||
tracing.setTracingImplementation(mockTracingImplementation);
|
||||
|
||||
const options: StartSpanOptions = { name: 'test-span' };
|
||||
const callback = vi.fn().mockResolvedValue('callback-result');
|
||||
|
||||
const result = await tracing.startNewTraceSpan(options, callback);
|
||||
|
||||
expect(mockTracingImplementation.startNewTraceSpan).toHaveBeenCalledWith(options, callback);
|
||||
expect(result).toBe('test-result');
|
||||
});
|
||||
|
||||
it('should use NoopTracing by default', async () => {
|
||||
const options: StartSpanOptions = { name: 'test-span' };
|
||||
const callback = vi.fn().mockResolvedValue('callback-result');
|
||||
|
||||
const result = await tracing.startNewTraceSpan(options, callback);
|
||||
|
||||
expect(callback).toHaveBeenCalledWith(expect.any(EmptySpan));
|
||||
expect(result).toBe('callback-result');
|
||||
});
|
||||
});
|
||||
|
||||
describe('startSpan', () => {
|
||||
it('should delegate to the current implementation', async () => {
|
||||
mockTracingImplementation.startSpan.mockResolvedValue('test-result');
|
||||
|
|
|
|||
|
|
@ -62,4 +62,11 @@ export class NoopTracing implements Tracer {
|
|||
async startSpan<T>(_options: StartSpanOpts, spanCb: (span: Span) => Promise<T>): Promise<T> {
|
||||
return await spanCb(this.emptySpan);
|
||||
}
|
||||
|
||||
async startNewTraceSpan<T>(
|
||||
options: StartSpanOpts,
|
||||
spanCb: (span: Span) => Promise<T>,
|
||||
): Promise<T> {
|
||||
return await this.startSpan(options, spanCb);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ import { SpanStatus, type Span, type StartSpanOpts, type Tracer } from './tracin
|
|||
* Tracing implementation that uses Sentry to trace spans
|
||||
*/
|
||||
export class SentryTracing implements Tracer {
|
||||
constructor(private readonly sentry: Pick<typeof Sentry, 'startSpan'>) {}
|
||||
constructor(private readonly sentry: Pick<typeof Sentry, 'startSpan' | 'startNewTrace'>) {}
|
||||
|
||||
async startSpan<T>(options: StartSpanOpts, spanCb: (span: Span) => Promise<T>): Promise<T> {
|
||||
return await this.sentry.startSpan(options, async (span) => {
|
||||
|
|
@ -24,4 +24,11 @@ export class SentryTracing implements Tracer {
|
|||
}
|
||||
});
|
||||
}
|
||||
|
||||
async startNewTraceSpan<T>(
|
||||
options: StartSpanOpts,
|
||||
spanCb: (span: Span) => Promise<T>,
|
||||
): Promise<T> {
|
||||
return await this.sentry.startNewTrace(async () => await this.startSpan(options, spanCb));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ export type SpanAttributes = SentrySpanAttributes;
|
|||
*/
|
||||
export interface Tracer {
|
||||
startSpan<T>(options: StartSpanOpts, spanCb: (span: Span) => Promise<T>): Promise<T>;
|
||||
startNewTraceSpan<T>(options: StartSpanOpts, spanCb: (span: Span) => Promise<T>): Promise<T>;
|
||||
}
|
||||
|
||||
const COMMON_TRACE_ATTRIBUTES = {
|
||||
|
|
@ -77,6 +78,18 @@ export class Tracing {
|
|||
return await this.tracer.startSpan(options, spanCb);
|
||||
}
|
||||
|
||||
/**
|
||||
* Start a span inside a brand-new root trace, detaching it from any trace
|
||||
* active on the current async context. Use for work that should be its own
|
||||
* transaction rather than a child of whatever trace it was scheduled from.
|
||||
*/
|
||||
async startNewTraceSpan<T>(
|
||||
options: StartSpanOpts,
|
||||
spanCb: (span: Span) => Promise<T>,
|
||||
): Promise<T> {
|
||||
return await this.tracer.startNewTraceSpan(options, spanCb);
|
||||
}
|
||||
|
||||
/** Pick common attributes for a workflow */
|
||||
pickWorkflowAttributes(workflow: Partial<Pick<IWorkflowBase, 'id' | 'name'>>): SpanAttributes {
|
||||
return {
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user