From ce3d905a7954d8af397ff2919e23bed0d59570d9 Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Sat, 14 Dec 2024 06:25:31 +0100 Subject: [PATCH] [exporter/elasticsearch] [chore] fix lint issues with golangci-lint 1.62 (#36798) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### 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: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/36638 --------- Co-authored-by: Carson Ip Co-authored-by: Tim Rühsen Co-authored-by: Christos Markou --- .../internal/exphistogram/exphistogram.go | 14 ++++++-- .../internal/objmodel/objmodel.go | 34 +++++++++++++------ .../internal/objmodel/objmodel_test.go | 1 + exporter/elasticsearchexporter/model.go | 22 ++++++++---- exporter/elasticsearchexporter/model_test.go | 6 ++-- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/exporter/elasticsearchexporter/internal/exphistogram/exphistogram.go b/exporter/elasticsearchexporter/internal/exphistogram/exphistogram.go index 255328f38f1d..6f5299946ffd 100644 --- a/exporter/elasticsearchexporter/internal/exphistogram/exphistogram.go +++ b/exporter/elasticsearchexporter/internal/exphistogram/exphistogram.go @@ -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) } @@ -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 + } +} diff --git a/exporter/elasticsearchexporter/internal/objmodel/objmodel.go b/exporter/elasticsearchexporter/internal/objmodel/objmodel.go index 6f8a9b41add8..0f514e06aaaa 100644 --- a/exporter/elasticsearchexporter/internal/objmodel/objmodel.go +++ b/exporter/elasticsearchexporter/internal/objmodel/objmodel.go @@ -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. @@ -77,6 +78,7 @@ const ( KindNil Kind = iota KindBool KindInt + KindUInt KindDouble KindString KindArr @@ -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) { @@ -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} } @@ -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. @@ -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" diff --git a/exporter/elasticsearchexporter/internal/objmodel/objmodel_test.go b/exporter/elasticsearchexporter/internal/objmodel/objmodel_test.go index 3d0a07b820d0..6805a958a019 100644 --- a/exporter/elasticsearchexporter/internal/objmodel/objmodel_test.go +++ b/exporter/elasticsearchexporter/internal/objmodel/objmodel_test.go @@ -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"}, diff --git a/exporter/elasticsearchexporter/model.go b/exporter/elasticsearchexporter/model.go index ba14a24d4fc6..f6ee644fb022 100644 --- a/exporter/elasticsearchexporter/model.go +++ b/exporter/elasticsearchexporter/model.go @@ -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() { @@ -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 } @@ -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 } @@ -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) @@ -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) } @@ -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()) @@ -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()) @@ -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 + } +} diff --git a/exporter/elasticsearchexporter/model_test.go b/exporter/elasticsearchexporter/model_test.go index 9b28e2459068..eda750a540e7 100644 --- a/exporter/elasticsearchexporter/model_test.go +++ b/exporter/elasticsearchexporter/model_test.go @@ -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)) @@ -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)