Since we are passing observabilityConfig to the non-static grpcInit we can construct configFilterHelper in there? Unless I am missing something?

May be the same thing for many other objects being constructed here and passed to the non-static grpcInit such as sink helper etc?

The reason I am saying is because the non-static grpcInit is unit tested/unit-testable so the more code it has the better it is. But we need to look into it carefully

Which other exception type made this necessary from IOException to Exception? Can we just add the additional types (if practical) instead of moving up to Exception?

Stackdriver<Stats/Trace>Exporter throws IllegalStateException. So I will add IllegalStateException to the list of exceptions instead of generic Exception.

This is unusual: catching an IOException and throwing another IOException with just the message modified. Wouldn't the original message be sufficient - so no need to do this? Same for line 181

Also I realized about the IllegalStateException: so we will get that for duplicate registration attempt. Ideally that should not happen because of the guard around grpcInit but a user could have registered the exporter on their own. I guess the only way to resolve that is to document that in the UG? Unless there is a use-case for the user to have a separate registration for their own purpose?

Removed catching and re-throwing exceptions.

This is formatting only change with no real code change - will be good to avoid it to minimize the amount of code showing up as different

Done.

Based on the latest change in the other PR, you should remove this check in the if. Even if empty you should now set the interceptors or tracers in the GlobalInterceptors.

Hopefully you can unit test this scenario since it's important for correct functioning

Added unit test to verify empty lists are returned when grpcInit is called and none of the observability features are enabled.

I can understand null, do we want to include empty to have the same semantics? 2 Qs

• will the ObservabilityConfig ever return empty string for the project ID?
• if it does do we want special or different semantics for empty project ID?

ObservabilityConfig will not return empty, if the value does not exist in config it returns null instead.
Updated the check.

Comparison of double numbers is tricky and because of precision (and conversion from String to double) issues this equality check may not go through. Has this been unit tested? https://www.baeldung.com/java-comparing-doubles#comparing_in_plain_Java

Updated. PTAL

Why not make this a class final constant with some comment (e.g. why 1e-6)?

Done

Would it be simpler/better if you just remove the check for enableCloudTracing and do it as follows (with epsilon defined as a class constant)?:

      if (samplingRate == null) {
        this.sampler = Samplers.probabilitySampler(0.0);
      } else {
        checkArgument(
            samplingRate >= 0.0 && samplingRate <= 1.0,
            "'global_trace_sampling_rate' needs to be between [0.0, 1.0]");
        // Using alwaysSample() instead of probabilitySampler() because according to
        // {@link io.opencensus.trace.samplers.ProbabilitySampler#shouldSample}
        // there is a (very) small chance of *not* sampling if probability = 1.00.
        if (Math.abs(1 - samplingRate) < EPSILON) {
          this.sampler = Samplers.alwaysSample();
        } else {
          this.sampler = Samplers.probabilitySampler(samplingRate);
        }
      }

Done

remove the commented line?

nit: Since samplingRate is always going to be <= 1.0 (because of the check on line 112) the Math.abs is strictly unnecessary. But I am okay to leave it there as a generalized double equality comparison logic

Removed Math.abs

another commented import line? Any reason to leave them here?

This was an oversight. Removed both.

observability is unused - I am surprised you don't get a compile or link warning. If that was the case you can rename this var to unused or use the SuppressWarning annotation.

Updated.