Instrumentation API

[anuraaga]

Proposal for an instrumentation API to assist in providing high quality, consistent instrumentation.

[anuraaga]

Related issues

@zionsofer open-telemetry/opentelemetry-specification#1724 - wanting callbacks for adding attributes to instrumentation

@jmacd open-telemetry/opentelemetry-specification#464 - dedicated instrument for timing. Possibly more appropriate for an instrumentation API having the concept of operations than the core API itself

@jkwatson open-telemetry/opentelemetry-specification#1747 - span modeling. I didn't include the shouldStart method we currently have in the Java prototype because span modeling is still not well defined. But it should be possible to capture such modeling in the instrumentation API without having to expose the confusion to instrumentation authors, I think

[carlosalberto]

There's a related group that may be responsible/help with this. @tedsuo ping

[mateuszrzeszutek]

Typo:

Suggested change

	An `InstrumentaterBuilder` stitches together `Instrumenter` and `Extractor`s, as well as consistently
	An `InstrumenterBuilder` stitches together `Instrumenter` and `Extractor`s, as well as consistently

[mateuszrzeszutek]

I wonder if we should pass any caught error to the listener too:

Suggested change

	- `void end(Context context, Attributes responseAttributes)`: Called at the end of a request
	- `void end(Context context, Attributes responseAttributes, Throwable error)`: Called at the end of a request

For example, if you'd like to implement tracing (or something similar to it) in a RequestListener you'd need that error to call span.recordException(). Everything a REQUEST or RESPONSE contains is already translated to Attributes, the error is the only thing missing.

[mateuszrzeszutek]

Should it be "response"?

Suggested change

	- `EndTimeExtractor`: Maps the request object to an end time. Rarely used but sometimes necessary.
	- `EndTimeExtractor`: Maps the response object to an end time. Rarely used but sometimes necessary.

Or maybe request and response, now that I think of it.

[iNikem]

These are good guidelines. It would be very nice to have POC implemented in some other language as well, but do we have any other SIG with enough capacity to do this?

[ahayworth]

I've been considering this OTEP for a few days, and I don't think I would recommend this in its current form. I left a few comments in-line on specifics, but I think my feedback falls along the following lines:

1. This OTEP still requires users to be familiar with the semantic conventions in order to implement the necessary extractors. They'll still have to read them, and then implement an extractor class to map them onto their domain objects. I don't think this accomplishes any reduction in cognitive load.

2. We wish to relieve library authors of the need to understand the OpenTelemetry API, but I don't think we've accomplished that here. For example, we still require library authors to keep track of span context between the start and end methods - and do not provide a way for common pitfalls to be easily avoided. I fear that this could actually lead to subtly buggy instrumentation, because we are only abstracting away one small portion of context management. Good instrumentation will still need to be familiar with the OpenTelemetry API (and tracing in general) in order to avoid these pitfalls.

3. Instead of learning the OpenTelemetry API, the library authors must now learn a new "instrumentation" API that has some significant overlap (start/end methods, for example). Then, they must implement the "instrumentation" API, and to me it seems that they will end up writing more code to comply with the "instrumentation" API than if they had just used the OpenTelemetry API directly.

4. This feels like a particularly java-centric way to do things (which makes sense, given its origins). It does not feel like something necessarily easy to implement or idiomatic in other languages.

5. It feels heavily request/response oriented, and requires wrapper objects for instrumentation that might only generate something like "internal" spans. I fear we may end up requiring library authors to implement wrappers to comply with the API that end up decreasing efficiency.

6. There is a significant amount of overhead involved in creating objects and dispatching methods in some languages. By attempting to make things easier by complying with (another) API, we introduce memory allocation overhead and CPU overhead in method dispatch. This is not universally free, or better.

---

All of that aside: I do believe there is much we can do to make things easier for instrumentation library authors. I wanted to point out a few alternative ideas that we have begun using in the ruby sdk, to great effect:

opentelemetry-instrumentation-base

