Skip to content

Commit

Permalink
Consistently use pointer receivers for core.Number (#375)
Browse files Browse the repository at this point in the history
Change all occurrences of value to pointer receivers
Add meta sys files to .gitignore
Code cleanup e.g.
- Don't capitalize error statements
- Fix ignored errors
- Fix ambiguous variable naming
- Remove unnecessary type casting
- Use named params

Fix #306
  • Loading branch information
elsesiy authored and rghetia committed Dec 9, 2019
1 parent 30795ef commit 1ab645f
Show file tree
Hide file tree
Showing 33 changed files with 103 additions and 95 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
.DS_Store
Thumbs.db

.tools/
.idea/
.vscode/
Expand Down
2 changes: 1 addition & 1 deletion api/core/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestValue(t *testing.T) {
name: "Key.Float64() correctly returns keys's internal float64 value",
value: k.Float64(42.1).Value,
wantType: core.FLOAT64,
wantValue: float64(42.1),
wantValue: 42.1,
},
{
name: "Key.Int32() correctly returns keys's internal int32 value",
Expand Down
42 changes: 21 additions & 21 deletions api/core/number.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,31 +94,31 @@ func NewUint64Number(u uint64) Number {
// - as x

// AsNumber gets the Number.
func (n Number) AsNumber() Number {
return n
func (n *Number) AsNumber() Number {
return *n
}

// AsRaw gets the uninterpreted raw value. Might be useful for some
// atomic operations.
func (n Number) AsRaw() uint64 {
return uint64(n)
func (n *Number) AsRaw() uint64 {
return uint64(*n)
}

// AsInt64 assumes that the value contains an int64 and returns it as
// such.
func (n Number) AsInt64() int64 {
func (n *Number) AsInt64() int64 {
return rawToInt64(n.AsRaw())
}

// AsFloat64 assumes that the measurement value contains a float64 and
// returns it as such.
func (n Number) AsFloat64() float64 {
func (n *Number) AsFloat64() float64 {
return rawToFloat64(n.AsRaw())
}

// AsUint64 assumes that the value contains an uint64 and returns it
// as such.
func (n Number) AsUint64() uint64 {
func (n *Number) AsUint64() uint64 {
return rawToUint64(n.AsRaw())
}

Expand Down Expand Up @@ -183,7 +183,7 @@ func (n *Number) AsUint64Ptr() *uint64 {

// CoerceToInt64 casts the number to int64. May result in
// data/precision loss.
func (n Number) CoerceToInt64(kind NumberKind) int64 {
func (n *Number) CoerceToInt64(kind NumberKind) int64 {
switch kind {
case Int64NumberKind:
return n.AsInt64()
Expand All @@ -199,7 +199,7 @@ func (n Number) CoerceToInt64(kind NumberKind) int64 {

// CoerceToFloat64 casts the number to float64. May result in
// data/precision loss.
func (n Number) CoerceToFloat64(kind NumberKind) float64 {
func (n *Number) CoerceToFloat64(kind NumberKind) float64 {
switch kind {
case Int64NumberKind:
return float64(n.AsInt64())
Expand All @@ -215,7 +215,7 @@ func (n Number) CoerceToFloat64(kind NumberKind) float64 {

// CoerceToUint64 casts the number to uint64. May result in
// data/precision loss.
func (n Number) CoerceToUint64(kind NumberKind) uint64 {
func (n *Number) CoerceToUint64(kind NumberKind) uint64 {
switch kind {
case Int64NumberKind:
return uint64(n.AsInt64())
Expand Down Expand Up @@ -498,7 +498,7 @@ func (n *Number) CompareAndSwapUint64(ou, nu uint64) bool {
// 0 if the numbers are equal
// -1 if the subject `n` is less than the argument `nn`
// +1 if the subject `n` is greater than the argument `nn`
func (n Number) CompareNumber(kind NumberKind, nn Number) int {
func (n *Number) CompareNumber(kind NumberKind, nn Number) int {
switch kind {
case Int64NumberKind:
return n.CompareInt64(nn.AsInt64())
Expand All @@ -514,7 +514,7 @@ func (n Number) CompareNumber(kind NumberKind, nn Number) int {

// CompareRaw compares two numbers, where one is input as a raw
// uint64, interpreting both values as a `kind` of number.
func (n Number) CompareRaw(kind NumberKind, r uint64) int {
func (n *Number) CompareRaw(kind NumberKind, r uint64) int {
return n.CompareNumber(kind, NewNumberFromRaw(r))
}

Expand All @@ -523,7 +523,7 @@ func (n Number) CompareRaw(kind NumberKind, r uint64) int {
// typical result of the compare function: -1 if the value is less
// than the other, 0 if both are equal, 1 if the value is greater than
// the other.
func (n Number) CompareInt64(i int64) int {
func (n *Number) CompareInt64(i int64) int {
this := n.AsInt64()
if this < i {
return -1
Expand All @@ -540,7 +540,7 @@ func (n Number) CompareInt64(i int64) int {
// greater than the other.
//
// Do not compare NaN values.
func (n Number) CompareFloat64(f float64) int {
func (n *Number) CompareFloat64(f float64) int {
this := n.AsFloat64()
if this < f {
return -1
Expand All @@ -555,7 +555,7 @@ func (n Number) CompareFloat64(f float64) int {
// typical result of the compare function: -1 if the value is less
// than the other, 0 if both are equal, 1 if the value is greater than
// the other.
func (n Number) CompareUint64(u uint64) int {
func (n *Number) CompareUint64(u uint64) int {
this := n.AsUint64()
if this < u {
return -1
Expand All @@ -568,17 +568,17 @@ func (n Number) CompareUint64(u uint64) int {
// - relations to zero

// IsPositive returns true if the actual value is greater than zero.
func (n Number) IsPositive(kind NumberKind) bool {
func (n *Number) IsPositive(kind NumberKind) bool {
return n.compareWithZero(kind) > 0
}

// IsNegative returns true if the actual value is less than zero.
func (n Number) IsNegative(kind NumberKind) bool {
func (n *Number) IsNegative(kind NumberKind) bool {
return n.compareWithZero(kind) < 0
}

// IsZero returns true if the actual value is equal to zero.
func (n Number) IsZero(kind NumberKind) bool {
func (n *Number) IsZero(kind NumberKind) bool {
return n.compareWithZero(kind) == 0
}

Expand All @@ -587,7 +587,7 @@ func (n Number) IsZero(kind NumberKind) bool {
// Emit returns a string representation of the raw value of the
// Number. A %d is used for integral values, %f for floating point
// values.
func (n Number) Emit(kind NumberKind) string {
func (n *Number) Emit(kind NumberKind) string {
switch kind {
case Int64NumberKind:
return fmt.Sprintf("%d", n.AsInt64())
Expand All @@ -602,7 +602,7 @@ func (n Number) Emit(kind NumberKind) string {

// AsInterface returns the number as an interface{}, typically used
// for NumberKind-correct JSON conversion.
func (n Number) AsInterface(kind NumberKind) interface{} {
func (n *Number) AsInterface(kind NumberKind) interface{} {
switch kind {
case Int64NumberKind:
return n.AsInt64()
Expand All @@ -617,7 +617,7 @@ func (n Number) AsInterface(kind NumberKind) interface{} {

// - private stuff

func (n Number) compareWithZero(kind NumberKind) int {
func (n *Number) compareWithZero(kind NumberKind) int {
switch kind {
case Int64NumberKind:
return n.CompareInt64(0)
Expand Down
9 changes: 6 additions & 3 deletions api/core/number_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,10 @@ func TestNumberZero(t *testing.T) {
}

func TestNumberAsInterface(t *testing.T) {
require.Equal(t, int64(10), NewInt64Number(10).AsInterface(Int64NumberKind).(int64))
require.Equal(t, float64(11.11), NewFloat64Number(11.11).AsInterface(Float64NumberKind).(float64))
require.Equal(t, uint64(100), NewUint64Number(100).AsInterface(Uint64NumberKind).(uint64))
i64 := NewInt64Number(10)
f64 := NewFloat64Number(11.11)
u64 := NewUint64Number(100)
require.Equal(t, int64(10), (&i64).AsInterface(Int64NumberKind).(int64))
require.Equal(t, 11.11, (&f64).AsInterface(Float64NumberKind).(float64))
require.Equal(t, uint64(100), (&u64).AsInterface(Uint64NumberKind).(uint64))
}
4 changes: 2 additions & 2 deletions api/distributedcontext/map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ func TestMap(t *testing.T) {
t.Errorf("Expected kv %v, but not found", kv)
return true
})
if len, exp := got.Len(), len(testcase.wantKVs); len != exp {
t.Errorf("+got: %d, -want: %d", len, exp)
if l, exp := got.Len(), len(testcase.wantKVs); l != exp {
t.Errorf("+got: %d, -want: %d", l, exp)
}
}
}
Expand Down
1 change: 0 additions & 1 deletion api/global/global_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func (*testMeterProvider) Meter(_ string) metric.Meter {
}

func TestMulitpleGlobalTracerProvider(t *testing.T) {

p1 := testTraceProvider{}
p2 := trace.NoopProvider{}
global.SetTraceProvider(&p1)
Expand Down
4 changes: 2 additions & 2 deletions api/propagators/b3_propagator_benchmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func BenchmarkExtractB3(b *testing.B) {
}

for _, tg := range testGroup {
propagator := propagators.B3{tg.singleHeader}
propagator := propagators.B3{SingleHeader: tg.singleHeader}
for _, tt := range tg.tests {
traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) {
ctx := context.Background()
Expand Down Expand Up @@ -97,7 +97,7 @@ func BenchmarkInjectB3(b *testing.B) {

for _, tg := range testGroup {
id = 0
propagator := propagators.B3{tg.singleHeader}
propagator := propagators.B3{SingleHeader: tg.singleHeader}
for _, tt := range tg.tests {
traceBenchmark(tg.name+"/"+tt.name, b, func(b *testing.B) {
req, _ := http.NewRequest("GET", "http://example.com", nil)
Expand Down
12 changes: 5 additions & 7 deletions api/propagators/b3_propagator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func TestExtractB3(t *testing.T) {
}

for _, tg := range testGroup {
propagator := propagators.B3{tg.singleHeader}
propagator := propagators.B3{SingleHeader: tg.singleHeader}
for _, tt := range tg.tests {
t.Run(tt.name, func(t *testing.T) {
req, _ := http.NewRequest("GET", "http://example.com", nil)
Expand Down Expand Up @@ -99,7 +99,7 @@ func TestInjectB3(t *testing.T) {

for _, tg := range testGroup {
id = 0
propagator := propagators.B3{tg.singleHeader}
propagator := propagators.B3{SingleHeader: tg.singleHeader}
for _, tt := range tg.tests {
t.Run(tt.name, func(t *testing.T) {
req, _ := http.NewRequest("GET", "http://example.com", nil)
Expand All @@ -117,21 +117,19 @@ func TestInjectB3(t *testing.T) {
t.Errorf("%s: %s, header=%s: -got +want %s", tg.name, tt.name, h, diff)
}
}
wantOk := false
for _, h := range tt.doNotWantHeaders {
v, gotOk := req.Header[h]
if diff := cmp.Diff(gotOk, wantOk); diff != "" {
if diff := cmp.Diff(gotOk, false); diff != "" {
t.Errorf("%s: %s, header=%s: -got +want %s, value=%s", tg.name, tt.name, h, diff, v)
}

}
})
}
}
}

func TestB3Propagator_GetAllKeys(t *testing.T) {
propagator := propagators.B3{false}
propagator := propagators.B3{SingleHeader: false}
want := []string{
propagators.B3TraceIDHeader,
propagators.B3SpanIDHeader,
Expand All @@ -144,7 +142,7 @@ func TestB3Propagator_GetAllKeys(t *testing.T) {
}

func TestB3PropagatorWithSingleHeader_GetAllKeys(t *testing.T) {
propagator := propagators.B3{true}
propagator := propagators.B3{SingleHeader: true}
want := []string{
propagators.B3SingleHeader,
}
Expand Down
6 changes: 3 additions & 3 deletions bridge/opentracing/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func (t *BridgeTracer) ContextWithSpanHook(ctx context.Context, span ot.Span) co

func otTagsToOtelAttributesKindAndError(tags map[string]interface{}) ([]otelcore.KeyValue, oteltrace.SpanKind, bool) {
kind := oteltrace.SpanKindInternal
error := false
err := false
var pairs []otelcore.KeyValue
for k, v := range tags {
switch k {
Expand All @@ -399,13 +399,13 @@ func otTagsToOtelAttributesKindAndError(tags map[string]interface{}) ([]otelcore
}
case string(otext.Error):
if b, ok := v.(bool); ok && b {
error = true
err = true
}
default:
pairs = append(pairs, otTagToOtelCoreKeyValue(k, v))
}
}
return pairs, kind, error
return pairs, kind, err
}

func otTagToOtelCoreKeyValue(k string, v interface{}) otelcore.KeyValue {
Expand Down
2 changes: 1 addition & 1 deletion bridge/opentracing/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (

oteltrace "go.opentelemetry.io/otel/api/trace"

migration "go.opentelemetry.io/otel/bridge/opentracing/migration"
"go.opentelemetry.io/otel/bridge/opentracing/migration"
)

type WrapperProvider struct {
Expand Down
6 changes: 3 additions & 3 deletions example/grpc/client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func main() {
if err != nil {
log.Fatalf("did not connect: %s", err)
}
defer conn.Close()
defer func() { _ = conn.Close() }()

c := api.NewHelloServiceClient(conn)

Expand All @@ -45,8 +45,8 @@ func main() {
"client-id", "web-api-client-us-east-1",
"user-id", "some-test-user-id",
)
context := metadata.NewOutgoingContext(context.Background(), md)
response, err := c.SayHello(context, &api.HelloRequest{Greeting: "World"})
ctx := metadata.NewOutgoingContext(context.Background(), md)
response, err := c.SayHello(ctx, &api.HelloRequest{Greeting: "World"})
if err != nil {
log.Fatalf("Error when calling SayHello: %s", err)
}
Expand Down
4 changes: 2 additions & 2 deletions example/grpc/middleware/tracing/tracing.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ func UnaryClientInterceptor(ctx context.Context, method string, req, reply inter

func setTraceStatus(ctx context.Context, err error) {
if err != nil {
status, _ := status.FromError(err)
trace.CurrentSpan(ctx).SetStatus(status.Code())
s, _ := status.FromError(err)
trace.CurrentSpan(ctx).SetStatus(s.Code())
} else {
trace.CurrentSpan(ctx).SetStatus(codes.OK)
}
Expand Down
2 changes: 1 addition & 1 deletion example/http-stackdriver/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func main() {
panic(err)
}
body, err = ioutil.ReadAll(res.Body)
res.Body.Close()
_ = res.Body.Close()
trace.CurrentSpan(ctx).SetStatus(codes.OK)

return err
Expand Down
2 changes: 1 addition & 1 deletion example/http/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func main() {
panic(err)
}
body, err = ioutil.ReadAll(res.Body)
res.Body.Close()
_ = res.Body.Close()
trace.CurrentSpan(ctx).SetStatus(codes.OK)

return err
Expand Down
2 changes: 1 addition & 1 deletion exporter/metric/internal/statsd/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ const (
var (
_ export.Exporter = &Exporter{}

ErrInvalidScheme = fmt.Errorf("Invalid statsd transport")
ErrInvalidScheme = fmt.Errorf("invalid statsd transport")
)

// NewExport returns a common implementation for exporters that Export
Expand Down
2 changes: 1 addition & 1 deletion exporter/metric/internal/statsd/conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ timer.B.D:%s|ms

var vfmt string
if nkind == core.Int64NumberKind {
fv := float64(value)
fv := value
vfmt = strconv.FormatInt(int64(fv), 10)
} else {
vfmt = strconv.FormatFloat(value, 'g', -1, 64)
Expand Down
1 change: 0 additions & 1 deletion exporter/metric/stdout/stdout.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ func (e *Exporter) Export(_ context.Context, checkpointSet export.CheckpointSet)
}
}
}

} else if lv, ok := agg.(aggregator.LastValue); ok {
if value, timestamp, err := lv.LastValue(); err != nil {
if err == aggregator.ErrNoLastValue {
Expand Down
2 changes: 1 addition & 1 deletion exporter/trace/jaeger/jaeger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func Test_spanDataToThrift(t *testing.T) {
messageEventValue := "event-test"
keyValue := "value"
statusCodeValue := int64(2)
doubleValue := float64(123.456)
doubleValue := 123.456
boolTrue := true
statusMessage := "Unknown"

Expand Down
Loading

0 comments on commit 1ab645f

Please sign in to comment.