Feedback on Instrumenter API

[iNikem]

I would like to start a discussion and collect feedback and ideas for Instrumenter API improvements before declaring it stable.

So far I have some specific feedback.

• Current Instrumenter API is very request-response heavy and this does not suite all cases. E.g. all database calls usually do not handle response in any way and we don't even capture it. Having to always use Void as response type seems artificial.
• We have to overwrite a lot of methods with empty bodies. E.g. almost all concrete subclasses of HttpCommonAttributesExtractor have around 5 empty methods. Seems untidy.
• It is cumbersome to implement custom inter-process context propagation. E.g. all cases for AWS specific frameworks.

[anuraaga]

Instrumenter API's main goals I think are

1. Be easy for common cases of instrumentation

2. Enforce compliance with semantic conventions

3. Allow consist configuration

I think only 1) and 2) are in scope for this issue

In particular they seem to be about 2)

Do the DB semantic conventions not capture information about the response that they should? DB is very much request-response heavy in nature so intuitively it does seem off that we always ignore the response.

Also do the HTTP common conventions just have too much? If many methods of the common conventions can't be implemented, it seems the conventions may not reflect reality. We should propose removing conventions - alternatively perhaps we can limit instrumentation API to only "required" attributes but I guess those aren't really well defined in practice. I think instrumentation API should target "normal" attributes - required as currently specd is probably too small, but full spec might be too big. But then we have nothing to target.

[anuraaga]

Also cloud SDKs are too broad to worry about IMO - AWS SDK propagation in particular is AWS-specific. With the instrumenter API allowing custom propagation I think it's doing it's job. The goal isn't to shoehorn every single possible use case into the API as long as there is an escape hatch back to OTel API for big gaps it should be ok.

[iNikem]

I don't see anything in DB semantic conventions that would require handling a response.

[iNikem]

open-telemetry/opentelemetry-specification#2028 :)

[anuraaga]

Yeah we don't see anything in conventions yet but that doesn't mean there shouldn't be any ;) One example is the number of rows affected by a query, this has been something I add myself to spans in the past and is pretty general I guess. Just saying that there is conceptually a response to DB queries too - perhaps it's not conceivable to spec it well enough though in which case we could force some semantic extractors like DB to have no response handling methods. Not sure it's better

[iNikem]

One more piece of feedback: it is unfortunate that subclassing of Instrumenters is practically impossible. I think we pass around too much Instrumenter<Request, Response> which is quite mouthful. I think it would be more convenient to operate with something like MongoInstrumenter.

[mateuszrzeszutek]

On a somewhat related note: I wish Java had type aliases; extending Instrumenter just to assign it a bit better name is just painful.