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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Decouple `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc/internal` from `go.opentelemetry.io/otel/exporters/otlp/internal` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal` using gotmpl. (#4400, #3846)
- Decouple `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp/internal` from `go.opentelemetry.io/otel/exporters/otlp/internal` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal` using gotmpl. (#4401, #3846)
- Do not block the metric SDK when OTLP metric exports are blocked in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc` and `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#3925, #4395)
- Fix resource detection data race in `go.opentelemetry.io/otel/sdk/resource`. (#4409)

## [1.16.0/0.39.0] 2023-05-18

Expand Down
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
27 changes: 27 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,29 @@ func TestWithContainer(t *testing.T) {
string(semconv.ContainerIDKey): fakeContainerID,
}, toMap(res))
}

func TestResourceConcurrentSafe(t *testing.T) {
// Creating Resources should also be free of any data races,
// because Resources are immutable.
var wg sync.WaitGroup
for i := 0; i < 2; i++ {
wg.Add(1)
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 concurrent-safe.
return resource.NewWithAttributes("https://opentelemetry.io/schemas/1.3.0"), nil
}

var _ resource.Detector = &fakeDetector{}