fix: default to otlp if OTEL_METRICS_EXPORTER is empty

[jeengbe]

Short description of the changes

The OTEL_METRICS_EXPORTER variable should default to otlp. It currently, if not specified, does nothing.

Type of change

• Bug fix (non-breaking change which fixes an issue)

This could potentially be considered a breaking change if a user produces metrics without configuring OTEL_METRICS_EXPORTER. What previously did nothing will now error with connect ECONNREFUSED 127.0.0.1:4318 when the SDK is shut down and metrics are flushed.

How Has This Been Tested?

• Unit tests adjusted

Checklist:

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

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: maryliag / name: Marylia Gutierrez (116edb9)

[jeengbe]

Apologies; all is fixed now. I also had to adjust some other (now incorrect) tests.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.21%. Comparing base (bc60b8d) to head (116edb9).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6092      +/-   ##
==========================================
+ Coverage   95.20%   95.21%   +0.01%     
==========================================
  Files         316      316              
  Lines        9388     9388              
  Branches     2166     2168       +2     
==========================================
+ Hits         8938     8939       +1     
+ Misses        450      449       -1     

Files with missing lines	Coverage Δ
...imental/packages/opentelemetry-sdk-node/src/sdk.ts	96.53% <100.00%> (+0.49%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[maryliag]

Suggested change

	const enabledExporters = getStringListFromEnv('OTEL_METRICS_EXPORTER') ?? [];
	const exportersType = Array.from(
	new Set(getStringListFromEnv('OTEL_METRICS_EXPORTER'))
	);

The function can return an empty array, so the ?? won't work on this case. Also adding a set to remove duplicates.
I'm working on some changes on this file, and this will all be gone, but I think the tests are worth adding, so let's get this fixed too for now :)

[jeengbe]

You can pick the tests from my branch and close this MR if it makes more sense to you - or what do you mean with "let's get this fixed too for now"

[maryliag]

I meant that is taking awhile for me to get reviews on my PRs that are changing this, so instead of waiting for my changes to actually get merged, I can approve/merge yours in the meantime, so we can get that fix in (even if this will change later on).

[maryliag]

Just commit my suggestion, resolve the conflict on changelog, and I can get that in 🚀

[jeengbe]

Got it. I would though like to defer new Set(...) to your MRs (or at least make it a separate change set) to keep this one minimal

The second comment didn't load initially 🙂 Will fix

[jeengbe]

Ready 🚀

[maryliag]

Will wait for the tests to pass later today :)
Thanks for adding the changes

[otelbot]

Thank you for your contribution @jeengbe! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.