Skip to content

Commit

Permalink
Make pdata.LogsToOtlp and pdata.LogsFromOtlp impossible to call from …
Browse files Browse the repository at this point in the history
…outside this module

LogsToOtlp and LogsFromOtlp are intended to be used only by OTLP exporter (for which
OTLP is the native data format) and fileexporter (for which we do want to output internal
representation for debugging purposes).

All other code is supposed to use public member functions of pdata.Logs struct.

This changes makes it impossible to use LogsToOtlp and LogsFromOtlp outside this module
by hiding them behind an intermediary struct that is declared in an internal package.

This is necessary to prevent accidental use of LogsToOtlp and LogsFromOtlp and the OTLP
data structs, which we prohibit to do since the OTLP data structs may change in the
future.
  • Loading branch information
Tigran Najaryan committed Sep 11, 2020
1 parent c0b3c61 commit 07db553
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 29 deletions.
27 changes: 16 additions & 11 deletions consumer/pdata/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package pdata
import (
"github.com/gogo/protobuf/proto"

"go.opentelemetry.io/collector/internal"
otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1"
)

Expand All @@ -32,31 +33,35 @@ type Logs struct {
orig *[]*otlplogs.ResourceLogs
}

// LogsFromOtlp creates the internal Logs representation from the ProtoBuf.
func LogsFromOtlp(orig []*otlplogs.ResourceLogs) Logs {
// NewLogs creates a new Logs.
func NewLogs() Logs {
orig := []*otlplogs.ResourceLogs(nil)
return Logs{&orig}
}

// LogsToOtlp converts the internal Logs to the ProtoBuf.
func LogsToOtlp(ld Logs) []*otlplogs.ResourceLogs {
return *ld.orig
// 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}
}

// NewLogs creates a new Logs.
func NewLogs() Logs {
orig := []*otlplogs.ResourceLogs(nil)
return Logs{&orig}
// 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}
}

// Clone returns a copy of Logs.
func (ld Logs) Clone() Logs {
otlp := LogsToOtlp(ld)
otlp := *ld.orig
resourceSpansClones := make([]*otlplogs.ResourceLogs, 0, len(otlp))
for _, resourceSpans := range otlp {
resourceSpansClones = append(resourceSpansClones,
proto.Clone(resourceSpans).(*otlplogs.ResourceLogs))
}
return LogsFromOtlp(resourceSpansClones)
return Logs{orig: &resourceSpansClones}
}

// LogRecordCount calculates the total number of log records.
Expand Down
15 changes: 8 additions & 7 deletions consumer/pdata/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/internal"
otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1"
)

Expand Down Expand Up @@ -46,26 +47,26 @@ func TestLogRecordCount(t *testing.T) {
}

func TestLogRecordCountWithNils(t *testing.T) {
assert.EqualValues(t, 0, LogsFromOtlp([]*otlplogs.ResourceLogs{nil, {}}).LogRecordCount())
assert.EqualValues(t, 0, LogsFromOtlp([]*otlplogs.ResourceLogs{
assert.EqualValues(t, 0, LogsFromInternalRep(internal.LogsFromOtlp([]*otlplogs.ResourceLogs{nil, {}})).LogRecordCount())
assert.EqualValues(t, 0, LogsFromInternalRep(internal.LogsFromOtlp([]*otlplogs.ResourceLogs{
{
InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{nil, {}},
},
}).LogRecordCount())
assert.EqualValues(t, 2, LogsFromOtlp([]*otlplogs.ResourceLogs{
})).LogRecordCount())
assert.EqualValues(t, 2, LogsFromInternalRep(internal.LogsFromOtlp([]*otlplogs.ResourceLogs{
{
InstrumentationLibraryLogs: []*otlplogs.InstrumentationLibraryLogs{
{
Logs: []*otlplogs.LogRecord{nil, {}},
},
},
},
}).LogRecordCount())
})).LogRecordCount())
}

func TestToFromLogProto(t *testing.T) {
otlp := []*otlplogs.ResourceLogs(nil)
td := LogsFromOtlp(otlp)
td := LogsFromInternalRep(internal.LogsFromOtlp(otlp))
assert.EqualValues(t, NewLogs(), td)
assert.EqualValues(t, otlp, LogsToOtlp(td))
assert.EqualValues(t, otlp, *td.orig)
}
3 changes: 2 additions & 1 deletion exporter/fileexporter/file_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/internal"
otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/logs/v1"
otlpmetrics "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/metrics/v1"
otlptrace "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/trace/v1"
Expand Down Expand Up @@ -55,7 +56,7 @@ func (e *fileExporter) ConsumeMetrics(_ context.Context, md pdata.Metrics) error

func (e *fileExporter) ConsumeLogs(_ context.Context, ld pdata.Logs) error {
request := otlplogs.ExportLogsServiceRequest{
ResourceLogs: pdata.LogsToOtlp(ld),
ResourceLogs: internal.LogsToOtlp(ld.InternalRep()),
}
return exportMessageAsLine(e, &request)
}
Expand Down
5 changes: 3 additions & 2 deletions exporter/fileexporter/file_exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/internal"
collectorlogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/logs/v1"
collectormetrics "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/metrics/v1"
collectortrace "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/trace/v1"
Expand Down Expand Up @@ -118,7 +119,7 @@ func TestFileLogsExporterNoErrors(t *testing.T) {
},
},
}
assert.NoError(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromOtlp(ld)))
assert.NoError(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromInternalRep(internal.LogsFromOtlp(ld))))
assert.NoError(t, exporter.Shutdown(context.Background()))

