Provide hooks for vendor-specific customization

[trask]

Some things vendors would like to be able to provide without having to fork this repo:

• ✅Custom configuration story
• Custom logging story
• ✅Custom IdsGenerator
• ✅Custom Resource
• ✅Custom Propagator

[iNikem]

In addition:

• ✅Add new instrumentation
• ✅Override any existing instrumentation
• ✅Add attributes to existing instrumentation's spans

Edit: last two point from above are addressed by #1623

[devinsba]

I would probably say:

• remove/disable (if not covered by config) instrumentation
• customize/intercept attributes on spans

[trask]

Also:

• ✅Custom sampler - #387 (comment)

[RashmiRam]

I am looking for this. @iNikem Do you want me to create a separate issue for this?

In addition:

• Add new instrumentation
• Override any existing instrumentation
• Add attributes to existing instrumentation's spans

[iNikem]

@RashmiRam I am afraid I don’t understand your question

[RashmiRam]

Sorry If I am not being clear. I assumed that the above comment from you & question from me in Gitter https://gitter.im/open-telemetry/opentelemetry-java-instrumentation?at=5efefb93bb149531ede7d726 is similar. Is it not?

[iNikem]

Ah :) If your need is for vendor-specific auto-instrumentation, then indeed this issue seems to cover that. I was under impression that you wanted for an option for manual instrumentation to add attributes to spans created by auto-instrumentation. I think they are somewhat different things.

[RashmiRam]

Yes. You are right. I was specifically looking for an option for manual instrumentation to add attributes to spans created by auto-instrumentation. Can't it be achieved by vendor specific customization too?

[iNikem]

Don't know yet :)

[trask]

hey @RashmiRam, can you give an example that you are trying to solve, e.g. which instrumentation's spans, and what attributes you would like to add to those spans? i agree with @iNikem, i don't think we've thought about this use case yet, so would be great to have a concrete example to think about. thx!

[RashmiRam]

Say, I have a multi-tenant application where each tenant id will be part of the incoming web request url. It would be useful to add the tenant-id as a custom attributes to all the child spans created for the request so that it could be seen if a particular tenant behaves erratically for these type of requests.

Incoming Web request url /api/<tenant-id>/resources is coming to an app using jetty web server.
Server span for this request already contain these following attributes when using auto instrumentation

http.method: "POST"
http.status_code: 204
http.url: "http://127.0.0.1/api/1234/resources"
internal.span.format: "proto"
net.peer.ip: <ip>
net.peer.port: 48458
span.kind: "server"
span.origin.type: "org.eclipse.jetty.server.handler.StatisticsHandler"
status.code: 0

I wanted to add the following attribute(s) to the server span created by auto instrumentation

tenant_id: 1234
some-app-tag:value

This is just an example. But, providing hooks to add dynamic request based custom attributes to spans created using auto instrumentation will be immensely helpful to capture application/business logic information of the traces.

[RashmiRam]

@iNikem I am able to add custom attributes to a currentSpan() programmatically. I have a dropwizard app with few custom ContainerRequestFilter. I have added the following code changes and it seemed to add this custom attributes to jetty server span(as that was the current span while I accessed it in my ContainerRequestFilter).

Span currentSpan = OpenTelemetry.getTracerProvider().get("any-name-it-doesn't-matter").getCurrentSpan();
currentSpan.setAttribute("tenant_id", 1234);

Dependencies:

 compile group: 'io.opentelemetry', name: 'opentelemetry-api', version: '0.6.0'

Is this intended & recommended way of adding our own custom attributes?

[iNikem]

That is a great question. While setting attributes on span seems totally fine, I feel uneasy about your way of getting hold to Tracer instance. @trask can you comment on that?

[RashmiRam]

I am able to add spans to the current context as well.

  Span mySpan = null;
  Tracer tracer = OpenTelemetry.getTracerProvider().get("any-name-it-doesn't-matter");
  Span currentSpan = tracer.getCurrentSpan();
  try(Scope scope = tracer.withSpan(currentSpan)) {
    mySpan = tracer.spanBuilder("Creating my Span").setSpanKind(Span.Kind.INTERNAL).startSpan();    
    mySpan.setAttribute("my_custom_attribute", "This is so cool");
    <<my logic>>
  }
  finally {
    mySpan.end();
    mySpan.setStatus(Status.OK);
  }

[trask]

I feel uneasy about your way of getting hold to Tracer instance. @trask can you comment on that?

Oh yes, you don't need a Tracer to get the current span 👍

TracingContextUtils.getCurrentSpan()

If you are adding child spans you will need a Tracer, and in that case the name may be relevant since some exporters will add the tracer name to the child span.

[pavolloffay]

Also:

• capture any or all request/response headers to spans #1316
• an ability to capture request and response bodies, (disabled by default) #1317

Another SPIs

• customize ByteBuddy agent #1613. It can be used by vendor to tweak bytebuddy behavior e.g. include instrumentations that are do not follow Instrumenter/InstrumenterModule approach or exclude addition classes from instrumentation.
• run arbitrary code in agent's classloader. It can be used to register implementations to commonly used API used by custom instrumentations #1619

[iNikem]

@trask Can you explain what do you mean by "Custom logging story"?

[trask]

ya, replacing slf4j simple logger with logback (e.g. to support log file rollover)

[iNikem]

Why do we do anything with logging at all? I mean, the simplest option is for us to just depend on slf4j API and that's it. As any other library. And let user provide any implementation they want.

Or our machinations with classloaders makes this impossible?

[pavolloffay]

A slightly orthogonal request - as a vendor I would like to change logging level/verbosity of for example when muzzle recognizes API mismatch and disables instrumentation. I am not sure if that is possible, or not, but it could be better documented.

[trask]

Why do we do anything with logging at all? I mean, the simplest option is for us to just depend on slf4j API and that's it. As any other library. And let user provide any implementation they want.

Or our machinations with classloaders makes this impossible?

ya, we shade slf4j so it can live in the bootstrap class loader and not cause conflicts for user

A slightly orthogonal request - as a vendor I would like to change logging level/verbosity of for example when muzzle recognizes API mismatch and disables instrumentation. I am not sure if that is possible, or not, but it could be better documented.

@pavolloffay do u mean something like log at a higher level than DEBUG if all instrumentation for a particular library (e.g. both cassandra-3.0 and cassandra-4.0) are muzzled? I think that would be a good improvement directly in this repo

[pavolloffay]

@pavolloffay do u mean something like log at a higher level than DEBUG if all instrumentation for a particular library (e.g. both cassandra-3.0 and cassandra-4.0) are muzzled? I think that would be a good improvement directly in this repo

Yes, the idea is to easily recognize API mismatch. With the debug level the mismatch can go without noticing and causing part of the application not being traced or broken context.

[trask]

Yes, the idea is to easily recognize API mismatch. With the debug level the mismatch can go without noticing and causing part of the application not being traced or broken context.

this would be great to add in this repo, no need for vendor hook I think

[iNikem]

I propose to close this issue as Done. All known requirements are addressed, except for "Custom logging story". I don't think we have to provide any customisation hook for that. If vendor distribution wants to replace our Simple logging with something else, they can just filter out Simple logger classes during repackaging and provide their our slf4j implementation.

@trask @pavolloffay your opinions?

[pavolloffay]

Sounds good we can open a follow-up issue for well-defined use-cases.