Skip to content

Commit

Permalink
Change duration to from Duration to uint64 (#142)
Browse files Browse the repository at this point in the history
For our use case, we will always be able to
encode durations as nanoseconds since the
start time. This avoids a protobuf message,
which adds to the number of bytes encoded
and adds a level of pointer indirection,
and more importantly, another heap allocation.
  • Loading branch information
axw authored Aug 14, 2023
1 parent f5da0e6 commit 48e4b20
Show file tree
Hide file tree
Showing 24 changed files with 128 additions and 263 deletions.
9 changes: 4 additions & 5 deletions codec/fullevent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"time"

"github.com/elastic/apm-data/model/modelpb"
"google.golang.org/protobuf/types/known/durationpb"
)

func fullEvent(t *testing.B) *modelpb.APMEvent {
Expand Down Expand Up @@ -52,7 +51,7 @@ func fullEvent(t *testing.B) *modelpb.APMEvent {
Resource: "destination_resource",
ResponseTime: &modelpb.AggregatedDuration{
Count: 3,
Sum: durationpb.New(4 * time.Second),
Sum: uint64(4 * time.Second),
},
},
Db: &modelpb.DB{
Expand Down Expand Up @@ -106,7 +105,7 @@ func fullEvent(t *testing.B) *modelpb.APMEvent {
},
SelfTime: &modelpb.AggregatedDuration{
Count: 6,
Sum: durationpb.New(7 * time.Second),
Sum: uint64(7 * time.Second),
},
RepresentativeCount: 8,
},
Expand Down Expand Up @@ -177,7 +176,7 @@ func fullEvent(t *testing.B) *modelpb.APMEvent {
Outcome: "outcome",
Duration: &modelpb.AggregatedDuration{
Count: 4,
Sum: durationpb.New(5 * time.Second),
Sum: uint64(5 * time.Second),
},
},
},
Expand Down Expand Up @@ -464,7 +463,7 @@ func fullEvent(t *testing.B) *modelpb.APMEvent {
Count: 1,
Sum: 2,
},
Duration: durationpb.New(3 * time.Second),
Duration: uint64(3 * time.Second),
Severity: 4,
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@
package modeldecoderutil

import (
"time"

"github.com/elastic/apm-data/model/modelpb"
"google.golang.org/protobuf/types/known/durationpb"
)

// SetInternalMetrics extracts well-known internal metrics from event.Metricset.Samples,
Expand Down Expand Up @@ -53,7 +50,7 @@ func SetInternalMetrics(event *modelpb.APMEvent) bool {
if event.Span.SelfTime == nil {
event.Span.SelfTime = &modelpb.AggregatedDuration{}
}
event.Span.SelfTime.Sum = durationpb.New(time.Duration(v.Value * 1000))
event.Span.SelfTime.Sum = uint64(v.Value * 1000)
haveMetrics = true
}
}
Expand Down
9 changes: 3 additions & 6 deletions input/elasticapm/internal/modeldecoder/rumv3/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/elastic/apm-data/input/elasticapm/internal/modeldecoder/modeldecoderutil"
"github.com/elastic/apm-data/input/elasticapm/internal/modeldecoder/nullable"
"github.com/elastic/apm-data/model/modelpb"
"google.golang.org/protobuf/types/known/durationpb"
)

