Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sdk/resource: Fix data race with emptyAttributes #4409

Merged
merged 14 commits into from
Aug 7, 2023
9 changes: 4 additions & 5 deletions sdk/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
}

var (
emptyResource Resource
defaultResource *Resource
defaultResourceOnce sync.Once
)
Expand Down Expand Up @@ -70,7 +69,7 @@
// of the attrs is known use NewWithAttributes instead.
func NewSchemaless(attrs ...attribute.KeyValue) *Resource {
if len(attrs) == 0 {
return &emptyResource
return &Resource{}
}

// Ensure attributes comply with the specification:
Expand All @@ -81,7 +80,7 @@

// If attrs only contains invalid entries do not allocate a new resource.
if s.Len() == 0 {
return &emptyResource
return &Resource{}
}

return &Resource{attrs: s} //nolint
Expand Down Expand Up @@ -195,7 +194,7 @@
// Empty returns an instance of Resource with no attributes. It is
// equivalent to a `nil` Resource.
func Empty() *Resource {
return &emptyResource
return &Resource{}
}

// Default returns an instance of Resource with a default
Expand All @@ -214,7 +213,7 @@
}
// If Detect did not return a valid resource, fall back to emptyResource.
if defaultResource == nil {
defaultResource = &emptyResource
defaultResource = &Resource{}

Check warning on line 216 in sdk/resource/resource.go

View check run for this annotation

Codecov / codecov/patch

sdk/resource/resource.go#L216

Added line #L216 was not covered by tests
}
})
return defaultResource
Expand Down
28 changes: 28 additions & 0 deletions sdk/resource/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"os"
"strings"
"sync"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -769,3 +770,30 @@ func TestWithContainer(t *testing.T) {
string(semconv.ContainerIDKey): fakeContainerID,
}, toMap(res))
}

func TestResourceRace(t *testing.T) {
jaredjenkins marked this conversation as resolved.
Show resolved Hide resolved
// Creating Resources should also be free of any ASAN issues,
jaredjenkins marked this conversation as resolved.
Show resolved Hide resolved
// because Resources are immutable.
var wg sync.WaitGroup
wg.Add(2)

for i := 0; i < 2; i++ {
jaredjenkins marked this conversation as resolved.
Show resolved Hide resolved
go func() {
defer wg.Done()
d := &fakeDetector{}
_, err := resource.Detect(context.Background(), d)
assert.NoError(t, err)
}()
}
wg.Wait()
}

type fakeDetector struct{}

func (f fakeDetector) Detect(_ context.Context) (*resource.Resource, error) {
// A bit pedantic, but resource.NewWithAttributes returns an empty Resource when
// no attributes specified. We want to make sure that this is memory-safe.
jaredjenkins marked this conversation as resolved.
Show resolved Hide resolved
return resource.NewWithAttributes("https://opentelemetry.io/schemas/1.3.0"), nil
}

var _ resource.Detector = &fakeDetector{}