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

[Metrics] ASync Instruments not fully implemented. #1387

Closed
marcalff opened this issue May 10, 2022 · 1 comment · Fixed by #1388
Closed

[Metrics] ASync Instruments not fully implemented. #1387

marcalff opened this issue May 10, 2022 · 1 comment · Fixed by #1388
Assignees
Labels
bug Something isn't working

Comments

@marcalff
Copy link
Member

Greetings,

I am a bit confused about the development status for async instruments in Metrics ...

There are no remaining issues left listed in the "Metrics SDK - Alpha release" milestone, great.

There are 5 unrelated open items in "Metrics SDK - Beta Release", and issue #1338 is about a more complex use case (observing N metrics at once), so this gives the impression that a basic Observable counter is already implemented.

If this is not supported yet, feel free to close this issue as a duplicate.
If supported, then it seems some bits are missing in the implementation:

Using #undef ENABLE_METRICS_PREVIEW

The method Meter::CreateLongObservableCounter() is available,
and builds a LongObservableCounter object, ok.

The constructors for:

  • LongObservableCounter
  • opentelemetry::metrics::ObservableCounter<long>
  • Asynchronous<long>
  • opentelemetry::metrics::AsynchronousInstrument

keep the instrument name + callback + description + unit,
but never build an instrument descriptor with it.

In the code path of Meter::CreateLongObservableCounter(),
the counter returned is not registered with any AsyncStorage,
and nothing keeps a reference to all the observable counters created,
so the ::Collect methods will never invoke callbacks on async instruments.

For synchronous instruments, methods like Meter::CreateLongCounter() build an InstrumentDescriptor object and call RegisterMetricStorage(), so I would expect some similar code, but found none.

Note that the class AsyncMetricStorage exists, but is never used in the production code (there is one use only, but from a unit test).

While testing, the callback function given to Meter::CreateLongObservableCounter() is never called,
and async instruments are not exported (tested with the Prometheus exporter)

What am I missing (or not understanding) ?

Regards.

@marcalff marcalff added the bug Something isn't working label May 10, 2022
@lalitb
Copy link
Member

lalitb commented May 10, 2022

Thanks, will look into this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants