Make pdata.LogsToOtlp and pdata.LogsFromOtlp impossible to call from …

…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.
open-telemetry · Sep 1, 2020 · f0a882f · f0a882f
1 parent 82525f3
commit f0a882f
Show file tree

Hide file tree

Showing 13 changed files with 89 additions and 116 deletions.
diff --git a/consumer/pdata/log.go b/consumer/pdata/log.go
@@ -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"
 )
 
@@ -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.

diff --git a/consumer/pdata/log_test.go b/consumer/pdata/log_test.go
@@ -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"
 )
 
@@ -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)
 }
diff --git a/exporter/fileexporter/file_exporter.go b/exporter/fileexporter/file_exporter.go
@@ -25,6 +25,7 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/consumer/pdatautil"
+	"go.opentelemetry.io/collector/internal"
 	"go.opentelemetry.io/collector/internal/data"
 	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"
@@ -57,7 +58,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)
 }

diff --git a/exporter/fileexporter/file_exporter_test.go b/exporter/fileexporter/file_exporter_test.go
@@ -24,6 +24,7 @@ import (
 
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/consumer/pdatautil"
+	"go.opentelemetry.io/collector/internal"
 	"go.opentelemetry.io/collector/internal/data"
 	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"
@@ -120,7 +121,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{}
@@ -211,7 +212,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()))
 		})
 	}

diff --git a/exporter/otlpexporter/otlp.go b/exporter/otlpexporter/otlp.go
@@ -31,6 +31,7 @@ import (
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/consumer/pdatautil"
 	"go.opentelemetry.io/collector/exporter/exporterhelper"
+	"go.opentelemetry.io/collector/internal"
 	"go.opentelemetry.io/collector/internal/data"
 	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"
@@ -104,7 +105,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)
 

diff --git a/internal/data/testdata/log.go b/internal/data/testdata/log.go
@@ -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"
 )
 
@@ -54,7 +55,7 @@ func generateLogOtlpOneEmptyResourceLogs() []*otlplogs.ResourceLogs {
 }
 
 func GenerateLogDataOneEmptyOneNilResourceLogs() pdata.Logs {
-	return pdata.LogsFromOtlp(generateLogOtlpOneEmptyOneNilResourceLogs())
+	return pdata.LogsFromInternalRep(internal.LogsFromOtlp(generateLogOtlpOneEmptyOneNilResourceLogs()))
 
 }
 
@@ -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 {
@@ -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 {

diff --git a/internal/data/testdata/log_test.go b/internal/data/testdata/log_test.go
@@ -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"
 )
 
@@ -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)
 		})
 	}

diff --git a/internal/empty_test.go b/internal/empty_test.go
@@ -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
diff --git a/internal/internal.go b/internal/internal.go
diff --git a/internal/internal_test.go b/internal/internal_test.go
diff --git a/internal/otlp_wrapper.go b/internal/otlp_wrapper.go
@@ -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}
+}
diff --git a/receiver/otlpreceiver/logs/otlp.go b/receiver/otlpreceiver/logs/otlp.go
@@ -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"
 )
@@ -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

diff --git a/receiver/otlpreceiver/logs/otlp_test.go b/receiver/otlpreceiver/logs/otlp_test.go
@@ -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"
@@ -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,