Skip to content

fix(otlp-exporter-base)!: split node and browser config types in two#5917

Merged
pichlermarc merged 6 commits intoopen-telemetry:mainfrom
dynatrace-oss-contrib:fix/webpack-failing
Sep 10, 2025
Merged

fix(otlp-exporter-base)!: split node and browser config types in two#5917
pichlermarc merged 6 commits intoopen-telemetry:mainfrom
dynatrace-oss-contrib:fix/webpack-failing

Conversation

@pichlermarc
Copy link
Copy Markdown
Member

@pichlermarc pichlermarc commented Sep 8, 2025 •
edited
Loading

Which problem is this PR solving?

See #5908, browser exporters are broken due to dynamic import of node:http in the defaults. What is happening is that getHttpConfigurationDefaults() is called, which uses the httpAgentFactoryFromOptions() function to create an agent factory, which does not work in the browser.

This PR fixes this, by introducing a new type OtlpNodeHttpConfiguration along with node-specific merge and default functions, that are based on the ones for OtlpHttpConfiguration. This is a breaking change as the name of the type changes, however it gives us more flexibility in the future to have these configurations diverge further without breaking one another.

Related PR: #5719
Fixes #5908
Related #4744

Type of change

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

How Has This Been Tested?

  • Manually tested against examples/opentelemetry-web
  • Added webpack bundler test (for browser target) as well as a workflow to run these tests on each PR and main

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 8, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.07%. Comparing base (b37bfa7) to head (54f736f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #5917   +/-   ##
=======================================
  Coverage   95.06%   95.07%           
=======================================
  Files         307      308    +1     
  Lines        8031     8039    +8     
  Branches     1627     1627           
=======================================
+ Hits         7635     7643    +8     
  Misses        396      396
Files with missing lines Coverage Δ
.../configuration/convert-legacy-node-http-options.ts 100.00% <100.00%> (ø)
...ase/src/configuration/legacy-node-configuration.ts 100.00% <ø> (ø)
...-base/src/configuration/otlp-http-configuration.ts 100.00% <ø> (ø)
.../src/configuration/otlp-node-http-configuration.ts 100.00% <100.00%> (ø)
.../configuration/otlp-node-http-env-configuration.ts 95.83% <100.00%> (ø)
...packages/otlp-exporter-base/src/index-node-http.ts 100.00% <100.00%> (ø)
...tlp-exporter-base/src/otlp-http-export-delegate.ts 100.00% <ø> (ø)
...rter-base/src/transport/http-exporter-transport.ts 95.83% <100.00%> (ø)
...xporter-base/src/transport/http-transport-utils.ts 100.00% <ø> (ø)
🚀 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.

@pichlermarc pichlermarc marked this pull request as ready for review September 8, 2025 15:13
@pichlermarc pichlermarc requested a review from a team as a code owner September 8, 2025 15:13
@pichlermarc pichlermarc added bug Something isn't working priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc pkg:otlp-exporter-base labels Sep 8, 2025
@pichlermarc
Copy link
Copy Markdown
Member Author

pichlermarc commented Sep 9, 2025

@maryliag thank you for taking a look. 🙂

Since you mentioned it here: I've been working on the follow-up test (#4744 like) in the background today - I just pushed them to this PR. You should be able to see them failing over at this draft #5919.

The test itself is very simple (it only runs webpack and checks if the thing builds or not), but should help us catch regressions like this in the future. 🙂

@pichlermarc
Copy link
Copy Markdown
Member Author

pichlermarc commented Sep 9, 2025

I had planned the extra tests to be a follow-up but it turned out to be simpler than I thought. :)

@pichlermarc pichlermarc mentioned this pull request Sep 9, 2025
Merged
@pichlermarc pichlermarc requested a review from maryliag September 9, 2025 16:49
@maryliag
Copy link
Copy Markdown
Contributor

maryliag commented Sep 9, 2025

awesome! thank you for adding the test! 🚀

@pichlermarc pichlermarc added this pull request to the merge queue Sep 10, 2025
Merged via the queue into open-telemetry:main with commit 9a8c6e0 Sep 10, 2025
25 checks passed
@pichlermarc pichlermarc deleted the fix/webpack-failing branch September 10, 2025 08:51
@opentelemetrybot opentelemetrybot mentioned this pull request Sep 10, 2025
Merged
@svrnm svrnm mentioned this pull request Sep 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maryliag maryliag maryliag approved these changes

Assignees

No one assigned

Labels

bug Something isn't working pkg:otlp-exporter-base priority:p1 Bugs which cause problems in end-user applications such as crashes, data inconsistencies, etc

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Webpack build failure using the experimental v0.204.0 packages

2 participants

@pichlermarc @maryliag