This is our take on "how to make this easy for library authors". It does a few useful things:

• For SDK authors, it provides an "instrumentation registry" that knows about any auto-instrumentation also using opentelemetry-instrumentation-base. SDKs can use that registry to activate or install instrumentation as it needs, and it allows SDKs to provide an easy configuration point for end-users. For our end-users, that looks something like:

OpenTelemetry::SDK.configure do |c|
  c.use 'OpenTelemetry::Instrumentation::SomeInstrumentedLibrary', { options: "if you have them" }
end

• For auto-instrumentation authors, it provides a number of useful hooks ("how can I make sure any pre-requisites are available" or "how can I easily accept options from my end-users"). The README section for auto-instrumentation authors shows how it can be used by library authors. Additionally, the instrumentation registry provides either no-op API tracers or real SDK tracers (once configured) to the library author at runtime.

Tracer.in_span

We provide a convenience method in our SDK to create (and activate) a new span around a specific block of code - the documentation is here, but usage is something like:

tracer.in_span("span name", span_kind: :internal, attributes: {}) do |span|
  # ... your instrumented code goes here
  # maybe also you want to add attributes
  span.add_attribute("foo", "bar")
end

We catch and record exceptions during that block, and also manage the context for you completely - so for the common case of "I just want to make a span appear right here", it works really well. Our Net::HTTP instrumentation is really helpful here, as it shows how we leverage this helper to easily do work before and after the block of code being instrumented - including context propagation and setting post-request span attributes: https://github.com/open-telemetry/opentelemetry-ruby/blob/main/instrumentation/net_http/lib/opentelemetry/instrumentation/net/http/patches/instrumentation.rb

This in_span method also ends up being really handy for end-users as well - it's very ruby-like.

OpenTelemetry::Common::HTTP::ClientContext

A lot of things we instrument end up being thin wrappers around HTTP clients. Rather than re-invent the wheel each time, we provide a really useful helper in opentelemetry-common, which is available for instrumentation authors: OpenTelemetry::Common::HTTP::ClientContext

It's intended to be used whenever we realize that an instrumentation library could simply be augmenting an HTTP span that will be generated further down the call stack. For example:

• Our koala instrumentation adds a few koala-specific semantic attributes for the imminently-outgoing HTTP request.
• Any auto-instrumented HTTP library, such as our Net::HTTP instrumentation, merge those into the span attributes for the outgoing request.

This is one way that we reduce burden on our instrumentation library authors for the rather common case of "this library is really just an HTTP request maker" - instrumentation is reduced to the bare minimum to register itself with opentelemetry-instrumentation-base and to decorate the soon-to-be-created HTTP request span with additional attributes that are helpful. Plus, it makes traces just a bit less messy in our view.

---

For what it's worth, I am very happy that we are talking about how to make this easier on instrumentation library authors. I think there are some really good improvements that can be made, and I like the way we're doing things over in opentelemetry-ruby quite a bit. This OTEP seems like it really makes things simpler in opentelemetry-java-instrumentation, and that's excellent. I'm not involved with that part of the project, so my concerns about "how does this make it easier" do not invalidate the great value that it seems to be providing there - but rather, it just means "I'm not sure how we could apply this proposal elsewhere."

Nor does it mean that the examples of how things are done in opentelemetry-ruby should be taken as the right way to do things, but rather I wanted to show that there are significantly different ways we are addressing them in other languages.

My hope is that we can learn from both approaches to make recommendations that are applicable across all SDKs without requiring non-idiomatic approaches in those other SDKs.

[ahayworth]

Suggested change

	`AttribuesExtractor` allows populating attributes from a library's request and response domain
	`AttributesExtractor` allows populating attributes from a library's request and response domain

[ahayworth]

This allows all HTTP attributes to be filled without specific knowledge of the semantic conventions themselves.

