diff --git a/api/core/number_test.go b/api/core/number_test.go index 0fb75b89294f..67583686c54c 100644 --- a/api/core/number_test.go +++ b/api/core/number_test.go @@ -67,14 +67,12 @@ func TestNumber(t *testing.T) { cmpsForPos := [3]int{1, 1, 0} type testcase struct { - // 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 + n Number kind NumberKind pos bool zero bool neg bool + nums [3]Number cmps [3]int } testcases := []testcase{ diff --git a/api/metric/alignment_test.go b/api/metric/alignment_test.go deleted file mode 100644 index 92ca9937a0ab..000000000000 --- a/api/metric/alignment_test.go +++ /dev/null @@ -1,24 +0,0 @@ -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()) -} diff --git a/api/metric/api.go b/api/metric/api.go index 2e546c697a4a..f96bd75d5bc1 100644 --- a/api/metric/api.go +++ b/api/metric/api.go @@ -85,9 +85,8 @@ type MeasureOptionApplier interface { // values. Instances of this type should be created by instruments // (e.g., Int64Counter.Measurement()). type Measurement struct { - // number needs to be aligned for 64-bit atomic operations. - number core.Number instrument InstrumentImpl + number core.Number } // Instrument returns the instrument that created this measurement. diff --git a/internal/metric/mock.go b/internal/metric/mock.go index 8bd37574f5bd..68b7d7582a9f 100644 --- a/internal/metric/mock.go +++ b/internal/metric/mock.go @@ -41,10 +41,9 @@ type ( } Batch struct { - // Measurement needs to be aligned for 64-bit atomic operations. - Measurements []Measurement Ctx context.Context LabelSet *LabelSet + Measurements []Measurement } MeterProvider struct { @@ -59,9 +58,8 @@ type ( Kind int8 Measurement struct { - // Number needs to be aligned for 64-bit atomic operations. - Number core.Number Instrument *Instrument + Number core.Number } ) diff --git a/internal/metric/mock_test.go b/internal/metric/mock_test.go deleted file mode 100644 index 7995b17d02d5..000000000000 --- a/internal/metric/mock_test.go +++ /dev/null @@ -1,28 +0,0 @@ -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()) -} diff --git a/internal/testing/alignment.go b/internal/testing/alignment.go deleted file mode 100644 index 92b8b1e5834c..000000000000 --- a/internal/testing/alignment.go +++ /dev/null @@ -1,57 +0,0 @@ -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 -} diff --git a/internal/trace/mock_tracer.go b/internal/trace/mock_tracer.go index 98d973f8b5b5..d6d033d03be0 100644 --- a/internal/trace/mock_tracer.go +++ b/internal/trace/mock_tracer.go @@ -28,14 +28,12 @@ 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) diff --git a/internal/trace/mock_tracer_test.go b/internal/trace/mock_tracer_test.go deleted file mode 100644 index fd8791b18ce8..000000000000 --- a/internal/trace/mock_tracer_test.go +++ /dev/null @@ -1,24 +0,0 @@ -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()) -} diff --git a/sdk/metric/aggregator/array/array.go b/sdk/metric/aggregator/array/array.go index 3855e8cc28ca..f9b79d60df9c 100644 --- a/sdk/metric/aggregator/array/array.go +++ b/sdk/metric/aggregator/array/array.go @@ -28,11 +28,10 @@ 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 diff --git a/sdk/metric/aggregator/array/array_test.go b/sdk/metric/aggregator/array/array_test.go index 53af84153165..aa5ec5252903 100644 --- a/sdk/metric/aggregator/array/array_test.go +++ b/sdk/metric/aggregator/array/array_test.go @@ -18,34 +18,16 @@ 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 diff --git a/sdk/metric/aggregator/counter/counter.go b/sdk/metric/aggregator/counter/counter.go index 7d4b8cd0ed45..93660e0d2d92 100644 --- a/sdk/metric/aggregator/counter/counter.go +++ b/sdk/metric/aggregator/counter/counter.go @@ -25,11 +25,9 @@ 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 } diff --git a/sdk/metric/aggregator/counter/counter_test.go b/sdk/metric/aggregator/counter/counter_test.go index 4c66a78e4a11..7ce5d16cdac0 100644 --- a/sdk/metric/aggregator/counter/counter_test.go +++ b/sdk/metric/aggregator/counter/counter_test.go @@ -16,39 +16,17 @@ 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() diff --git a/sdk/metric/aggregator/gauge/gauge.go b/sdk/metric/aggregator/gauge/gauge.go index a9e73f73de87..5b86d7585bfd 100644 --- a/sdk/metric/aggregator/gauge/gauge.go +++ b/sdk/metric/aggregator/gauge/gauge.go @@ -45,8 +45,6 @@ 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. diff --git a/sdk/metric/aggregator/gauge/gauge_test.go b/sdk/metric/aggregator/gauge/gauge_test.go index 34fd7f53da88..aeff19643f1f 100644 --- a/sdk/metric/aggregator/gauge/gauge_test.go +++ b/sdk/metric/aggregator/gauge/gauge_test.go @@ -17,14 +17,11 @@ 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" @@ -34,21 +31,6 @@ 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() diff --git a/sdk/metric/aggregator/minmaxsumcount/mmsc.go b/sdk/metric/aggregator/minmaxsumcount/mmsc.go index 8d9b67c7d6aa..24876979fa28 100644 --- a/sdk/metric/aggregator/minmaxsumcount/mmsc.go +++ b/sdk/metric/aggregator/minmaxsumcount/mmsc.go @@ -26,15 +26,12 @@ type ( // Aggregator aggregates measure events, keeping only the max, // sum, and count. Aggregator struct { - // current has to be aligned for 64-bit atomic operations. - current state - // checkpoint has to be aligned for 64-bit atomic operations. + current state 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 diff --git a/sdk/metric/aggregator/minmaxsumcount/mmsc_test.go b/sdk/metric/aggregator/minmaxsumcount/mmsc_test.go index 21feacd19e17..fb2142fcd76f 100644 --- a/sdk/metric/aggregator/minmaxsumcount/mmsc_test.go +++ b/sdk/metric/aggregator/minmaxsumcount/mmsc_test.go @@ -18,14 +18,11 @@ import ( "context" "math" "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" @@ -62,41 +59,6 @@ var ( } ) -// 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), - }, - { - Name: "state.count", - Offset: unsafe.Offsetof(state{}.count), - }, - { - Name: "state.sum", - Offset: unsafe.Offsetof(state{}.sum), - }, - { - Name: "state.min", - Offset: unsafe.Offsetof(state{}.min), - }, - { - Name: "state.max", - Offset: unsafe.Offsetof(state{}.max), - }, - } - if !ottest.Aligned8Byte(fields, os.Stderr) { - os.Exit(1) - } - - os.Exit(m.Run()) -} - func TestMinMaxSumCountAbsolute(t *testing.T) { test.RunProfiles(t, func(t *testing.T, profile test.Profile) { minMaxSumCount(t, profile, positiveOnly) diff --git a/sdk/metric/aggregator/test/test.go b/sdk/metric/aggregator/test/test.go index 582183fc189f..3bd484c813f0 100644 --- a/sdk/metric/aggregator/test/test.go +++ b/sdk/metric/aggregator/test/test.go @@ -17,13 +17,10 @@ package test import ( "context" "math/rand" - "os" "sort" "testing" - "unsafe" "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" ) @@ -66,25 +63,9 @@ func RunProfiles(t *testing.T, f func(*testing.T, Profile)) { } } -// Ensure local struct alignment prior to running tests. -func TestMain(m *testing.M) { - fields := []ottest.FieldOffset{ - { - Name: "Numbers.numbers", - Offset: unsafe.Offsetof(Numbers{}.numbers), - }, - } - if !ottest.Aligned8Byte(fields, os.Stderr) { - os.Exit(1) - } - - os.Exit(m.Run()) -} - type Numbers struct { - // numbers has to be aligned for 64-bit atomic operations. - numbers []core.Number kind core.NumberKind + numbers []core.Number } func NewNumbers(kind core.NumberKind) Numbers { diff --git a/sdk/metric/alignment_test.go b/sdk/metric/alignment_test.go deleted file mode 100644 index 9e00ae9e827d..000000000000 --- a/sdk/metric/alignment_test.go +++ /dev/null @@ -1,36 +0,0 @@ -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: "record.refcount", - Offset: unsafe.Offsetof(record{}.refcount), - }, - { - Name: "record.collectedEpoch", - Offset: unsafe.Offsetof(record{}.collectedEpoch), - }, - { - Name: "record.modifiedEpoch", - Offset: unsafe.Offsetof(record{}.modifiedEpoch), - }, - { - Name: "record.reclaim", - Offset: unsafe.Offsetof(record{}.reclaim), - }, - } - if !ottest.Aligned8Byte(fields, os.Stderr) { - os.Exit(1) - } - - os.Exit(m.Run()) -} diff --git a/sdk/metric/monotone_test.go b/sdk/metric/monotone_test.go index 9fb6ee99fcb7..0c3c5dadd888 100644 --- a/sdk/metric/monotone_test.go +++ b/sdk/metric/monotone_test.go @@ -31,12 +31,11 @@ import ( ) type monotoneBatcher struct { - // currentValue needs to be aligned for 64-bit atomic operations. - currentValue *core.Number + t *testing.T + collections int + currentValue *core.Number currentTime *time.Time - - t *testing.T } func (*monotoneBatcher) AggregatorFor(*export.Descriptor) export.Aggregator { diff --git a/sdk/metric/sdk.go b/sdk/metric/sdk.go index 5c15eca14bcc..30cb13e79013 100644 --- a/sdk/metric/sdk.go +++ b/sdk/metric/sdk.go @@ -97,39 +97,31 @@ type ( // `record` in existence at a time, although at most one can // be referenced from the `SDK.current` map. record struct { + // labels is the LabelSet passed by the user. + labels *labels + + // descriptor describes the metric instrument. + descriptor *export.Descriptor + // refcount counts the number of active handles on // referring to this record. active handles prevent // removing the record from the current map. - // - // refcount has to be aligned for 64-bit atomic operations. refcount int64 // collectedEpoch is the epoch number for which this // record has been exported. This is modified by the // `Collect()` method. - // - // collectedEpoch has to be aligned for 64-bit atomic operations. collectedEpoch int64 // modifiedEpoch is the latest epoch number for which // this record was updated. Generally, if // modifiedEpoch is less than collectedEpoch, this // record is due for reclaimation. - // - // modifiedEpoch has to be aligned for 64-bit atomic operations. modifiedEpoch int64 // reclaim is an atomic to control the start of reclaiming. - // - // reclaim has to be aligned for 64-bit atomic operations. reclaim int64 - // labels is the LabelSet passed by the user. - labels *labels - - // descriptor describes the metric instrument. - descriptor *export.Descriptor - // recorder implements the actual RecordOne() API, // depending on the type of aggregation. If nil, the // metric was disabled by the exporter. diff --git a/sdk/metric/stress_test.go b/sdk/metric/stress_test.go index a7149f810689..6e17802dc28d 100644 --- a/sdk/metric/stress_test.go +++ b/sdk/metric/stress_test.go @@ -51,7 +51,6 @@ const ( type ( testFixture struct { - // stop has to be aligned for 64-bit atomic operations. stop int64 expected sync.Map received sync.Map // Note: doesn't require synchronization @@ -94,7 +93,6 @@ type ( // where a race condition causes duplicate records. We always // take the later timestamp. gaugeState struct { - // raw has to be aligned for 64-bit atomic operations. raw core.Number ts time.Time }