feat(otlp-exporter-base): allow agent override in Node exporter
Which problem is this PR solving?
Currently, the OTLP HTTP exporters in Node environments only allow providing a configuration for the underlying Agent used when making the requests, however, there are instances in which one might want to opt-out of using the default node http/https Agent, and prefer using something like agentkeepalive. One of these for example is in order to enforce a TTL on the sockets, which does not seem doable with the current setup.
Short description of the changes
Allow providing an Agent directly, instead of only providing a way of providing a configuration for the agent. This comes as a change to the OTLPExporterNodeBase class and its' configuration type.
Type of change
- [X] New feature (non-breaking change which adds functionality)
How Has This Been Tested?
Currently not covered, looking for feedback on how this could be covered. One possibility is to do it via the various extensions on the base class, i.e OTLPTraceExporter?
Checklist:
- [X] Followed the style guidelines of this project
- [ ] Unit tests have been added
- [X] Documentation has been updated (Updated the types)
Codecov Report
Merging #3935 (b68f8f6) into main (de5cd0f) will decrease coverage by
0.04%. Report is 164 commits behind head on main. The diff coverage is60.00%.
:exclamation: Current head b68f8f6 differs from pull request most recent head 334786d. Consider uploading reports for the commit 334786d to get more accurate results
Additional details and impacted files
@@ Coverage Diff @@
## main #3935 +/- ##
==========================================
- Coverage 93.14% 93.11% -0.04%
==========================================
Files 298 298
Lines 8869 8873 +4
Branches 1826 1830 +4
==========================================
+ Hits 8261 8262 +1
- Misses 608 611 +3
| Files | Coverage Δ | |
|---|---|---|
| ...ages/otlp-exporter-base/src/platform/node/types.ts | 100.00% <ø> (ø) |
|
| ...ter-base/src/platform/node/OTLPExporterNodeBase.ts | 50.00% <60.00%> (ø) |
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.
I still would like to see this, so commenting to remove stale
I can see that this feature could be helpful, though I'm hesitant to accept new features to the OTLP exporters before some extensive refactoring is done on them. :thinking:
There was a proposal a very long time ago to introduce a transport layer to the exporters (#422). I've opened another issue (#4116) to track this in a more recent context, as #422 was created when OTLP exporters did not exist yet. I believe that we need to take care of this first, as we're increasingly adding features to the OTLP exporters while having difficulty in adequately testing them.
@open-telemetry/javascript-maintainers wdyt? IMO, we should feature-freeze OTLP exporters until this refactoring is done.
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.