I may have misunderstood something here.... however, based on my reading of this OTEP, I don't understand how this Extractor pattern results in less work re: semantic conventions for library authors? How can I build an Extractor class that map[s] from a request or response object into attributes without knowing the actual semantic conventions involved?

Is there perhaps a concise snippet of code you could share that would illustrate how this might be used by a third-party library to instrument their library for OpenTelemetry? I found a Spring integration PR that uses this framework in the opentelemetry-java-instrumentation repo, but that particular example doesn't really clear up much for me.

[ahayworth]

This seems as though it would be difficult to things that do not have a clear request/response cycle. If, for instance, one were building instrumentation for something like ActionView in Rails and wished to create a span when a template were rendered into the HTML response, we'd need to then construct some kind of artificial request abstraction to pass to this method.

[anuraaga]

The instrumentation API is meant to make standard telemetry instrumentation easier, I could try to write that but a bit unsure the best way to phrase it. Internal implementation details like a view may be best with the OpenTelemetry API especially if it doesn't semantic conventions - the main reason to use a mapping of request to attributes is to enforce the conventions. Internal instrumentation also doesn't need to worry about propagation, nesting, etc that this API tries to tackle

[ahayworth]

I mentioned that specific example because I've been working on optional auto-instrumentation for the opentelemetry-ruby project that instruments parts of Rails like that automatically, and I think other frameworks might fall into that category too. I agree that end-users need not use this Instrumentation API, but people like me (writing auto-instrumentation for others) would. That's the kind of thing I had in mind, if that helps clarify why I mentioned it?

[ahayworth]

If we are to make instrumentation easier for library authors, then we should consider if there is anything else we could to to reduce manual context management. For example, is there a way to build this Instrumentation API such that "dangling spans" (those that the library author forgot to end) are automatically cleaned up?

[anuraaga]

If context could be handled by magic without a javaagent then life would be great :) No ideas on how to improve the general pattern of context management, though at least having 1 method that returns a Context and another that is documented to accept it is still a win over OpenTelemetry API I think - I've seen code that creates a context (possibly not parenting correctly) to create a span, then passes in an arbitrary Context.current() when ending instead of the instrumentation context.

For dangling spans, I think this is quite language specific. In Java, we can implement a "GC hook" as a span processor to catch spans that get GC'd without end being called. Main reason for not having implemented this yet is not knowing what to do with those spans as it probably needs some specification (Zipkin exports the span context (not the span, it's already collected) with with flush=true attribute).

[ahayworth]

If context could be handled by magic without a javaagent then life would be great :)

True. 😄

No ideas on how to improve the general pattern of context management, though at least having 1 method that returns a Context and another that is documented to accept it is still a win over OpenTelemetry API I think

I wonder if this is an opportunity to improve the next iteration of the Context / Span APIs themselves?

For dangling spans, I think this is quite language specific.

This is probably true, and I think we can probably just ignore my comments in this section because of that. Although, maybe mentioning here that languages implementing the Instrumentation API "SHOULD provide an automatic cleanup mechanism for authors", but the value of that may be debatable still.

[ahayworth]

Suggested change

	interface methods is provided. This is to ensure it is a conscience decision by the instrumentation library
	interface methods is provided. This is to ensure it is a conscious decision by the instrumentation library

[ahayworth]

Required by whom? The instrumentation library author or the SDK?

[anuraaga]

The Instrumentation API, at least in this case by using abstract methods in Java. Indeed this may not translate to other languages but even without the strict requirement, it could be more discoverable and ensure filling in attributes compared to setAttribute I think

[ahayworth]

Sorry - let me rephrase. Are we saying here that we expect instrumentation authors to implement all of the abstract methods, even if they don't use them? Or that we expect the people who actually build the instrumentation API itself to implement no-op implementations of all attributes?

I do like the focus on explicitness, in general, but whether or not I personally like this part of the idea depends on who we expect to be implementing these methods, and when. 😄

[ahayworth]

I respectfully disagree. 😄

