Skip to content

Commit

Permalink
[exporter/elasticsearch] [chore] fix lint issues with golangci-lint 1…
Browse files Browse the repository at this point in the history
….62 (open-telemetry#36798)

#### Description

The golangci-lint upgrades causes a lot gosec issues due to a new check
for integer conversion overflow.
This PR checks for integer overflows within the elasticsearch exporter
package.


#### Link to tracking issue
Related:
open-telemetry#36638

---------

Co-authored-by: Carson Ip <[email protected]>
Co-authored-by: Tim Rühsen <[email protected]>
Co-authored-by: Christos Markou <[email protected]>
  • Loading branch information
4 people authored and sbylica-splunk committed Dec 17, 2024
1 parent 2e17d0a commit ce3d905
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ func ToTDigest(dp pmetric.ExponentialHistogramDataPoint) (counts []int64, values
}
lb := -LowerBoundary(offset+i+1, scale)
ub := -LowerBoundary(offset+i, scale)
counts = append(counts, int64(count))
counts = append(counts, safeUint64ToInt64(count))
values = append(values, lb+(ub-lb)/2)
}

if zeroCount := dp.ZeroCount(); zeroCount != 0 {
counts = append(counts, int64(zeroCount))
counts = append(counts, safeUint64ToInt64(zeroCount))
values = append(values, 0)
}

Expand All @@ -59,8 +59,16 @@ func ToTDigest(dp pmetric.ExponentialHistogramDataPoint) (counts []int64, values
}
lb := LowerBoundary(offset+i, scale)
ub := LowerBoundary(offset+i+1, scale)
counts = append(counts, int64(count))
counts = append(counts, safeUint64ToInt64(count))
values = append(values, lb+(ub-lb)/2)
}
return
}

