opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

feat(exporters)!: use transport interface in node.js exporters

Open pichlermarc opened this issue 1 year ago • 1 comments
trafficstars

Which problem is this PR solving?

Exporters are very hard to test as everything has to be integration-tested at the moment (see #4116).

This PR does the following for the Node.js exporters:

  • moves the ExporterTransport interface previuously only used by the grpc exporters (introduced in #4432) to the exporter base package
  • untangles the HTTP transport and retry logic by making them both implement the interface
  • makes the HTTP transport and retry logic individually testable
  • removes the header property to reduce the API surface of the OTLPExporterNodeBase and its child classes
    • headers have been set differently across exporters, sometimes overriding what the base constructor does, this change also streamlines how headers are set by the exporters by requiring them to be passed to the base-class constructor.
  • removes the compression property to reduce the API surface of the OTLPExporterNodeBase and its child-classes

A follow-up will do the same for browser transports, eventually removing the need to have different exporter-bases and retry implementations for Node.js and the Browser. This follow-up will conclude the tasks defined in #4116 and enabling implementation of feature-requests like #4631

Another follow-up will address config, splitting the config defaults and env-var code from the actual exporters. Then the tests will be adapted so that changes to underlying structures don't have to be propagated to the signal-/transport-specific exporters.

Breaking changes:

  • (user-facing) headers was intended for internal use has been removed from all exporters
  • (user-facing) compression was intended for internal use and has been removed from all exporters
  • (user-facing) hostname was intended for use in tests and is not used by any exporters, it will be removed in a future
  • (internal) OTLPExporterNodeBase constructor has changed and now expects singalSpecificHeaders to be passed to the constructor.

Updates #4116

Type of change

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

How Has This Been Tested?

  • [x] Adapted existing tests
  • [x] Added Unit tests

pichlermarc avatar May 31 '24 10:05 pichlermarc