Skip to content

Commit b309c89

Browse files
committed
Bug 1990006 - Handle UDF corner cases.
1 parent a6bd06d commit b309c89

2 files changed

Lines changed: 122 additions & 10 deletions

File tree

  • sql/mozfun/gecko_trace

sql/mozfun/gecko_trace/build_root_span/udf.sql

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,19 @@ RETURNS JSON
33
LANGUAGE js AS r"""
44
const spansById = new Map();
55
let rootSpanId;
6+
if (spans.length === 0) return null;
7+
8+
// Validation: no null span_ids
9+
const nullSpanIds = spans.filter(s => !s.span_id).length;
10+
if (nullSpanIds > 0) {
11+
throw new Error(`Invalid spans: ${nullSpanIds} span(s) have null or missing span_id`);
12+
}
13+
14+
// Validation: at most one explicit root
15+
const explicitRoots = spans.filter(s => !s.parent_span_id).length;
16+
if (explicitRoots > 1) {
17+
throw new Error(`Invalid span tree: found ${explicitRoots} spans with null parent_span_id, expected at most 1`);
18+
}
619
720
spans.forEach((span) => {
821
const spanId = span.span_id;
@@ -39,6 +52,27 @@ LANGUAGE js AS r"""
3952
rootSpanId = missingRoots[0].span_id;
4053
}
4154
55+
// Cycle detection: verify no circular references
56+
const visited = new Set();
57+
const visiting = new Set();
58+
const hasCycle = (spanId) => {
59+
if (visited.has(spanId)) return false;
60+
if (visiting.has(spanId)) return true;
61+
visiting.add(spanId);
62+
const span = spansById.get(spanId);
63+
if (span?.childSpans) {
64+
for (const child of span.childSpans) {
65+
if (hasCycle(child.span_id)) return true;
66+
}
67+
}
68+
visiting.delete(spanId);
69+
visited.add(spanId);
70+
return false;
71+
};
72+
if (hasCycle(rootSpanId)) {
73+
throw new Error(`Invalid span tree: circular dependency detected`);
74+
}
75+
4276
return spansById.get(rootSpanId);
4377
""";
4478

@@ -64,4 +98,29 @@ SELECT
6498
),
6599
"$.span_id"
66100
)
101+
),
102+
-- Test multiple roots validation (should throw)
103+
assert.error(
104+
() => gecko_trace.build_root_span(
105+
[
106+
JSON '{"span_id": "root1", "parent_span_id": null}',
107+
JSON '{"span_id": "root2", "parent_span_id": null}'
108+
]
109+
),
110+
"found 2 spans with null parent_span_id"
111+
),
112+
-- Test null span_id validation (should throw)
113+
assert.error(
114+
() => gecko_trace.build_root_span([JSON '{"span_id": null, "parent_span_id": null}']),
115+
"have null or missing span_id"
116+
),
117+
-- Test circular dependency detection (should throw)
118+
assert.error(
119+
() => gecko_trace.build_root_span(
120+
[
121+
JSON '{"span_id": "a", "parent_span_id": "b"}',
122+
JSON '{"span_id": "b", "parent_span_id": "a"}'
123+
]
124+
),
125+
"circular dependency"
67126
);

sql/mozfun/gecko_trace/calculate_signature/udf.sql

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,30 +34,47 @@ LANGUAGE js AS r"""
3434
3535
const ATTRS_TO_SKIP = {"gecko_process_internal_id": null}
3636
const hashAttrs = (attrs) => {
37+
if (!attrs) return; // Gracefully skip null/undefined attributes
3738
for (const [key, value] of Object.entries(attrs)) {
3839
if (key in ATTRS_TO_SKIP) continue;
39-
hash(key);
40-
hash(value);
40+
hash(String(key));
41+
hash(String(value));
4142
}
4243
}
4344
4445
const hashEvents = (events) => {
46+
if (!events) return; // Gracefully skip null/undefined events array
4547
for (const event of events) {
46-
hash(event.name);
48+
if (!event) continue; // Skip null events in the array
49+
if (event.name) hash(event.name);
4750
hashAttrs(event.attributes);
4851
}
4952
};
5053
54+
if (rootSpan === null) return null;
5155
const stack = [rootSpan];
5256
while (stack.length > 0) {
5357
const span = stack.pop();
54-
hashAttrs(span.resource.attributes);
55-
hash(span.scope.name);
56-
hash(span.name);
58+
if (!span) continue; // Safety check for null span in stack
59+
// Guard on optional resource field per OpenTelemetry spec
60+
if (span.resource) {
61+
hashAttrs(span.resource.attributes);
62+
}
63+
// Guard on optional scope field
64+
if (span.scope) {
65+
hash(span.scope.name);
66+
}
67+
// Guard on optional name field
68+
if (span.name) {
69+
hash(span.name);
70+
}
5771
if (span.events) {
58-
hashEvents(span.events);
72+
hashEvents(span.events);
73+
}
74+
// Guard on childSpans - may be null, undefined, or missing
75+
if (span.childSpans) {
76+
stack.push(...span.childSpans);
5977
}
60-
stack.push(...span.childSpans);
6178
}
6279
6380
return digest;
@@ -80,5 +97,41 @@ SELECT
8097
JSON '{"span_id": "root", "name": "test", "scope": {"name": "test_scope"}, "resource": {"attributes": {}}, "childSpans": []}'
8198
)
8299
),
83-
-- Test that null input returns empty string
84-
assert.equals("", gecko_trace.calculate_signature(NULL));
100+
-- Test that null input returns null
101+
assert.null(gecko_trace.calculate_signature(NULL)),
102+
-- Test robustness: missing resource
103+
assert.not_null(
104+
gecko_trace.calculate_signature(
105+
JSON '{"span_id": "root", "name": "test", "scope": {"name": "test"}, "resource": null, "childSpans": []}'
106+
)
107+
),
108+
-- Test robustness: missing scope
109+
assert.not_null(
110+
gecko_trace.calculate_signature(
111+
JSON '{"span_id": "root", "name": "test", "scope": null, "resource": {"attributes": {}}, "childSpans": []}'
112+
)
113+
),
114+
-- Test robustness: missing childSpans
115+
assert.not_null(
116+
gecko_trace.calculate_signature(
117+
JSON '{"span_id": "root", "name": "test", "scope": {"name": "test"}, "resource": {"attributes": {}}, "childSpans": null}'
118+
)
119+
),
120+
-- Test robustness: null event in events array
121+
assert.not_null(
122+
gecko_trace.calculate_signature(
123+
JSON '{"span_id": "root", "name": "test", "scope": {"name": "test"}, "resource": {"attributes": {}}, "events": [null, {"name": "valid_event", "attributes": {}}], "childSpans": []}'
124+
)
125+
),
126+
-- Test robustness: missing name
127+
assert.not_null(
128+
gecko_trace.calculate_signature(
129+
JSON '{"span_id": "root", "name": null, "scope": {"name": "test"}, "resource": {"attributes": {}}, "childSpans": []}'
130+
)
131+
),
132+
-- Test robustness: null event attributes
133+
assert.not_null(
134+
gecko_trace.calculate_signature(
135+
JSON '{"span_id": "root", "name": "test", "scope": {"name": "test"}, "resource": {"attributes": {}}, "events": [{"name": "event1", "attributes": null}], "childSpans": []}'
136+
)
137+
);

0 commit comments

Comments
 (0)