frigus02 / opentelemetry-application-insights

OpenTelemetry exporter for Azure Application Insights
MIT License
22 stars 12 forks source link

gzip compression #46

Closed birkmose closed 2 years ago

birkmose commented 2 years ago

Hi!,

Thank you for a great crate! I really appreciate the effort you have put into this.

I have been testing this crate with a very high traffic system and I noticed that bandwidth seems to be a limiting factor. I noticed that you do not compress when uploading traces. The application insights endpoint should support compression as I understand it - for instance, see:

https://docs.microsoft.com/en-us/answers/questions/464764/application-insight-compression-in-rest-post.html

Any thoughts on supporting this?

frigus02 commented 2 years ago

Hi there. Thanks for reporting. Support for gzip sounds like a good idea. The link suggests gzip compression is even enabled by default in the .NET SDK. What's the Rust crate others are generally using for that?

frigus02 commented 2 years ago

This crate currently requires users to provide an HTTP client. In theory this client could do the gzip compression. I assume most users are going to use reqwest or surf. Sadly it seems neither of those support compression of request bodies. Reqwest only supports decompression of response bodies.

Apart from that, it's probably a good idea to do the compression in this crate anyway. Then we only have to test what Application Insights supports once. I think most crates depend on flate2 for that.

birkmose commented 2 years ago

Yes, when looking at the .NET SDK source it looks like they just create a gzip stream and push bytes through that - so I imagine this crate could do something similar to that - regardless of the HTTP client?

Reqwest supports gzip (for decompression - based on content-encoding). It appears they use the crate async-compression for that feature (which in turn uses the flate2 crate)

I noticed this issue tracing 2.5million events in a 10seconds stress test, and realized it was taking many minutes for the traces to get uploaded. I imagine something of the vicinity of a 10x reduction in bandwidth should not be out of reach, turning on the gzip compression. Would be a nice optimization!

frigus02 commented 2 years ago

Agree. I'm going to see if I can make this happen in the next days.

frigus02 commented 2 years ago

I noticed this issue tracing 2.5million events in a 10seconds stress test

A test like this sounds quite useful to have in this repo, too. Do you have this prepared in any way, that you'd be willing to contribute? 🙂

birkmose commented 2 years ago

The test I did has a lot of internal domain-specific dependencies unfortunatly, but I could make a synthetic one for sure - if that is useful? Happy to help out!

frigus02 commented 2 years ago

It would help in testing the impact of this feature, I think. Though to be honest, I'm not entirely sure what else I'd like to get out of a test like this, yet. If it's not too much work, I think it could be nice to have. But don't worry about it otherwise.

birkmose commented 2 years ago

I can create a stress test example in the example folder, perhaps with an env variable to control how many spans you want to generate. How does that sound?

frigus02 commented 2 years ago

Yes, that's exactly what I had in mind. 👍

frigus02 commented 2 years ago

I released version 0.20.0.

Thank you for the suggestion and for your help testing it, @birkmose.