Streamline API between exception capture and tracing

[roncohen]

Today, exception/message capture methods are exposed on Client while the tracing APIs are available on elasticapm. We should look into streamlining this.

[basepi]

From Slack, for context (courtesy of @beniwohli)

Why doesn't capture_span require a Client object?

the transaction is stored in a thread local (or contextvar), and it does have a reference to the client

but to be perfectly honest, capture_span not needing a client instance turned out to make things like the open-tracing bridge and asyncio support quite tedious. Most other python APM agents need an explicit client/tracer/... instance, which makes a lot of the asyncio pain go away

[basepi]

It seems to me that since capture_span actually does require a Client, but we do magic to find it, making all of our API require explicit Client objects would be a good way to standardize. But I ended up exploring both routes.

---

Solution 1: elasticapm.get_client() (Everything requires Client object)

I've seen some users using elasticapm.contrib.django.client.get_client, even though it's not in our public API. Would it make sense to create a public API endpoint get_client which would either retrieve the thread local/contextvar Client object, or create a new Client object for the user if one doesn't already exist?

There are pitfalls. If you look at the issue I mentioned above you'll see that re-using the Client object sometimes runs into problems, specifically in forking models. We could potentially overcome these with arguments:

def get_client(force_new=False, settings=None, **kwargs):

This also breaks down when you start using capture_span as a decorator (which, I'd argue, is probably one of the primary ways in which it is used). If capture_span is suddenly a member function of the Client object, we'll have users calling elasticapm.get_client in the global context of their modules, so that they have a Client object from which they can retrieve the decorator. This won't work. (The way we get around import-time vs execution-time in the current decorator is by only triggering the context manager behavior of the class at call time, at which point the current execution context can be evaluated)

I'm not convinced that losing the ease-of-use of this decorator is worth it in order to standardize the API.

---

Solution 2: Nothing requires a Client object

Alternatively, we could make all of our Client members (such as capture_message) into top-level members of our API. These new API functions would either retrieve an existing client, or create a new one. (We could also allow a user to pass in a Client object if they really wanted to)

This would allow us to retain the easy decorator behavior of capture_span. However, it would not give us the asyncio wins that @beniwohli mentioned above. It also adds magic, as opposed to removing it, which is always a compromise.

---

Summary

When I began writing this, I was favoring Solution 1. But the capture_span decorator usage feels like a deal-breaker, and I'm not sure how to get around it. Solution 2 seems like a win from a user-experience standpoint, but there may be dragons lurking in automatic Client creation that I'm missing since I'm still new to the codebase.

Thoughts?

[axw]

@beniwohli

the transaction is stored in a thread local (or contextvar), and it does have a reference to the client

but to be perfectly honest, capture_span not needing a client instance turned out to make things like the open-tracing bridge and asyncio support quite tedious. Most other python APM agents need an explicit client/tracer/... instance, which makes a lot of the asyncio pain go away

Can you expand on this? Don't you still need to fetch a client from contextvars, and maintain the span stack in contextvars? Maybe you could point at the bits code that were made more tedious/difficult.