[Feature]: add ability for with_resource to merge Resources on providers

[pitoniak32]

Related Problems?

Resulting from a conversation with @cijothomas about ResourceBuilder and Signal Providers

Would like to hear thoughts on this!

Describe the solution you'd like:

Instead of replacing the Resource on a Provider if .with_resource is called multiple times, it would be good if we could build up a resource from many calls on the provider.

A few on how this could work:

We could add something like .with_resource_builder(…) that will aggregate on top of the existing resource / other builders in the provider.

Or update the .with_resource to just merge with existing resources so it can be called multiple times.

Or maybe both

Considered Alternatives

No response

Additional Context

relates to: #2320

inspired by:

• SetResourceBuilder is overly restrictive, no way to cooperatively initialize Resources opentelemetry-dotnet#2909
• https://github.com/open-telemetry/opentelemetry-dotnet/pull/3307/files

[cijothomas]

Or update the .with_resource to just merge with existing resources so it can be called multiple times.

@lalitb @utpilla any thoughts on this? That looks reasonable to me from a Quick Look.

[lalitb]

Or update the .with_resource to just merge with existing resources so it can be called multiple times.
@lalitb @utpilla any thoughts on this? That looks reasonable to me from a Quick Look.

Looks reasonable to me too.

[utpilla]

Or update the .with_resource to just merge with existing resources so it can be called multiple times.

@lalitb @utpilla any thoughts on this? That looks reasonable to me from a Quick Look.

That sounds fine but if we are doing that then maybe we should rename the method to something more intuitive like add_resource or append_resource?

[cijothomas]

Moving out of 0.28 milestone.

[cijothomas]

Given we are at RC stage for some components, it is better to avoid breaking change by renaming with_resource.
with_resource_builder is additive change, but we still need to define rules about what if with_resource and with_resource_builder are used etc.

Perhaps it is okay to keep the same name with_resource and provide "merge" behavior. For most of the "with" style APIs on builder, it does replacement of previous value, if any. with_processor is different, as it appends a processor to an existing chain. We can follow the similar idea for with_resource and do the merge. In short, anything that has the notion of supporting multiple, we can accept 'chain/merge' behavior instead of replacement. For anything that only has one value possible (eg: Sampler), then it'd replace previous value, if any.

[martintmk]

The #2677 preserves existing APIs and makes the with_resource calls additive with the "merge" behavior.

What would be the advantage of having the with_resource_builder? What would multiple calls to with_resource_builder do?

[martintmk]

Just some alternatives how this can be approached:

• Additive with_resource

builder
   .with_resource(resource1)
   .with_resource(resource2) // merges with the previous one

• with_resource + set_resource

builder
   .with_resource(resource1)
   .with_resource(resource2) // merges with the previous one
   .set_resource(resource3) // resets the resource to resource3

• Explicit behavior with_resource_additive

builder
   .with_resource(resource1)
   .with_resource_additive(resource2)  // merges with the previous one
   .with_resource(resource3) // resets back to resource 3

• Embrace the builder

This is similar to what .NET has. Having builder also introduces ambiguity between with_resource and with_resource_builder. Would be better to just keep one variant, albeit this introduces breaking changes.

builder
   .with_resource_builder(builder) // resets the current resource builder
   .configure_resource(|builder| {
      builder.with_attribute(KeyValue::new("a", "b"))
   })
   .configure_resource(|builder| {
      builder.with_attribute(KeyValue::new("c", "d"))
   })

Perhaps it is okay to keep the same name with_resource and provide "merge" behavior. For most of the "with" style APIs on builder, it does replacement of previous value, if any. with_processor is different, as it appends a processor to an existing chain. We can follow the similar idea for with_resource and do the merge. In short, anything that has the notion of supporting multiple, we can accept 'chain/merge' behavior instead of replacement. For anything that only has one value possible (eg: Sampler), then it'd replace previous value, if any.

I feel, from these options, the with_resource that merges resources is the simplest and still gives a lot of flexibility. If someone complains and wants the "reset" behavior, the set_resource method can be introduced later without any breaking changes.

[lalitb]

I feel, from these options, the with_resource that merges resources is the simplest and still gives a lot of flexibility. If someone complains and wants the "reset" behavior, the set_resource method can be introduced later without any breaking changes.

Thanks for adding possible alternatives. Thinking again, I agree - option 1, is much simpler. And doesn't affect the existing behavior where users would be invoking it only once.