Skip to content

Commit

Permalink
Merge branch 'main' into sdk-enabled-param
Browse files Browse the repository at this point in the history
  • Loading branch information
pellared committed Sep 13, 2024
2 parents 7e9e7ce + 80e18a5 commit f08ebd6
Show file tree
Hide file tree
Showing 29 changed files with 230 additions and 92 deletions.
6 changes: 6 additions & 0 deletions attribute/value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ func TestValue(t *testing.T) {
wantType: attribute.INT64,
wantValue: int64(42),
},
{
name: "Key.Int64() correctly returns negative keys's internal int64 value",
value: k.Int64(-42).Value,
wantType: attribute.INT64,
wantValue: int64(-42),
},
{
name: "Key.Int64Slice() correctly returns keys's internal []int64 value",
value: k.Int64Slice([]int64{42, -3, 12}).Value,
Expand Down
6 changes: 3 additions & 3 deletions bridge/opencensus/internal/ocmetric/metric.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func convertHistogram(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.Time
Attributes: attrs,
StartTime: t.StartTime,
Time: p.Time,
Count: uint64(dist.Count),
Count: uint64(max(0, dist.Count)), // nolint:gosec // A count should never be negative.
Sum: dist.Sum,
Bounds: dist.BucketOptions.Bounds,
BucketCounts: bucketCounts,
Expand All @@ -166,7 +166,7 @@ func convertBuckets(buckets []ocmetricdata.Bucket) ([]uint64, []metricdata.Exemp
err = errors.Join(err, fmt.Errorf("%w: %q", errNegativeBucketCount, bucket.Count))
continue
}
bucketCounts[i] = uint64(bucket.Count)
bucketCounts[i] = uint64(max(0, bucket.Count)) // nolint:gosec // A count should never be negative.

if bucket.Exemplar != nil {
exemplar, exemplarErr := convertExemplar(bucket.Exemplar)
Expand Down Expand Up @@ -357,7 +357,7 @@ func convertSummary(labelKeys []ocmetricdata.LabelKey, ts []*ocmetricdata.TimeSe
Attributes: attrs,
StartTime: t.StartTime,
Time: p.Time,
Count: uint64(summary.Count),
Count: uint64(max(0, summary.Count)), // nolint:gosec // A count should never be negative.
QuantileValues: convertQuantiles(summary.Snapshot),
Sum: summary.Sum,
}
Expand Down
2 changes: 1 addition & 1 deletion bridge/opencensus/internal/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (s *Span) SetName(name string) {

// SetStatus sets the status of this span, if it is recording events.
func (s *Span) SetStatus(status octrace.Status) {
s.otelSpan.SetStatus(codes.Code(status.Code), status.Message)
s.otelSpan.SetStatus(codes.Code(max(0, status.Code)), status.Message) // nolint:gosec // Overflow checked.
}

// AddAttributes sets attributes in this span.
Expand Down
41 changes: 33 additions & 8 deletions bridge/opencensus/internal/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,40 @@ func TestSpanSetStatus(t *testing.T) {
s := &span{recording: true}
ocS := internal.NewSpan(s)

c, d := codes.Error, "error"
status := octrace.Status{Code: int32(c), Message: d}
ocS.SetStatus(status)
for _, tt := range []struct {
name string

if s.sCode != c {
t.Error("span.SetStatus failed to set OpenTelemetry status code")
}
if s.sMsg != d {
t.Error("span.SetStatus failed to set OpenTelemetry status description")
code int32
message string

wantCode codes.Code
}{
{
name: "with an error code",
code: int32(codes.Error),
message: "error",

wantCode: codes.Error,
},
{
name: "with a negative/invalid code",
code: -42,
message: "error",

wantCode: codes.Unset,
},
} {
t.Run(tt.name, func(t *testing.T) {
status := octrace.Status{Code: tt.code, Message: tt.message}
ocS.SetStatus(status)

if s.sCode != tt.wantCode {
t.Errorf("span.SetStatus failed to set OpenTelemetry status code. Expected %d, got %d", tt.wantCode, s.sCode)
}
if s.sMsg != tt.message {
t.Errorf("span.SetStatus failed to set OpenTelemetry status description. Expected %s, got %s", tt.message, s.sMsg)
}
})
}
}

Expand Down
5 changes: 3 additions & 2 deletions exporters/otlp/otlplog/otlploggrpc/internal/transform/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord {
// year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The
// result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
nano := t.UnixNano()
if nano < 0 {
return 0
}
return uint64(t.UnixNano())
return uint64(nano) // nolint:gosec // Overflow checked.
}

// AttrIter transforms an [attribute.Iterator] into OTLP key-values.
Expand Down
28 changes: 28 additions & 0 deletions exporters/otlp/otlplog/otlploggrpc/internal/transform/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ var (
ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0))
obs = ts.Add(30 * time.Second)

// A time before unix 0.
negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC)

alice = api.String("user", "alice")
bob = api.String("user", "bob")

Expand Down Expand Up @@ -158,6 +161,20 @@ var (
Resource: res,
}.NewRecord())

out = append(out, logtest.RecordFactory{
Timestamp: negativeTs,
ObservedTimestamp: obs,
Severity: sevB,
SeverityText: "B",
Body: bodyB,
Attributes: []api.KeyValue{bob},
TraceID: trace.TraceID(traceIDB),
SpanID: trace.SpanID(spanIDB),
TraceFlags: trace.TraceFlags(flagsB),
InstrumentationScope: &scope,
Resource: res,
}.NewRecord())

return out
}()

Expand Down Expand Up @@ -206,6 +223,17 @@ var (
TraceId: traceIDB,
SpanId: spanIDB,
},
{
TimeUnixNano: 0,
ObservedTimeUnixNano: uint64(obs.UnixNano()),
SeverityNumber: pbSevB,
SeverityText: "B",
Body: pbBodyB,
Attributes: []*cpb.KeyValue{pbBob},
Flags: uint32(flagsB),
TraceId: traceIDB,
SpanId: spanIDB,
},
}

