Fix trace exporter

[psx95]

PR to attempt fixes for #141.

---

These changes prevent a crash and tests verified that traces are being generated. This is an early PR for any feedback.

Startup code for testing -

private static void initializeOTel() {
		traceExporter = TraceExporter.createWithConfiguration(TraceConfiguration.builder()
				.setProjectId(PROJECT_ID)); // contained a live project

		sdkTracerProvider = SdkTracerProvider.builder()
				.setResource(resource)
				.addSpanProcessor(BatchSpanProcessor.builder(traceExporter).build())
				.build();

		openTelemetry = OpenTelemetrySdk.builder()
				.setTracerProvider(sdkTracerProvider)
				.buildAndRegisterGlobal();

		System.out.println("OpenTelemetry initialized Successfully !!!");
}

User Facing Changes (NONE)

The way users interact with this API has not changed, but this PR changes the internal implementation of how a TraceExporter object is initialized. The initialization is now deferred till the very first export is called.

Summary of Changes (Including implementation details) -

TraceExporter

1. The original TraceExporter class has been converted to InternalTraceExporter - this class contains all the logic required and core logic for exporting traces is unchanged.
2. The current TraceExporter is a wrapper around the InternalTraceExporter and delegates call to InternalTraceExporter at the appropriate time.
3. TraceExporter holds a reference to InternalTraceExporter which gets initialized when the very first export is called. This initialization is only done once in a thread-safe manner (via memoization).

TraceConfiguration & its interaction with TraceExporter

1. User may not provide explicit null or empty values for project ID.
2. User provided value for the project ID is internally used in a Supplier. If the user does not call setProjectId to provide a Project ID, the generated Supplier will provide default Project ID via ServiceOptions#getDefaultProjectId. The supplier used is memoized so the value is retrieved only once.
3. Building the TraceConfiguration object using the builder no longer attempts to fetch the defaultProjectId immediately via ServiceOptions.getDefaultProjectId() call.
4. Building the TraceConfiguration object using builder will now generate a Supplier which will resolve to a project ID later when get() is called on it. This is done by InternalTraceExporter when it is lazy loaded.
5. From the information in (4), this can be inferred that TraceConfiguration.getProjectId() will always return whatever user has specified as project ID.
6. In case the call made to ServiceOptions.getDefaultProjectId() (for retrieving default project ID) fails, the TraceExporter implementation will be changed to a noop implementation and the user will be notified of this via logs.

[punya]

Why do we need this wrapper?

[psx95]

So one thing to note here is that this is the exact same class as the TraceExporter in the current main.

It has just been renamed to InternalTraceExporter. In order to minimize the changes, I did not edit any existing logic.

[punya]

• An auto-value class should not have concrete fields or setters.
• This should be named projectIdSupplier to be consistent with the type name.

One way to do this would be

1. Remove the field
2. Add an abstract method getProjectIdSupplier to TraceConfiguration
3. Change the existing abstract method getProjectId to a concrete, memoized method that calls getProjectIdSupplier().get().
4. Add a concrete method setProjectId to TraceConfiguration.Bulder that calls setProjectIdSupplier(() -> id).

[punya]

Can we unconditionally return success for this method, without even checking whether the field has been set?

[psx95]

Technically, we can. But the purpose of this class it defer business logic to the internalTraceExporter.

However, I was thinking if failure is the correct code here. Any thoughts ? This condition would happen if flush is called before export somehow.

[punya]

I'm confused by the synchronization model here - usually an AtomicReference is used to avoid synchronization. You might be able to avoid a lot of this complexity by changing internalTraceExporter to Suppliers.memoized(() -> new InternalTraceExporter(this.config)) and making it do all the work.

[punya]

Consider wrapping the IOException at this layer and removing it from the signature. Since our external API no longer has an IOException, this just adds noise to our implementation.

[psx95]

So this exception may be thrown from two possible places, handling it at the InternalTraceExporter level makes the implementation a little ugly since both need to be handled. Also, InternalTraceExporter is the class that actually has the implementation so handling it there would introduce more noise to our implementation detail.

I think it makes sense to keep it in the function signature here since it has a package private access and this exception is handled in TraceExporter for both cases.

[punya]

🎉

[jsuereth]

Nice use of the memoize function.

[jsuereth]

The Noop version should not be returning any failures.