Skip to content

Commit 2b0037a

Browse files
portertechjpkrohling
authored andcommitted
[processor/tailsampling] Late span age histogram should include sampled traces (open-telemetry#37180)
Currently, the [late span age histogram](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/981e9f5cc58149ea37c8909a4a6eb30faee96351/processor/tailsamplingprocessor/internal/metadata/generated_telemetry.go#L106-L110) only includes non-sampled traces. The histogram should also include sampled traces. This pull request includes anything with a decision time. _Note: The decision cache still wrecks it 😅_ --------- Signed-off-by: Sean Porter <[email protected]> Co-authored-by: Juraci Paixão Kröhling <[email protected]>
1 parent 5ed328a commit 2b0037a

File tree

3 files changed

+58
-15
lines changed

3 files changed

+58
-15
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: tailsamplingprocessor
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Late span age histogram should include sampled traces.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [37180]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: [user]

processor/tailsamplingprocessor/processor.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -518,11 +518,14 @@ func (tsp *tailSamplingSpanProcessor) processTraces(resourceSpans ptrace.Resourc
518518
tsp.releaseSampledTrace(tsp.ctx, id, traceTd)
519519
case sampling.NotSampled:
520520
tsp.nonSampledIDCache.Put(id, true)
521-
tsp.telemetry.ProcessorTailSamplingSamplingLateSpanAge.Record(tsp.ctx, int64(time.Since(actualData.DecisionTime)/time.Second))
522521
default:
523522
tsp.logger.Warn("Encountered unexpected sampling decision",
524523
zap.Int("decision", int(finalDecision)))
525524
}
525+
526+
if !actualData.DecisionTime.IsZero() {
527+
tsp.telemetry.ProcessorTailSamplingSamplingLateSpanAge.Record(tsp.ctx, int64(time.Since(actualData.DecisionTime)/time.Second))
528+
}
526529
}
527530
}
528531

processor/tailsamplingprocessor/processor_telemetry_test.go

+27-14
Original file line numberDiff line numberDiff line change
@@ -455,10 +455,10 @@ func TestProcessorTailSamplingSamplingLateSpanAge(t *testing.T) {
455455
PolicyCfgs: []PolicyCfg{
456456
{
457457
sharedPolicyCfg: sharedPolicyCfg{
458-
Name: "never-sample",
458+
Name: "sample-half",
459459
Type: Probabilistic,
460460
ProbabilisticCfg: ProbabilisticCfg{
461-
SamplingPercentage: 0,
461+
SamplingPercentage: 50,
462462
},
463463
},
464464
},
@@ -476,22 +476,24 @@ func TestProcessorTailSamplingSamplingLateSpanAge(t *testing.T) {
476476
err = proc.Start(context.Background(), componenttest.NewNopHost())
477477
require.NoError(t, err)
478478

479-
traces := simpleTraces()
480-
traceID := traces.ResourceSpans().At(0).ScopeSpans().AppendEmpty().Spans().AppendEmpty().TraceID()
481-
482-
lateSpan := ptrace.NewTraces()
483-
lateSpan.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty().Spans().AppendEmpty().SetTraceID(traceID)
484-
485479
// test
486-
err = proc.ConsumeTraces(context.Background(), traces)
487-
require.NoError(t, err)
480+
traceIDs, batches := generateIDsAndBatches(10)
481+
for _, batch := range batches {
482+
err = proc.ConsumeTraces(context.Background(), batch)
483+
require.NoError(t, err)
484+
}
488485

489486
tsp := proc.(*tailSamplingSpanProcessor)
490487
tsp.policyTicker.OnTick() // the first tick always gets an empty batch
491488
tsp.policyTicker.OnTick()
492489

493-
err = proc.ConsumeTraces(context.Background(), lateSpan)
494-
require.NoError(t, err)
490+
for _, traceID := range traceIDs {
491+
lateSpan := ptrace.NewTraces()
492+
lateSpan.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty().Spans().AppendEmpty().SetTraceID(traceID)
493+
494+
err = proc.ConsumeTraces(context.Background(), lateSpan)
495+
require.NoError(t, err)
496+
}
495497

496498
// verify
497499
var md metricdata.ResourceMetrics
@@ -503,11 +505,22 @@ func TestProcessorTailSamplingSamplingLateSpanAge(t *testing.T) {
503505
Unit: "s",
504506
Data: metricdata.Histogram[int64]{
505507
Temporality: metricdata.CumulativeTemporality,
506-
DataPoints: []metricdata.HistogramDataPoint[int64]{{}},
508+
DataPoints: []metricdata.HistogramDataPoint[int64]{
509+
{
510+
Count: 10,
511+
Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000},
512+
BucketCounts: []uint64{10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
513+
Min: metricdata.NewExtrema[int64](0),
514+
Max: metricdata.NewExtrema[int64](0),
515+
Sum: 0,
516+
},
517+
},
507518
},
508519
}
520+
509521
got := s.getMetric(m.Name, md)
510-
metricdatatest.AssertEqual(t, m, got, metricdatatest.IgnoreTimestamp(), metricdatatest.IgnoreValue())
522+
523+
metricdatatest.AssertEqual(t, m, got, metricdatatest.IgnoreTimestamp())
511524
}
512525

513526
type testTelemetry struct {

0 commit comments

Comments
 (0)