frigus02 / opentelemetry-application-insights

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

Temporality selector #74

Closed cijothomas closed 4 months ago

cijothomas commented 4 months ago

How does this crate select temporality? Application Insights works with delta temporality only, but that does not look like handled here.

Note: I work in Application Insights team and also part of opentelemetry-rust! Was curious on how folks are writing custom exporters for otel metrics!

frigus02 commented 4 months ago

Hi. To be honest, I'm not sure if anyone is using this crate to export metrics. I've been using this for traces only and added metrics because someone asked for it in #39.

You can customize the temporality selector using https://github.com/frigus02/opentelemetry-application-insights/blob/8fe79de9c695607ed22f1e5a25e2dbdbbc7b1b25/src/lib.rs#L766

But if Application Insights supports delta temporality only, that customization is probably a bad idea. How is temporality configured in other exporters? Is it only done in the exporter or is there another temporality config for the SDK somewhere?

cijothomas commented 4 months ago

But if Application Insights supports delta temporality only, that customization is probably a bad idea

Yes.! You can hardcode to emit delta temporality, and not allow any customization. It must be "delta" as defined in: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk_exporters/otlp.md#additional-configuration

Delta: Choose Delta aggregation temporality for Counter, Asynchronous Counter and Histogram instrument kinds, choose Cumulative aggregation for UpDownCounter and Asynchronous UpDownCounter instrument kinds

(note: even in delta,, UpDownCounters are still expected to be cumulative...something I have seen a lot of people make mistakes!)

How is temporality configured in other exporters? If you are referring to other exporters to application insights, then they hard code it, without allowing customization.

(I simply reported this issue as I noticed it! Its totally okay to do nothing, unless there are some real users affected!)

frigus02 commented 4 months ago

Thanks. I will try to fix that in the next days.

If you are referring to other exporters to application insights, then they hard code it, without allowing customization.

No, to other Rust-based exporters. I was wondering if the SDK gets the temporality selector from the exporter or if users need to configure it elsewhere, too. I think I was thinking of an older version, though. It's clearly defined by the exporter.

So if I understand correctly, I need a temporality selector like this: https://github.com/open-telemetry/opentelemetry-rust/blob/b98b5896d2f1b47a1fb496af09d12505d233205b/opentelemetry-otlp/src/metric.rs#L271-L288. Hm, what happened to Gauge?

Oh, and I also just realized I forgot to implement the https://docs.rs/opentelemetry_sdk/latest/opentelemetry_sdk/metrics/data/struct.ExponentialHistogram.html