Add comments and test for 64-bit field alignment

api/core/number_test.go

@@ -67,12 +67,14 @@ func TestNumber(t *testing.T) {
 	cmpsForPos := [3]int{1, 1, 0}
 
 	type testcase struct {
-		n    Number
+		// n needs to be aligned for 64-bit atomic operations.
+		n Number
+		// nums needs to be aligned for 64-bit atomic operations.
+		nums [3]Number
 		kind NumberKind
 		pos  bool
 		zero bool
 		neg  bool
-		nums [3]Number
 		cmps [3]int
 	}
 	testcases := []testcase{

api/metric/alignment_test.go

@@ -0,0 +1,24 @@
+package metric
+
+import (
+	"os"
+	"testing"
+	"unsafe"
+
+	ottest "go.opentelemetry.io/otel/internal/testing"
+)
+
+// Ensure struct alignment prior to running tests.
+func TestMain(m *testing.M) {
+	fields := []ottest.FieldOffset{
+		{
+			Name:   "Measurement.number",
+			Offset: unsafe.Offsetof(Measurement{}.number),
+		},
+	}
+	if !ottest.Aligned8Byte(fields, os.Stderr) {
+		os.Exit(1)
+	}
+
+	os.Exit(m.Run())
+}

api/metric/api.go

@@ -85,8 +85,9 @@ type MeasureOptionApplier interface {
 // values. Instances of this type should be created by instruments
 // (e.g., Int64Counter.Measurement()).
 type Measurement struct {
-	instrument InstrumentImpl
+	// number needs to be aligned for 64-bit atomic operations.
 	number     core.Number
+	instrument InstrumentImpl
 }
 
 // Instrument returns the instrument that created this measurement.

internal/metric/mock.go

@@ -41,9 +41,10 @@ type (
 	}
 
 	Batch struct {
+		// Measurement needs to be aligned for 64-bit atomic operations.
+		Measurements []Measurement
 		Ctx          context.Context
 		LabelSet     *LabelSet
-		Measurements []Measurement
 	}
 
 	MeterProvider struct {
@@ -58,8 +59,9 @@ type (
 	Kind int8
 
 	Measurement struct {
-		Instrument *Instrument
+		// Number needs to be aligned for 64-bit atomic operations.
 		Number     core.Number
+		Instrument *Instrument
 	}
 )
 

internal/metric/mock_test.go

@@ -0,0 +1,28 @@
+package metric
+
+import (
+	"os"
+	"testing"
+	"unsafe"
+
+	ottest "go.opentelemetry.io/otel/internal/testing"
+)
+
+// Ensure struct alignment prior to running tests.
+func TestMain(m *testing.M) {
+	fields := []ottest.FieldOffset{
+		{
+			Name:   "Batch.Measurments",
+			Offset: unsafe.Offsetof(Batch{}.Measurements),
+		},
+		{
+			Name:   "Measurement.Number",
+			Offset: unsafe.Offsetof(Measurement{}.Number),
+		},
+	}
+	if !ottest.Aligned8Byte(fields, os.Stderr) {
+		os.Exit(1)
+	}
+
+	os.Exit(m.Run())
+}

internal/testing/alignment.go

@@ -0,0 +1,57 @@
+package testing
+
+/*
+This file contains common utilities and objects to validate memory alignment
+of Go types. The primary use of this functionality is intended to ensure
+`struct` fields that need to be 64-bit aligned so they can be passed as
+arguments to 64-bit atomic operations.
+
+The common workflow is to define a slice of `FieldOffset` and pass them to the
+`Aligned8Byte` function from within a `TestMain` function from a package's
+tests. It is important to make this call from the `TestMain` function prior
+to running the rest of the test suit as it can provide useful diagnostics
+about field alignment instead of ambiguous nil pointer dereference and runtime
+panic.
+
+For more information:
+https://github.com/open-telemetry/opentelemetry-go/issues/341
+*/
+
+import (
+	"fmt"
+	"io"
+)
+
+// FieldOffset is a preprocessor representation of a struct field alignment.
+type FieldOffset struct {
+	// Name of the field.
+	Name string
+
+	// Offset of the field in bytes.
+	//
+	// To compute this at compile time use unsafe.Offsetof.
+	Offset uintptr
+}
+
+// Aligned8Byte returns if all fields are aligned modulo 8-bytes.
+//
+// Error messaging is printed to out for any fileds determined misaligned.
+func Aligned8Byte(fields []FieldOffset, out io.Writer) bool {
+	misaligned := make([]FieldOffset, 0)
+	for _, f := range fields {
+		if f.Offset%8 != 0 {
+			misaligned = append(misaligned, f)
+		}
+	}
+
+	if len(misaligned) == 0 {
+		return true
+	}
+
+	fmt.Fprintln(out, "struct fields not aligned for 64-bit atomic operations:")
+	for _, f := range misaligned {
+		fmt.Fprintf(out, "  %s: %d-byte offset\n", f.Name, f.Offset)
+	}
+
+	return false
+}

internal/trace/mock_tracer.go

@@ -28,12 +28,14 @@ import (
 // It only supports ChildOf option. SpanId is atomically increased every time a
 // new span is created.
 type MockTracer struct {
-	// Sampled specifies if the new span should be sampled or not.
-	Sampled bool
-
 	// StartSpanID is used to initialize spanId. It is incremented by one
 	// every time a new span is created.
+	//
+	// StartSpanID has to be aligned for 64-bit atomic operations.
 	StartSpanID *uint64
+
+	// Sampled specifies if the new span should be sampled or not.
+	Sampled bool
 }
 
 var _ apitrace.Tracer = (*MockTracer)(nil)

internal/trace/mock_tracer_test.go

@@ -0,0 +1,24 @@
+package trace
+
+import (
+	"os"
+	"testing"
+	"unsafe"
+
+	ottest "go.opentelemetry.io/otel/internal/testing"
+)
+
+// Ensure struct alignment prior to running tests.
+func TestMain(m *testing.M) {
+	fields := []ottest.FieldOffset{
+		{
+			Name:   "MockTracer.StartSpanID",
+			Offset: unsafe.Offsetof(MockTracer{}.StartSpanID),
+		},
+	}
+	if !ottest.Aligned8Byte(fields, os.Stderr) {
+		os.Exit(1)
+	}
+
+	os.Exit(m.Run())
+}

sdk/metric/aggregator/array/array.go

@@ -28,10 +28,11 @@ import (
 
 type (
 	Aggregator struct {
+		// ckptSum needs to be aligned for 64-bit atomic operations.
+		ckptSum    core.Number
 		lock       sync.Mutex
 		current    points
 		checkpoint points
-		ckptSum    core.Number
 	}
 
 	points []core.Number

sdk/metric/aggregator/array/array_test.go

@@ -18,16 +18,34 @@ import (
 	"context"
 	"fmt"
 	"math"
+	"os"
 	"testing"
+	"unsafe"
 
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel/api/core"
+	ottest "go.opentelemetry.io/otel/internal/testing"
 	export "go.opentelemetry.io/otel/sdk/export/metric"
 	"go.opentelemetry.io/otel/sdk/export/metric/aggregator"
 	"go.opentelemetry.io/otel/sdk/metric/aggregator/test"
 )
 
+// Ensure struct alignment prior to running tests.
+func TestMain(m *testing.M) {
+	fields := []ottest.FieldOffset{
+		{
+			Name:   "Aggregator.ckptSum",
+			Offset: unsafe.Offsetof(Aggregator{}.ckptSum),
+		},
+	}
+	if !ottest.Aligned8Byte(fields, os.Stderr) {
+		os.Exit(1)
+	}
+
+	os.Exit(m.Run())
+}
+
 type updateTest struct {
 	count    int
 	absolute bool

sdk/metric/aggregator/counter/counter.go

@@ -25,9 +25,11 @@ import (
 // Aggregator aggregates counter events.
 type Aggregator struct {
 	// current holds current increments to this counter record
+	// current needs to be aligned for 64-bit atomic operations.
 	current core.Number
 
 	// checkpoint is a temporary used during Checkpoint()
+	// checkpoint needs to be aligned for 64-bit atomic operations.
 	checkpoint core.Number
 }
 

sdk/metric/aggregator/counter/counter_test.go

@@ -16,17 +16,39 @@ package counter
 
 import (
 	"context"
+	"os"
 	"testing"
+	"unsafe"
 
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel/api/core"
+	ottest "go.opentelemetry.io/otel/internal/testing"
 	export "go.opentelemetry.io/otel/sdk/export/metric"
 	"go.opentelemetry.io/otel/sdk/metric/aggregator/test"
 )
 
 const count = 100
 
+// Ensure struct alignment prior to running tests.
+func TestMain(m *testing.M) {
+	fields := []ottest.FieldOffset{
+		{
+			Name:   "Aggregator.current",
+			Offset: unsafe.Offsetof(Aggregator{}.current),
+		},
+		{
+			Name:   "Aggregator.checkpoint",
+			Offset: unsafe.Offsetof(Aggregator{}.checkpoint),
+		},
+	}
+	if !ottest.Aligned8Byte(fields, os.Stderr) {
+		os.Exit(1)
+	}
+
+	os.Exit(m.Run())
+}
+
 func TestCounterMonotonic(t *testing.T) {
 	ctx := context.Background()
 

sdk/metric/aggregator/gauge/gauge.go

@@ -45,6 +45,8 @@ type (
 	// a sequence number to determine the winner of a race.
 	gaugeData struct {
 		// value is the int64- or float64-encoded Set() data
+		//
+		// value needs to be aligned for 64-bit atomic operations.
 		value core.Number
 
 		// timestamp indicates when this record was submitted.

sdk/metric/aggregator/gauge/gauge_test.go

@@ -17,11 +17,14 @@ package gauge
 import (
 	"context"
 	"math/rand"
+	"os"
 	"testing"
+	"unsafe"
 
 	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/otel/api/core"
+	ottest "go.opentelemetry.io/otel/internal/testing"
 	export "go.opentelemetry.io/otel/sdk/export/metric"
 	"go.opentelemetry.io/otel/sdk/export/metric/aggregator"
 	"go.opentelemetry.io/otel/sdk/metric/aggregator/test"
@@ -31,6 +34,21 @@ const count = 100
 
 var _ export.Aggregator = &Aggregator{}
 
+// Ensure struct alignment prior to running tests.
+func TestMain(m *testing.M) {
+	fields := []ottest.FieldOffset{
+		{
+			Name:   "gaugeData.value",
+			Offset: unsafe.Offsetof(gaugeData{}.value),
+		},
+	}
+	if !ottest.Aligned8Byte(fields, os.Stderr) {
+		os.Exit(1)
+	}
+
+	os.Exit(m.Run())
+}
+
 func TestGaugeNonMonotonic(t *testing.T) {
 	ctx := context.Background()
 

sdk/metric/aggregator/minmaxsumcount/mmsc.go

@@ -26,12 +26,15 @@ type (
 	// Aggregator aggregates measure events, keeping only the max,
 	// sum, and count.
 	Aggregator struct {
-		current    state
+		// current has to be aligned for 64-bit atomic operations.
+		current state
+		// checkpoint has to be aligned for 64-bit atomic operations.
 		checkpoint state
 		kind       core.NumberKind
 	}
 
 	state struct {
+		// all fields have to be aligned for 64-bit atomic operations.
 		count core.Number
 		sum   core.Number
 		min   core.Number