add metric annotation instrumentation

[Duncan-tree-zhou]

refer to #7030
add metric annotation instrumentation

[steverao]

There are some CI failures, firstly you can solve them by referring to https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/contributing/running-tests.md#troubleshooting-ci-test-failures

[Duncan-tree-zhou]

Hi @steverao, I found that there are lots of fails "to connect to localhost/127.0.0.1:4318". So I want to rerun the task, but I can't see the rerun button. Is it disabled for contributors?

[steverao]

Is it disabled for contributors?

Yes, you can fix the problems firstly and push relevant commits. It will trigger to rerun the CI tasks.

[Duncan-tree-zhou]

I mean I guest the failure is caused by the CI run time environment crush..... if it's able to rerun a task, It might save some time....

[Duncan-tree-zhou]

I turn it ready for reviewing, and get some questions for discuss:

1. the module naming of the @Counted and @Timed instrumentation.
   • I avoid using the opentelemetry-instrumentation-annotations prefix because it's already been used for @Withspan. I am not sure if using opentelemetry-instrumentation-annotations-coutned would cause some problem somewhere.
   • I have no idea if there is a better module naming rule for metrics annotations instrumentations.
2. I am not sure if we should put the instance of meter, LongCounter and DoubleHistogram in another module where can be loaded without javaagent?
3. I am doubted about allowing the return value to be an attributes because most of time the return value is unpredictable. would it be possible to design some static api to allow user to post the attributes to the metrics via threadlocal?

[amexboy]

Ping,

I'm really looking forward to this. Are these on track to be available soon?

[fmhwong]

Line 32 and 33 are the same

[Duncan-tree-zhou]

Sorry for late reply, I went to a family travel last week and come back just now. I think it's testing the key overlay case, so change it to:

  @Counted("example.with.static.attributes.count")
  @StaticAttribute(name = "key1", value = "value1")
  @StaticAttribute(name = "key2", value = "value3")
  @StaticAttribute(name = "key2", value = "value2")

and for TimedExample, change it to:

  @Timed(name = "example.with.static.attributes.duration")
  @StaticAttribute(name = "key1", value = "value1")
  @StaticAttribute(name = "key2", value = "value3")
  @StaticAttribute(name = "key2", value = "value2")
  public void exampleWithStaticAttributes() {}

[fmhwong]

LGTM for the renaming of value to name

[brunobat]

Sorry for continuing of this.
I still have doubts about these async interfaces and how generic their names are.
This line is a perfect example of the problem. The interface is AsyncOperationCallback but we end up calling the attribute handler because it's more convenient but too abstract to actually mean anything in the code. What is it handling? business logic, telemetry?

If we cannot use the name of the interface as a name of an instance in the code, something is wrong.

On this comment #11354 (comment),
@Duncan-tree-zhou suggested AsyncEndHandler or AsyncCallback#onEnd and we ended up with AsyncOperationCallback#onEnd. In my mind I always ask: AsyncOperationCallback why? To where?

We also have AsyncOperationEndStrategy.java, AsyncOperationEndSupport

If we look at the package name:
io.opentelemetry.instrumentation.api.annotation.support.async.AsyncOperationCallback, it already includes async.
Probably we need some brainstorm around these names and what they actually do. Imagine some newbie looking at code using this... At first glance it doesn't seem related with telemetry.

What if we use one of:

• InstrumentationCallback
• TelemetryCallback
• TelemetryObserver

I think InstrumentationCallback better aligns with the current OTel practice and also answer: why we do the callback and to where.

[Duncan-tree-zhou]

Thank you @brunobat for raising it again, I get your point. I think both InstrumentationCallback and TelemetryCallback work for me in the code context, it's clear for new comer to understand the usage of the interface. What do you think @laurit and @trask ?

[hannahchan]

I'm really looking forward for this to be merged. However I've noticed that code.namespace and code.function have been deprecated in the semantic conventions and has been replaced by code.function.name in the which is the fully qualified name. Should this be updated in this PR as well?