Skip to content

Commit 550fe32

Browse files
waleedlatif1waleedlatif1
authored
fix(logs): name display fields and real span counts in trace projection warnings (#9690)
1 parent 17b5a70 commit 550fe32

4 files changed

Lines changed: 120 additions & 31 deletions

File tree

‎apps/sim/lib/logs/execution/trace-secret-projection.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1091,7 +1091,7 @@ describe('projectTraceSpansForSecrets whole-trace content budget', () => {
10911091
}
10921092
expect(JSON.stringify(projected)).not.toContain('top-secret')
10931093
expect(warnMock).toHaveBeenCalledWith(
1094-
'Trace content exceeded the projection budget; withholding the remaining spans',
1094+
'Trace content exceeded the projection budget; withholding the remaining content',
10951095
expect.objectContaining({
10961096
executionId: 'execution-1',
10971097
phase: 'read',

‎apps/sim/lib/logs/execution/trace-secret-projection.ts‎

Lines changed: 77 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ interface ProjectionContext {
8585
/** Content fields omitted so far; a span compares it across its own fields to set `contentWithheld`. */
8686
omittedContentFields: number
8787
logDetails: ProjectionLogDetails
88+
displayFieldsBySpanId?: ReadonlyMap<string, string>
8889
}
8990

9091
interface OversizedHydrationGate {
@@ -124,6 +125,11 @@ export interface ProjectTraceSpansForSecretsOptions {
124125
store: LargeValueStoreContext
125126
/** Read-only display projections omit ref-backed content instead of writing another ref. */
126127
allowLargeValueWrites?: boolean
128+
/**
129+
* Synthetic spans that each carry one display-envelope value, keyed by span id. Warnings name
130+
* the envelope field instead of the synthetic span and count it apart from real spans.
131+
*/
132+
displayFieldsBySpanId?: ReadonlyMap<string, string>
127133
}
128134

129135
class TraceSecretProjectionError extends Error {
@@ -188,7 +194,9 @@ function describeProjectionFailure(error: unknown): { name: string; reason?: str
188194
function createProjectionContext(
189195
matcher: ResolvedSecretMatcher,
190196
store: LargeValueStoreContext,
191-
allowLargeValueWrites: boolean
197+
allowLargeValueWrites: boolean,
198+
logDetails: ProjectionLogDetails,
199+
displayFieldsBySpanId?: ReadonlyMap<string, string>
192200
): ProjectionContext {
193201
return {
194202
matcher,
@@ -206,7 +214,8 @@ function createProjectionContext(
206214
sourceLargeValueBytes: 0,
207215
storedLargeValueBytes: 0,
208216
storedLargeValueCount: 0,
209-
logDetails: projectionLogDetails(store, allowLargeValueWrites ? 'write' : 'read'),
217+
logDetails,
218+
displayFieldsBySpanId,
210219
}
211220
}
212221

@@ -849,11 +858,12 @@ async function sanitizeContentField(
849858
try {
850859
return await sanitizeMaterializedValue(value, context)
851860
} catch (error) {
861+
const displayField = context.displayFieldsBySpanId?.get(owner.id)
852862
logger.warn('Omitting trace content that could not be sanitized', {
853863
...context.logDetails,
854-
spanId: owner.id,
855-
spanType: owner.type,
856-
field,
864+
...(displayField === undefined
865+
? { spanId: owner.id, spanType: owner.type, field }
866+
: { displayField }),
857867
failure: describeProjectionFailure(error),
858868
})
859869
context.omittedContentFields += 1
@@ -1269,32 +1279,41 @@ function projectBoundedTraceSpans(
12691279
state: BoundedTraceStructureState,
12701280
depth: number
12711281
) => TraceSpan | undefined,
1282+
logDetails: ProjectionLogDetails,
12721283
state: BoundedTraceStructureState = { nodes: 0, truncated: false }
12731284
): TraceSpan[] {
12741285
const projected = projectBoundedArray(traceSpans, (span) => project(span, state, 0))
12751286
if (state.truncated) {
1276-
logger.warn('Trace structure exceeded projection limits; truncating projected structure')
1287+
logger.warn(
1288+
'Trace structure exceeded projection limits; truncating projected structure',
1289+
logDetails
1290+
)
12771291
}
12781292
return projected
12791293
}
12801294

12811295
/** Pass `state` to continue a structure budget that earlier spans of the same trace already spent. */
12821296
function structuralOnlyTraceSpans(
12831297
traceSpans: TraceSpan[],
1298+
logDetails: ProjectionLogDetails,
12841299
state?: BoundedTraceStructureState
12851300
): TraceSpan[] {
12861301
try {
1287-
return projectBoundedTraceSpans(traceSpans, structuralOnlySpan, state)
1302+
return projectBoundedTraceSpans(traceSpans, structuralOnlySpan, logDetails, state)
12881303
} catch (error) {
12891304
logger.warn('Trace structure could not be safely traversed; omitting projected spans', {
1305+
...logDetails,
12901306
failure: describeProjectionFailure(error),
12911307
})
12921308
return []
12931309
}
12941310
}
12951311

1296-
function cloneTraceSpansForProjection(traceSpans: TraceSpan[]): TraceSpan[] {
1297-
return projectBoundedTraceSpans(traceSpans, cloneTraceSpanForProjection)
1312+
function cloneTraceSpansForProjection(
1313+
traceSpans: TraceSpan[],
1314+
logDetails: ProjectionLogDetails
1315+
): TraceSpan[] {
1316+
return projectBoundedTraceSpans(traceSpans, cloneTraceSpanForProjection, logDetails)
12981317
}
12991318

13001319
function assertNoPlaintext(
@@ -1600,9 +1619,10 @@ async function verifyPostTransformLargeValues(
16001619
async function assertPostTransformTraceSpansAreSafe(
16011620
traceSpans: TraceSpan[],
16021621
matcher: ResolvedSecretMatcher,
1603-
store: LargeValueStoreContext
1622+
store: LargeValueStoreContext,
1623+
logDetails: ProjectionLogDetails
16041624
): Promise<void> {
1605-
const projection = createProjectionContext(matcher, store, false)
1625+
const projection = createProjectionContext(matcher, store, false, logDetails)
16061626
const context: PostTransformInvariantContext = {
16071627
projection,
16081628
plaintext: {
@@ -1623,6 +1643,32 @@ async function assertPostTransformTraceSpansAreSafe(
16231643
await verifyPostTransformLargeValues(refs, context, new Set<string>())
16241644
}
16251645

1646+
/**
1647+
* Counts the real spans kept before `withheldFrom` and withheld from it on, naming the withheld
1648+
* display-envelope fields apart from them.
1649+
*/
1650+
function describeWithheldContent(
1651+
traceSpans: readonly TraceSpan[],
1652+
withheldFrom: number,
1653+
displayFieldsBySpanId: ReadonlyMap<string, string> | undefined
1654+
): { retainedSpanCount: number; withheldSpanCount: number; withheldDisplayFields?: string[] } {
1655+
let retainedDisplayFieldCount = 0
1656+
const withheldDisplayFields: string[] = []
1657+
if (displayFieldsBySpanId?.size) {
1658+
for (let index = 0; index < traceSpans.length; index += 1) {
1659+
const displayField = displayFieldsBySpanId.get(traceSpans[index].id)
1660+
if (displayField === undefined) continue
1661+
if (index < withheldFrom) retainedDisplayFieldCount += 1
1662+
else withheldDisplayFields.push(displayField)
1663+
}
1664+
}
1665+
return {
1666+
retainedSpanCount: withheldFrom - retainedDisplayFieldCount,
1667+
withheldSpanCount: traceSpans.length - withheldFrom - withheldDisplayFields.length,
1668+
...(withheldDisplayFields.length > 0 ? { withheldDisplayFields } : {}),
1669+
}
1670+
}
1671+
16261672
export interface EnforceTraceSpanSecretInvariantOptions {
16271673
registry?: ResolvedSecretTraceRegistry
16281674
store: LargeValueStoreContext
@@ -1638,22 +1684,23 @@ export async function enforceTraceSpanSecretInvariant(
16381684
traceSpans: TraceSpan[],
16391685
options: EnforceTraceSpanSecretInvariantOptions
16401686
): Promise<TraceSpan[]> {
1687+
const logDetails = projectionLogDetails(options.store, 'write')
16411688
try {
1642-
if (!options.registry?.isComplete()) return structuralOnlyTraceSpans(traceSpans)
1689+
if (!options.registry?.isComplete()) return structuralOnlyTraceSpans(traceSpans, logDetails)
16431690

16441691
const matcher = createResolvedSecretMatcher(options.registry.getActiveMatches(), {
16451692
preserveNamedProvenanceLabels: true,
16461693
})
16471694
if (!matcher) return traceSpans
16481695

1649-
await assertPostTransformTraceSpansAreSafe(traceSpans, matcher, options.store)
1696+
await assertPostTransformTraceSpansAreSafe(traceSpans, matcher, options.store, logDetails)
16501697
return traceSpans
16511698
} catch (error) {
16521699
logger.warn('Trace secret invariant failed; retaining structural spans only', {
1653-
...projectionLogDetails(options.store, 'write'),
1700+
...logDetails,
16541701
failure: describeProjectionFailure(error),
16551702
})
1656-
return structuralOnlyTraceSpans(traceSpans)
1703+
return structuralOnlyTraceSpans(traceSpans, logDetails)
16571704
}
16581705
}
16591706

@@ -1665,19 +1712,25 @@ export async function projectTraceSpansForSecrets(
16651712
traceSpans: TraceSpan[],
16661713
options: ProjectTraceSpansForSecretsOptions
16671714
): Promise<TraceSpan[]> {
1715+
const allowLargeValueWrites = options.allowLargeValueWrites !== false
1716+
const logDetails = projectionLogDetails(options.store, allowLargeValueWrites ? 'write' : 'read')
16681717
if (!options.registry?.isComplete()) {
1669-
return structuralOnlyTraceSpans(traceSpans)
1718+
return structuralOnlyTraceSpans(traceSpans, logDetails)
16701719
}
16711720

1672-
const allowLargeValueWrites = options.allowLargeValueWrites !== false
1673-
const logDetails = projectionLogDetails(options.store, allowLargeValueWrites ? 'write' : 'read')
16741721
try {
16751722
const matcher = createResolvedSecretMatcher(options.registry.getActiveMatches(), {
16761723
preserveNamedProvenanceLabels: true,
16771724
})
1678-
if (!matcher) return cloneTraceSpansForProjection(traceSpans)
1725+
if (!matcher) return cloneTraceSpansForProjection(traceSpans, logDetails)
16791726

1680-
const context = createProjectionContext(matcher, options.store, allowLargeValueWrites)
1727+
const context = createProjectionContext(
1728+
matcher,
1729+
options.store,
1730+
allowLargeValueWrites,
1731+
logDetails,
1732+
options.displayFieldsBySpanId
1733+
)
16811734

16821735
const projected: TraceSpan[] = []
16831736
const state: BoundedTraceStructureState = { nodes: 0, truncated: false }
@@ -1698,17 +1751,16 @@ export async function projectTraceSpansForSecrets(
16981751
* spans left it, so the trace keeps exactly the structure a whole-trace fallback would.
16991752
*/
17001753
logger.warn(
1701-
'Trace content exceeded the projection budget; withholding the remaining spans',
1754+
'Trace content exceeded the projection budget; withholding the remaining content',
17021755
{
17031756
...logDetails,
1704-
retainedSpanCount: index,
1705-
withheldSpanCount: traceSpans.length - index,
1757+
...describeWithheldContent(traceSpans, index, options.displayFieldsBySpanId),
17061758
failure: describeProjectionFailure(error),
17071759
}
17081760
)
17091761
return [
17101762
...projected,
1711-
...structuralOnlyTraceSpans(traceSpans.slice(index), {
1763+
...structuralOnlyTraceSpans(traceSpans.slice(index), logDetails, {
17121764
nodes: retainedStructureNodes,
17131765
truncated: false,
17141766
}),
@@ -1721,6 +1773,6 @@ export async function projectTraceSpansForSecrets(
17211773
...logDetails,
17221774
failure: describeProjectionFailure(error),
17231775
})
1724-
return structuralOnlyTraceSpans(traceSpans)
1776+
return structuralOnlyTraceSpans(traceSpans, logDetails)
17251777
}
17261778
}

‎apps/sim/lib/logs/execution/trace-store.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const materializeLargeValueRefMock = executionPayloadStoreMockFns.mockMaterializ
2828
const storeExecutionTraceArchiveMock = executionPayloadStoreMockFns.mockStoreExecutionTraceArchive
2929

3030
const mockLogger = getMockLogger('TraceStore')
31+
const projectionLogger = getMockLogger('TraceSecretProjection')
3132

3233
const decryptSecretMock = encryptionMockFns.mockDecryptSecret
3334

@@ -555,6 +556,17 @@ describe('projectExecutionDataForDisplay envelope isolation', () => {
555556
expect(displayData.error).toBe('Request failed with {{OPENAI_API_KEY}}')
556557
expect(displayData.message).toBe('Run failed')
557558
expect(displayData).not.toHaveProperty('blockExecutions')
559+
expect(projectionLogger.warn).toHaveBeenCalledWith(
560+
'Omitting trace content that could not be sanitized',
561+
{
562+
executionId: CONTEXT.executionId,
563+
workflowId: CONTEXT.workflowId,
564+
workspaceId: CONTEXT.workspaceId,
565+
phase: 'read',
566+
displayField: 'blockExecutions',
567+
failure: expect.objectContaining({ name: 'TraceSecretProjectionError' }),
568+
}
569+
)
558570
expect(displayData.traceSpans).toEqual([
559571
expect.objectContaining({ id: 'span-1', output: { result: '{{OPENAI_API_KEY}}' } }),
560572
])
@@ -600,6 +612,14 @@ describe('projectExecutionDataForDisplay envelope isolation', () => {
600612

601613
expect(displayData.error).toBe('Request failed with {{OPENAI_API_KEY}}')
602614
expect(displayData).not.toHaveProperty('blockExecutions')
615+
expect(projectionLogger.warn).toHaveBeenCalledWith(
616+
'Trace content exceeded the projection budget; withholding the remaining content',
617+
expect.objectContaining({
618+
retainedSpanCount: 1,
619+
withheldSpanCount: 0,
620+
withheldDisplayFields: ['blockExecutions'],
621+
})
622+
)
603623
const [span] = displayData.traceSpans as TraceSpan[]
604624
expect(span.output?.result).toBe('{{OPENAI_API_KEY}}')
605625
expect(span).not.toHaveProperty('contentWithheld')

‎apps/sim/lib/logs/execution/trace-store.ts‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -476,10 +476,11 @@ export async function materializeExecutionDataForDisplayWithBlockOutputs(
476476
registry = blockImport.registry
477477
}
478478
const now = new Date().toISOString()
479+
const blockOutputSpanId = `${LOG_DISPLAY_PROJECTION_SPAN_ID}-block-output`
479480
const [projected] = await projectTraceSpansForSecrets(
480481
[
481482
{
482-
id: `${LOG_DISPLAY_PROJECTION_SPAN_ID}-block-output`,
483+
id: blockOutputSpanId,
483484
name: 'Block Output Display Projection',
484485
type: 'display',
485486
duration: 0,
@@ -488,7 +489,12 @@ export async function materializeExecutionDataForDisplayWithBlockOutputs(
488489
output: { value: blockState.output },
489490
},
490491
],
491-
{ registry, allowLargeValueWrites: false, store: projectionStore }
492+
{
493+
registry,
494+
allowLargeValueWrites: false,
495+
store: projectionStore,
496+
displayFieldsBySpanId: new Map([[blockOutputSpanId, `blockOutput:${blockId}`]]),
497+
}
492498
)
493499
if (projected?.output && Object.hasOwn(projected.output, 'value')) {
494500
blockOutputs.set(blockId, projected.output.value)
@@ -723,10 +729,11 @@ export async function projectExecutionDataForDisplay(
723729
exactRegistry.markIncomplete('untrusted-provenance', { origin: 'traceStore.exactProvenance' })
724730
}
725731

732+
const exactValueSpanId = `${LOG_DISPLAY_PROJECTION_SPAN_ID}-${valueKey}`
726733
const [projected] = await projectTraceSpansForSecrets(
727734
[
728735
{
729-
id: `${LOG_DISPLAY_PROJECTION_SPAN_ID}-${valueKey}`,
736+
id: exactValueSpanId,
730737
name: 'Exact Log Value Display Projection',
731738
type: 'display',
732739
duration: 0,
@@ -735,7 +742,12 @@ export async function projectExecutionDataForDisplay(
735742
output: { value: executionData[valueKey] },
736743
},
737744
],
738-
{ registry: exactRegistry, allowLargeValueWrites: false, store: projectionStore }
745+
{
746+
registry: exactRegistry,
747+
allowLargeValueWrites: false,
748+
store: projectionStore,
749+
displayFieldsBySpanId: new Map([[exactValueSpanId, valueKey]]),
750+
}
739751
)
740752
if (projected?.output && Object.hasOwn(projected.output, 'value')) {
741753
exactValueProjections.set(valueKey, projected.output.value)
@@ -781,7 +793,12 @@ export async function projectExecutionDataForDisplay(
781793
...spansToProject,
782794
...envelopeSpans(TRAILING_LOG_DISPLAY_CONTENT_KEYS),
783795
],
784-
{ registry, allowLargeValueWrites: false, store: projectionStore }
796+
{
797+
registry,
798+
allowLargeValueWrites: false,
799+
store: projectionStore,
800+
displayFieldsBySpanId: envelopeKeysBySpanId,
801+
}
785802
)
786803

787804
const displayData = omit(executionData, [

0 commit comments

Comments
 (0)