From df9f5650412ab905c72070c42271e04e7a8a365a Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Wed, 17 Mar 2021 07:49:34 -0700 Subject: [PATCH] Use otlp request in wrapper, hide members in the wrapper (#2692) Tested in contrib and was able to access InternalWrapper.Orig, with this change this is no longer possible. Also removes one extra allocation for the request object when using otlp receiver/exporter since we keep the initial pointer around. Signed-off-by: Bogdan Drutu --- consumer/pdata/log.go | 57 ++++----- consumer/pdata/log_test.go | 45 ++++--- exporter/fileexporter/file_exporter.go | 7 +- exporter/fileexporter/file_exporter_test.go | 134 ++++++++++---------- exporter/otlpexporter/otlp.go | 6 +- exporter/otlpexporter/otlp_test.go | 6 +- internal/otlp_wrapper.go | 18 +-- internal/testdata/log.go | 131 ++++++++++--------- internal/testdata/log_test.go | 6 +- receiver/otlpreceiver/logs/otlp.go | 2 +- receiver/otlpreceiver/logs/otlp_test.go | 32 +++-- testbed/testbed/data_providers.go | 2 +- 12 files changed, 228 insertions(+), 218 deletions(-) diff --git a/consumer/pdata/log.go b/consumer/pdata/log.go index 958e0cded23..8610f3adc2f 100644 --- a/consumer/pdata/log.go +++ b/consumer/pdata/log.go @@ -29,56 +29,51 @@ import ( // Must use NewLogs functions to create new instances. // Important: zero-initialized instance is not valid for use. type Logs struct { - orig *[]*otlplogs.ResourceLogs + orig *otlpcollectorlog.ExportLogsServiceRequest } // NewLogs creates a new Logs. func NewLogs() Logs { - orig := []*otlplogs.ResourceLogs(nil) - return Logs{&orig} + return Logs{orig: &otlpcollectorlog.ExportLogsServiceRequest{}} } // LogsFromInternalRep creates the internal Logs representation from the ProtoBuf. Should // not be used outside this module. This is intended to be used only by OTLP exporter and // File exporter, which legitimately need to work with OTLP Protobuf structs. -func LogsFromInternalRep(logs internal.OtlpLogsWrapper) Logs { - return Logs{logs.Orig} +func LogsFromInternalRep(logs internal.LogsWrapper) Logs { + return Logs{orig: internal.LogsToOtlp(logs)} +} + +// LogsFromOtlpProtoBytes converts OTLP Collector ExportLogsServiceRequest +// ProtoBuf bytes to the internal Logs. +// +// Returns an invalid Logs instance if error is not nil. +func LogsFromOtlpProtoBytes(data []byte) (Logs, error) { + req := otlpcollectorlog.ExportLogsServiceRequest{} + if err := req.Unmarshal(data); err != nil { + return Logs{}, err + } + return Logs{orig: &req}, nil } // InternalRep returns internal representation of the logs. Should not be used outside // this module. This is intended to be used only by OTLP exporter and File exporter, // which legitimately need to work with OTLP Protobuf structs. -func (ld Logs) InternalRep() internal.OtlpLogsWrapper { - return internal.OtlpLogsWrapper{Orig: ld.orig} +func (ld Logs) InternalRep() internal.LogsWrapper { + return internal.LogsFromOtlp(ld.orig) } // ToOtlpProtoBytes returns the internal Logs to OTLP Collector ExportTraceServiceRequest // ProtoBuf bytes. This is intended to export OTLP Protobuf bytes for OTLP/HTTP transports. func (ld Logs) ToOtlpProtoBytes() ([]byte, error) { - logs := otlpcollectorlog.ExportLogsServiceRequest{ - ResourceLogs: *ld.orig, - } - return logs.Marshal() -} - -// FromOtlpProtoBytes converts OTLP Collector ExportLogsServiceRequest -// ProtoBuf bytes to the internal Logs. Overrides current data. -// Calling this function on zero-initialized structure causes panic. -// Use it with NewLogs or on existing initialized Logs. -func (ld Logs) FromOtlpProtoBytes(data []byte) error { - logs := otlpcollectorlog.ExportLogsServiceRequest{} - if err := logs.Unmarshal(data); err != nil { - return err - } - *ld.orig = logs.ResourceLogs - return nil + return ld.orig.Marshal() } // Clone returns a copy of Logs. func (ld Logs) Clone() Logs { - rls := NewResourceLogsSlice() - ld.ResourceLogs().CopyTo(rls) - return Logs(rls) + cloneLd := NewLogs() + ld.ResourceLogs().CopyTo(cloneLd.ResourceLogs()) + return cloneLd } // LogRecordCount calculates the total number of log records. @@ -99,15 +94,11 @@ func (ld Logs) LogRecordCount() int { // SizeBytes returns the number of bytes in the internal representation of the // logs. func (ld Logs) SizeBytes() int { - size := 0 - for i := range *ld.orig { - size += (*ld.orig)[i].Size() - } - return size + return ld.orig.Size() } func (ld Logs) ResourceLogs() ResourceLogsSlice { - return ResourceLogsSlice(ld) + return newResourceLogsSlice(&ld.orig.ResourceLogs) } // SeverityNumber is the public alias of otlplogs.SeverityNumber from internal package. diff --git a/consumer/pdata/log_test.go b/consumer/pdata/log_test.go index 9ba57586ec5..811e748d4d4 100644 --- a/consumer/pdata/log_test.go +++ b/consumer/pdata/log_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/internal" + otlpcollectorlog "go.opentelemetry.io/collector/internal/data/protogen/collector/logs/v1" otlplogs "go.opentelemetry.io/collector/internal/data/protogen/logs/v1" ) @@ -48,17 +49,24 @@ func TestLogRecordCount(t *testing.T) { } func TestLogRecordCountWithEmpty(t *testing.T) { - assert.EqualValues(t, 0, LogsFromInternalRep(internal.LogsFromOtlp([]*otlplogs.ResourceLogs{{}})).LogRecordCount()) - assert.EqualValues(t, 0, LogsFromInternalRep(internal.LogsFromOtlp([]*otlplogs.ResourceLogs{ - { - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{{}}, + assert.Zero(t, NewLogs().LogRecordCount()) + assert.Zero(t, LogsFromInternalRep(internal.LogsFromOtlp(&otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{{}}, + })).LogRecordCount()) + assert.Zero(t, LogsFromInternalRep(internal.LogsFromOtlp(&otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{{}}, + }, }, })).LogRecordCount()) - assert.EqualValues(t, 1, LogsFromInternalRep(internal.LogsFromOtlp([]*otlplogs.ResourceLogs{ - { - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ - { - Logs: []*otlplogs.LogRecord{{}}, + assert.Equal(t, 1, LogsFromInternalRep(internal.LogsFromOtlp(&otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ + { + Logs: []*otlplogs.LogRecord{{}}, + }, }, }, }, @@ -66,10 +74,10 @@ func TestLogRecordCountWithEmpty(t *testing.T) { } func TestToFromLogProto(t *testing.T) { - otlp := []*otlplogs.ResourceLogs(nil) - td := LogsFromInternalRep(internal.LogsFromOtlp(otlp)) - assert.EqualValues(t, NewLogs(), td) - assert.EqualValues(t, otlp, *td.orig) + wrapper := internal.LogsFromOtlp(&otlpcollectorlog.ExportLogsServiceRequest{}) + ld := LogsFromInternalRep(wrapper) + assert.EqualValues(t, NewLogs(), ld) + assert.EqualValues(t, &otlpcollectorlog.ExportLogsServiceRequest{}, ld.orig) } func TestLogsToFromOtlpProtoBytes(t *testing.T) { @@ -78,14 +86,13 @@ func TestLogsToFromOtlpProtoBytes(t *testing.T) { bytes, err := send.ToOtlpProtoBytes() assert.NoError(t, err) - recv := NewLogs() - err = recv.FromOtlpProtoBytes(bytes) + recv, err := LogsFromOtlpProtoBytes(bytes) assert.NoError(t, err) assert.EqualValues(t, send, recv) } func TestLogsFromInvalidOtlpProtoBytes(t *testing.T) { - err := NewLogs().FromOtlpProtoBytes([]byte{0xFF}) + _, err := LogsFromOtlpProtoBytes([]byte{0xFF}) assert.EqualError(t, err, "unexpected EOF") } @@ -127,8 +134,8 @@ func BenchmarkLogsFromOtlp(b *testing.B) { b.ResetTimer() b.ReportAllocs() for n := 0; n < b.N; n++ { - traces := NewLogs() - require.NoError(b, traces.FromOtlpProtoBytes(buf)) - assert.Equal(b, baseLogs.ResourceLogs().Len(), traces.ResourceLogs().Len()) + logs, err := LogsFromOtlpProtoBytes(buf) + require.NoError(b, err) + assert.Equal(b, baseLogs.ResourceLogs().Len(), logs.ResourceLogs().Len()) } } diff --git a/exporter/fileexporter/file_exporter.go b/exporter/fileexporter/file_exporter.go index 3835f394bc8..f0a8a5b4554 100644 --- a/exporter/fileexporter/file_exporter.go +++ b/exporter/fileexporter/file_exporter.go @@ -25,7 +25,6 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/internal" - otlplogs "go.opentelemetry.io/collector/internal/data/protogen/collector/logs/v1" otlpmetrics "go.opentelemetry.io/collector/internal/data/protogen/collector/metrics/v1" otlptrace "go.opentelemetry.io/collector/internal/data/protogen/collector/trace/v1" ) @@ -55,10 +54,8 @@ func (e *fileExporter) ConsumeMetrics(_ context.Context, md pdata.Metrics) error } func (e *fileExporter) ConsumeLogs(_ context.Context, ld pdata.Logs) error { - request := otlplogs.ExportLogsServiceRequest{ - ResourceLogs: internal.LogsToOtlp(ld.InternalRep()), - } - return exportMessageAsLine(e, &request) + request := internal.LogsToOtlp(ld.InternalRep()) + return exportMessageAsLine(e, request) } func exportMessageAsLine(e *fileExporter, message proto.Message) error { diff --git a/exporter/fileexporter/file_exporter_test.go b/exporter/fileexporter/file_exporter_test.go index 2881b3c1d0b..0d7f413b7a6 100644 --- a/exporter/fileexporter/file_exporter_test.go +++ b/exporter/fileexporter/file_exporter_test.go @@ -73,105 +73,109 @@ func TestFileLogsExporterNoErrors(t *testing.T) { require.NotNil(t, exporter) now := time.Now() - ld := []*logspb.ResourceLogs{ - { - Resource: otresourcepb.Resource{ - Attributes: []otlpcommon.KeyValue{ - { - Key: "attr1", - Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value1"}}, - }, - }, - }, - InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ - { - Logs: []*logspb.LogRecord{ - { - TimeUnixNano: uint64(now.UnixNano()), - Name: "logA", - }, + otlp := &collectorlogs.ExportLogsServiceRequest{ + ResourceLogs: []*logspb.ResourceLogs{ + { + Resource: otresourcepb.Resource{ + Attributes: []otlpcommon.KeyValue{ { - TimeUnixNano: uint64(now.UnixNano()), - Name: "logB", + Key: "attr1", + Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value1"}}, }, }, }, - }, - }, - { - Resource: otresourcepb.Resource{ - Attributes: []otlpcommon.KeyValue{ + InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ { - Key: "attr2", - Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value2"}}, + Logs: []*logspb.LogRecord{ + { + TimeUnixNano: uint64(now.UnixNano()), + Name: "logA", + }, + { + TimeUnixNano: uint64(now.UnixNano()), + Name: "logB", + }, + }, }, }, }, - InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ - { - Logs: []*logspb.LogRecord{ + { + Resource: otresourcepb.Resource{ + Attributes: []otlpcommon.KeyValue{ { - TimeUnixNano: uint64(now.UnixNano()), - Name: "logC", + Key: "attr2", + Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value2"}}, + }, + }, + }, + InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ + { + Logs: []*logspb.LogRecord{ + { + TimeUnixNano: uint64(now.UnixNano()), + Name: "logC", + }, }, }, }, }, }, } - assert.NoError(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromInternalRep(internal.LogsFromOtlp(ld)))) + assert.NoError(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromInternalRep(internal.LogsFromOtlp(otlp)))) assert.NoError(t, exporter.Shutdown(context.Background())) var unmarshaler = &jsonpb.Unmarshaler{} var j collectorlogs.ExportLogsServiceRequest assert.NoError(t, unmarshaler.Unmarshal(mf, &j)) - assert.EqualValues(t, ld, j.ResourceLogs) + assert.EqualValues(t, otlp.ResourceLogs, j.ResourceLogs) } func TestFileLogsExporterErrors(t *testing.T) { now := time.Now() - ld := []*logspb.ResourceLogs{ - { - Resource: otresourcepb.Resource{ - Attributes: []otlpcommon.KeyValue{ - { - Key: "attr1", - Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value1"}}, - }, - }, - }, - InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ - { - Logs: []*logspb.LogRecord{ - { - TimeUnixNano: uint64(now.UnixNano()), - Name: "logA", - }, + otlp := &collectorlogs.ExportLogsServiceRequest{ + ResourceLogs: []*logspb.ResourceLogs{ + { + Resource: otresourcepb.Resource{ + Attributes: []otlpcommon.KeyValue{ { - TimeUnixNano: uint64(now.UnixNano()), - Name: "logB", + Key: "attr1", + Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value1"}}, }, }, }, - }, - }, - { - Resource: otresourcepb.Resource{ - Attributes: []otlpcommon.KeyValue{ + InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ { - Key: "attr2", - Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value2"}}, + Logs: []*logspb.LogRecord{ + { + TimeUnixNano: uint64(now.UnixNano()), + Name: "logA", + }, + { + TimeUnixNano: uint64(now.UnixNano()), + Name: "logB", + }, + }, }, }, }, - InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ - { - Logs: []*logspb.LogRecord{ + { + Resource: otresourcepb.Resource{ + Attributes: []otlpcommon.KeyValue{ { - TimeUnixNano: uint64(now.UnixNano()), - Name: "logC", + Key: "attr2", + Value: otlpcommon.AnyValue{Value: &otlpcommon.AnyValue_StringValue{StringValue: "value2"}}, + }, + }, + }, + InstrumentationLibraryLogs: []*logspb.InstrumentationLibraryLogs{ + { + Logs: []*logspb.LogRecord{ + { + TimeUnixNano: uint64(now.UnixNano()), + Name: "logC", + }, }, }, }, @@ -210,7 +214,7 @@ func TestFileLogsExporterErrors(t *testing.T) { exporter := &fileExporter{file: mf} require.NotNil(t, exporter) - assert.Error(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromInternalRep(internal.LogsFromOtlp(ld)))) + assert.Error(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromInternalRep(internal.LogsFromOtlp(otlp)))) assert.NoError(t, exporter.Shutdown(context.Background())) }) } diff --git a/exporter/otlpexporter/otlp.go b/exporter/otlpexporter/otlp.go index 0b1f72e96d8..a2907eecba3 100644 --- a/exporter/otlpexporter/otlp.go +++ b/exporter/otlpexporter/otlp.go @@ -85,10 +85,8 @@ func (e *exporterImp) pushMetricsData(ctx context.Context, md pdata.Metrics) err return nil } -func (e *exporterImp) pushLogData(ctx context.Context, logs pdata.Logs) error { - request := &otlplogs.ExportLogsServiceRequest{ - ResourceLogs: internal.LogsToOtlp(logs.InternalRep()), - } +func (e *exporterImp) pushLogData(ctx context.Context, ld pdata.Logs) error { + request := internal.LogsToOtlp(ld.InternalRep()) if err := e.w.exportLogs(ctx, request); err != nil { return fmt.Errorf("failed to push log data via OTLP exporter: %w", err) } diff --git a/exporter/otlpexporter/otlp_test.go b/exporter/otlpexporter/otlp_test.go index 421e49849cc..9118fd7d657 100644 --- a/exporter/otlpexporter/otlp_test.go +++ b/exporter/otlpexporter/otlp_test.go @@ -33,6 +33,7 @@ import ( "go.opentelemetry.io/collector/config/configgrpc" "go.opentelemetry.io/collector/config/configtls" "go.opentelemetry.io/collector/consumer/pdata" + "go.opentelemetry.io/collector/internal" otlplogs "go.opentelemetry.io/collector/internal/data/protogen/collector/logs/v1" otlpmetrics "go.opentelemetry.io/collector/internal/data/protogen/collector/metrics/v1" otlptraces "go.opentelemetry.io/collector/internal/data/protogen/collector/trace/v1" @@ -515,10 +516,7 @@ func TestSendLogData(t *testing.T) { // A request with 2 log entries. td = testdata.GenerateLogDataTwoLogsSameResource() - - expectedOTLPReq := &otlplogs.ExportLogsServiceRequest{ - ResourceLogs: testdata.GenerateLogOtlpSameResourceTwoLogs(), - } + expectedOTLPReq := internal.LogsToOtlp(td.Clone().InternalRep()) err = exp.ConsumeLogs(context.Background(), td) assert.NoError(t, err) diff --git a/internal/otlp_wrapper.go b/internal/otlp_wrapper.go index f367322d7a5..f069f130b14 100644 --- a/internal/otlp_wrapper.go +++ b/internal/otlp_wrapper.go @@ -14,19 +14,21 @@ package internal -import otlplogs "go.opentelemetry.io/collector/internal/data/protogen/logs/v1" +import ( + otlpcollectorlog "go.opentelemetry.io/collector/internal/data/protogen/collector/logs/v1" +) -// OtlpLogsWrapper is an intermediary struct that is declared in an internal package +// LogsWrapper is an intermediary struct that is declared in an internal package // as a way to prevent certain functions of pdata.Logs data type to be callable by // any code outside of this module. -type OtlpLogsWrapper struct { - Orig *[]*otlplogs.ResourceLogs +type LogsWrapper struct { + req *otlpcollectorlog.ExportLogsServiceRequest } -func LogsToOtlp(l OtlpLogsWrapper) []*otlplogs.ResourceLogs { - return *l.Orig +func LogsToOtlp(l LogsWrapper) *otlpcollectorlog.ExportLogsServiceRequest { + return l.req } -func LogsFromOtlp(logs []*otlplogs.ResourceLogs) OtlpLogsWrapper { - return OtlpLogsWrapper{Orig: &logs} +func LogsFromOtlp(req *otlpcollectorlog.ExportLogsServiceRequest) LogsWrapper { + return LogsWrapper{req: req} } diff --git a/internal/testdata/log.go b/internal/testdata/log.go index 2d5cbfb862a..acdae00b78d 100644 --- a/internal/testdata/log.go +++ b/internal/testdata/log.go @@ -19,6 +19,7 @@ import ( "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/internal/data" + otlpcollectorlog "go.opentelemetry.io/collector/internal/data/protogen/collector/logs/v1" otlpcommon "go.opentelemetry.io/collector/internal/data/protogen/common/v1" otlplogs "go.opentelemetry.io/collector/internal/data/protogen/logs/v1" ) @@ -33,8 +34,8 @@ func GenerateLogDataEmpty() pdata.Logs { return ld } -func generateLogOtlpEmpty() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs(nil) +func generateLogOtlpEmpty() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{} } func GenerateLogDataOneEmptyResourceLogs() pdata.Logs { @@ -43,9 +44,11 @@ func GenerateLogDataOneEmptyResourceLogs() pdata.Logs { return ld } -func generateLogOtlpOneEmptyResourceLogs() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs{ - {}, +func generateLogOtlpOneEmptyResourceLogs() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + {}, + }, } } @@ -56,10 +59,12 @@ func GenerateLogDataNoLogRecords() pdata.Logs { return ld } -func generateLogOtlpNoLogRecords() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs{ - { - Resource: generateOtlpResource1(), +func generateLogOtlpNoLogRecords() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + Resource: generateOtlpResource1(), + }, }, } } @@ -72,14 +77,16 @@ func GenerateLogDataOneEmptyLogs() pdata.Logs { return ld } -func generateLogOtlpOneEmptyLogs() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs{ - { - Resource: generateOtlpResource1(), - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ - { - Logs: []*otlplogs.LogRecord{ - {}, +func generateLogOtlpOneEmptyLogs() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + Resource: generateOtlpResource1(), + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ + { + Logs: []*otlplogs.LogRecord{ + {}, + }, }, }, }, @@ -97,13 +104,15 @@ func GenerateLogDataOneLogNoResource() pdata.Logs { return ld } -func generateLogOtlpOneLogNoResource() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs{ - { - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ - { - Logs: []*otlplogs.LogRecord{ - generateOtlpLogOne(), +func generateLogOtlpOneLogNoResource() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ + { + Logs: []*otlplogs.LogRecord{ + generateOtlpLogOne(), + }, }, }, }, @@ -121,14 +130,16 @@ func GenerateLogDataOneLog() pdata.Logs { return ld } -func generateLogOtlpOneLog() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs{ - { - Resource: generateOtlpResource1(), - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ - { - Logs: []*otlplogs.LogRecord{ - generateOtlpLogOne(), +func generateLogOtlpOneLog() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + Resource: generateOtlpResource1(), + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ + { + Logs: []*otlplogs.LogRecord{ + generateOtlpLogOne(), + }, }, }, }, @@ -146,16 +157,18 @@ func GenerateLogDataTwoLogsSameResource() pdata.Logs { return ld } -// GenerateLogOtlpSameResourceTwologs returns the OTLP representation of the GenerateLogOtlpSameResourceTwologs. -func GenerateLogOtlpSameResourceTwoLogs() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs{ - { - Resource: generateOtlpResource1(), - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ - { - Logs: []*otlplogs.LogRecord{ - generateOtlpLogOne(), - generateOtlpLogTwo(), +// generateLogOtlpSameResourceTwologs returns the OTLP representation of the GenerateLogOtlpSameResourceTwologs. +func generateLogOtlpSameResourceTwoLogs() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + Resource: generateOtlpResource1(), + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ + { + Logs: []*otlplogs.LogRecord{ + generateOtlpLogOne(), + generateOtlpLogTwo(), + }, }, }, }, @@ -180,25 +193,27 @@ func GenerateLogDataTwoLogsSameResourceOneDifferent() pdata.Logs { return ld } -func generateLogOtlpTwoLogsSameResourceOneDifferent() []*otlplogs.ResourceLogs { - return []*otlplogs.ResourceLogs{ - { - Resource: generateOtlpResource1(), - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ - { - Logs: []*otlplogs.LogRecord{ - generateOtlpLogOne(), - generateOtlpLogTwo(), +func generateLogOtlpTwoLogsSameResourceOneDifferent() *otlpcollectorlog.ExportLogsServiceRequest { + return &otlpcollectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplogs.ResourceLogs{ + { + Resource: generateOtlpResource1(), + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ + { + Logs: []*otlplogs.LogRecord{ + generateOtlpLogOne(), + generateOtlpLogTwo(), + }, }, }, }, - }, - { - Resource: generateOtlpResource2(), - InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ - { - Logs: []*otlplogs.LogRecord{ - generateOtlpLogThree(), + { + Resource: generateOtlpResource2(), + InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{ + { + Logs: []*otlplogs.LogRecord{ + generateOtlpLogThree(), + }, }, }, }, diff --git a/internal/testdata/log_test.go b/internal/testdata/log_test.go index 7c213b52118..27eb489c349 100644 --- a/internal/testdata/log_test.go +++ b/internal/testdata/log_test.go @@ -21,13 +21,13 @@ import ( "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/internal" - otlplogs "go.opentelemetry.io/collector/internal/data/protogen/logs/v1" + otlpcollectorlog "go.opentelemetry.io/collector/internal/data/protogen/collector/logs/v1" ) type logTestCase struct { name string ld pdata.Logs - otlp []*otlplogs.ResourceLogs + otlp *otlpcollectorlog.ExportLogsServiceRequest } func generateAllLogTestCases() []logTestCase { @@ -65,7 +65,7 @@ func generateAllLogTestCases() []logTestCase { { name: "two-records-same-resource", ld: GenerateLogDataTwoLogsSameResource(), - otlp: GenerateLogOtlpSameResourceTwoLogs(), + otlp: generateLogOtlpSameResourceTwoLogs(), }, { name: "two-records-same-resource-one-different", diff --git a/receiver/otlpreceiver/logs/otlp.go b/receiver/otlpreceiver/logs/otlp.go index ae9434893cd..ef41687d7b8 100644 --- a/receiver/otlpreceiver/logs/otlp.go +++ b/receiver/otlpreceiver/logs/otlp.go @@ -54,7 +54,7 @@ func (r *Receiver) Export(ctx context.Context, req *collectorlog.ExportLogsServi // We need to ensure that it propagates the receiver name as a tag ctxWithReceiverName := obsreport.ReceiverContext(ctx, r.instanceName, receiverTransport) - ld := pdata.LogsFromInternalRep(internal.LogsFromOtlp(req.ResourceLogs)) + ld := pdata.LogsFromInternalRep(internal.LogsFromOtlp(req)) err := r.sendToNextConsumer(ctxWithReceiverName, ld) if err != nil { return nil, err diff --git a/receiver/otlpreceiver/logs/otlp_test.go b/receiver/otlpreceiver/logs/otlp_test.go index 43aa8fefd18..fe3f1008aae 100644 --- a/receiver/otlpreceiver/logs/otlp_test.go +++ b/receiver/otlpreceiver/logs/otlp_test.go @@ -55,16 +55,18 @@ func TestExport(t *testing.T) { unixnanos := uint64(12578940000000012345) traceID := [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1} spanID := [8]byte{8, 7, 6, 5, 4, 3, 2, 1} - resourceLogs := []*otlplog.ResourceLogs{ - { - InstrumentationLibraryLogs: []*otlplog.InstrumentationLibraryLogs{ - { - Logs: []*otlplog.LogRecord{ - { - TraceId: data.NewTraceID(traceID), - SpanId: data.NewSpanID(spanID), - Name: "operationB", - TimeUnixNano: unixnanos, + otlp := &collectorlog.ExportLogsServiceRequest{ + ResourceLogs: []*otlplog.ResourceLogs{ + { + InstrumentationLibraryLogs: []*otlplog.InstrumentationLibraryLogs{ + { + Logs: []*otlplog.LogRecord{ + { + TraceId: data.NewTraceID(traceID), + SpanId: data.NewSpanID(spanID), + Name: "operationB", + TimeUnixNano: unixnanos, + }, }, }, }, @@ -74,13 +76,9 @@ func TestExport(t *testing.T) { // Keep log data to compare the test result against it // Clone needed because OTLP proto XXX_ fields are altered in the GRPC downstream - traceData := pdata.LogsFromInternalRep(internal.LogsFromOtlp(resourceLogs)).Clone() - - req := &collectorlog.ExportLogsServiceRequest{ - ResourceLogs: resourceLogs, - } + ld := pdata.LogsFromInternalRep(internal.LogsFromOtlp(otlp)).Clone() - resp, err := traceClient.Export(context.Background(), req) + resp, err := traceClient.Export(context.Background(), otlp) require.NoError(t, err, "Failed to export trace: %v", err) require.NotNil(t, resp, "The response is missing") @@ -88,7 +86,7 @@ func TestExport(t *testing.T) { require.Equal(t, 1, len(logSink.AllLogs()), "unexpected length: %v", len(logSink.AllLogs())) - assert.EqualValues(t, traceData, logSink.AllLogs()[0]) + assert.EqualValues(t, ld, logSink.AllLogs()[0]) } func TestExport_EmptyRequest(t *testing.T) { diff --git a/testbed/testbed/data_providers.go b/testbed/testbed/data_providers.go index 58257cf9944..cf8c2bf5338 100644 --- a/testbed/testbed/data_providers.go +++ b/testbed/testbed/data_providers.go @@ -365,7 +365,7 @@ func NewFileDataProvider(filePath string, dataType configmodels.DataType) (*File } message = &msg - md := pdata.LogsFromInternalRep(internal.LogsFromOtlp(msg.ResourceLogs)) + md := pdata.LogsFromInternalRep(internal.LogsFromOtlp(&msg)) dataPointCount = md.LogRecordCount() }