var (
Expand Down Expand Up @@ -410,7 +409,7 @@ func mapToTransactionMetricsetModel(from *transactionMetricset, event *modelpb.A
}
if value := from.Samples.SpanSelfTimeSum.Value; value.IsSet() {
event.Span.SelfTime = populateNil(event.Span.SelfTime)
event.Span.SelfTime.Sum = durationpb.New(time.Duration(value.Val * 1000))
event.Span.SelfTime.Sum = uint64(value.Val * 1000)
ok = true
}
}
Expand Down Expand Up @@ -589,8 +588,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
}
if from.Duration.IsSet() {
event.Event = populateNil(event.Event)
duration := time.Duration(from.Duration.Val * float64(time.Millisecond))
event.Event.Duration = durationpb.New(duration)
event.Event.Duration = uint64(from.Duration.Val * float64(time.Millisecond))
}
if from.ID.IsSet() {
out.Id = from.ID.Val
Expand Down Expand Up @@ -725,8 +723,7 @@ func mapToTransactionModel(from *transaction, event *modelpb.APMEvent) {
}
if from.Duration.IsSet() {
event.Event = populateNil(event.Event)
duration := time.Duration(from.Duration.Val * float64(time.Millisecond))
event.Event.Duration = durationpb.New(duration)
event.Event.Duration = uint64(from.Duration.Val * float64(time.Millisecond))
}
if from.ID.IsSet() {
out.Id = from.ID.Val
Expand Down
11 changes: 4 additions & 7 deletions input/elasticapm/internal/modeldecoder/v2/decoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import (
"github.com/elastic/apm-data/input/elasticapm/internal/modeldecoder/nullable"
"github.com/elastic/apm-data/input/otlp"
"github.com/elastic/apm-data/model/modelpb"
"google.golang.org/protobuf/types/known/durationpb"

"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/ptrace"
Expand Down Expand Up @@ -325,7 +324,7 @@ func mapToDroppedSpansModel(from []transactionDroppedSpanStats, tx *modelpb.Tran
to.Duration.Count = uint64(f.Duration.Count.Val)
sum := f.Duration.Sum
if sum.IsSet() {
to.Duration.Sum = durationpb.New(time.Duration(sum.Us.Val) * time.Microsecond)
to.Duration.Sum = uint64(time.Duration(sum.Us.Val) * time.Microsecond)
}
}
if f.ServiceTargetType.IsSet() {
Expand Down Expand Up @@ -1130,8 +1129,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
}
if from.Duration.IsSet() {
event.Event = populateNil(event.Event)
duration := time.Duration(from.Duration.Val * float64(time.Millisecond))
event.Event.Duration = durationpb.New(duration)
event.Event.Duration = uint64(from.Duration.Val * float64(time.Millisecond))
}
if from.ID.IsSet() {
out.Id = from.ID.Val
Expand Down Expand Up @@ -1183,7 +1181,7 @@ func mapToSpanModel(from *span, event *modelpb.APMEvent) {
// event.Timestamp should have been initialized to the time the
// payload was received; offset that by "start" milliseconds for
// RUM.
event.Timestamp += uint64(time.Duration(float64(time.Millisecond) * from.Start.Val).Nanoseconds())
event.Timestamp += uint64(float64(time.Millisecond) * from.Start.Val)
}
if from.TraceID.IsSet() {
event.Trace = &modelpb.Trace{
Expand Down Expand Up @@ -1331,9 +1329,8 @@ func mapToTransactionModel(from *transaction, event *modelpb.APMEvent) {
}
}
if from.Duration.IsSet() {
duration := time.Duration(from.Duration.Val * float64(time.Millisecond))
event.Event = populateNil(event.Event)
event.Event.Duration = durationpb.New(duration)
event.Event.Duration = uint64(from.Duration.Val * float64(time.Millisecond))
}
if from.ID.IsSet() {
out.Id = from.ID.Val
Expand Down
9 changes: 4 additions & 5 deletions input/elasticapm/internal/modeldecoder/v2/metricset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/durationpb"

"github.com/elastic/apm-data/input/elasticapm/internal/decoder"
"github.com/elastic/apm-data/input/elasticapm/internal/modeldecoder"
Expand Down Expand Up @@ -268,7 +267,7 @@ func TestDecodeMetricsetInternal(t *testing.T) {
Subtype: "span_subtype",
SelfTime: &modelpb.AggregatedDuration{
Count: 123,
Sum: durationpb.New(456 * time.Microsecond),
Sum: uint64(456 * time.Microsecond),
},
},
}}, batch, protocmp.Transform()))
Expand Down Expand Up @@ -325,7 +324,7 @@ func TestDecodeMetricsetServiceName(t *testing.T) {
Subtype: "span_subtype",
SelfTime: &modelpb.AggregatedDuration{
Count: 123,
Sum: durationpb.New(456 * time.Microsecond),
Sum: uint64(456 * time.Microsecond),
},
},
}}, batch, protocmp.Transform()))
Expand Down Expand Up @@ -384,7 +383,7 @@ func TestDecodeMetricsetServiceNameAndVersion(t *testing.T) {
Subtype: "span_subtype",
SelfTime: &modelpb.AggregatedDuration{
Count: 123,
Sum: durationpb.New(456 * time.Microsecond),
Sum: uint64(456 * time.Microsecond),
},
},
}}, batch, protocmp.Transform()))
Expand Down Expand Up @@ -442,7 +441,7 @@ func TestDecodeMetricsetServiceVersion(t *testing.T) {
Subtype: "span_subtype",
SelfTime: &modelpb.AggregatedDuration{
Count: 123,
Sum: durationpb.New(456 * time.Microsecond),
Sum: uint64(456 * time.Microsecond),
},
},
}}, batch, protocmp.Transform()))
Expand Down
2 changes: 1 addition & 1 deletion input/elasticapm/internal/modeldecoder/v2/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestDecodeNestedSpan(t *testing.T) {
require.Len(t, batch, 1)
require.NotNil(t, batch[0].Span)
assert.Equal(t, eventBase.Timestamp+uint64((143*time.Millisecond).Nanoseconds()), batch[0].Timestamp)
assert.Equal(t, 100*time.Millisecond, batch[0].Event.Duration.AsDuration())
assert.Equal(t, uint64(100*time.Millisecond), batch[0].Event.Duration)
assert.Equal(t, "parent-123", batch[0].ParentId, protocmp.Transform())
assert.Equal(t, &modelpb.Trace{Id: "trace-ab"}, batch[0].Trace, protocmp.Transform())
assert.Empty(t, cmp.Diff(&modelpb.Span{
Expand Down
5 changes: 2 additions & 3 deletions input/elasticapm/internal/modeldecoder/v2/transaction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/durationpb"
"google.golang.org/protobuf/types/known/structpb"

"github.com/elastic/apm-data/input/elasticapm/internal/decoder"
Expand Down Expand Up @@ -201,15 +200,15 @@ func TestDecodeMapToTransactionModel(t *testing.T) {
Outcome: "success",
Duration: &modelpb.AggregatedDuration{
Count: 2,
Sum: durationpb.New(time.Duration(durationSumUs) * time.Microsecond),
Sum: uint64(time.Duration(durationSumUs) * time.Microsecond),
},
},
{
DestinationServiceResource: "mysql://mysql:3306",
Outcome: "unknown",
Duration: &modelpb.AggregatedDuration{
Count: 10,
Sum: durationpb.New(time.Duration(durationSumUs) * time.Microsecond),
Sum: uint64(time.Duration(durationSumUs) * time.Microsecond),
},
},
},
Expand Down
3 changes: 1 addition & 2 deletions input/otlp/traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ import (
"go.uber.org/zap"
"google.golang.org/grpc/codes"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/durationpb"

"github.com/elastic/apm-data/model/modelpb"
)
Expand Down Expand Up @@ -160,7 +159,7 @@ func (c *Consumer) convertSpan(
}
}
event.Event = populateNil(event.Event)
event.Event.Duration = durationpb.New(duration)
event.Event.Duration = uint64(duration)
event.Event.Outcome = spanStatusOutcome(otelSpan.Status())
if parentID != "" {
event.ParentId = parentID
Expand Down
4 changes: 2 additions & 2 deletions input/otlp/traces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -820,8 +820,8 @@ func TestConsumeTracesExportTimestamp(t *testing.T) {
assert.Equal(t, uint64((exceptionOffset - transactionOffset).Nanoseconds()), (*batch)[2].Timestamp-(*batch)[0].Timestamp)

// Durations should be unaffected.
assert.Equal(t, transactionDuration, (*batch)[0].GetEvent().GetDuration().AsDuration())
assert.Equal(t, spanDuration, (*batch)[1].GetEvent().GetDuration().AsDuration())
assert.Equal(t, transactionDuration, time.Duration((*batch)[0].GetEvent().GetDuration()))
assert.Equal(t, spanDuration, time.Duration((*batch)[1].GetEvent().GetDuration()))

for _, b := range *batch {
// telemetry.sdk.elastic_export_timestamp should not be sent as a label.
Expand Down
9 changes: 4 additions & 5 deletions model/modeljson/apmevent.pb.json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/elastic/apm-data/model/modelpb"
"github.com/stretchr/testify/require"
"go.elastic.co/fastjson"
durationpb "google.golang.org/protobuf/types/known/durationpb"
)

func TestFullEvent(t *testing.T) {
Expand Down Expand Up @@ -73,7 +72,7 @@ func fullEvent(t testing.TB) *modelpb.APMEvent {
Resource: "destination_resource",
ResponseTime: &modelpb.AggregatedDuration{
Count: 3,
Sum: durationpb.New(4 * time.Second),
Sum: uint64(4 * time.Second),
},
},
Db: &modelpb.DB{
Expand Down Expand Up @@ -127,7 +126,7 @@ func fullEvent(t testing.TB) *modelpb.APMEvent {
},
SelfTime: &modelpb.AggregatedDuration{
Count: 6,
Sum: durationpb.New(7 * time.Second),
Sum: uint64(7 * time.Second),
},
RepresentativeCount: 8,
},
Expand Down Expand Up @@ -198,7 +197,7 @@ func fullEvent(t testing.TB) *modelpb.APMEvent {
Outcome: "outcome",
Duration: &modelpb.AggregatedDuration{
Count: 4,
Sum: durationpb.New(5 * time.Second),
Sum: uint64(5 * time.Second),
},
},
},
Expand Down Expand Up @@ -485,7 +484,7 @@ func fullEvent(t testing.TB) *modelpb.APMEvent {
Count: 1,
Sum: 2,
},
Duration: durationpb.New(3 * time.Second),
Duration: uint64(3 * time.Second),
Severity: 4,
},
}
Expand Down
2 changes: 1 addition & 1 deletion model/modeljson/event.pb.json.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func EventModelJSON(e *modelpb.Event, out *modeljson.Event) {
Kind: e.Kind,
Category: e.Category,
Type: e.Type,
Duration: int64(e.Duration.AsDuration().Nanoseconds()),
Duration: e.Duration,
Severity: e.Severity,
}
if e.SuccessCount != nil {
Expand Down
5 changes: 2 additions & 3 deletions model/modeljson/event.pb.json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/elastic/apm-data/model/modelpb"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/types/known/durationpb"
)

const (
Expand Down Expand Up @@ -56,7 +55,7 @@ func TestEventToModelJSON(t *testing.T) {
Count: 1,
Sum: 2,
},
Duration: durationpb.New(3 * time.Second),
Duration: uint64(3 * time.Second),
Severity: 4,
Received: modelpb.FromTime(now),
},
Expand All @@ -71,7 +70,7 @@ func TestEventToModelJSON(t *testing.T) {
Count: 1,
Sum: 2,
},
Duration: int64(3 * time.Second),
Duration: uint64(3 * time.Second),
Severity: 4,
Received: modeljson.Time(now),
},
Expand Down
2 changes: 1 addition & 1 deletion model/modeljson/internal/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ type Event struct {
Received Time `json:"received,omitempty"`
Type string `json:"type,omitempty"`
SuccessCount SummaryMetric `json:"success_count,omitempty"`
Duration int64 `json:"duration,omitempty"`
Duration uint64 `json:"duration,omitempty"`
Severity uint64 `json:"severity,omitempty"`
}
2 changes: 1 addition & 1 deletion model/modeljson/internal/marshal_fastjson.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions model/modeljson/span.pb.json.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func SpanModelJSON(e *modelpb.Span, out *modeljson.Span) {
if e.SelfTime != nil {
out.SelfTime = modeljson.AggregatedDuration{
Count: e.SelfTime.Count,
Sum: e.SelfTime.Sum.AsDuration(),
Sum: time.Duration(e.SelfTime.Sum),
}
}
if e.Db != nil {
Expand All @@ -89,7 +89,7 @@ func SpanModelJSON(e *modelpb.Span, out *modeljson.Span) {
if e.DestinationService.ResponseTime != nil {
out.Destination.Service.ResponseTime = modeljson.AggregatedDuration{
Count: e.DestinationService.ResponseTime.Count,
Sum: e.DestinationService.ResponseTime.Sum.AsDuration(),
Sum: time.Duration(e.DestinationService.ResponseTime.Sum),
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions model/modeljson/span.pb.json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/elastic/apm-data/model/modelpb"
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/require"
"google.golang.org/protobuf/types/known/durationpb"
)

func TestSpanToModelJSON(t *testing.T) {
Expand Down Expand Up @@ -72,7 +71,7 @@ func TestSpanToModelJSON(t *testing.T) {
Resource: "destination_resource",
ResponseTime: &modelpb.AggregatedDuration{
Count: 3,
Sum: durationpb.New(4 * time.Second),
Sum: uint64(4 * time.Second),
},
},
Db: &modelpb.DB{
Expand All @@ -98,7 +97,7 @@ func TestSpanToModelJSON(t *testing.T) {
},
SelfTime: &modelpb.AggregatedDuration{
Count: 6,
Sum: durationpb.New(7 * time.Second),
Sum: uint64(7 * time.Second),
},
RepresentativeCount: 8,
},
Expand Down
Loading

0 comments on commit 48e4b20

Please sign in to comment.