Add explicit setup method to API.

[Oberon00]

This allows programmatically setting up the API, which is useful if you
need to access runtime configuration during initialization that is
cumbersome to get when you are in a class newly constructed by the SPI
loader mechanism.

EDIT: Addresses #552, #747

[arminru]

@jkwatson Will this resolve your #552 or are you missing anything here?

[bogdandrutu]

@Oberon00 I think this has a lot of problems with initialization order. If you want to support this probably we should have a "DelayedInitializedTracerProvider" which is load with spi that allows to set a real concrete "TracerProvider" later.

[jkwatson]

@jkwatson Will this resolve your #552 or are you missing anything here?

definitely. 👍

[Oberon00]

@bogdandrutu

I think this has a lot of problems with initialization order.

No problems, just restrictions 😛 But all the same restrictions/problems apply to calling setSystemProperty, just the failure mode is IMHO more convenient with the method introduced in this PR.

Yes this method is very inflexible, but I think problems that can occur are mostly straightforward to discover. Worst case, you can't use the method and will have to wait until someone implements your DelayedInitializedTraceProvider SPI provider along with your DelayInitializedTracerProvider (which will have a performance impact BTW, unlike this method).

[codecov-io]

Codecov Report

Merging #1058 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #1058      +/-   ##
============================================
+ Coverage     85.47%   85.50%   +0.03%     
- Complexity     1045     1054       +9     
============================================
  Files           136      136              
  Lines          3848     3856       +8     
  Branches        338      342       +4     
============================================
+ Hits           3289     3297       +8     
  Misses          429      429              
  Partials        130      130              

Impacted Files	Coverage Δ	Complexity Δ
.../src/main/java/io/opentelemetry/OpenTelemetry.java	98.11% <100.00%> (+0.33%)	26.00 <7.00> (+9.00)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f97c4b...f9f230d. Read the comment docs.

[jkwatson]

@Oberon00 I know I originally was pushing for something along these lines, but since open-telemetry/oteps#74 still hasn't been approved, and I think the auto-instrumentation use-case has been solved, are we sure this is necessary?

Do we have a specific use-case that this will solve? If not, I think we should hold off on this for now. If we do, let's work with @bogdandrutu to resolve the initialization order "issues".

[Oberon00]

I need this feature myself. But how did auto-instrumentation solve it?
EDIT: The use case is in the PR description.

[jkwatson]

I need this feature myself. But how did auto-instrumentation solve it?
EDIT: The use case is in the PR description.

I'll let @trask answer that one in detail, but IIRC, they just make sure they do the initialization as soon as possible in the startup lifecycle.

[trask]

I'm not sure if the initialization issue we had in auto-instrumentation is the same. As John mentioned, we solved by forcing early initialization: https://github.com/open-telemetry/opentelemetry-auto-instr-java/blob/v0.2.0/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/AgentInstaller.java#L79-L82

[jkwatson]

EDIT: The use case is in the PR description.

I don't understand what you mean in the description. Can you provide more background?

[Oberon00]

In pseudo code:

1. Different components calculate resource values
2. Put resource values together
3. Create TracerProvider with resource from 2
4. Use TracerProvider

Without the setup function in this PR, I would have to implement this by creating a SPI TraceProvider (!= TracerProvider) implementation that calls the "put together" component. With the setup function, I can just have some initialization component call the "put together" component and don't have to write a custom SPI. And I can use values calculated in main(), possibly from the command line args to put into resources. With the SPI, I'd have to introduce some static variables for that.

So I think that everything is possible without the setup function here, but more cumbersome, and with requiring more global variables (aka static variables).

[jkwatson]

Thanks for the details! Is the use-case purely around the Resource creation/assignment? Maybe there's an approach that would allow updating the Resource in the active SDK? I know this has been discussed briefly in the specs SIG, but I don't know exactly where it ended up. @bogdandrutu what do you think about the ability to update the Resource used in the SDK?

[Oberon00]

Maybe there's an approach that would allow updating the Resource in the active SDK

I'm against allowing that, and got open-telemetry/opentelemetry-specification#515 merged, so the spec now explictly forbids changing resources (not only modifying objects but also changing the resource associated with a tracer, see https://github.com/open-telemetry/opentelemetry-specification/pull/515/files#r393752660).

[jkwatson]

I wonder what good the merge functionality is, on the Resource, if you can't actually end up using the result in the SDK.

[Oberon00]

Well, you can. If different components create different resources, you merge them together before creating the SDK. Of course you could also keep everything in a Map<String, AttributeValue> and only create a resource in the final step, so from that point of view, Merge is just a convenience operation.

[Oberon00]

@jkwatson

Is the use-case purely around the Resource creation/assignment?

My own use-case is, yes. But there might theoretically be use cases for any immutable property of the TracerSdkProvider, which currently are only Resource, Clock and IdsGenerator .

[jkwatson]

Cool. I'm totally convinced. :) (I just wanted to make very sure before we add things to the public API that we can't remove). I'd like @bogdandrutu 's concerns to be understood and resolved before we merge.

[carlosalberto]

we solved by forcing early initialization

This sounds like a workaround we forced you to do @trask So I'm definitely up for this change. Lets discuss the details but we should proceed with it IMHO (hoping it will help you ;) )

[Oberon00]

This effectively exists now.