Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: non-root spans with FollowsFrom reference incorrectly represent as trace in collector metrics #4380

Closed
ChenX1993 opened this issue Apr 12, 2023 · 2 comments · Fixed by #4382
Labels

Comments

@ChenX1993
Copy link
Contributor

ChenX1993 commented Apr 12, 2023

What happened?

We noticed that non-root spans with FollowsFrom reference are showing as traces in collector metrics, e.g. traces.received, traces.rejected metrics.

The reason is that in collector metrics, ParentSpanID() is used to determine if a span is a root span, e.g. below

m.countSpansByServiceName(serviceName, span.Flags.IsDebug())
if span.ParentSpanID() == 0 {
m.countTracesByServiceName(serviceName, span.Flags.IsDebug(), span.
GetSamplerType())
}

And the current implementation of ParentSpanID() only considers ChildOf relationship. So if a span has only FollowsFrom reference, the ParentSpanID() doesn't consider that and returns 0.

jaeger/model/span.go

Lines 99 to 109 in 506adaf

// ParentSpanID returns ID of a parent span if it exists.
// It searches for the first child-of reference pointing to the same trace ID.
func (s *Span) ParentSpanID() SpanID {
for i := range s.References {
ref := &s.References[i]
if ref.TraceID == s.TraceID && ref.RefType == ChildOf {
return ref.SpanID
}
}
return SpanID(0)
}

Not sure any particular reason that FollowsFrom is not considered in the current implementation of ParentSpanID().

The proposal is to either 1) fix in collector metric to replace the ParentSpanID(), 2) fix in ParentSpanID().
Prefer 2) because we use this method a lot to determine a span is a root span internally, but changing its behavior might break other places in jaeger OSS.

Steps to reproduce

Create spans with FollowsFrom relationship, and check the collector trace metrics.

Expected behavior

non-root spans with FollowsFrom reference should not show as trace in collector metrics.

Relevant log output

No response

Screenshot

No response

Additional context

No response

Jaeger backend version

No response

SDK

No response

Pipeline

No response

Stogage backend

No response

Operating system

No response

Deployment model

No response

Deployment configs

No response

@ChenX1993 ChenX1993 added the bug label Apr 12, 2023
@kubarydz
Copy link
Contributor

Would chaning ParentSpanID() behaviour in 2) way also resolve this (rather old one) TODO?

// finds all spans that have no parent, i.e. where parentID is either 0
// or points to an ID for which there is no span.
func (a *clockSkewAdjuster) buildSubGraphs() {
a.roots = make(map[model.SpanID]*node)
for _, n := range a.spans {
// TODO handle FOLLOWS_FROM references
if n.span.ParentSpanID() == 0 {
a.roots[n.span.SpanID] = n
continue
}

@ChenX1993
Copy link
Contributor Author

I think so.

yurishkuro added a commit that referenced this issue Apr 16, 2023
## Which problem is this PR solving?
Resolves #4380

## Short description of the changes
handle FOLLOWS_FROM as a parent span ID

---------

Signed-off-by: Jakub Rydz <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants