feat(resource)!: Remove resource class export in favor of functions and types only to aid in cross-version compatibility

[dyladan]

Fixes #5358

• Renames Resource class to ResourceImpl
• Renames IResource interface to Resource
• Introduce function resourceFromAttributes to create a resource from a set of attributes
• Only export types and functions. This aids in cross-version compatibility and makes it more easily extensible in the future.

[codecov]

Codecov Report

Attention: Patch coverage is 95.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 94.90%. Comparing base (2d5671a) to head (f401b42).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...ntelemetry-browser-detector/src/BrowserDetector.ts	0.00%	2 Missing ⚠️
...ckages/opentelemetry-resources/src/ResourceImpl.ts	98.27%	1 Missing ⚠️
...es/opentelemetry-resources/src/detect-resources.ts	83.33%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5421   +/-   ##
=======================================
  Coverage   94.90%   94.90%           
=======================================
  Files         308      308           
  Lines        7971     7975    +4     
  Branches     1677     1677           
=======================================
+ Hits         7565     7569    +4     
  Misses        406      406           

Files with missing lines	Coverage Δ
...ry-exporter-prometheus/src/PrometheusSerializer.ts	95.77% <100.00%> (ø)
...imental/packages/opentelemetry-sdk-node/src/sdk.ts	96.03% <100.00%> (ø)
...l/packages/otlp-transformer/src/common/internal.ts	100.00% <100.00%> (ø)
...ckages/otlp-transformer/src/logs/internal-types.ts	100.00% <ø> (ø)
...tal/packages/otlp-transformer/src/logs/internal.ts	100.00% <ø> (ø)
...ges/otlp-transformer/src/metrics/internal-types.ts	100.00% <ø> (ø)
...kages/otlp-transformer/src/trace/internal-types.ts	100.00% <ø> (ø)
...al/packages/otlp-transformer/src/trace/internal.ts	98.38% <100.00%> (ø)
experimental/packages/sdk-logs/src/LogRecord.ts	98.11% <ø> (ø)
...perimental/packages/sdk-logs/src/LoggerProvider.ts	97.77% <100.00%> (ø)
... and 12 more

[trentm]

[Notes to self when understanding the API change.]

The most comment usage of creating a resource to pass to an SDK:

// old (showing the API from before #5350)
import { Resource } from '@opentelemetry/resources';
new Resource({'service.name': 'myservice'});

// new
import { resourceFromAttributes } from '@opentelemetry/resources';
resourceFromAttributes({'service.name': 'myservice'});

When creating the lower-level providers directly, typically one must merge with the default resource:

// old
Resource.default().merge(new Resource({'service.name': 'myservice'});

// new
import { DEFAULT_RESOURCE, resourceFromAttributes } from '@opentelemetry/resources';
DEFAULT_RESOURCE.merge(resourceFromAttributes({'service.name': 'myservice'});

[trentm]

This doesn't export resourceFromDetectedAttributes, I notice. Will the detector implementations in opentelemetry-js-contrib.git need that? (I'm not confident on the differences between resourceFromDetectedAttributes and resourceFromAttributes.

Perhaps change the PR title to:
feat(resource)!: Remove resource class export in favor of functions and types only to aid in cross-version compatibility

I think there are still soem examples in README.md files that should be updated, in particular the README for the resources package itself.

README.md
85:  resource: new Resource({

experimental/packages/opentelemetry-browser-detector/README.md
21:  let resource= new Resource({

experimental/packages/exporter-logs-otlp-proto/README.md
34:const logProvider = new LoggerProvider({resource: new Resource({'service.name': 'testApp'})});

packages/opentelemetry-resources/README.md
22:const resource = new Resource({
26:const anotherResource = new Resource({

[dyladan]

I'm not confident on the differences between resourceFromDetectedAttributes and resourceFromAttributes

I asume you mean resourceFromDetectedResource

They're basically the same. resourceFromAttributes is just slightly more convenient. resourceFromDetectedResource. The DetectedResource contains DetectedAttributes so having a direct attributes helper is there for convenience. In the future there would also be resourceFromDetectedEntities. For now I only export resourceFromAttributes to keep the API small. I thought about only exporting detectResources but direct creation of resources is used in testing a handful of places.

Perhaps change the PR title to: feat(resource)!: Remove resource class export in favor of functions and types only to aid in cross-version compatibility

I think there are still soem examples in README.md files that should be updated, in particular the README for the resources package itself.

I'll make these changes

[trentm]

I asume you mean resourceFromDetectedResource

Yes. My bad.

[dyladan]

Perhaps change the PR title to: feat(resource)!: Remove resource class export in favor of functions and types only to aid in cross-version compatibility
I think there are still soem examples in README.md files that should be updated, in particular the README for the resources package itself.

I'll make these changes

Done

[pichlermarc]

looks good, thank you for working on this.
just one q around the empty/default resource consts.

[pichlermarc]

I wonder if we'd want to make these two functions instead? 🤔

Today, there's no such thing, but if we ever have a function on a resource that modifies the state of the resource is some way, we may allow end-users to modify the empty resource. That would then get updated everywhere its used. Given that these are only used a few times on startup, I think the performance overhead of creating a new empty resource everytime it is called would be negligible.

Also people may use may start relying on the fact that you can check for reference equality, which is not given between different versions of @opentelemetry/resources and may lead to trouble.

These are all very artificial examples, but I've seen people do some gnarly things. 😅

It would also nicely align them with the resourceFromAttributes() function. 🙂

[pichlermarc]

Anyway, my comment here is not blocking, but resolution of #5358 is a release blocker - I'll open another PR to suggest these changes 🙂