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

Registering multiple asynchronous instruments #3549

Closed
fabianishere opened this issue Aug 25, 2021 · 7 comments
Closed

Registering multiple asynchronous instruments #3549

fabianishere opened this issue Aug 25, 2021 · 7 comments
Labels
Feature Request Suggest an idea for this project metrics SDK

Comments

@fabianishere
Copy link

Describe the bug
I have been experimenting with the latest OpenTelemetry Metrics release (version 1.5.0-alpha), but it has occurred to me that registering multiple asynchronous instruments with the same name will result in only one of the callbacks being invoked during a collection cycle.

I want to implement a pattern like this:

public class Host(public val uid: UUID, meter: Meter, ....) {
    init {
        meter.gaugeBuilder("cpu.usage")
            .setDescription("The amount of CPU resources used by the host")
            .setUnit("MHz")
            .buildWithCallback { result ->
                val cpuUsage = ...
                result.observe(cpuUsage, Attributes.of(ResourceAttributes.HOST_ID, uid.toString()))
            }
    }
}

Steps to reproduce

  1. Create multiple asynchronous instruments with the same name
  2. Observe that only one callback is invoked during the collection cycle

What did you expect to see?
Every asynchronous instrument with the same should have its callback invoked every collection cycle.

What did you see instead?
Only one of the asynchronous instrument callbacks is invoked every collection cycle.

What version and what artifacts are you using?
This is the Gradle version catalog I use for the OpenTelemetry artifacts:

[versions]
opentelemetry-main = "1.5.0"
opentelemetry-metrics = "1.5.0-alpha"
opentelemetry-semconv = "1.5.0-alpha"

[libraries]
opentelemetry-api-main = { module = "io.opentelemetry:opentelemetry-api", version.ref = "opentelemetry-main" }
opentelemetry-sdk-main = { module = "io.opentelemetry:opentelemetry-sdk", version.ref = "opentelemetry-main" }
opentelemetry-api-metrics = { module = "io.opentelemetry:opentelemetry-api-metrics", version.ref = "opentelemetry-metrics" }
opentelemetry-sdk-metrics = { module = "io.opentelemetry:opentelemetry-sdk-metrics", version.ref = "opentelemetry-metrics" }
opentelemetry-semconv = { module = "io.opentelemetry:opentelemetry-semconv", version.ref = "opentelemetry-semconv" }

Environment
Compiler: Oracle JDK 16.0.2
OS: macOS Big Sur

@fabianishere fabianishere added the Bug Something isn't working label Aug 25, 2021
@jkwatson
Copy link
Contributor

Hi @fabianishere. This is an interesting use-case. I assume you don't know all the host UUIDs ahead of time, and they will be generated/discovered at runtime?

Another way to implement this would be to save a collection of UUIDs that a single instrument/callback will use make the recordings.

You are definitely correct that you only get one callback per Instrument, and you can't register the same instrument repeatedly.

@fabianishere
Copy link
Author

fabianishere commented Aug 26, 2021

Correct, the hosts are generated at runtime.

I think the way to implement this use-case with the current API would be to have a central component above the Host class to register the asychronous instrument and have it collect the metrics from each of the know hosts.

My only reservation with this approach is that now the instruments for the same entity (the host) are spread across multiple classes: synchronous instruments in Host and asynchronous instruments in the other class.

@jkwatson
Copy link
Contributor

yep. sounds like you'll have to manage some coordination there. Due to the nature of "observable" instruments, I think it is very unlikely that this behavior will change, at least not anytime soon. @jsuereth any thoughts here?

@jsuereth
Copy link
Contributor

This is a known limitation right now in the API. I've been holding off on fixing this prior to aggregator refactor. I'm happy to prioritize it next, before finishing up exemplar or multi-export.

As jkwatson@ mentions it's a limitation in the spec. If you want multiple async instruments, use different meters and then ignore their names later (as a workaround).

I'd love to lift this limitation at some point, but for now we can't tell if re-registering the same instrument is a user error or on purpose.

@fabianishere
Copy link
Author

@jsuereth Thanks for the clarification! Should I keep the issue open to document the limitation for now?

@jsuereth
Copy link
Contributor

Sorry for delay, yes keep this issue open. We just had a specification discussion around this, and there's a bit to figure out here.

@jack-berg
Copy link
Member

Solved in #4143.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project metrics SDK
Projects
None yet
Development

No branches or pull requests

4 participants