Skip to content

Commit

Permalink
Return merged Resource on schema conflict
Browse files Browse the repository at this point in the history
  • Loading branch information
MrAlias committed Jan 31, 2024
1 parent fecb92e commit b2905b3
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 22 deletions.
20 changes: 18 additions & 2 deletions sdk/resource/auto.go
Expand Up @@ -41,15 +41,31 @@ type Detector interface {
// must never be done outside of a new major release.
}

// Detect calls all input detectors sequentially and merges each result with the previous one.
// It returns the merged error too.
// Detect returns a new [Resource] merged from all the Resources each of the
// detectors produces. Each of the detectors are called sequentially, in the
// order they are passed, merging the produced resource into the previous.
//
// This may return a partial Resource along with an error containing
// [ErrPartialResource] if that error is returned from a detector. It may also
// return a merge-conflicting Resource along with an error containing
// [ErrSchemaURLConflict] if merging Resources from different detectors results
// in a schema URL conflict. It is up to the caller to determine if this
// returned Resource should be used or not.
//
// If one of the detectors returns an error that is not [ErrPartialResource],
// the resource produced by the detector will not be merged and the returned
// error will wrap that detector's error.
func Detect(ctx context.Context, detectors ...Detector) (*Resource, error) {
r := new(Resource)
return r, detect(ctx, r, detectors)
}

// detect runs all detectors using ctx and merges the result into res. This
// assumes res is allocated and not nil, it will panic otherwise.
//
// If the detectors or merging resources produces any errors (i.e.
// [ErrPartialResource] [ErrSchemaURLConflict]), a single error wrapping all of
// these errors will be returned. Otherwise, nil is returned.
func detect(ctx context.Context, res *Resource, detectors []Detector) error {
var (
r *Resource
Expand Down
56 changes: 43 additions & 13 deletions sdk/resource/resource.go
Expand Up @@ -40,9 +40,19 @@ var (
defaultResourceOnce sync.Once
)

var errMergeConflictSchemaURL = errors.New("cannot merge resource due to conflicting Schema URL")
// ErrSchemaURLConflict is an error returned with two Resources are merged
// together that contain different, non-empty, schema URLs.
var ErrSchemaURLConflict = errors.New("conflicting Schema URL")

// New returns a Resource combined from the user-provided detectors.
// New returns a [Resource] combined from the user-provided opts.
//
// This may return a partial Resource along with an error containing
// [ErrPartialResource] if options that provide [Detector] are used and it is
// error is returned from one or more of the Detector. It may also return a
// merge-conflicting Resource along with an error containing
// [ErrSchemaURLConflict] if merging Resources from the opts results in a
// schema URL conflict (see [Resource.Merge] for more information). It is up to
// the caller to determine if this returned Resource should be used or not.
func New(ctx context.Context, opts ...Option) (*Resource, error) {
cfg := config{}
for _, opt := range opts {
Expand Down Expand Up @@ -146,16 +156,31 @@ func (r *Resource) Equal(eq *Resource) bool {
return r.Equivalent() == eq.Equivalent()
}

// Merge creates a new resource by combining resource a and b.
// Merge creates a new [Resource] by combining resource a and b.
//
// If there are common keys between a and b, then the value from b will
// overwrite the value from a, even if b's value is empty.
//
// The SchemaURL of the resources will be merged according to the
// [OpenTelemetry specification rules]:
//
// - If the a's Schema URL is empty then the returned Resource's Schema URL
// will be set to the Schema URL of b,
// - Else if the b's Schema URL is empty then the returned Resource's Schema
// URL will be set to the Schema URL of a,
// - Else if the Schema URLs of a and b are the same then that will be the
// Schema URL of the returned Resource,
// - Else this is a merging error (this is the case when the Schema URL of b
// and a are not empty and are different). See below for how this is
// handled.
//
// If there are common keys between resource a and b, then the value
// from resource b will overwrite the value from resource a, even
// if resource b's value is empty.
// If the resources have different, non-empty, schema URLs an error containing
// [ErrSchemaURLConflict] will be returned with the merged Resource. It may be
// the case that some unintended attributes have been overwritten or old
// semantic conventions persisted in the returned Resource. It is up to the
// caller to determine if this returned Resource should be used or not.
//
// The SchemaURL of the resources will be merged according to the spec rules:
// https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/resource/sdk.md#merge
// If the resources have different non-empty schemaURL an empty resource and an error
// will be returned.
// [OpenTelemetry specification rules]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/specification/resource/sdk.md#merge
func Merge(a, b *Resource) (*Resource, error) {
if a == nil && b == nil {
return Empty(), nil
Expand All @@ -168,7 +193,10 @@ func Merge(a, b *Resource) (*Resource, error) {
}

// Merge the schema URL.
var schemaURL string
var (
schemaURL string
err error
)
switch true {
case a.schemaURL == "":
schemaURL = b.schemaURL
Expand All @@ -177,7 +205,9 @@ func Merge(a, b *Resource) (*Resource, error) {
case a.schemaURL == b.schemaURL:
schemaURL = a.schemaURL
default:
return Empty(), errMergeConflictSchemaURL
// Return the merged resource with an appropriate error. It is up to
// the user to decide if the returned resource can be used or not.
err = ErrSchemaURLConflict
}

// Note: 'b' attributes will overwrite 'a' with last-value-wins in attribute.Key()
Expand All @@ -188,7 +218,7 @@ func Merge(a, b *Resource) (*Resource, error) {
combine = append(combine, mi.Attribute())
}
merged := NewWithAttributes(schemaURL, combine...)
return merged, nil
return merged, err
}

// Empty returns an instance of Resource with no attributes. It is
Expand Down
24 changes: 17 additions & 7 deletions sdk/resource/resource_test.go
Expand Up @@ -183,7 +183,7 @@ func TestMerge(t *testing.T) {
name: "Merge with different schemas",
a: resource.NewWithAttributes("https://opentelemetry.io/schemas/1.4.0", kv41),
b: resource.NewWithAttributes("https://opentelemetry.io/schemas/1.3.0", kv42),
want: nil,
want: []attribute.KeyValue{kv42},
isErr: true,
},
}
Expand Down Expand Up @@ -406,9 +406,14 @@ func TestNew(t *testing.T) {
),
resource.WithSchemaURL("https://opentelemetry.io/schemas/1.1.0"),
},
resourceValues: map[string]string{},
schemaURL: "",
isErr: true,
resourceValues: map[string]string{
string(semconv.HostNameKey): func() (hostname string) {
hostname, _ = os.Hostname()
return hostname
}(),
},
schemaURL: "",
isErr: true,
},
{
name: "With conflicting detector schema urls",
Expand All @@ -420,9 +425,14 @@ func TestNew(t *testing.T) {
),
resource.WithSchemaURL("https://opentelemetry.io/schemas/1.2.0"),
},
resourceValues: map[string]string{},
schemaURL: "",
isErr: true,
resourceValues: map[string]string{
string(semconv.HostNameKey): func() (hostname string) {
hostname, _ = os.Hostname()
return hostname
}(),
},
schemaURL: "",
isErr: true,
},
}
for _, tt := range tc {
Expand Down

0 comments on commit b2905b3

Please sign in to comment.