opentelemetry-js
opentelemetry-js copied to clipboard
feat(exporters)!: collapse base classes into one
Which problem is this PR solving?
Through recent changes I've been working on reducing code-duplication across the exporters and separating config-code, transport code and base exporter code. I'm doing this for three reasons:
- improve how well we can test parts of the exporter
- address some design-level bugs that we've introduced over the year
- reduce the public API to maintainable levels
In the changes I made previously, we have seen a convergence of the OTLPExporterNodeBase, OTLPExporterBrowserBase, and OTLPGRPCExporterBase classes. This PR removes all of these classes, and replaces them with an implementation of a new interface: IOTLPExportDelegate
Collapsing these into a single implementation with a single name has the benefit of fixing #4794, which was caused by differently named platform exports for Node.js and the Browser. I'm fixing the problem of having different public interfaces for different transports by introducing new entrypoints: node-http and browser-http which are only imported by their respective platform-specific implementations.
This export delegate contains all the behavior that is present in the OTLP exporter specification and shared across all signals. It can be created via a factory function to avoid exposing the internal API as public. Specific exporters are expected to use this delegate to eventually compose an exporter though different transports and serializers. Composition is used over inheritance as polymorphism is not of relevance when using an OTLP exporter: It is either a PushMetricExporter, a SpanExporter, or a LogRecordExporter - having an OTLPExporterBase does not add any value to the end-user as the public interface ends up being the same. On top of that it was only used internally as it was the easy way of getting base functionality to the individual exporters. In doing so it also exposed a lot of internals as part of the public API.
OTLPExporterBase still exists but now only takes an IOTLPExportDelegate and will eventually be removed. I've decided to keep it around to have a base for the individual exporters, which can be instantiated with new. In a future revision, I plan to introduce factory functions for each exporter which returns a PushMetricExporter, a SpanExporter, or a LogRecordExporter instead of a specific exporter class. The factory functions will also do away with some duplicate ways of configuring transports.
As a way to transition to these factory functions, this PR introduces "translation" helpers for the "old" configuration to the "new" one which is already used internally (these legacy conversions can be identified by the word legacy in their names).
Fixes #4794
Breaking changes
- feat(otlp-exporter-base)!: collapse base classes into one
OTLPExporterNodeBasehas been removed in favor of a platform-agnostic implementation (OTLPExporterBase)OTLPExporterBrowserBasehas been removed in favor of a platform-agnostic implementation (OTLPExporterBase)ExportServiceErrorwas intended for internal use and has been dropped from exportsvalidateAndNormalizeHeaderswas intended for internal use and has been dropped from exportsOTLPExporterBaseall properties are now private, the constructor now takes anIOTLPExportDelegate, the type parameter for config type has been dropped.- This type is scheduled for removal in a future version of this package, please treat all exporters as
SpanExporter,PushMetricExporterorLogRecordExporter, based on their respective type.
- This type is scheduled for removal in a future version of this package, please treat all exporters as
- feat(otlp-grpc-exporter-base)!: collapse base classes into one
OTLPGRPCExporterNodeBasehas been removed in favor of a platform-agnostic implementation (OTLPExporterBasefrom@opentelemetry/otlp-exporter-base)
TODOS
- more tests
- properly labeling deprecated types
Type of change
Please delete options that are not relevant.
- [x] Bug fix
- [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
How Has This Been Tested?
- [x] Added unit tests, adapted existing tests