Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce internal exporter http client #3577

Closed
wants to merge 1 commit into from

Conversation

legendecas
Copy link
Member

@legendecas legendecas commented Jan 30, 2023

Which problem is this PR solving?

Introduce an internal http client that chooses the available http implementations in XMLHttpRequest, fetch, sendBeacon, and node:http based on feature detection. The http client is designed to be used with exporters only. The exposed HTTP Client is a feature-limited one. Exporters don't care about the response details of the request.

This enables exporters to support environments like Deno, ServiceWorker, that don't support XMLHttpRequest or sendBeacon. And reduces the duplications between exporters.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • Browser test - XHRHttpRequest
  • Browser test - sendBeacon
  • Fetch
  • node:http
  • Retry Export
  • Zipkin Exporter
  • OTLP Exporter

Checklist:

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

@legendecas legendecas requested a review from a team January 30, 2023 03:58
@legendecas legendecas marked this pull request as draft January 30, 2023 03:58
@legendecas legendecas changed the title feat: introduce internal http client feat: introduce internal exporter http client Jan 30, 2023
@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #3577 (1df055e) into main (d1f9594) will decrease coverage by 1.06%.
The diff coverage is 83.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3577      +/-   ##
==========================================
- Coverage   93.88%   92.83%   -1.06%     
==========================================
  Files         255      253       -2     
  Lines        7757     7413     -344     
  Branches     1612     1554      -58     
==========================================
- Hits         7283     6882     -401     
- Misses        474      531      +57     
Impacted Files Coverage Δ
...ntelemetry-core/src/internal/http-clients/fetch.ts 17.39% <17.39%> (ø)
...try-core/src/platform/node/internal/send-beacon.ts 66.66% <66.66%> (ø)
...entelemetry-core/src/platform/node/internal/xhr.ts 66.66% <66.66%> (ø)
...metry-core/src/platform/node/internal/node-http.ts 87.50% <87.50%> (ø)
...ckages/opentelemetry-exporter-zipkin/src/zipkin.ts 91.66% <92.85%> (-8.34%) ⬇️
...ges/opentelemetry-core/src/internal/http-export.ts 98.03% <98.03%> (ø)
...ges/opentelemetry-core/src/internal/http-client.ts 100.00% <100.00%> (ø)
packages/opentelemetry-core/src/internal/index.ts 100.00% <100.00%> (ø)
packages/opentelemetry-sdk-trace-web/src/utils.ts 65.83% <0.00%> (-29.20%) ⬇️
... and 9 more

@github-actions
Copy link

github-actions bot commented Apr 3, 2023

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Apr 3, 2023
@github-actions
Copy link

This PR was closed because it has been stale for 14 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant