evanderkoogh / otel-cf-workers

An OpenTelemetry compatible library for instrumenting and exporting traces for Cloudflare Workers
BSD 3-Clause "New" or "Revised" License
210 stars 40 forks source link

[Feature Request] Support multiple exporter targets #19

Closed AdiRishi closed 1 year ago

AdiRishi commented 1 year ago

Motivation

When evaluating different monitoring and tracing products in the market it is very useful to be able to be able to send the metrics to multiple different sources without major code changes.

Currently I have to pick and choose which workers send traces to which destination. In an ideal world, we could just send the same traces to multiple targets.

Config proposal

This might be a good way to expose the new setting. Accept an array of exporter targets in the config function.

Potential config structure

const config: ResolveConfigFn = (env: Env, _trigger) => {
    return {
        exporter: [
            {
                url: 'https://api.honeycomb.io/v1/traces',
                headers: { 'x-honeycomb-team': env.HONEYCOMB_API_KEY },
            },
            {
                url: 'https://api.axiom.co',
                headers: { authorization: env.AXIOM_API_KEY, 'x-axiom-dataset': env.AXIOM_DATASET },
            }
        ],
        service: { name: 'greetings' },
    }
}

Performance considerations

If my understanding of how this library works is correct, this shouldn't impact performance significantly? The requests to the telemetry targets are done in ctx.waitUntil so they wouldn't block the main request from returning.

evanderkoogh commented 1 year ago

While it is certainly possible to add the functionality to accept an array of Exporters and thus export to multiple vendors, but it gets pretty messy pretty quickly once you start thinking about what to do if one errors and not the other.

A better way IMO is to create an Exporter that can take multiple Exporters and delegates to them. So you can handle all other Logic in there.

Does that make sense?

AdiRishi commented 1 year ago

A better way IMO is to create an Exporter that can take multiple Exporters and delegates to them. So you can handle all other Logic in there.

I confess that I don't know the source code of this project very well, so there is likely some subtlety that I'm missing here, but I don't get why this is a cleaner solution? In my mind, each exporter has a single responsibility, be given a bunch of data that it needs to send to a target destination? Something like this line I found in the source

export(spans: ReadableSpan[], resultCallback: (result: ExportResult) => void): void;

So if that's how exporters behave, then when you have multiple, they don't care about each other right? If one fails the other continues like nothing happened.

Again, this is based on my limited understanding. In the end, whatever option you think is best. What matters is that the end user (developer) can specify multiple targets for their data.

evanderkoogh commented 1 year ago

Absolutely no need to apologise for asking good questions 😀

You are completely correct that if one fails the other one can just continue. But the challenge is that an exporter reports if it had been successful or not. And the question becomes, what is the status of an export where one of the exporters failed and one of them succeeded?

If I change the SpanProcessor to accept multiple exporters, which I totally could do ofc, it is the SpanProcessor that has to make that decision and it will be the same for every single user.

If you make an exporter that delegates to multiple exporters, you can make that decision for yourself. You can have Logic that says it is the success of the production one that matters, or that it is succeeded if one of them succeeds, or both.

Right now we don’t do much with the result information much, but I can certainly see a future where we do retries/queues etc. And I don’t want to paint us into a corner now.

Does that make sense?

AdiRishi commented 1 year ago

Right now we don’t do much with the result information much, but I can certainly see a future where we do retries/queues etc. And I don’t want to paint us into a corner now.

Oooh, yeah that totally makes sense, by having the library make a decision on what to do with a failure, you restrict yourself from making these kind of improvements down the line.

So is it currently possible for us to make custom exporters? I see in the readme we can assign things like const exporter = new ConsoleSpanExporter() and pass that in. I haven't read into it, but can I customize this exporter to handle multiple targets? If so, that's cool, I'll try doing that at some point and maybe write some documentation on it so others can follow.

evanderkoogh commented 1 year ago

Yes exactly. There is an interface SpanExporter that you can just implement.

And if you write it a MultiExportExporter, I have no issues including that as an optional utility in the code base btw. I just don’t want the SpanProcessor to have to deal with it.

evanderkoogh commented 1 year ago

Yes exactly. There is an interface SpanExporter that you can just implement.

And if you write a MultiExportExporter, I have no issues including that as an optional utility in the code base btw. I just don’t want the SpanProcessor to have to deal with it.

willie commented 11 months ago

@AdiRishi Did you end up writing one?

AdiRishi commented 11 months ago

@willie I did not write one yet. This and a few other things with this repo are on my radar, just haven't gotten around to it yet. I'll be sure to update this issue when I do.

willie commented 10 months ago

@AdiRishi I went ahead and wrote one, see if it helps you. https://github.com/evanderkoogh/otel-cf-workers/pull/44

evanderkoogh commented 10 months ago

It is included in the lastest 1.0.0-rc.8 release :)

Let me know if you run into any issues..