chore(otl-exporter-base): make _delegate protected

[Joozty]

Which problem is this PR solving?

We are extending the OTLExporter functionality and need to access the delegate property. However, this property is currently private and not exposed to the derived class, which prevents us from using it.

Type of change

Please delete options that are not relevant.

How Has This Been Tested?

N/A

Checklist:

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

[pichlermarc]

However, this property is currently private and not exposed to the derived class, which prevents us from using it.

This was the intention. We're trying to get completely away from allowing the sub classing of exporters. The state of the exporters about a year ago was so dire as everything was public/protected that we were not able to make even small changes anymore without breaking someone. This made fixing bugs virtually impossible.

I strongly oppose exposing more than what's needed for this reason. Maybe we can work something else out that would benefit both the project and you? Can you provide some details to what you're trying to achieve?

[Joozty]

I thought that was the reason. I maintain the @splunk/otel-web package, which is an agent that heavily depends on OpenTelemetry with some custom adjustments. I’m trying to upgrade our OTel packages since they’re currently very, very outdated, and it’s proving to be quite challenging.

We have currently three reasons to extend exporters:

Right now, we have three main reasons for extending exporters:

1. Global attribute hook:
   We have a hook called onAttributesSerializing that allows customers to modify any attributes before they are sent. We call this hook inside the exporter’s send method..

2. Dropping unwanted spans:
   We need the ability to drop certain spans, and also allow customers to drop spans based on their own criteria. For example, if a third-party error is spamming your logs, you might not want to track it. This gives customers the flexibility to filter out unwanted spans before sending.

3. Persisting failed spans:
   Sometimes spans fail to send, typically when a page is unloading. We want to persist these spans in localStorage or IndexedDB, so that on the next page load, we can attempt to resend them.

[Joozty]

Another use case is reacting to status codes. For example, we have some rate limiting in place and need to handle the 429 error code, so hooking into the send method makes sense for us. Some of the things mentioned above could be implemented directly in OTel , but others are product-specific. In those cases, having hooks or the ability to override methods seems reasonable to me.

[pichlermarc]

Looking at the use-cases, it looks to me like none of them would actually benefit from having the actual OTLP exporter as a base class. It looks to me that many of these can be solved via composition and using the SpanExporter interfaces as the basis for a custom exporter, as that is what the SDK accepts.

That should give you a way to accomplish the following by intercepting the ReadableSpans before passing it to the underlying exporter:

• Global attribute hook
• Dropping unwanted spans

If you add some code after passing it to the underlying exporter (in the callback), then you should also be able to do:

• Persisting failed spans

As for handling 429 error codes, this should already be handled by the exporter today - it also parses the retry-after header and retries after some time (unless it's using sendBeacon, where the response code is lost).

[oreporan]

Adding the 429 use case
In older versions of the exporter we parsed the status code on the error and had specific handling for 413 (Too Large) responses,
in later versions this status code is no longer available for us when override

export(metrics: ResourceMetrics, resultCallback: (result: ExportResult) => void)

What other options do we have if we want the underline http status code?

Thanks!

[pichlermarc]

I see what you're trying to accomplish, but I fail to see how this change here would move the needle significantly towards you accomplishing your goals. From my POV this PR reduces the need for you to have around 20 lines of boilerplate code, but does not move you any closer towards what you're telling me that you're actually trying to do.

[pichlermarc]

So what I'm trying to avoid is merging a PR that expands our public interface but still ends up being for nothing in the end.

[oreporan]

@pichlermarc
I see, any other workaround to let me see the underlying fetch status code without having to write a wrapper around my entire app's fetch?

[pichlermarc]

@oreporan please open an issue or link it here if it already exists - ideally with information of what exactly the handling is that you're looking for. 🙂 In the past, we've added features to the exporters without much discussion about how features are supposed to be used by the end-users. This made us end up with a lot of bloat in the exporters that made development extremely difficult.

I want to help, but we need to sort out the requirements first, so that me and other @open-telemetry/javascript-approvers to adequately review the contents of a PR - looking at this PR here I don't see the full picture yet and I feel like I'm missing a whole lot of context about how the PR helps with achieving a goal, which is the reason I cannot even suggest any notable improvements or alternatives.

A side note: usually an implementation of SpanProcessor/LogRecordProcessor is used to partition batches of spans that are being sent to the exporters for the sake of limiting export size.