opentelemetry-ruby icon indicating copy to clipboard operation
opentelemetry-ruby copied to clipboard

Support no code change auto instrumentation initialize of OTel SDK

Open NathanielRN opened this issue 2 years ago • 2 comments

Description

In my experience with OTel Python and JS, “auto instrumentation” refers to a feature that allows users to get traces without having to write 1 line of code and can just use environment variables instead. This is also the definition in the specification.

In OTel Python there is an executable we provide that wraps your regular Python program. The executable initializes OTel and then uses the regular Python executable to run the argument provided as a parameter.

In OTel JS, you can just include an initialization script by configuring the NODE_OPTIONS environment variable and OTel is initialized before your application starts.

In Java they have the Java Agent for auto instrumentation which uses the instrumentation API to alter the byte code that is loaded in a JVM to give it OTel tracing.

In Go I know they were trying something similar to Python but it hasn’t been ironed out yet.

A “monkey patching script” sounds like it might be possible in ruby too, but just not an option yet 🙂 The goal is to give users no code to maintain at all, and still give them OTel tracing with all the features they’d hope for.

All this should be considering in light that Auto Instrumentation is still pre-1.0 so there is no canonical way to implement it, although there are great precedents 🙂

I also think we should update the docs to remove references to "auto instrumentation" which do not match this definition:

  • https://github.com/open-telemetry/opentelemetry-ruby/blame/cf01d9a9927e33e308fcf63665763089166a402e/website_docs/manual_instrumentation.md#L6
  • https://github.com/open-telemetry/opentelemetry-ruby/blame/cf01d9a9927e33e308fcf63665763089166a402e/website_docs/manual_instrumentation.md#L27

and in several other places:

image

For example, we can probably say "'OpenTelemetry Ruby Instrumentation packages' in this specific case:

Auto-instrumentation is the easiest way to get started with instrumenting your code, but in order to get the most insight into your system, you should add manual instrumentation where appropriate.

NathanielRN avatar Jan 18 '22 21:01 NathanielRN

tl;dr Appreciate the time and effort you put into this issue @NathanielRN, but i disagree.

IMO what we offer is absolutely "automatic instrumentation", and I have no real interest in jumping through linguistic hoops, potentially reducing end-users adoption, just to avoid usage of the term.

Before I go further, what you're broadly looking for in ruby can probably be achieved with a "railtie" which is a ruby on rails specific initializer you can include, but is not generic to all ruby web frameworks (See, https://api.rubyonrails.org/classes/Rails/Railtie.html, and a previous discussion of it here https://github.com/open-telemetry/opentelemetry-ruby/issues/1017). I think a PR that adds this would be welcome (although the specifics of how it can be included, what should be on by default, and so on, need to be thought about very carefully), but not having it should not preclude opentelemetry-ruby from using the term "auto-instrumentation".

That being said, i'll try to be very specific with my disagreement. This is the part of the specification you're referencing above IIUC:

Automatic Instrumentation

Refers to telemetry collection methods that do not require the end-user to write or access application code to use the OpenTelemetry APIs. Methods vary by programming language, and examples include bytecode injection or monkey patching.

Put frankly I think you're misinterpreting what the specification is asking for. My understanding is that this is meant to describe a user not having to instrument, say, the start and finish for each span in each controller action, or wrapping each database query, or view rendering, or so on. The rote work of "manual instrumentation" so to speak. This isn't meant to describe not adding a configuration file once to denote what gems/portions of your application you want to monkeypatch, which is what our auto-instrumentation approach calls for at the moment.

Here's another way to think about your interpretation of the specification. If it's the case the "auto-instrumentation" is meant to refer to "Turn on a bunch of monkeypatching or alter byte code based on 1 env var" Then, well, why isn't any Environment Variable for this behavior actually defined in the specification? And additionally, why isn't an environment variable for an "instrumentation package's options" defined in the specification? Wouldn't both those things be common sense things to expect in a specification if we were to accept your definition of what "auto instrumentation" means? The reason, to me at least, is because you're mis-interpreting the specification and your specific narrow definition of "auto instrumentation" is incorrect.

And finally let me say this. I think the goal you state is well meaning but ultimately an anti-pattern.

The goal is to give users no code to maintain at all, and still give them OTel tracing with all the features they’d hope for.

"No code to maintain at all" and "all the feature they'd hope for" seem incongruous to me. The fact of the matter is that different users will want different things from their tracing instrumentation, and trying to abstract that fact away with "a single env var" is just simply sacrificing one group of users for another. One obvious example is to look at AWS's own preferred instrumentation patterns for say, SQS. Is there a "right way" to propagate trace context across SQS? Should SQS be represented via parent/child relationship, or via span "links", or not connected at all? The answer is that different users will want this represented differently (discussed in an issue here https://github.com/open-telemetry/opentelemetry-ruby/discussions/1057). Or, a broader example. Does a user want all incoming http requests to all endpoints to be traced? Again, the answer will vary depending on the end-user, the specifics of the application being instrumented, and so on. The approach we have now in ruby for automatic instrumentation (a configuration file where you supply a code block that can specify all the options you want along with the instrumentations you want) allows a user to address those questions and details, and still get all the time-saving OOTB benefits of auto-instrumenting their applications. The approach you're proposing, OTOH, it's currently not possible to address any of those questions without a user having to go and add the very configuration that you're looking to exclude from the definition of "auto instrumentation" ( unless your otel-sdk has created it's own non-spec environment variable format like Python has done with things like OTEL_PYTHON_EXCLUDED_URLS https://github.com/open-telemetry/opentelemetry-python-contrib/pull/790 :) ).

I'm open to other opinions here, but that's my two cents at present. At the very least, if it's agreed that we're significantly off-spec here, then i'd like to see the specification language clarified/updated to reflect that. I think a more productive use of time here would be to formalize all these non-spec Env vars and features like OTEL_PYTHON_EXCLUDED_URLS and others (ruby has quite a few too!) into sdk language-agnostic specification.

ericmustin avatar Jan 18 '22 23:01 ericmustin

Note that it is entirely possible to create a wrapper gem for OpenTelemetry Ruby that automatically installs instrumentation and configures exporters, etc. This can then be added as a dependency in an application's Gemfile with no code changes required, which is on par with the wrapper script approach, for example in terms of its intrusiveness. In fact, we have done this at Shopify and I believe GitHub has something similar.

We have previously discussed extracting common elements of Shopify's and GitHub's internal wrapper gems to provide a more consumable Open Source wrapper gem, and we should definitely do this.

IMO the need for these wrapper gems does not imply that we don't provide auto-instrumentation. The mechanisms exist in this project, but at present the policies are specific to individual organizations consuming the project.

fbogsany avatar Jan 19 '22 16:01 fbogsany

👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot.

github-actions[bot] avatar Apr 01 '24 01:04 github-actions[bot]