[processor/spanmetrics] Possibility to acces index out of range and crash

[Tenaria]

Describe the bug
It is possible for the span metrics processor to access an index out of range hence causing the collector to crash.

The bug occurs on this line and can happen as the latencyInMilliseconds value is searched for in the slice p.latencyBounds. However, the last value in the bounds representing the "infinity bucket" is defined by math.MaxInt64 but the latencyInMilliseconds value is a float64 in which the maximum value can exceed the maximum value defined by an int64. This means that the binary search won't find the value and will instead default to returning the index where the value should be inserted i.e an index that currently does not exist. The index is then accessed on this line at which point an out of bounds error is thrown.

Steps to reproduce
Set the end time of a span to the current time and the start time of a span to 0 and the application will crash. (This usually should not happen but is a way to trigger the bug)

What did you expect to see?
The last value in the slice should be incremented (the value representing "infinity" in histograms).

What did you see instead?
The span metrics processor caused the collector to crash due to accessing an index that does not exist. Error: panic: runtime error: index out of range [17] with length 17

What version did you use?
Version: v0.42.0

[jpkrohling]

cc @albertteoh

[tbthanh90]

I am facing the same issue too, do we have plan to fix it soon ? Thanks

[crobertson-conga]

I replicate this when EndTime is before StartTime on the spans. A negative value for duration causes the binary search to overflow and try to find a value greater than the max duration already set.

I think a sane option here might just be to check for the case where the index is out of bounds and drop the data point. i.e. on line 387 of the current processor do something like

	// Binary search to find the latencyInMilliseconds bucket index.
	index := sort.SearchFloat64s(p.latencyBounds, latencyInMilliseconds)
	// Negative durations wrap and so will result in out of bounds panic exception
	if index >= len(p.latencyBounds) {
		index = 0
		latencyInMilliseconds = 0
	}

Note, I've seen negative durations specifically leveraging spans generated from opentelemetry-js browser integrations.

[crobertson-conga]

I think this is now fixed with #9891

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.