feat(otlp-exporter-base): internally accept a http header provider function only

[pichlermarc]

Which problem is this PR solving?

Changes internal code to always expect a header provider function, instead of a direct object. This enables us to implement #2903 later.

This does not fully implement #2903 as I want to migrate the current way of constructing an OTLP exporter to a factory function. That factory function will have a slightly different options to the existing OTLP exporters. Goal of introducing the factory function with a different interface is to:

• reduce the public API (hide private properties and avoid that users can inherit from exporters - offering them a composition-based approach instead)
• move to a single way of setting options (like keepAlive, or inheaders in this case)
  • a single case to handle makes the underlying code simpler and makes setup code more consistent across users, which should aid in hunting down bugs

The changes in this PR are currently non-breaking as it's not part of the public API in any released version. So it's just internally breaking if we merge it before we release the changes from #5031.

Enables #2903

Short description of the changes

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

• existing unit tests
• adapted unit tests

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.56%. Comparing base (91b9abd) to head (909666f).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5179      +/-   ##
==========================================
- Coverage   94.57%   94.56%   -0.01%     
==========================================
  Files         314      314              
  Lines        7956     7966      +10     
  Branches     1600     1601       +1     
==========================================
+ Hits         7524     7533       +9     
- Misses        432      433       +1     

Files with missing lines	Coverage Δ
.../configuration/convert-legacy-node-http-options.ts	100.00% <100.00%> (ø)
...-base/src/configuration/otlp-http-configuration.ts	100.00% <100.00%> (ø)
...e/src/configuration/otlp-http-env-configuration.ts	95.83% <100.00%> (+0.08%)	⬆️
...ter-base/src/configuration/shared-configuration.ts	100.00% <100.00%> (ø)
...xporter-base/src/transport/http-transport-utils.ts	100.00% <ø> (ø)
...perimental/packages/otlp-exporter-base/src/util.ts	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[pichlermarc]

Note for reviewers: this was in the wrong spot earlier, so moved it up here.

[bozzelliandrea]

Hi @pichlermarc, I have done a similar pr about that for string or function support #5118
What do you think??

[pichlermarc]

Hi @bozzelliandrea - I'd prefer having only one option internally (as my PR here proposes) to avoid the code becoming too complex. By only dealing with provider functions internally we can handle both static and dynamic use-cases via the same code-path.

[bozzelliandrea]

Hi @pichlermarc, do you have any idea when the team will release the version with this commit? thanks

[bozzelliandrea]

Hi @pichlermarc I've tested this commit locally but it's not enough for the dynamically headers support, the base exporter configuration interface expect a Record<string,string> and not a function.

So this feat seems only for internal purpose and not exposed externally.

[pichlermarc]

Hi @pichlermarc I've tested this commit locally but it's not enough for the dynamically headers support, the base exporter configuration interface expect a Record<string,string> and not a function.

So this feat seems only for internal purpose and not exposed externally.

@bozzelliandrea yes, this is on purpose. We're restructuring the exporters. Legacy config is not intended to be modified since it leads to churn while we do it and delays other features - I will introduce a new interface that will expose this feature eventually. See the PR description where I already explained this.

We have asked users to avoid opening PRs for exporters while we work on it. See #5149 for the reasoning behind that.

[bozzelliandrea]

@pichlermarc thanks for the reply, I've actually already open a pr for the dynamic header support on legacy exporter, #5221 (I hadn't seen the issue #5149 before). the change doesn't have a big impact and ensures backwards compatibility, can we consider making the feature available for legacy classes too?