pbScopeLogs = &lpb.ScopeLogs{
Expand Down
5 changes: 3 additions & 2 deletions exporters/otlp/otlplog/otlploghttp/internal/transform/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord {
// year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The
// result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
nano := t.UnixNano()
if nano < 0 {
return 0
}
return uint64(t.UnixNano())
return uint64(nano) // nolint:gosec // Overflow checked.
}

// AttrIter transforms an [attribute.Iterator] into OTLP key-values.
Expand Down
28 changes: 28 additions & 0 deletions exporters/otlp/otlplog/otlploghttp/internal/transform/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ var (
ts = time.Date(2000, time.January, 0o1, 0, 0, 0, 0, time.FixedZone("GMT", 0))
obs = ts.Add(30 * time.Second)

// A time before unix 0.
negativeTs = time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC)

alice = api.String("user", "alice")
bob = api.String("user", "bob")

Expand Down Expand Up @@ -158,6 +161,20 @@ var (
Resource: res,
}.NewRecord())

out = append(out, logtest.RecordFactory{
Timestamp: negativeTs,
ObservedTimestamp: obs,
Severity: sevB,
SeverityText: "B",
Body: bodyB,
Attributes: []api.KeyValue{bob},
TraceID: trace.TraceID(traceIDB),
SpanID: trace.SpanID(spanIDB),
TraceFlags: trace.TraceFlags(flagsB),
InstrumentationScope: &scope,
Resource: res,
}.NewRecord())

return out
}()

Expand Down Expand Up @@ -206,6 +223,17 @@ var (
TraceId: traceIDB,
SpanId: spanIDB,
},
{
TimeUnixNano: 0,
ObservedTimeUnixNano: uint64(obs.UnixNano()),
SeverityNumber: pbSevB,
SeverityText: "B",
Body: pbBodyB,
Attributes: []*cpb.KeyValue{pbBob},
Flags: uint32(flagsB),
TraceId: traceIDB,
SpanId: spanIDB,
},
}

pbScopeLogs = &lpb.ScopeLogs{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) {
// timeUnixNano on the zero Time returns 0.
// The result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
return 0
}
return uint64(t.UnixNano())
return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked.
}

// Exemplars returns a slice of OTLP Exemplars generated from exemplars.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,7 @@ func Temporality(t metricdata.Temporality) (mpb.AggregationTemporality, error) {
// timeUnixNano on the zero Time returns 0.
// The result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
return 0
}
return uint64(t.UnixNano())
return uint64(max(0, t.UnixNano())) // nolint:gosec // Overflow checked.
}

// Exemplars returns a slice of OTLP Exemplars generated from exemplars.
Expand Down
8 changes: 4 additions & 4 deletions exporters/otlp/otlptrace/internal/tracetransform/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ func span(sd tracesdk.ReadOnlySpan) *tracepb.Span {
SpanId: sid[:],
TraceState: sd.SpanContext().TraceState().String(),
Status: status(sd.Status().Code, sd.Status().Description),
StartTimeUnixNano: uint64(sd.StartTime().UnixNano()),
EndTimeUnixNano: uint64(sd.EndTime().UnixNano()),
StartTimeUnixNano: uint64(max(0, sd.StartTime().UnixNano())), // nolint:gosec // Overflow checked.
EndTimeUnixNano: uint64(max(0, sd.EndTime().UnixNano())), // nolint:gosec // Overflow checked.
Links: links(sd.Links()),
Kind: spanKind(sd.SpanKind()),
Name: sd.Name(),
Expand Down Expand Up @@ -178,7 +178,7 @@ func buildSpanFlags(sc trace.SpanContext) uint32 {
flags |= tracepb.SpanFlags_SPAN_FLAGS_CONTEXT_IS_REMOTE_MASK
}

return uint32(flags)
return uint32(flags) // nolint:gosec // Flags is a bitmask and can't be negative
}

