feat: add default resource in TracerProvider if user didn't provide one.

[TommyCpp]

• Update MeterProviders.
• Remove the same logic from Jaeger.
  - [ ] Add more resource detector? Will probably do it in another PR.

fix #558

This PR:

• Added a SdkProvidedResourceDetector to detect attributes that should be provided by SDK if not specified otherwise. As for now, it's service.name
• Move the logic that detects service name from the environment variable from exporter builder to the SDK.
• Note that to have better performance, all exporter builders will store the service name in the exporter instead of a resource. But the SDK will store the service name in resource if users don't provide a resource, which could cause a performance overhead because of the clones that happen with the resource.

[codecov]

Codecov Report

Merging #571 (6cd1d19) into main (db20b9c) will increase coverage by 0.0%.
The diff coverage is 49.0%.

@@          Coverage Diff           @@
##            main    #571    +/-   ##
======================================
  Coverage   54.7%   54.7%            
======================================
  Files        101     102     +1     
  Lines       8667    8840   +173     
======================================
+ Hits        4744    4839    +95     
- Misses      3923    4001    +78     

Impacted Files	Coverage Δ
opentelemetry-datadog/src/exporter/mod.rs	12.2% <0.0%> (-4.2%)	⬇️
opentelemetry-jaeger/src/exporter/env.rs	73.3% <ø> (+1.1%)	⬆️
opentelemetry-zipkin/src/exporter/mod.rs	0.0% <0.0%> (ø)
opentelemetry/src/sdk/trace/config.rs	27.5% <0.0%> (-1.6%)	⬇️
opentelemetry/src/testing/metric.rs	0.0% <0.0%> (ø)
opentelemetry-jaeger/src/exporter/mod.rs	47.7% <43.1%> (-0.1%)	⬇️
opentelemetry/src/sdk/resource/env.rs	86.4% <94.7%> (+3.9%)	⬆️
opentelemetry/src/sdk/metrics/mod.rs	24.5% <96.1%> (+5.9%)	⬆️
opentelemetry/src/sdk/resource/mod.rs	90.2% <100.0%> (+3.2%)	⬆️
opentelemetry/src/sdk/trace/provider.rs	81.8% <100.0%> (+7.1%)	⬆️
... and 2 more

---

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 db20b9c...6cd1d19. Read the comment docs.

[jtescher]

We may want to consider the perf impact here, I believe that cloning the resource had a measurable impact on spans with few attributes.

[TommyCpp]

Sounds reasonable. But I'd argue this usually only happens when the application starts so it may not impact performance that much. I can create sdk_provided_resource only when there isn't a resource already configured so that could reduce some overhead.

[jtescher]

Was thinking more about the clone each time a span is exported, could check

opentelemetry-rust/opentelemetry/benches/trace.rs

Line 27 in efbc842

trace_benchmark_group(c, "start-end-span", |tracer| tracer.start("foo").end());

at some point

[TommyCpp]

Yeah, you are right. This will add an attribute to each span by default. But users can avoid that by passing an empty resource in the config. Will benchmark it before mark this ready.

[TommyCpp]

I ran the bench. With default resource & required attributes:

start-end-span/always-sample                                                                             
                        time:   [536.05 ns 538.61 ns 541.21 ns]
                        change: [+18.240% +18.917% +19.533%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild
start-end-span/never-sample                                                                            
                        time:   [166.50 ns 166.98 ns 167.66 ns]
                        change: [+0.1332% +0.4798% +0.8373%] (p = 0.01 < 0.05)
                        Change within noise threshold.
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe

The performance impact is concerning. But looking at the spec

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.

I think it's clear we need to build a resource with service.name and associate it with TracerProvider.

[TommyCpp]

Hmm, I don't think that works for resources created from environment variables? Because we cannot know the value in the compile time. I am thinking if users can somehow explicitly says they don't want any resource maybe we could avoid the overhead.

[TommyCpp]

@jtescher I guess a workaround here is to explicitly pass an empty resource into the config to indicate users don't want any resource including the one provided by SDK by default. Then we won't have any performance impact on users.

We can check if the resource is empty in Builder::build() and if it's empty we just replace it with None because Some(Resource::empty()) is the same in this case.

[jtescher]

Hm yeah that's a possibility. Wonder if there should be an explicit method in the builder or otherwise more discoverable method. People doing comparisons like https://github.com/tikv/minitrace-rust/blob/master/benches/compare.rs will likely miss the possible optimizations otherwise.

[TommyCpp]

Added a with_no_resource method for that purpose.

[jtescher]

ok can merge this for now and add a check in the 1.0 readiness to see if breaking changes are necessary for perf reasons

[jtescher]

Looks good for now, we can re-assess if changes are needed to fix the perf issues before 1.0

[dherbert97]

Okay thank goodness I'm going to be in the car