What is the expectation of dependencies already manually instrumented?

[gabizou]

In a repository of "libraries" that has been instrumented for various reasons, does Orchestrion require commenting //orchestrion:ignore on the particular code block that provides such a dependency?

Case in point:

package main

import "github.com/mysuperawesome/library/deps"

func main() {

  //orchestrion:ignore
  db, err := deps.NewDB()
  defer db.Close()

  //...
  
}

Even though db is a database/sql instance, will Orchestrion abide by the ignore directive in this particular block?

[eliottness]

👋 Hey @gabizou

The answer to your question is: it depends on the library and how it's instrumented it.

For example:

• database/sql: For this one, [dd-trace-go] implemented the Driver interface and the instrumentation aspect only replaces calls to sql.Open with their equivalent from dd-trace-go. This means that doubly-instrumenting this is a noop since there is no more calls to sql.Open to latch on for orchestrion.
• go-chi/chi: This http router has a built-in middleware system so dd-trace-go and orchestrion use this. Since middlewares can be chained independently, doubly-instrumenting this would make 2 spans for each request. In this case adding a //orchestrion:ignore comment or removing the manual instrumentation code makes sense.
• net/http: We inject dd-trace-go code directly inside the net/http package so it is not possible to //orchestrion:ignore it. Doubly-instrumenting it will also create 2 spans for each request. The only solution is to remove the manual instrumentation.

We are actively working on better configuration to detect and act in case we found cases of "doubly-instrumentation". Could you describe in more details what you would expect from orchestrion in cases like this ?

[RomainMuller]

I guess this is also one case where having a report of what is being instrumented (and how) for a given app would be useful to gain quick insight into what happens where; as that'd then help determine how to change that configuration we're trying to design

[gabizou]

Could you describe in more details what you would expect from orchestrion in cases like this?

I believe @RomainMuller has a brilliant suggestion on having an outlined report of what is being instrumented (and how).

I'd expect orchestrion to detect if a dependency is already being potentially orchestrated by a dependency method like NewDB() or NewRouter. The trouble is from the library that provides our flavor of options for configuration is very flexible, so it's not always guaranteed to instrument the dependency, but it always has the configuration option available like so:

type RouterOption = func(next http.Handler) http.Handler
type Router interface {
	Handle(path string, handler http.Handler) Route
	ServeHTTP(w http.ResponseWriter, r *http.Request)
}
func NewRouter(options []RouterOption) Router {
	chiRouter := chi.NewRouter()
	for _, o := range options {
		chiRouter.Use(o)
	}
	return &router{
		Router: chiRouter,
	}
}

And separately, we've modularized our DataDog dependencies (makes updating our library modules a heck of a lot easier) so that we can easily include datadoghttp.TraceOption() http.RouterOption when building the app. I guess the difficulty is how to determine on orchestrion's side that that is a possibility (and honestly, I wouldn't expect it to).

Which lead me to the question of how would it best be considered that the application that is trying to adopt orchestrion, to declare a dependency should not be instrumented as it's already possibly instrumented based on runtime configurations.

[RomainMuller]

I'll note that as of v1.1.0 you're able to manually determine what integrations are going to be applied or not by controlling the contents of your orchestrion.tool.go file... That is kind of an "all-or-nothing" still (all net/http clients or none; not per individual client); but it gives you more control there.