Handling empty resource vs required resource attributes

[pellared]

The "combination" of these requirements is not very clear.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/README.md#semantic-attributes-with-sdk-provided-default-value

These are the attributes which MUST be provided by the SDK as specified in the Resource SDK specification:

• service.name
• telemetry.sdk group

AND

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#the-empty-resource

It is recommended, but not required, to provide a way to quickly create an empty
resource.

AND

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#sdk-provided-resource-attributes

The SDK MUST provide access to a Resource with at least the attributes listed at Semantic Attributes with SDK-provided Default Value. This resource MUST be associated with a TracerProvider or MeterProvider if another resource was not explicitly specified.

Note: This means that it is possible to create and associate a resource that does not have all or any of the SDK-provided attributes present. However, that does not happen by default. If a user wants to combine custom attributes with the default resource, they can use Merge with their custom resource or specify their attributes by implementing Custom resource detectors instead of explicitly associating a resource.

Questions

Should the SDK give the possibility to not emit the required attributes: service.name, telemetry.sdk.name, telemetry.sdk.version, telemetry.sdk.language? In my understanding, no.

(rephrasing) Should the tracer/meter/logger emit these attributes even if the user set an empty resource? In my understanding, yes.

Do you agree? Are the SDKs implemented in this way? From my memory (which is buggy), it is not how .NET and Go SDKs are working.

EDIT: I changed my mind here. 🤦 Still I find the specification not clear or at least confusing.

Context: open-telemetry/opentelemetry-dotnet-instrumentation#2415 (comment)

[Oberon00]

It seems we somehow lost a fundamental thing in the definition of requirement levels: They only apply if you intend to use a particular semantic convention. E.g. even though http.method is required, you don't have to put that attribute on every span, you only need to set it if you want to report a span following the HTTP conventions.

CC @trask @lmolkova @carlosalberto

[Oberon00]

NB: There are other reasons why this does not apply in the particular case of this issue, e.g. the API for creating resources is not an something like an instrumentation library, so requirement levels don't apply there.

[pellared]

It seems we somehow lost a fundamental thing in the definition of requirement levels: They only apply if you intend to use a particular semantic convention.

Then shouldn't the Telemetry SDK semantic convention be followed by the SDK? 😉

Side note:

I find this sentence bringing more confusion:

opentelemetry-specification/specification/resource/semantic_conventions/README.md

Lines 125 to 126 in 40e0457

	The default OpenTelemetry SDK provided by the OpenTelemetry project MUST set `telemetry.sdk.name`
	to the value `opentelemetry`.

This gives a feeling that the SDK has to set this attribute. I will try to make a PR to address my concern.

EDIT: I created #3392

[Oberon00]

Then shouldn't the Telemetry SDK semantic convention be followed by the SDK?

Maybe, but that's not what the fields here define. That would need to be stated explicitly somewhere. And it still is unrelated to the ability to create an empty Resource. That particular API will only create some in-process representation of a resource. Even if you allow to create empty resources, you can still ensure in the SDK implementation that when the resource is attached e.g. to a TracerProvider, that any always-required attributes are added.

[pellared]

you can still ensure in the SDK implementation that when the resource is attached e.g. to a TracerProvider, that any always-required attributes are added

That make sense, but I am not sure if any of the SDK does that...

There is also https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#sdk-provided-resource-attributes

But I am not sure if it is well-defined that the SDK Tracer/Meter/Logger MUST emit these resource attributes (even if the user has set an "empty resource").

EDIT: I also edited the issue issue description (first comment).

[MrAlias]

(rephrasing) Should the tracer/meter/logger emit these attributes even if the user set an empty resource? In my understanding, yes.

Seems reasonable to me.

[pellared]

After reading it the third time I have now a different opinion

These are the attributes which MUST be provided by the SDK as specified in the Resource SDK specification:

"MUST be provided" probably means that the resource detector is a MUST HAVE for in the SDK, but it does not force the user to use it.

Can we add a clarification in the spec?

I am adding two proposals as comment below.
Can you please 👍 on the comment which proposal you find correct?

[pellared]

If a user configures the SDK with a resource
that misses any of the SDK-provided attributes,
the SDK MUST add them on its own.

[pellared]

If a user configures the SDK with a resource
that misses any of the SDK-provided attributes,
the SDK MUST respect the user configuration
and not add them.

[Oberon00]

I like not enforcing them, but the SDK should make it hard for the user to accidentally drop them. For example, just relying on the user to know that they should probably use CreateDefault instead of CreateEmpty might make it too easy to do the wrong thing, so maybe an explicit SetResourceWithoutDefaults is needed. To be considered separately in each language, according to usual guidelines there.

[joaopgrassi]

Maybe first we need to think more about the following:

1. Do we think most people will want to have these attributes? VS
2. Do we think most people don't see value on them and want them not present by default?

That would lead us to find out if we should enforce the attributes or not.

Moreover, I think the confusing part with the existing text in the Resource SDK spec:

Note: This means that it is possible to create and associate a resource that does not have all or any of the SDK-provided attributes present. However, that does not happen by default. If a user wants to combine custom attributes with the default resource, they can use Merge with their custom resource or specify their attributes by implementing Custom resource detectors instead of explicitly associating a resource.

Is that it is implicitly saying the SDK will always add the default attributes, if users add their own resource via Merge or Resource detectors, but doesn't say what happens when they set their resources in other ways.

I tend to think the attributes are valuable by default. Given that, I'd say the SDK should always populate the fields. If a user passes a resource without them (whatever how, via code, via detectors via env vars), the SDK still adds it. To address the case for people that explicitly don't want them, we could edit the spec and maybe list that SDKs should offer a way to opt-out of such attributes.

I definitely agree with @Oberon00 that if we make it "easy" people will use it wrong and then the attributes will be basically rendered useless.

[pellared]

I like not enforcing them, but the SDK should make it hard for the user to accidentally drop them. For example, just relying on the user to know that they should probably use CreateDefault instead of CreateEmpty might make it too easy to do the wrong thing, so maybe an explicit SetResourceWithoutDefaults is needed. To be considered separately in each language, according to usual guidelines there.

First I want to clarify if it is possible to drop the SDK-provided attributes. The next step could be considering to change the spec to recommend how it could be designed. These are two separate things.

[pellared]

Is that it is implicitly saying the SDK will always add the default attributes,

I cannot find how and where it is stated.

Maybe first we need to think more about the following:

1. Do we think most people will want to have these attributes? VS
2. Do we think most people don't see value on them and want them not present by default?

The problem with always adding them would be:

1. Would require changes in most (if not all) of the SDKs
2. Less configurable

While I do not see many scenarios where it would be useful to not emit the SDK-provided attributes, I strongly feel it is safer to allow opt-out.

[jsuereth]

The intention behind the Resource + Telemetry SDK values was that they are out of the box and provide bare minimum of:

• Resource based "joinable telemetry"
• Self-observability of what OpenTelemetry did.

As such the default should be these are on.

Would require changes in most (if not all) of the SDKs

What SDKs would this require changes in? Can you provide a list?

I've only done the formal review for 3 of the OTEL SDKs, but this was something we looked for in the Resource implementation.

[pellared]

@jsuereth

I was not referring to existing implementation of "default" resource.

This was about

That would lead us to find out if we should enforce the attributes or not.

and also

Is that it is implicitly saying the SDK will always add the default attributes

[joaopgrassi]

Is that it is implicitly saying the SDK will always add the default attributes,

I cannot find how and where it is stated.

As I said, it's implicitly saying it, at least in my interpretation. I will try to mark in bold the parts of the text that seems to imply this:

Note: This means that it is possible to create and associate a resource that does not have all or any of the SDK-provided attributes present. However, that does not happen by default. If a user wants to combine custom attributes with the default resource, they can use Merge with their custom resource or specify their attributes by implementing Custom resource detectors instead of explicitly associating a resource.

• However, that does not happen by default: This makes clear to me the SDK by default will always add the attributes
• wants to combine custom attributes with the default resource: Again, offers way to combine with the default resource that already contains the default attributes.

My interpretation of the above then is:

1. If I don't use a custom resource, the SDK will "use the default" that contains the attributes.
2. If I add a custom resource via Merge or Resource detector it will also combine with the default resource. Also having the default attributes.

What the text lacks saying is: What will happen when I add a custom resource, and I do it another way. E.g., not using CreateDefault, Merge or a detector.

Also note the text above

The SDK MUST provide access to a Resource with at least the attributes listed at Semantic Attributes with SDK-provided Default Value.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/sdk.md#sdk-provided-resource-attributes

[pellared]

If I add a custom resource via Merge or Resource detector it will also combine with the default resource. Also having the default attributes.

Somebody may Merge WITHOUT using the default resource and may not use a resource detector which adds the SDK-provided attributes.

What the text lacks saying is: What will happen when I add a custom resource, and I do it another way. E.g., not using CreateDefault, Merge or a detector.

👍

I agree with @Oberon00 #3382 (comment)

[joaopgrassi]

Somebody may Merge WITHOUT using the default resource and may not use a resource detector which adds the SDK-provided attributes.

Yeah but that's what I think the text and the MUST above is implicitly saying that the SDK should always add the default attributes if you use those forms of adding it. Maybe it needs to be more explicit?

I agree with @Oberon00 #3382 (comment)

Me too :)