As a career Rubyist, this doesn't feel like something I could easily or idiomatically adapt to in the Ruby world. That is not to say it is a good proposal or bad proposal, but rather to say that this does feel rather Java-specific to me, at least.

[anuraaga]

Thanks for the feedback, all great stuff. I think one aspect that you may not have considered is the third goal - "Provide a consistent user experience for users of instrumentation". So some parts may require more code and feel like it's somewhat harder to write the instrumentation, but the key point is that the desire is that it's easier to write good instrumentation. This is why an attempt is made to push the attributes directly onto the author, and the knobs such as setting custom extractors, listeners, is spec'd out as standard to ensure all instrumentation support them. It takes some more code to support these features, though I don't think a lot thanks to the abstractions

The intent of the code is definitely to illustrate the concepts and maybe actually writing out method signatures was too much. I think the key features may be summarized as

• Contracts that push, if not force, instrumentation authors to consider all the OTel semantic conventions when writing instrumentation. To ensure high conformance with OTel spec and data fidelity. In Java, this is an abstract class but I definitely expect this to be drastically different in other languages.

• A well defined set of knobs that all instrumentation allows configuring. Attribute extractors based on request/response objects, request listeners, and the others I mentioned here. The implementation doesn't have to be a builder like here but having the knobs standardized seems to fall in OTel's goals of making sure all instrumentation supports a common set of functionality. I linked to a few threads asking for these features, and have even more in internal discussions and some other conversations with users. These are a big point that is currently missing from the OTel API I think.

I think these still could be cross-language concepts?

Is there perhaps a concise snippet of code you could share that would illustrate how this might be used by a third-party library to instrument their library for OpenTelemetry?

Not sure why GitHub isn't giving me a comment box on this one so answering here. What illustrates it most for me is some third party instrumentation I've seen. For example, couchbase instrumentation has their own attributes based on opentracing and then just maps span.setattribute

https://github.com/couchbase/couchbase-jvm-clients/blob/master/tracing-opentelemetry/src/main/java/com/couchbase/client/tracing/opentelemetry/OpenTelemetryRequestSpan.java#L59

This means OTel conventions are likely not being handled well. If using the instrumentation API where the couchbase request would have to be mapped into the extractors, it would be obvious that the OTel conventions are being handled. It would be harder for them to write the instrumentation, but we want to make sure it's high quality.

[anuraaga]

The instrumentation API is meant to make standard telemetry instrumentation easier, I could try to write that but a bit unsure the best way to phrase it. Internal implementation details like a view may be best with the OpenTelemetry API especially if it doesn't semantic conventions - the main reason to use a mapping of request to attributes is to enforce the conventions. Internal instrumentation also doesn't need to worry about propagation, nesting, etc that this API tries to tackle

[anuraaga]

If context could be handled by magic without a javaagent then life would be great :) No ideas on how to improve the general pattern of context management, though at least having 1 method that returns a Context and another that is documented to accept it is still a win over OpenTelemetry API I think - I've seen code that creates a context (possibly not parenting correctly) to create a span, then passes in an arbitrary Context.current() when ending instead of the instrumentation context.

For dangling spans, I think this is quite language specific. In Java, we can implement a "GC hook" as a span processor to catch spans that get GC'd without end being called. Main reason for not having implemented this yet is not knowing what to do with those spans as it probably needs some specification (Zipkin exports the span context (not the span, it's already collected) with with flush=true attribute).

[anuraaga]

The Instrumentation API, at least in this case by using abstract methods in Java. Indeed this may not translate to other languages but even without the strict requirement, it could be more discoverable and ensure filling in attributes compared to setAttribute I think

[iNikem]

Suggested change

	`AttribuetsExtractor` allows populating attributes from a library's request and response domain
	`AttributesExtractor` allows populating attributes from a library's request and response domain

[ahayworth]

I think one aspect that you may not have considered is the third goal - "Provide a consistent user experience for users of instrumentation".

