open-telemetry / opentelemetry-cpp

The OpenTelemetry C++ Client
https://opentelemetry.io/
Apache License 2.0
880 stars 420 forks source link

[API] Remove Meters #2184

Closed marcalff closed 1 year ago

marcalff commented 1 year ago

Related to:

Context

Using opentelemetry-cpp 1.9.1 Using metrics, with asynchronous instruments. Using callbacks located in a shared library.

Use case:

Measurements are taken invoking the callback, and are exported.

Then:

From this point, no new measurements are called.

The OTLP HTTP exporter however, still exports data about the metrics no longer measured, as seen in the opentelemetry-collector logs in the endpoint.

In my understanding, this is due to the aggregations used:

Please provide:

so that exporters no longer report anything about defunct Meter and Metrics.

This is blocking: in practice, using Metrics with shared libraries is not possible currently with opentelemetry-cpp.

lalitb commented 1 year ago

Also related - https://github.com/open-telemetry/opentelemetry-cpp/discussions/2168. This is related to the aggregation, which probably doesn't get cleaned-up. @marcalff - Thanks for raising the issue. If you don't have plan to work on it, should I assign it to myself. I think I will have some bandwidth during next week to fix it.

marcalff commented 1 year ago

Thanks @lalitb .

I am working on Factory::Create() methods for metrics, to get me started on this part of the code base.

Assigning to you then. I can help with testing and review.

marcalff commented 1 year ago

Adding the blocking tag, as using opentelemetry-cpp for metrics is not possible without this fix, when instrumenting meters in shared libraries (that can disappear upon unload)

marcalff commented 1 year ago

After additional analysis, there is no real use case for removing instruments individually.

The entry point for an instrumented library is MeterProvider::GetMeter(), and an instrumented library will define its own meters.

As a result, removing a meter, which imply to remove all the associated instruments, at once, is sufficient.