func safeUint64ToInt64(v uint64) int64 {
if v > math.MaxInt64 {
return math.MaxInt64
} else {
return int64(v) // nolint:goset // overflow checked
}
}
34 changes: 23 additions & 11 deletions exporter/elasticsearchexporter/internal/objmodel/objmodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ type field struct {

// Value type that can be added to a Document.
type Value struct {
kind Kind
primitive uint64
dbl float64
str string
arr []Value
doc Document
ts time.Time
kind Kind
ui uint64
i int64
dbl float64
str string
arr []Value
doc Document
ts time.Time
}

// Kind represent the internal kind of a value stored in a Document.
Expand All @@ -77,6 +78,7 @@ const (
KindNil Kind = iota
KindBool
KindInt
KindUInt
KindDouble
KindString
KindArr
Expand Down Expand Up @@ -168,6 +170,11 @@ func (doc *Document) AddInt(key string, value int64) {
doc.Add(key, IntValue(value))
}

// AddUInt adds an unsigned integer value to the document.
func (doc *Document) AddUInt(key string, value uint64) {
doc.Add(key, UIntValue(value))
}

// AddAttributes expands and flattens all key-value pairs from the input attribute map into
// the document.
func (doc *Document) AddAttributes(key string, attributes pcommon.Map) {
Expand Down Expand Up @@ -424,7 +431,10 @@ func (doc *Document) iterJSONDedot(w *json.Visitor, otel bool) error {
func StringValue(str string) Value { return Value{kind: KindString, str: str} }

// IntValue creates a new value from an integer.
func IntValue(i int64) Value { return Value{kind: KindInt, primitive: uint64(i)} }
func IntValue(i int64) Value { return Value{kind: KindInt, i: i} }

// UIntValue creates a new value from an unsigned integer.
func UIntValue(i uint64) Value { return Value{kind: KindUInt, ui: i} }

// DoubleValue creates a new value from a double value..
func DoubleValue(d float64) Value { return Value{kind: KindDouble, dbl: d} }
Expand All @@ -435,7 +445,7 @@ func BoolValue(b bool) Value {
if b {
v = 1
}
return Value{kind: KindBool, primitive: v}
return Value{kind: KindBool, ui: v}
}

// ArrValue combines multiple values into an array value.
Expand Down Expand Up @@ -519,9 +529,11 @@ func (v *Value) iterJSON(w *json.Visitor, dedot bool, otel bool) error {
case KindNil:
return w.OnNil()
case KindBool:
return w.OnBool(v.primitive == 1)
return w.OnBool(v.ui == 1)
case KindInt:
return w.OnInt64(int64(v.primitive))
return w.OnInt64(v.i)
case KindUInt:
return w.OnUint64(v.ui)
case KindDouble:
if math.IsNaN(v.dbl) || math.IsInf(v.dbl, 0) {
// NaN and Inf are undefined for JSON. Let's serialize to "null"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ func TestValue_Serialize(t *testing.T) {
"bool value: true": {value: BoolValue(true), want: "true"},
"bool value: false": {value: BoolValue(false), want: "false"},
"int value": {value: IntValue(42), want: "42"},
"uint value": {value: UIntValue(42), want: "42"},
"double value: 3.14": {value: DoubleValue(3.14), want: "3.14"},
"double value: 1.0": {value: DoubleValue(1.0), want: "1.0"},
"NaN is undefined": {value: DoubleValue(math.NaN()), want: "null"},
Expand Down
22 changes: 15 additions & 7 deletions exporter/elasticsearchexporter/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ func (m *encodeModel) upsertMetricDataPointValueOTelMode(documents map[uint32]ob

if dp.HasMappingHint(hintDocCount) {
docCount := dp.DocCount()
document.AddInt("_doc_count", int64(docCount))
document.AddUInt("_doc_count", docCount)
}

switch value.Type() {
Expand Down Expand Up @@ -387,7 +387,7 @@ func (dp summaryDataPoint) Value() (pcommon.Value, error) {
vm := pcommon.NewValueMap()
m := vm.Map()
m.PutDouble("sum", dp.Sum())
m.PutInt("value_count", int64(dp.Count()))
m.PutInt("value_count", safeUint64ToInt64(dp.Count()))
return vm, nil
}

Expand All @@ -413,7 +413,7 @@ func (dp exponentialHistogramDataPoint) Value() (pcommon.Value, error) {
vm := pcommon.NewValueMap()
m := vm.Map()
m.PutDouble("sum", dp.Sum())
m.PutInt("value_count", int64(dp.Count()))
m.PutInt("value_count", safeUint64ToInt64(dp.Count()))
return vm, nil
}

Expand Down Expand Up @@ -460,7 +460,7 @@ func (dp histogramDataPoint) Value() (pcommon.Value, error) {
vm := pcommon.NewValueMap()
m := vm.Map()
m.PutDouble("sum", dp.Sum())
m.PutInt("value_count", int64(dp.Count()))
m.PutInt("value_count", safeUint64ToInt64(dp.Count()))
return vm, nil
}
return histogramToValue(dp.HistogramDataPoint)
Expand Down Expand Up @@ -518,7 +518,7 @@ func histogramToValue(dp pmetric.HistogramDataPoint) (pcommon.Value, error) {
value = explicitBounds.At(i-1) + (explicitBounds.At(i)-explicitBounds.At(i-1))/2.0
}

counts.AppendEmpty().SetInt(int64(count))
counts.AppendEmpty().SetInt(safeUint64ToInt64(count))
values.AppendEmpty().SetDouble(value)
}

Expand Down Expand Up @@ -674,7 +674,7 @@ func (m *encodeModel) encodeSpanOTelMode(resource pcommon.Resource, resourceSche
document.AddSpanID("parent_span_id", span.ParentSpanID())
document.AddString("name", span.Name())
document.AddString("kind", span.Kind().String())
document.AddInt("duration", int64(span.EndTimestamp()-span.StartTimestamp()))
document.AddUInt("duration", uint64(span.EndTimestamp()-span.StartTimestamp()))

m.encodeAttributesOTelMode(&document, span.Attributes())

Expand Down Expand Up @@ -985,7 +985,7 @@ func valueHash(h hash.Hash, v pcommon.Value) {
h.Write(buf)
case pcommon.ValueTypeInt:
buf := make([]byte, 8)
binary.LittleEndian.PutUint64(buf, uint64(v.Int()))
binary.LittleEndian.PutUint64(buf, uint64(v.Int())) // nolint:gosec // Overflow assumed. We prefer having high integers over zero.
h.Write(buf)
case pcommon.ValueTypeBytes:
h.Write(v.Bytes().AsRaw())
Expand Down Expand Up @@ -1074,3 +1074,11 @@ func mergeGeolocation(attributes pcommon.Map) {
}
}
}

func safeUint64ToInt64(v uint64) int64 {
if v > math.MaxInt64 {
return math.MaxInt64
} else {
return int64(v) // nolint:goset // overflow checked
}
}
6 changes: 3 additions & 3 deletions exporter/elasticsearchexporter/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1109,8 +1109,8 @@ func TestEncodeLogOtelMode(t *testing.T) {
// helper function that creates the OTel LogRecord from the test structure
func createTestOTelLogRecord(t *testing.T, rec OTelRecord) (plog.LogRecord, pcommon.InstrumentationScope, pcommon.Resource) {
record := plog.NewLogRecord()
record.SetTimestamp(pcommon.Timestamp(uint64(rec.Timestamp.UnixNano())))
record.SetObservedTimestamp(pcommon.Timestamp(uint64(rec.ObservedTimestamp.UnixNano())))
record.SetTimestamp(pcommon.Timestamp(uint64(rec.Timestamp.UnixNano()))) //nolint:gosec // this input is controlled by tests
record.SetObservedTimestamp(pcommon.Timestamp(uint64(rec.ObservedTimestamp.UnixNano()))) //nolint:gosec // this input is controlled by tests

record.SetTraceID(pcommon.TraceID(rec.TraceID))
record.SetSpanID(pcommon.SpanID(rec.SpanID))
Expand Down Expand Up @@ -1245,7 +1245,7 @@ func TestEncodeLogBodyMapMode(t *testing.T) {
resourceLogs := logs.ResourceLogs().AppendEmpty()
scopeLogs := resourceLogs.ScopeLogs().AppendEmpty()
logRecords := scopeLogs.LogRecords()
observedTimestamp := pcommon.Timestamp(time.Now().UnixNano())
observedTimestamp := pcommon.Timestamp(time.Now().UnixNano()) // nolint:gosec // UnixNano is positive and thus safe to convert to signed integer.

logRecord := logRecords.AppendEmpty()
logRecord.SetObservedTimestamp(observedTimestamp)
Expand Down

0 comments on commit ce3d905

Please sign in to comment.