Specify the treatment of duplicate instruments and observations - #2270

Does this mean that registering multiple callbacks in a single async instrument is officially supported? This is related to issue #2249 - it is currently unclear whether this behavior is allowed; and the Java SDK rejects duplicate callbacks and only calls the first one.

That was my intention, yes. There's a historical perspective on this: OpenCensus permitted duplicate instrument registration and it was part of the early thinking for synchronous instruments. The only reason duplicate-identical instruments ever seemed contentious to me is due to asynchronous instruments. The question is whether users are harmed or helped by allowing this, and I didn't have an answer to this question.

I believe #2249 is suggesting the answer ought to favor allowing duplicate registration and multiple callbacks. I have come to adopt this position as well, but it is entirely driven by thinking this is best for users, i.e., that it creates less confusion. This is independent of the question of whether and how an SDK is required to treat duplicate observations. That is, even without duplicate callbacks the SDK should be doing something to prevent duplicate observations from being double-counted.

I believe #2249 is suggesting the answer ought to favor allowing duplicate registration and multiple callbacks. I have come to adopt this position as well, but it is entirely driven by thinking this is best for users, i.e., that it creates less confusion.

Yes, I can agree with that -- having recently implemented a bridge from another metrics API to OTel, I can confirm that not being able to register multiple callbacks just forces the instrumentation author to manage that themselves. And that can get unwieldy and hacky.

Is the "official" way of registering multiple callbacks going to be multiple registration of asynchronous instruments? If multiple callbacks are going to be supported and the user wants them, shouldn't the user be allowed to simply to add more than one callback when creating an asynchronous instrument?

@ocelotl I agree this would be awkward. I think it is not meant to be the official way, but is just how multiple registrations should be handled as a specific case. See #2281, specifically "The API MUST provide a way to associate instruments with callback functions responsible for reporting the associated".

under (belonging to?) the same Meter? See #2276

Is this compatible with the following phrasing describing meters?

It is unspecified whether or under which conditions the same or different Meter instances are returned from this functions.

If its undefined whether or not a meter provider must return the same or a separate instance of a meter for the same name, then an implementation might end up with two different meters for the same name. This would allow instruments to have the same name, but different type and unit under meters which have the same name.

We could resolve this by adding language that clarified that the identity of the meter is tied to the meter name, similar to the new phrasing here "Instruments are associated with the Meter during creation and are
identified by instrument name".

What does this mean?

the identity of the meter is tied to the meter name

If 2 or more different meters may have the same name then how can the meter name be used to identify a meter?

If 2 or more different meters may have the same name then how can the meter name be used to identify a meter?

I should be able to obtain a meter by the same name multiple times. And from the meter (or meters - it shouldn't matter whether the same exact instance is returned) I should be able to obtain the same instrument multiple times.

Using an example, the following should work without any errors, and with all calls to add(..) recording against the same metric stream:

meterProvider.get("my-meter").counterBuilder("my-counter").build().add(10);
meterProvider.get("my-meter").counterBuilder("my-counter").build().add(10);

Meter myMeter =  meterProvider.get("my-meter");
myMeter.counterBuilder("my-counter").build().add(10);
myMeter.counterBuilder("my-counter").build().add(10);

This is in the other file.

You'll need to update the ToC to make the check pass (make markdown-toc).

Perhaps "MUST NOT"?

See #2317, major changes.

I've changed this detail in #2317. I think it's fine to be strict about description, as long as we're tolerating duplicate identities.

I am wondering if instrument registration is considered part of the initialization. 🤔
I am worried that this might be a bit unwieldy to implement with the requirement not to throw after initialization in this spec.

The API or SDK may fail fast and cause the application to fail on initialization, e.g. because of a bad user config or environment, but MUST NOT cause the application to fail later at run time, e.g. due to dynamic config settings received from the Collector.

Instrument registration is not part of initialization because libraries can get loaded after the initialization is done, and the library being loaded could register more instruments.

Following the discussion in #2226, I've updated this part of the proposal in a new PR please see #2317.

This recommendation would be better declared as a requirement. There does not seem to be a situation where there is a valid reason to ignore this recommendation and doing so would present compatibility issues. If one OTel implementation allows this interference a user will not be able to implement an application in that language the a same as another because they cannot use the same instruments.

Fixed in #2317

Make the normative statement be a positive one about OTel implementation behavior, not user expectations.

Also, is there a valid reason to not use the last value observed? Should this be a requirement and not a recommendation? Not using the last value observed would lead to inconsistent behavior across OTel implementations. Meaning having this as a requirement would make sense.

On this note please see #2318. I removed this paragraph in a future iteration, simply stating at this point in the document that the behavior is unspecified. I view this being unspecified is a problem, but I don't want to fix it here.

... the implementation MUST return a registration error...

Is the use of "return an error" language-specific? I know that there are languages whose error handling consists of returning errors, but others raise exceptions.

Is the intention of this part of the specification to indicate something must me returned or that the error can be handled in a way that better suits every implementation?

See #2317. There's still an error, it's just a question of where it gets surfaced.

What is a non-valid instrument?

Removed in #2317

What is a registration error?

Eliminated in #2317 😀

Fixed in #2317

"the implementation MUST return a valid instrument" would this be a breaking change given the API spec is already stable?

Take this to #2317
I believe we can change a condition formerly specified as error into correct behavior. To me, the error state was effectively unspecified.