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

Track span count and optionally add it to root #532

Merged
merged 8 commits into from
Oct 6, 2022
Merged

Conversation

kentquirk
Copy link
Contributor

Which problem is this PR solving?

Adds the ability to add the trace's span count to the root span of the trace, and does it in a way that allows the rules engine to query it as meta.span_count. This would allow rules that depend on span count -- for example, a rule that adjusts sample rate so that larger traces are kept less often.

Note that this value is not available if the root span hasn't arrived when the TraceTimeout completes; at this point, the rules will be evaluated without a span_count because there is no root span to put it on.

If the root span arrives late, it will be updated with the total number of spans that have arrived at that point, so that the value stored in Honeycomb is close to correct. If more spans arrive later, they will not update the value in the root span.

Closes #524
Closes #483

@kentquirk kentquirk force-pushed the kent.add_span_count branch from a319e19 to 77a31ed Compare October 3, 2022 18:53
@kentquirk kentquirk marked this pull request as ready for review October 4, 2022 19:52
@kentquirk kentquirk requested review from a team and JamieDanielson October 4, 2022 19:52
@kentquirk kentquirk added type: enhancement New feature or request version: bump minor A PR that adds behavior, but is backwards-compatible. labels Oct 5, 2022
@vreynolds vreynolds requested a review from TylerHelmuth October 5, 2022 15:17
@kentquirk kentquirk assigned kentquirk and TylerHelmuth and unassigned kentquirk Oct 5, 2022
sample/rules.go Show resolved Hide resolved
collect/collect_test.go Show resolved Hide resolved
@TylerHelmuth
Copy link
Contributor

@kentquirk looks like there are some build issues. Rerunning them didn't work.

Copy link
Contributor

@TylerHelmuth TylerHelmuth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -68,6 +68,11 @@ func (t *Trace) GetSpans() []*Span {
return t.spans
}

// SpanCount gets the number of spans currently in this trace as int64
func (t *Trace) SpanCount() int64 {
return int64(len(t.spans))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #555 for correcting the reported value.

ghost pushed a commit to opsramp/tracing-proxy that referenced this pull request Jul 5, 2024
## Which problem is this PR solving?

Adds the ability to add the trace's span count to the root span of the
trace, and does it in a way that allows the rules engine to query it as
`meta.span_count`. This would allow rules that depend on span count --
for example, a rule that adjusts sample rate so that larger traces are
kept less often.

Note that this value is not available if the root span hasn't arrived
when the TraceTimeout completes; at this point, the rules will be
evaluated without a `span_count` because there is no root span to put it
on.

If the root span arrives late, it will be updated with the total number
of spans that have arrived at that point, so that the value stored in
Honeycomb is close to correct. If more spans arrive later, they will not
update the value in the root span.

Closes honeycombio#524  
Closes honeycombio#483
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request version: bump minor A PR that adds behavior, but is backwards-compatible.
Projects
None yet
3 participants