All of the goals in this are really good, but this one is in tension with the others. That dynamic is definitely what we need to figure out: how to balance quality experience for the users against the ease of implementation for third-party library authors. The potential downsides are pretty large, too - if we make it too difficult for third-party library authors, then they will likely not do it (foisting the work directly back onto the OpenTelemetry community to maintain the instrumentation, rather than spreading it around).

The intent of the code is definitely to illustrate the concepts and maybe actually writing out method signatures was too much.

That's a reasonable point, and I think maybe I agree too. Part of what makes this difficult for me to support is the how specific it already is in implementation details. For example, I might be able to build something that fulfills the spirit of what we're trying to do here in ruby, but do it in a way that is idiomatic and comfortable for language authors - but not if I had to implement the specific contracts and patterns that this lays out.

Contracts that push, if not force, instrumentation authors to consider all the OTel semantic conventions when writing instrumentation. To ensure high conformance with OTel spec and data fidelity.

We should probably be careful with this? Attributes - and as far as I know, even using the ones defined in the Semantic Conventions - are optional. The specification even says "In OpenTelemetry spans can be created freely and it’s up to the implementor to annotate them with attributes specific to the represented operation."

So while we want people to use those attributes, we should be careful not to require them from folks, because we haven't really agreed that they are required otherwise. Not to mention, the semantic conventions are still "Experimental" at this point, anyways.

I think we'd probably do better at making it easier to use the right semantic conventions, rather than forcing people to use them?

[lzchen]

Has there been any discussion on how to handle instrumenting more than once to avoid duplicate telemetry? In Python we simply utilize a property on the base class that all instrumentations share. Probably would be something cool to have in the API.

[anuraaga]

@ahayworth Thanks - going to rewrite much of this to remove code and focus more on the concepts and use cases in a bit.

@lzchen We have open-telemetry/opentelemetry-specification#1747 - indeed, such a concern can ideally be handled by this instrumentation API / infrastructure rather than each library instrumentation having to do it.

[jmacd]

focus more on the concepts and use cases in a bit.

@anuraaga I'm starting to understand the concepts and use-cases in this OTEP.

The biggest area for research and discovery that I see here is the point that we have two correlated problems: (1) the need for an idiomatic API for creating complex instrumentation which is sensitive to cost, (2) the need for a way to configure which instrumentation takes effect at runtime through a declarative configuration.

I have a feeling the APIs that result from this work could end up being language-specific, but that there are general concepts to be found. What if we focus on the second part first-- configuration-- under the assumption that every language SIG will find its own idiomatic API?

[carlosalberto]

I have a feeling the APIs that result from this work could end up being language-specific, but that there are general concepts to be found. What if we focus on the second part first-- configuration-- under the assumption that every language SIG will find its own idiomatic API?

+1 to this - even if the first part could easily be done for all languages, the second one (configuration) seems to be more important right now.

[srikanthccv]

I have a feeling the APIs that result from this work could end up being language-specific, but that there are general concepts to be found. What if we focus on the second part first-- configuration-- under the assumption that every language SIG will find its own idiomatic API?

+1 to this - even if the first part could easily be done for all languages, the second one (configuration) seems to be more important right now.

I second this. There is a varying difference between each SIG in terms of config part. For example ability to ignore tracing certain routes (/health) in web framework, request & response hooks to customise more attributes, and many other. The use cases are mostly same across languages and can be better uniformly defined to make it lot easier for instrumentation users and potentially the library authors.

[anuraaga]

Thanks - I agree that configuration for the end user and developer experience for instrumentation authors are probably separate topics, and the former would have more shared concepts across languages or a set of shared knobs.

Developer experience for instrumentation is also important, but perhaps communicating data that is shared among concepts (HTTP route used both as attribute and span name) is not quite as important as having a good test suite per semantic convention namespace, which could potentially even be implemented out of process.

[tedsuo]

@anuraaga we are marking OTEPs which are no longer being updated as stale. We will close this PR in one week. If you wish to resurrect this PR, that is fine.