Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protobuffer support #68

Open
cdloh opened this issue Oct 11, 2023 · 8 comments
Open

Protobuffer support #68

cdloh opened this issue Oct 11, 2023 · 8 comments
Labels
enhancement New feature or request external There is an external dependency or integration

Comments

@cdloh
Copy link

cdloh commented Oct 11, 2023

Love the start of the library!

We use Dynatrace as part of our monitoring stack and presently they only support HTTP & Protobuffer as a transport mechanism for span traces and the such - https://www.dynatrace.com/support/help/shortlink/otel-getstarted-otlpexport

Any chance you would add protobuffer support at all?

Thanks!

@RichiCoder1
Copy link

RichiCoder1 commented Oct 11, 2023

I had proto support in https://github.com/RichiCoder1/opentelemetry-sdk-workers so I know it works. Only issue I had with it was size and complexity.

https://github.com/RichiCoder1/opentelemetry-sdk-workers#otlphttp-protobuf-support

Shouldn't be too difficult to add to this lib since it's more compliant with the source libs.

@evanderkoogh
Copy link
Owner

Theoretically it should be possible to use the OpenTelemetry-JS protobuf exporter? I haven’t tried it myself, but it uses the same interface for that reason..

So if you could give that a try I would greatly appreciate it so we can document that better for when the next person comes along.

Also let us know if you run into any problems ofc and we’ll see what we can come up with 😄

@cdloh
Copy link
Author

cdloh commented Oct 13, 2023

I've had a go at this but turns out the OTEL exporter library uses XMLHttpRequest to send the data.

There is an MR to rework this but not clear how far off it is to landing open-telemetry/opentelemetry-js#3542

@evanderkoogh
Copy link
Owner

Then we might need to look at adapting @RichiCoder1's code and porting it over to here.. Would you be able/have time to have a look at what would need to be done for that @RichiCoder1?

@RichiCoder1
Copy link

RichiCoder1 commented Oct 15, 2023

I'm happy to advise, but I'm afraid I can't make any promises about contributing it myself.

There's a few complexities with pulling my code:

That said, there's no reason it couldn't/wouldn't work verbatim with this package, just would be pulling over a lot of junk. Might be easier to the OTEL JS PR merged, though one might have to ping the slack/WG to get it accelerated.

@evanderkoogh
Copy link
Owner

I know you were very strapped for time, but this is exactly what I was hoping for.. an idea of what would be involved so we can figure out how to best move forward.

Getting a PR merged sounds like it would be by far the best option. In the interim I guess we could fork, merge the PR ourselves and release that.. but that sounds a bit messy?

@evanderkoogh evanderkoogh added enhancement New feature or request external There is an external dependency or integration labels Oct 15, 2023
@cdloh
Copy link
Author

cdloh commented Nov 3, 2023

So I've managed to get a branch of this all working and there's a few things that will likely need fixing on this end.

At present when the exporter calls fetch it resolves to the proxy which then causes it to start a new span and export it again resulting in a never ending loop of spans that keep sending till the Worker exhausts it's request limits.

Not sure how I can unwrap it within the exporter or get the Proxy to detect that it's the exporter that's using fetch vs the worker.

@evanderkoogh
Copy link
Owner

That is awesome news 😄

the unwrapping happens with the unwrap method. Here is an example:

unwrap(fetch)(this.url, params)

does that help?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request external There is an external dependency or integration
Projects
None yet
Development

No branches or pull requests

3 participants