feat(sdk-*): add option to opt-out from merging the resource with Resource.default()

[pichlermarc]

Description

The spec states that

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.

However, we merge with Resource.default() regardless of if another resource was explicitly specified or not.

This leaves us with a difficult choice:

1. stop merging with Resource.default() - this would likely break telemetry for users that rely on our non spec-compliant behavior of merging all the time, resource attributes that used to be there, would not be there anymore. (I'm not in favor of this)
2. allow users to explicitly turn this behavior off. It's not optimal as this will still leave us spec non-compliant, but slightly less so. Users will be able to choose.
3. do something else

This PR chooses option 2, providing a flag to each SDK and sdk-node that allows the users to opt-in to the spec compliant behavior. We can make the spec compliant behavior the default in SDK 2.0

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.19%. Comparing base (4b5c21c) to head (44fd9d8).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4617      +/-   ##
==========================================
+ Coverage   93.17%   93.19%   +0.02%     
==========================================
  Files         315      315              
  Lines        8086     8098      +12     
  Branches     1617     1623       +6     
==========================================
+ Hits         7534     7547      +13     
+ Misses        552      551       -1     

Files with missing lines	Coverage Δ
...imental/packages/opentelemetry-sdk-node/src/sdk.ts	95.54% <100.00%> (+0.05%)	⬆️
...perimental/packages/sdk-logs/src/LoggerProvider.ts	98.00% <100.00%> (+0.17%)	⬆️
experimental/packages/sdk-logs/src/config.ts	100.00% <ø> (ø)
...elemetry-sdk-trace-base/src/BasicTracerProvider.ts	95.57% <100.00%> (+0.03%)	⬆️
...ackages/opentelemetry-sdk-trace-base/src/config.ts	88.63% <ø> (ø)
packages/sdk-metrics/src/MeterProvider.ts	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

[trentm]

Some notes, thinking out loud.

How to create NodeSDK without getting Resource.default() merged in?

// - Option 1: If we had to do it all over again it would be: If you explicitly
//   pass in a `resource` then `.default()` is NOT used.
//   Cons: Breaks backward compat. NodeSDK *is* pre-1.0, so we *could*, but
//   probably don't want to until a major version bump.
const sdk = new NodeSDK({ resource: new Resource({foo: 'bar'}) }); // no defaults
const sdk = new NodeSDK({ resource: Resource.empty() }); // no defaults
const sdk = new NodeSDK({ resource: Resource.default() }); // yes defaults
const sdk = new NodeSDK(); // yes defaults

// - Option 2: Boolean option to say not to merge defaults.
//   Defaults to always merge with Resource.default() for now.
//
//   Could consider later (for SDK 2.0?) changing `mergeResourceWithDefaults`
//   to a tri-state: true, false, or unspecified. Unspecified  means it
//   depends on whether `resource` is given. Eventually then the behaviour
//   would be as in Option 1, with the option to explicitly keep it stable
//   by specifying `mergeResourceWithDefaults`.
const sdk = new NodeSDK({ mergeResourceWithDefaults: false }); // no defaults

[github-actions]

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[trentm]

@pichlermarc Interested in adding a doc section for this in the sdk-node/README.md and then taking this out of draft?

See discussion at #4930

[pichlermarc]

@trentm sorry for letting this sit for so long. I'm picking this up again. I don't have good answers to these questions yet but I'll comment here and will take it out of draft once I figured out what's best for NodeSDK.

When we merge this I'll also follow up on the next branch to restore a spec-compliant behavior. I think in 2.0 we can then have a Default Resource detector in NodeSDK that would add the default resource and can be turned off via env var just like the other detectors.

Maybe for now we just allow the same flag as proposed in the NodeSDK for this PR.

[pichlermarc]

@trentm I've gone the route of adding the same flag to the NodeSDK - this way we stay consistent in what we do in all the SDKs (logs, metrics, traces) as well as the wrapper (sdk-node). I've added the docs section to the NodeSDK.

I suggest we make that spec-compliant behavior the default and drop the option in 2.0. I added a changelog entry to announce that plan.

[trentm]

I suggest we make that spec-compliant behavior the default and drop the option in 2.0. I added a changelog entry to announce that plan.

Should we add #4930 to the SDK 2.0 milestone?

[pichlermarc]

I suggest we make that spec-compliant behavior the default and drop the option in 2.0. I added a changelog entry to announce that plan.

Should we add #4930 to the SDK 2.0 milestone?

Yes, I'll refine that request into a new issue once I've merged this 🙂

[pichlermarc]

I opted to close their issue as this PR seems to address all their requirements for now. I created a follow-up issue to fully align with the spec in SDK 2.0 (#5132)