Skip to content

Commit

Permalink
[processor/spanmetrics] Clamp negative duration times to 0 bucket. (#…
Browse files Browse the repository at this point in the history
…9891)

* Clamp negative duration times to 0 bucket.

* Added changelog entry.

* Fixed tracking issues since it doesn't seem like you can reference other project issues in the list.
  • Loading branch information
crobertson-conga authored Aug 3, 2022
1 parent 9dead84 commit facab6e
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 1 deletion.
8 changes: 7 additions & 1 deletion processor/spanmetricsprocessor/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,13 @@ func (p *processorImp) aggregateMetricsForServiceSpans(rspans ptrace.ResourceSpa
}

func (p *processorImp) aggregateMetricsForSpan(serviceName string, span ptrace.Span, resourceAttr pcommon.Map) {
latencyInMilliseconds := float64(span.EndTimestamp()-span.StartTimestamp()) / float64(time.Millisecond.Nanoseconds())
// Protect against end timestamps before start timestamps. Assume 0 duration.
latencyInMilliseconds := float64(0)
startTime := span.StartTimestamp()
endTime := span.EndTimestamp()
if endTime > startTime {
latencyInMilliseconds = float64(endTime-startTime) / float64(time.Millisecond.Nanoseconds())
}

// Binary search to find the latencyInMilliseconds bucket index.
index := sort.SearchFloat64s(p.latencyBounds, latencyInMilliseconds)
Expand Down
21 changes: 21 additions & 0 deletions processor/spanmetricsprocessor/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,13 @@ func TestProcessorConsumeTraces(t *testing.T) {
verifier: verifyConsumeMetricsInputDelta,
traces: []ptrace.Traces{buildSampleTrace(), buildSampleTrace()},
},
{
// Consumptions with improper timestamps
name: "Test bad consumptions (Delta).",
aggregationTemporality: cumulative,
verifier: verifyBadMetricsOkay,
traces: []ptrace.Traces{buildBadSampleTrace()},
},
}

for _, tc := range testcases {
Expand Down Expand Up @@ -404,6 +411,10 @@ func verifyConsumeMetricsInputCumulative(t testing.TB, input pmetric.Metrics) bo
return verifyConsumeMetricsInput(t, input, pmetric.MetricAggregationTemporalityCumulative, 1)
}

func verifyBadMetricsOkay(t testing.TB, input pmetric.Metrics) bool {
return true // Validating no exception
}

// verifyConsumeMetricsInputDelta expects one accumulation of metrics, and marked as delta
func verifyConsumeMetricsInputDelta(t testing.TB, input pmetric.Metrics) bool {
return verifyConsumeMetricsInput(t, input, pmetric.MetricAggregationTemporalityDelta, 1)
Expand Down Expand Up @@ -539,6 +550,16 @@ func verifyMetricLabels(dp metricDataPoint, t testing.TB, seenMetricIDs map[metr
seenMetricIDs[mID] = true
}

func buildBadSampleTrace() ptrace.Traces {
badTrace := buildSampleTrace()
span := badTrace.ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0)
now := time.Now()
// Flipping timestamp for a bad duration
span.SetEndTimestamp(pcommon.NewTimestampFromTime(now))
span.SetStartTimestamp(pcommon.NewTimestampFromTime(now.Add(sampleLatencyDuration)))
return badTrace
}

// buildSampleTrace builds the following trace:
// service-a/ping (server) ->
// service-a/ping (client) ->
Expand Down
17 changes: 17 additions & 0 deletions unreleased/spanmetrics_7250.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: spanmetricsprocessor

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Modifies spanmetrics processor to handle negative durations without crashing. Related to open-telemetry/opentelemetry-js-contrib#1013


# One or more tracking issues related to the change
issues: [7250]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: Sets negative durations to count towards the smallest histogram bucket.

0 comments on commit facab6e

Please sign in to comment.