open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.65k stars 765 forks source link

PeriodicExportingReader should have a separate observation and export interval #3668

Open jdmarshall opened 1 year ago

jdmarshall commented 1 year ago

NB: Before opening a feature request against this repo, consider whether the feature should/could be implemented in the other OpenTelemetry client libraries. If so, please open an issue on opentelemetry-specification first.

Is your feature request related to a problem? Please describe.

If you want a high sampling rate for Observables, you have to set a high export rate, which the otel-collector doesn't handle anywhere near as gracefully as statsd. It would be better for all involved if flushes were several times less frequent than observations. Setting a debugger on an ObservableGauge callback shows that the call interval is approximately equal to the exporting interval, and tracing the call graph shows this to be true.

Describe the solution you'd like

A second option to the Exporter that allows for an observation interval and timeout

Describe alternatives you've considered

In our preprod environment I had to reduce from 1 second to 10 seconds in order to avoid a CPU utilization that would have reached much higher on our production servers, which use a lot more cores per collector.

Right now we're using a compromise between visibility and system load by doing a few samples per minute, but that means that everything we've gained by more accurate histograms is now lost due to dramatically less accurate gauges.

Additional context

dyladan commented 1 year ago

This seems like a reasonable feature request but I think it might have to involve some spec changes. I think other languages are likely to run into similar issues. I am a bit surprised that the collector isn't handling this gracefully. Can you give me an idea of how many metric streams you're reporting to a single collector?

jdmarshall commented 1 year ago

That's the problem with coming from statsd. You can chew on data points like candy. In a candy factory.

At least 400 data points, with distinct labels per point (1-20, median probably around 4). Times 30 processes per machine, times a cluster, times blue/green deployment.

One of my surlier coworkers asked if "this is a toy?" I'm not... unsympathetic to that question. We are running a tiny fraction of what some other industries run. Wikipedia handles per day what we wouldn't be able to handle in a month.

statsd stays at 2-3% on a loaded machine in production. Our otel collector stays around 2% on an idle box, and before we made some changes could go up to 10-30% under load.

jdmarshall commented 1 year ago

clarification: not 10-20% of one core, 10-20% of all cores.

legendecas commented 1 year ago

Please correct me if I'm getting wrong. How does the observation useful if the observation is not exported? The intermediate metric points are going to be coalesced into the final exports by the aggregators.

Would you mind expanding on your use cases and the metric examples you are collecting with the observation pattern? Thank you!

jdmarshall commented 1 year ago

If you look at the data exported by OTLP (as opposed to prometheus), there are timestamps on the data. I don't see any reason why a single push could not include multiple time offsets for the same key.

danopia commented 1 year ago

Hi, I have also been looking for a decoupled observation and reporting period. In my case I have several meters that are somewhat expensive to gather and I wanted to observe them less often than others. (I addressed my usecase by having my observer callback just do nothing most of the time it's called.)

As noted above, OTLP appears to allow for submitting one list of datapoints with differing timestamps. I'm coming from Datadog, which has a similar submission API to OTLP exports. This API is effectively used in the Datadog ecosystem to report down to 1s granularity while sending HTTP-friendly batches only every 30s or so.

One possibility that I see for OP's usecase would be creating a BatchingPeriodicMetricReader which gathers more frequently and concatenates the datapoint lists for less frequent submission. I'm not sure what the performance impact would be, and the change would affect every metric coming from that SDK. :/

In general, I think OTLP would benefit from Opentelemetry leaning more towards a datapoint submission concept, instead of the scrape-export model from Prometheus. The opentelemetry-js metrics SDK doesn't seem to allow for alternative models like only sending metrics with new datapoints.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

jdmarshall commented 1 year ago

This is a terrible way to run a bug database.

github-actions[bot] commented 8 months ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

jdmarshall commented 8 months ago

Still waiting.

jdmarshall commented 8 months ago

@danopia

If I understand your issue, I have maybe the reverse problem of you, where my high res values are rather limited and the low res ones add up to expensive, so I use an interval to gather the high res values as a histogram and the rest are only gathered on user activity.

To an extent it still adds up to the same problem.