Difficulty using Resource::from_detectors to set `service.name` correctly

[BrynCooke]

SdkProvidedResource detector currently has to know about all other detectors to correctly set service.name.

Detectors currently fill out resources in a last one wins fashion. In the following situation EnvResourceDetector, upon finding service.name will overwrite the value even if the SdkProvidedResourceDetector obtained it from OTEL_SERVICE_NAME.

let resource = Resource::from_detectors(
            time::Duration::from_secs(5),
            vec![
                Box::new(SdkProvidedResourceDetector),
                Box::new(EnvResourceDetector::new()),
            ],
        );

It's possible to reverse to ordering of the resource detectors to get the right result, but this doesn't support adding other resource detectors.

let resource = Resource::from_detectors(
            time::Duration::from_secs(5),
            vec![
                Box::new(MyResourceDetector::new()),
                Box::new(EnvResourceDetector::new()),
                Box::new(SdkProvidedResourceDetector),
            ],
        );

In this case SdkProvidedResourceDetector will override any value obtained by MyResourceDetector with unknown_service if it can't find anything from env.

Relates to #1295

[BrynCooke]

The issue seems to be that if a detector does defaulting then it leads to issues. The following works:

 let resource = Resource::from_detectors(
            time::Duration::from_secs(5),
            vec![
                Box::new(MyResourceDetector::new()),
                Box::new(EnvResourceDetector::new()),
                Box::new(EnvServiceNameDetector::new()), //Only sets service.name if it exists in env.
            ],
        );
        

Proposal: If instead Resouce itself is responsible for defaulting service name then it can take a look at the attributes after all detectors have been resolved and see if service name is missing.

[flokli]

I'm interested in this as well. I initially made an attempt extending SdkProvidedResourceDetector with a fallback_service_name: Option<String> (deriving Default) and adding a constructor for the fallback name, but it requires everyone to "properly" create an instance (you can't just use SdkProvidedResourceDetector.detect anymore).

I think your approach of letting Resource take care of defaulting a service name if unset is nicer, and I favor it.

[cijothomas]

@pitoniak32 Is this something you can help triage, as you already helping in this area?

[pitoniak32]

I started looking into this one

Resource detector packages MAY detect resource information from multiple possible sources and merge the result using the Merge operation described above.

The spec seems to state that the merge of the result of multiple detectors should be resolved according to https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#merge.

I just made the change so thats accurate, I will add some tests around this to make sure we have the correct behavior. I will also see how that aligns with what is being requested in this post.

I tend to agree that it's not a great experience that defaults will override existing values.

[cijothomas]

Moving out of 0.28 milestone.

@pitoniak32 Please continue to work on it, I simply moved out of 0.28 which is about to be shipped.

[cijothomas]

Removing from Logging SDK Stable milestone.