fix(resource): make properties for async resource resolution optional

[pichlermarc]

Which problem is this PR solving?

See #3676, existing detectors that previously worked with 1.9.1 do not compile anymore. This PR fixes this, but will break the use of resource.waitForAsyncAttributes() that was introduced in the most recent version. Looks like this is the only way to restore compatibility. I'm open to any suggestions to improve this.

Fixes #3676
Fixes open-telemetry/opentelemetry-js-contrib#1429

Short description of the changes

• make properties for async resource resolution optional

Type of change

• Bug fix, restoring backwards compatibility, see semver:

  What do I do if I accidentally release a backwards incompatible change as a minor version?

  As soon as you realize that you’ve broken the Semantic Versioning spec, fix the problem and release a new minor version that corrects the problem and restores backwards compatibility. Even under this circumstance, it is unacceptable to modify versioned releases. If it’s appropriate, document the offending version and inform your users of the problem so that they are aware of the offending version.

How Has This Been Tested?

• Existing tests
• Added an unmodified 1.9.1-style Detector as a regression test that should fail compilation when incompatible changes are made in the future.

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added
• Documentation has been updated

[codecov]

Codecov Report

Merging #3677 (0794001) into main (3653f5c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 0794001 differs from pull request most recent head c4b75ec. Consider uploading reports for the commit c4b75ec to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3677   +/-   ##
=======================================
  Coverage   93.70%   93.70%           
=======================================
  Files         277      277           
  Lines        8451     8454    +3     
  Branches     1755     1761    +6     
=======================================
+ Hits         7919     7922    +3     
  Misses        532      532           

Impacted Files	Coverage Δ
packages/opentelemetry-resources/src/Resource.ts	100.00% <100.00%> (ø)
...y-sdk-trace-base/src/export/SimpleSpanProcessor.ts	92.10% <100.00%> (+0.67%)	⬆️

[Flarna]

What about the existing compat tests here?

Are they still needed/helpful?
And why haven't they shown the compat problems or do they focus on something completely different?

[dyladan]

Those tests focus on using the old version of @opentelemetry/resources with a new resource detector. I believe what marc here is showing is the opposite? Using an old detector with the newest version of @opentelemetry/resources

[pichlermarc]

Exactly, they're the other way around. Even without the ts-ignore that I removed in 0887706, this would not have been caught using the existing tests.

[dyladan]

So what happened is that we added a new method to the Resource class which doesn't exist in resources used by old resource detectors

[dyladan]

This is only a problem because the old detect method referenced the class Resource instead of a Resource interface. Yet again concrete class references in types coming to bite us.

https://github.com/open-telemetry/opentelemetry-js/blob/v1.9.1/packages/opentelemetry-resources/src/types.ts#L33

[dyladan]

Discussed in SIG. We decided that the breaking change introduced in 1.10.0 was itself a bug. This PR fixes that break and is itself a bugfix and NOT a break. Even though it is technically "breaking" from 1.10.0, it is compatible with 1.9.x and previous versions.

/cc @open-telemetry/javascript-approvers @open-telemetry/javascript-maintainers for visibility on this decision

[dyladan]

@pichlermarc i'm going to leave this until tomorrow before merge/release just to be sure people have the chance to see the decision above. Since the bug only affects people who implement their own resource detectors and the fix is easy for those people to unblock themselves if they need I think this isn't the biggest rush bug.