feat: add the ability to set the views via the SDK constructor

[weyert]

Which problem is this PR solving?

Recently the ability to define views has been moved to the constructor of the MeterProvider when you are using the node-sdk you aren't not able to pass views anymore. This PR attempts to resolve this problem.

Fixes #3125 (issue)

Short description of the changes

Add the views: Views[]-parameter to the configuration type

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Used yalc to verify it works

Checklist:

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

[weyert]

Does anyone have a good idea how I can access the views of the meter provider to ensure they got passed in?

[codecov]

Codecov Report

Merging #3124 (2931a15) into main (6807def) will decrease coverage by 0.02%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##             main    #3124      +/-   ##
==========================================
- Coverage   93.21%   93.18%   -0.03%     
==========================================
  Files         196      196              
  Lines        6414     6431      +17     
  Branches     1350     1359       +9     
==========================================
+ Hits         5979     5993      +14     
- Misses        435      438       +3     

Impacted Files	Coverage Δ
...imental/packages/opentelemetry-sdk-node/src/sdk.ts	91.02% <87.50%> (-2.42%)	⬇️

[dyladan]

Does anyone have a good idea how I can access the views of the meter provider to ensure they got passed in?

You can access private properties using obj["_somePrivateProperty"] syntax. I believe the views are in provider["_sharedState"].viewRegistry["_registeredViews"]. You can also do provider["_sharedState"].viewRegistry.findViews(instrument, meter) if you have access to the instrument.

[weyert]

Does anyone have a good idea how I can access the views of the meter provider to ensure they got passed in?

You can access private properties using obj["_somePrivateProperty"] syntax. I believe the views are in provider["_sharedState"].viewRegistry["_registeredViews"]. You can also do provider["_sharedState"].viewRegistry.findViews(instrument, meter) if you have access to the instrument.

Thank you for the tip. I have added the unit test :)

[legendecas]

Sorry for the delay, this is purely a suggestion: You can also test with the resulting metric streams with a MetricReader, like adding a renaming view and checking that the metric instrument is being renamed when exported, without accessing the internal states of the meter provider.

[legendecas]

The generated dts files are erasing the types of private properties. So linked packages accessing the private properties will not get the type information.

[weyert]

Sorry for the delay, this is purely a suggestion: You can also test with the resulting metric streams with a MetricReader, like adding a renaming view and checking that the metric instrument is being renamed when exported, without accessing the internal states of the meter provider.

I will give it shot :) If I understand you correctly you want me to test with like the in memory exporter?

[legendecas]

If I understand you correctly you want me to test with like the in memory exporter?

Yeah, or more cleanly testing it with InMemoryExporter will be great. This is where InMemoryExporter is best suited :)

[dyladan]

@weyert what is the plan here? To update the tests to use the in memory exporter?

[dyladan]

If this is not ready for review please mark as draft, otherwise just comment and I will review it.

[weyert]

@weyert what is the plan here? To update the tests to use the in memory exporter?

Yes, that's the plan. I hope to sort it out before the end of the weekend. Only I am unable to switch the PR to draft

[pichlermarc]

@weyert thanks for adding this and taking the time to switch over the tests. 🚀
I marked the PR as draft now, you should be able to mark it as ready at any point 🙂

[weyert]

@pichlermarc I have updated the test, let me know, if this test with in memory exporter makes sense for you. If so, feel free to change the status of the PR to Ready for review :)

[pichlermarc]

Looking good, thanks for adapting the tests. 🙂
I think this should be ready once the linting errors are addressed (there are a few unused imports and missing semicolons). 🙂

[pichlermarc]

Looks good with a few minor comments. 🙂
Sorry for the churn. 🙁

[dyladan]

LGTM pending conflict resolution

[dyladan]

Want to make sure @legendecas is happy since he had comments but this can be merged IMO

[legendecas]

Thank you!