Refactor OTLP Metrics Exporters

[legendecas]

Currently, we have OTLP trace exporters to be implemented in an inherited class pattern:

OTLPTraceExporter (grpc) -> OTLPGRPCExporterNodeBase -> OTLPExporterBase
OTLPTraceExporter (HTTP)-> OTLPExporterNodeBase -> OTLPExporterBase
OTLPTraceExporter (proto) -> OTLPProtoExporterNodeBase -> OTLPExporterNodeBase -> OTLPExporterBase

On the other hand, the OTLP metric exporters are not in this pattern. The base OTLP metric exporter, a MetricExporter, has an internal metric exporter, which also implements the MetricExporter interface. This pattern looks abstruse to me.

Can the OTLP metric exporter be implemented without the intermediate exporter?

/cc @pichlermarc

[pichlermarc]

Yes implementing this way should be possible. I originally introduced this as to not bloat up the PRs (#2871, #2893), as they were already way too large at that time.

The reason for that was that the OTLP Exporter expects a list of objects passed to the export() function. This worked for the Trace exporters as they are expecting a list of spans, but not for the Metrics Exporters, that pass a single ResourceMetrics instance. My solution for that was to introduce this Proxy, which kept the diff manageable, but looks, as you already said, abstruse and is far from the optimal solution. 🙁

My original intention was always to follow up with more refactoring PRs to simplify the exporters (and their tests), but I unfortunately never came around to doing it. If needed, I can prioritize this. 🙂

[legendecas]

My original intention was always to follow up with more refactoring PRs to simplify the exporters (and their tests), but I unfortunately never came around to doing it. If needed, I can prioritize this. 🙂

No, not in a hurry as it is working now. I was opening this issue to see if this is intentional and if not we can track the progress here. Thank you for the clarification.

[github-actions]

This issue 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]

This issue 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.

[pichlermarc]

With #5031 this now becomes a bit more actionable. I'll refine this issue once the PR is merged to reflect what needs to be done in a bit more detail. 🙂

[pichlermarc]

Actually, the leftovers from this can be handled in one simple follow-up.

#5031 removes the last of these proxy implementations. Once it is merged, I'll follow up with another PR to removing the array-wrapping inside the exporters, as that can be handled by the json/proto serializers (away from the public API) now.

[pichlermarc]

Assigning myself since #5159 will fix this 🙂