Allow supplying a callback for adding new attributes when spans start

[zionsofer]

Adding attributes to spans is a crucial part of OpenTelemtry to allow extra information passing.
The instrumentation libraries create their spans in a very specific part of the flow, "wrapping" existing functionality to allow span creation correctly.

Sometimes, as part of our recurring flows in the system (for example, server http requests), we want to add extra attributes to the existing spans, in a single place for all relevant flows (for example, adding information about the user that made the request as attributes).
Unfortunately, that span only exists and created in a specific point in time, and any attempt to add any attribute to it before that, as expected, results in nothing.

My proposition is to be able to configure the tracer with some kind of callback that will be applied each time a span is created to allow extra configuration, for example settings additional attributes, without having to explicitly do it in code which is supposed to be agnostic to opentelemetry instrumentation and does not rely on it running.

For example, assume I have a python service based on flask.
This service uses instrumentation:

otlp_exporter = OTLPSpanExporter()
span_processor = BatchSpanProcessor(otlp_exporter)
trace.set_tracer_provider(
    TracerProvider(
        active_span_processor=span_processor,
        id_generator=AwsXRayIdGenerator(),
    )
)

app = Flask(__name__)
FlaskInstrumentor().instrument_app(app)

@app.route("/ping")
def ping():
    span = trace.get_current_span()
    span.set_attribute("name", "flask_otel")
    return {"message": "ok"}

In this case, the setting of the attribute would work because this is inside the route itself, which means it's being done inside the created span of OpenTelemetry.
But, let's assume I have a flask middleware which adds information for all requests:

@app.before_request
def add_otel_attribute():
    span = trace.get_current_span()
    span.set_attribute("name", "flask_otel")

So, this would be done once for all endpoints instead of setting it at each route.
This, unfortunately, won't work, because at this point the span created by OpenTelemetry does not exist yet, and thus will be an invalid (noop) span.

If we had, however, some way to supply a callback to add attributes to each span creation, we would not rely on when and how the spans are created, and won't need to "manoeuvre" our way to find when we can add our attributes.
This callback can return an object (key-value store) of attributes, just like we would have added manually.

Something like this:

def add_otel_flask_attributes():
    return {"name": "flask_otel"}

otlp_exporter = OTLPSpanExporter()
span_processor = BatchSpanProcessor(otlp_exporter)
trace.set_tracer_provider(
    TracerProvider(
        active_span_processor=span_processor,
        id_generator=AwsXRayIdGenerator()
    )
)

app = Flask(__name__)
FlaskInstrumentor(span_attr_callback=add_otel_flask_attributes).instrument_app(app)

Then, this callback will be applied inside the span creation to add attributes the moment the span is created.

[carlosalberto]

We used to have this in some OpenTracing instrumentation, btw. Do you need to add the attributes to all Spans or only a few, depending on the specific instrumented library? Your case seems to be the former, in which case you could use a SpanProcessor:

class MySpanProcessor implements SpanProcessor {
  public onStart(Context parentContext, ReadWriteSpan span) {
    span.setAttribute("foo", "bar");
    // etc
  }
}

Either way, OTel needs to provide some out-of-the-box functionality to satisfy this :)

[jkwatson]

Either way, OTel needs to provide some out-of-the-box functionality to satisfy this :)

I disagree. The SpanProcessor is perfectly suited for this use-case. I don't think we should introduce more APIs where we have one that works today.

[carlosalberto]

I meant providing (in contrib) a SpanProcessor that does this ;)

[jkwatson]

I meant providing (in contrib) a SpanProcessor that does this ;)

I don't think that needs to be something in the spec, though...any language could provide this as a contrib extension as-needed.

[zionsofer]

@carlosalberto Right, it does seem from my description that I want the former. It's my bad, I was actually talking about the latter - for specific instrumentation libraries only do we want to add them, because most of the times the data we want to insert concerns specific library/implementation information (such as request information, which only exists when we're running an http server that is instrumented). I updated my description to match the needs.

If there's any way to do it out of the box in the current spec, then please do tell and I'll do it, I just haven't found anything in the spec or implementation about it

[jkwatson]

This sounds like something that would be provided by the specific instrumentation library, not by otel APIs directly. Is that correct?

[zionsofer]

Perhaps you're right, but in my mind I do feel this should be supported at the OTEL API,, as this support should be available for all libraries as it's not only relevant for any specific library.

If you feel this FR should be opened in anywhere else I'd be happy to, I just didn't see any single place to open this other than here.

[anuraaga]

Java instrumentation provides this now as AttributesExtractor which can be provided by a user.

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/instrumenter/AttributesExtractor.java

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/grpc-1.6/library/src/main/java/io/opentelemetry/instrumentation/grpc/v1_6/GrpcTracingBuilder.java#L37

It is true that this is not a concern of either the OpenTelemetry API or SDK, which are low level data collection libraries. However we can add an "Instrumentation API" as a third concept in the spec and this functionality definitely needs to be provided by it.

[zionsofer]

@anuraaga Agreed, indeed it doesn't really fit in either current specs, so a third spec might be the right choice.

[srikanthccv]

@zionsofer Did you take a look at the python instrumentatons? We support the request/response hooks to enable users to set custom attributes / or any kind of operation on span. May be all the instrumentation not have the support yet but we plan to. Here is how you do it Django web framework for example https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/instrumentation/opentelemetry-instrumentation-django#request-and-response-hooks.

[zionsofer]

@lonewolf3739 I see, that sounds like exactly what I'm looking for, but it doesn't seem to exist in other instrumentation libraries (not even Flask).
What I'm suggesting is maybe "enforcing" all libraries to support it by a so-called Instrumentation API like @anuraaga suggested.

[srikanthccv]

There is an ongoing effort to add request/response hooks for all the instrumentations. Here is the PR for Flask framework open-telemetry/opentelemetry-python-contrib#416.

I think this doesn't have to be part of spec and no enforcing. I would want each language SIG to have ability to add their own features which fits with the ecosystem and flexibility to not implement something which doesn't.

[zionsofer]

I understand, and if you feel that we do we can close this FR, but IMO this is a feature that is not specific to any language or library, but crosses all these concerns and is relevant for anything.
Taking the "each language and library will implement it" approach will just make all these inconsistencies between them, and will create a lot of feature requests from people that want to add this functionality in many different places.

[cijothomas]

similar feature provided by some instrumentations in .NET https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Instrumentation.AspNetCore#enrich

[carlosalberto]

I think this doesn't have to be part of spec and no enforcing. I would want each language SIG to have ability to add their own features which fits with the ecosystem and flexibility to not implement something which doesn't.

Probably we could have a document specifying a set of guidelines or recommendations for instrumentation, which then include mention of such hooks.

[srikanthccv]

I think this doesn't have to be part of spec and no enforcing. I would want each language SIG to have ability to add their own features which fits with the ecosystem and flexibility to not implement something which doesn't.

Probably we could have a document specifying a set of guidelines or recommendations for instrumentation, which then include mention of such hooks.

This echoes my thoughts. We have a set of guidelines in python contrib repo we recommend people to follow when they add a new instrumentation https://github.com/open-telemetry/opentelemetry-python-contrib/blob/main/CONTRIBUTING.md#guideline-for-instrumentations. I believe there is already a group entirely focusing on instrumentation efforts, Probably this can be part of that design document. This may eventually end up as concept in spec but enforcing and spec'ing it now seems premature.