Skip to content

Commit

Permalink
fix: Don’t consider log events as root spans (#1208)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?

Log events should never be considered root spans even though they do not
contain a `trace.parent_id`. A log event that is part of a trace would
mark the trace complete as if it's considered a root span.

This change updates the check for whether a span is considered a root
span to also check for the meta field `meta.signal_type`. If the field
exists and is set to "log", we return false.

## Short description of the changes
- Update `InMemoryCollector.isRootSpan` to check if `meta.signal_type`
and has a value of "log"
- Add unit tests for InMemoryCollector to verify expected behaviour for
isRootSpan
  • Loading branch information
MikeGoldsmith authored Jun 17, 2024
1 parent 4260ba8 commit eb76cf4
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 0 deletions.
5 changes: 5 additions & 0 deletions collect/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,11 @@ func mergeTraceAndSpanSampleRates(sp *types.Span, traceSampleRate uint, dryRunMo
}

func (i *InMemCollector) isRootSpan(sp *types.Span) bool {
// log event should never be considered a root span, check for that first
if signalType, _ := sp.Data["meta.signal_type"]; signalType == "log" {
return false
}
// check if the event has a parent id using the configured parent id field names
for _, parentIdFieldName := range i.Config.GetParentIdFieldNames() {
parentId := sp.Data[parentIdFieldName]
if _, ok := parentId.(string); ok {
Expand Down
63 changes: 63 additions & 0 deletions collect/collect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1364,3 +1364,66 @@ func TestSpanWithRuleReasons(t *testing.T) {
}
transmission.Mux.RUnlock()
}

func TestIsRootSpan(t *testing.T) {
tesCases := []struct {
name string
span *types.Span
expected bool
}{
{
name: "root span - no parent id",
span: &types.Span{
Event: types.Event{
Data: map[string]interface{}{},
},
},
expected: true,
},
{
name: "non-root span - empty parent id",
span: &types.Span{
Event: types.Event{
Data: map[string]interface{}{
"trace.parent_id": "",
},
},
},
expected: false,
},
{
name: "non-root span - parent id",
span: &types.Span{
Event: types.Event{
Data: map[string]interface{}{
"trace.parent_id": "some-id",
},
},
},
expected: false,
},
{
name: "non-root span - no parent id but has signal_type of log",
span: &types.Span{
Event: types.Event{
Data: map[string]interface{}{
"meta.signal_type": "log",
},
},
},
expected: false,
},
}

collector := &InMemCollector{
Config: &config.MockConfig{
ParentIdFieldNames: []string{"trace.parent_id", "parentId"},
},
}

for _, tc := range tesCases {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expected, collector.isRootSpan(tc.span))
})
}
}

0 comments on commit eb76cf4

Please sign in to comment.