Being able to specify Resource manually

[carlosalberto]

Currently the only way to set Resources for the global OpenTelemetry is through environment variables - but we need to allow users to set them manually (through code) or through system properties (in a similar fashion to the configuration supported for BatchSpanProcessor and siblings).

I see two possible options to do this:

1. Set the Resource in the loaded TracerSdkProvider and siblings once, and report errors if the user tries to do this again.
2. Allow TracerSdkProvider and siblings to be set manually, in addition to the SPI path. For this, @Oberon00's PR would help us achieve this purpose (while also allowing global initialization, of course).

Option 2) is the most reasonable to me, and can help get it to the finish line, but want to get initial feedback (mostly from @bogdandrutu )

EDIT: Updated the description to specifically address the global case.

[Oberon00]

Currently the only way to set Resources is through environment variables

That's technically not true: There is TracerSdkProvider.Builder.setResource https://github.com/open-telemetry/opentelemetry-java/blob/v0.6.0/sdk/src/main/java/io/opentelemetry/sdk/trace/TracerSdkProvider.java#L174
It is cumbersome to use without #1058 but still possible if you provide your own SPI.

[anuraaga]

Yeah as an example I'm doing this

https://github.com/aws-samples/aws-xray-sdk-with-opentelemetry-sample/blob/both-otel-and-xray/awsagentprovider/src/main/java/com/softwareaws/xray/opentelemetry/exporters/AwsTracerProviderFactory.java#L35

Because we have a pattern of generating many different Resource and merging them, an SPI that collects all the Resource and merges them would be very natural as a default. Manual initialization probably needs to provide two options of merging into such a default or replacing it completely. I'd expect most users to merge into the default with their own domain-specific attributes though.

Is there a difference between this issue and #1149?

[carlosalberto]

That's technically not true

Well, there's also the possibility of use reflection to achieve that ;) But in all seriousness, being able to set Resource easily for the common case, i.e. the global OpenTelemetry part.

I honestly cannot imagine users having to extend the SPI part to only override Resource.

[carlosalberto]

Manual initialization probably needs to provide two options of merging into such a default or replacing it completely. I'd expect most users to merge into the default with their own domain-specific attributes though.

Yes, we need to support this IMHO.

Is there a difference between this issue and #1149?

Hopefully there's no difference and we can close this issue after initial feedback (and deciding what path to go with).

[anuraaga]

Maybe we can repurpose this or a new issue into the resourc merging story? I feel that in most cases, users will be happy adding various artifacts for different resources to their classpath to get the information they need - otel-resource-k8s, otel-resource-gcp, etc. I don't think we've committed to a story for this yet.

[carlosalberto]

I feel that in most cases, users will be happy adding various artifacts for different resources to their classpath to get the information they need

We indeed need to discuss how to achieve this in a nice way (probably through additional SPI). I suggest we keep this issue for the simple case of an user setting Resource locally (and probably marked as a duplicate of #1149), and have a new ticket for discussing how to add Resource for the cases you mentioned.

[malafeev]

My case is to use it in custom agent extending java-instrumentation agent.

In the java-instrumentation shading is done in such way that sdk classes are not available (classes are moved to "inst" directory and extension .class is renamed to .classdata)

@anuraaga in his example added dependency on opentelemetry-sdk to be able to provide custom TracerProviderFactory.
But he also did similar java-instrumentation shading of added dependencies.
Looks complicated and I was not able to repeat the same steps using maven (maven-shade-plugin).

Having system property would solve the issue in one line of code (System.setProperty(...))

I expect any SPI solution will not work with java-instrumentation without special shading cc @trask

[dengliming]

Another scenario. follow open-telemetry/opentelemetry-java-instrumentation#864 In spring boot starter
OtlpGrpcSpanExporterProperties.serviceName is not being used and Currently the way to set service name for OTLP is via OTEL_RESOURCE_ATTRIBUTES. Should we set OpenTelemetry Resource Attributes programmatically or a way to set the serviceName directly on the OtlpGrpcSpanExporter (e.g. which would then override service.name set via Resource Attributes).

[jkwatson]

If a user were to supply a Resource manually, would it get merged with any others, automatically, or should it be considered to be complete?

Would we want to enforce the "required" Resource attributes as specified here? https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/resource/semantic_conventions/README.md ? And, if so, what would be the desired behavior if the required attributes weren't supplied? [note: I think this is still an open question for non-manual setting of the Resource, but it would apply to this case as well].

[Oberon00]

I don't think "required" is enforced for spans, so why should it be enforced for resources? Also, open-telemetry/opentelemetry-specification#653

[jkwatson]

I don't think "required" is enforced for spans, so why should it be enforced for resources? Also, open-telemetry/opentelemetry-specification#653

oh, I know. :) Just wanted to raise it, since of all the things, not having a service.name is probably going to make most backends not work right.