var unmarshaler = &jsonpb.Unmarshaler{}
Expand Down Expand Up @@ -209,7 +210,7 @@ func TestFileLogsExporterErrors(t *testing.T) {
exporter := &fileExporter{file: mf}
require.NotNil(t, exporter)

assert.Error(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromOtlp(ld)))
assert.Error(t, exporter.ConsumeLogs(context.Background(), pdata.LogsFromInternalRep(internal.LogsFromOtlp(ld))))
assert.NoError(t, exporter.Shutdown(context.Background()))
})
}
Expand Down
3 changes: 2 additions & 1 deletion exporter/otlpexporter/otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"go.opentelemetry.io/collector/consumer/consumererror"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/exporter/exporterhelper"
"go.opentelemetry.io/collector/internal"
otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/logs/v1"
otlpmetrics "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/metrics/v1"
otlptrace "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/trace/v1"
Expand Down Expand Up @@ -101,7 +102,7 @@ func (e *exporterImp) pushMetricsData(ctx context.Context, md pdata.Metrics) (in

func (e *exporterImp) pushLogData(ctx context.Context, logs pdata.Logs) (int, error) {
request := &otlplogs.ExportLogsServiceRequest{
ResourceLogs: pdata.LogsToOtlp(logs),
ResourceLogs: internal.LogsToOtlp(logs.InternalRep()),
}
err := e.w.exportLogs(ctx, request)

Expand Down
7 changes: 4 additions & 3 deletions internal/data/testdata/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1"

"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/internal"
otlpcommon "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/common/v1"
)

Expand Down Expand Up @@ -54,7 +55,7 @@ func generateLogOtlpOneEmptyResourceLogs() []*otlplogs.ResourceLogs {
}

func GenerateLogDataOneEmptyOneNilResourceLogs() pdata.Logs {
return pdata.LogsFromOtlp(generateLogOtlpOneEmptyOneNilResourceLogs())
return pdata.LogsFromInternalRep(internal.LogsFromOtlp(generateLogOtlpOneEmptyOneNilResourceLogs()))

}

Expand Down Expand Up @@ -104,7 +105,7 @@ func generateLogOtlpOneEmptyLogs() []*otlplogs.ResourceLogs {
}

func GenerateLogDataOneEmptyOneNilLogRecord() pdata.Logs {
return pdata.LogsFromOtlp(generateLogOtlpOneEmptyOneNilLogRecord())
return pdata.LogsFromInternalRep(internal.LogsFromOtlp(generateLogOtlpOneEmptyOneNilLogRecord()))
}

func generateLogOtlpOneEmptyOneNilLogRecord() []*otlplogs.ResourceLogs {
Expand Down Expand Up @@ -173,7 +174,7 @@ func generateLogOtlpOneLog() []*otlplogs.ResourceLogs {
}

func GenerateLogDataOneLogOneNil() pdata.Logs {
return pdata.LogsFromOtlp(generateLogOtlpOneLogOneNil())
return pdata.LogsFromInternalRep(internal.LogsFromOtlp(generateLogOtlpOneLogOneNil()))
}

func generateLogOtlpOneLogOneNil() []*otlplogs.ResourceLogs {
Expand Down
5 changes: 3 additions & 2 deletions internal/data/testdata/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/stretchr/testify/assert"

"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/internal"
otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1"
)

Expand Down Expand Up @@ -96,9 +97,9 @@ func TestToFromOtlpLog(t *testing.T) {
for i := range allTestCases {
test := allTestCases[i]
t.Run(test.name, func(t *testing.T) {
ld := pdata.LogsFromOtlp(test.otlp)
ld := pdata.LogsFromInternalRep(internal.LogsFromOtlp(test.otlp))
assert.EqualValues(t, test.ld, ld)
otlp := pdata.LogsToOtlp(ld)
otlp := internal.LogsToOtlp(ld.InternalRep())
assert.EqualValues(t, test.otlp, otlp)
})
}
Expand Down
15 changes: 15 additions & 0 deletions internal/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package internal
32 changes: 32 additions & 0 deletions internal/otlp_wrapper.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package internal

import otlplogs "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1"

// OtlpLogsWrapper 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
}

func LogsToOtlp(l OtlpLogsWrapper) []*otlplogs.ResourceLogs {
return *l.Orig
}

func LogsFromOtlp(logs []*otlplogs.ResourceLogs) OtlpLogsWrapper {
return OtlpLogsWrapper{Orig: &logs}
}
3 changes: 2 additions & 1 deletion receiver/otlpreceiver/logs/otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"go.opentelemetry.io/collector/client"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/internal"
collectorlog "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/logs/v1"
"go.opentelemetry.io/collector/obsreport"
)
Expand Down Expand Up @@ -53,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, receiverTagValue)

ld := pdata.LogsFromOtlp(req.ResourceLogs)
ld := pdata.LogsFromInternalRep(internal.LogsFromOtlp(req.ResourceLogs))
err := r.sendToNextConsumer(ctxWithReceiverName, ld)
if err != nil {
return nil, err
Expand Down
3 changes: 2 additions & 1 deletion receiver/otlpreceiver/logs/otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/pdata"
"go.opentelemetry.io/collector/exporter/exportertest"
"go.opentelemetry.io/collector/internal"
collectorlog "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/collector/logs/v1"
otlplog "go.opentelemetry.io/collector/internal/data/opentelemetry-proto-gen/logs/v1"
"go.opentelemetry.io/collector/obsreport"
Expand Down Expand Up @@ -77,7 +78,7 @@ 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.LogsFromOtlp(resourceLogs).Clone()
traceData := pdata.LogsFromInternalRep(internal.LogsFromOtlp(resourceLogs)).Clone()

req := &collectorlog.ExportLogsServiceRequest{
ResourceLogs: resourceLogs,
Expand Down

0 comments on commit 07db553

Please sign in to comment.