// spanEvents transforms span Events to an OTLP span events.
Expand All @@ -192,7 +192,7 @@ func spanEvents(es []tracesdk.Event) []*tracepb.Span_Event {
for i := 0; i < len(es); i++ {
events[i] = &tracepb.Span_Event{
Name: es[i].Name,
TimeUnixNano: uint64(es[i].Time.UnixNano()),
TimeUnixNano: uint64(max(0, es[i].Time.UnixNano())), // nolint:gosec // Overflow checked.
Attributes: KeyValues(es[i].Attributes),
DroppedAttributesCount: clampUint32(es[i].DroppedAttributeCount),
}
Expand Down
10 changes: 9 additions & 1 deletion exporters/otlp/otlptrace/internal/tracetransform/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ func TestEmptySpanEvent(t *testing.T) {
func TestSpanEvent(t *testing.T) {
attrs := []attribute.KeyValue{attribute.Int("one", 1), attribute.Int("two", 2)}
eventTime := time.Date(2020, 5, 20, 0, 0, 0, 0, time.UTC)
negativeEventTime := time.Date(1969, 7, 20, 20, 17, 0, 0, time.UTC)
got := spanEvents([]tracesdk.Event{
{
Name: "test 1",
Expand All @@ -80,14 +81,21 @@ func TestSpanEvent(t *testing.T) {
Time: eventTime,
DroppedAttributeCount: 2,
},
{
Name: "test 3",
Attributes: attrs,
Time: negativeEventTime,
DroppedAttributeCount: 2,
},
})
if !assert.Len(t, got, 2) {
if !assert.Len(t, got, 3) {
return
}
eventTimestamp := uint64(1589932800 * 1e9)
assert.Equal(t, &tracepb.Span_Event{Name: "test 1", Attributes: nil, TimeUnixNano: eventTimestamp}, got[0])
// Do not test Attributes directly, just that the return value goes to the correct field.
assert.Equal(t, &tracepb.Span_Event{Name: "test 2", Attributes: KeyValues(attrs), TimeUnixNano: eventTimestamp, DroppedAttributesCount: 2}, got[1])
assert.Equal(t, &tracepb.Span_Event{Name: "test 3", Attributes: KeyValues(attrs), TimeUnixNano: 0, DroppedAttributesCount: 2}, got[2])
}

func TestNilLinks(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions exporters/zipkin/internal/internaltest/text_map_propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text
}

// InjectedN tests if p has made n injections to carrier.
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) {
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != n {
t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n)
return false
}
Expand Down
4 changes: 2 additions & 2 deletions internal/internaltest/text_map_propagator.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text
}

// InjectedN tests if p has made n injections to carrier.
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) {
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != n {
t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n)
return false
}
Expand Down
3 changes: 2 additions & 1 deletion internal/rawhelpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ func RawToBool(r uint64) bool {
}

func Int64ToRaw(i int64) uint64 {
return uint64(i)
// Assumes original was a valid int64 (overflow not checked).
return uint64(i) // nolint: gosec
}

func RawToInt64(r uint64) int64 {
Expand Down
4 changes: 2 additions & 2 deletions internal/shared/internaltest/text_map_propagator.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ func (p *TextMapPropagator) Inject(ctx context.Context, carrier propagation.Text
}

// InjectedN tests if p has made n injections to carrier.
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n int) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != uint64(n) {
func (p *TextMapPropagator) InjectedN(t *testing.T, carrier *TextMapCarrier, n uint64) bool {
if actual := p.stateFromCarrier(carrier).Injections; actual != n {
t.Errorf("TextMapPropagator{%q} injected %d times, not %d", p.name, actual, n)
return false
}
Expand Down
5 changes: 3 additions & 2 deletions internal/shared/otlp/otlplog/transform/log.go.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,11 @@ func LogRecord(record log.Record) *lpb.LogRecord {
// year 1678 or after 2262). timeUnixNano on the zero Time returns 0. The
// result does not depend on the location associated with t.
func timeUnixNano(t time.Time) uint64 {
if t.IsZero() {
nano := t.UnixNano()
if nano < 0 {
return 0
}
return uint64(t.UnixNano())
return uint64(nano) // nolint:gosec // Overflow checked.
}

// AttrIter transforms an [attribute.Iterator] into OTLP key-values.
Expand Down
Loading

0 comments on commit f08ebd6

Please sign in to comment.