[Oberon00]

Note that as per https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/resource/semantic_conventions/README.md#document-conventions

Certain attribute groups in this document have a Required column. For these groups if any attribute from the particular group is present in the Resource then all attributes that are marked as Required MUST be also present in the Resource. However it is also valid if the entire attribute group is omitted (i.e. none of the attributes from the particular group are present even though some of them are marked as Required in this document).

So as long as no other service attribute is present, it is OK to leave out service.name, even obeying required.

[jkwatson]

So as long as no other service attribute is present, it is OK to leave out service.name, even obeying required.

That may be currently allowable, but I think that will force all the exporters to require it as a field, kind of making any definition of the service.name in the Resource moot. I know this isn't the right place to be having this discussion, btw...I just wanted to highlight how I think the Resource spec is not really fulfilling some basic needs at the moment.

[iNikem]

As open-telemetry/opentelemetry-specification#799 got merged, I need some way for auto insrtumentation agent to provide new resource attribute. I was going to file an issue about adding ResourceProvide SPI when I noticed this issue. Can this one be closed by introducing ResourceProvide SPI or these two approaches are separate?

@carlosalberto @jkwatson @anuraaga @Oberon00

[jkwatson]

@iNikem I'd like to have an SPI-option for specifying the Resource. I'd also like a manual option (I'm not sure that SPI is Spring-friendly?). So, let's keep the 2 options as separate PRs. I'd like to have both.

[carlosalberto]

let's keep the 2 options as separate PRs. I'd like to have both.

+1

[iNikem]

I will try to submit a ResourceProvider SPI PR during this week.

[jkwatson]

With the SPI in place, could we defer this to after GA, or is it still needed pre-GA?

[carlosalberto]

Not a strong opinion (i.e. I'm not totally clear), but I'd say After GA.

[jkwatson]

I'm going to move it after-ga. If anyone disagrees, please yell.

[huntc]

Just to add my AU$0.02 - being able to programmatically specify things like a service name and IP address is something we could do back in the day of Open Tracing. My situation is that programs read their configuration and determine the service name (for example). Did this end up being too hard, or is it something I can help with?

[jkwatson]

Just to add my AU$0.02 - being able to programmatically specify things like a service name and IP address is something we could do back in the day of Open Tracing. My situation is that programs read their configuration and determine the service name (for example). Did this end up being too hard, or is it something I can help with?

I think the issue wasn't really that it is too hard, but that there are potential lifecycle issues to be hammered out. e.g. when is an SDK considered "ready" to accept telemetry. How does the manual method interact with the SPI method? A Resource shouldn't ever be altered on the SDK, so getting the lifecycle right is a little tricky, and we haven't had time to work on it. :) If you'd like to take a run at it, that would be great!

[huntc]

Happy to have a go at it.

I see that the underlying problem here is that we rely on global state. I don’t see a way around this other than to provide a means of not having to rely on global state and therefore have the state passed around. This is much like having to pass around an ActorSystem in Akka. Languages like Scala make this a little more transparent given implicits, but in Java, the OTEL state will have to be passed around. It’s worth pointing out that when using Java and Akka, the actor system is always passed explicitly.

Is there an appetite for providing an option to pass the state around, if we could make this a nice experience? I’d not see this precluding the existing global state approach. The local state approach would just be an alternative that the global state approach would build on.

[jkwatson]

@huntc I would love to have a non-global option for OpenTelemetry. I think it might be easier just to be able to get access to an instance of an OpenTelemetry class, and then keep everything else the same as it is now.