Allow adding transaction context fields via Public API

[mnylen]

Describe the solution you'd like I would like for the Public API to support setting transaction context fields such as 'context.request.url.pathname' and 'context.user.user-agent' for transactions created via the Public API. Currently the Public API does not expose a way to set this information. This would be useful when integrating frameworks outside of the supported list (I'm currently in progress of trying to integrate Clojure's ring)

For me, the best solution would be to have a Transaction#addContext method in similar fashion as we have Transaction#addTag

Transaction tx = ElasticApm.startTransaction();
tx.addContext("request.url.pathname", "/hello");
tx.addContext("user.user-agent", "some-client");

Another option would be to add separate setters for each context field described here: https://www.elastic.co/guide/en/apm/server/current/exported-fields-apm.html#_context_fields but I feel having a single addContext would be the easiest to maintain over time.

Describe alternatives you've considered I'm currently falling back to using Transaction#addTag to add this information to transactions. This works, but Kibana shows the information as a just flat list of key value pairs under "Tags". The Normally, context.request.* fields would be shown under "Request" tab with somewhat nicer presentation.

[felixbarny]

Hi and thanks for your issue. Would it be an option for you to contribute an agent plugin where you can use the internal API?

[mnylen]

I have no experience with JVM instrumentation, but as ring handlers are typically just clojure functions that operate on untyped request maps and are often composed together to form an "app", I feel like instrumenting that would be quite painful as there's not really one super class to target.

It might be possible if it targeted specific ring adapter (like jetty), but making a generic solution would be preferable.

I feel like having a way to build these kind of quick integrations for other JVM languages/frameworks that don't necessarily bend well for instrumentation would be a good idea. I've been able to go pretty far with the Public API and it seems like support for adding context is the only thing missing.

One alternative for me would be to create a completely separate library for clojure/ring integration that dropped the Java Agent completely and communicated with the APM server directly. But I would hate to do that, as the Java Agent has some useful instrumentations on JDBC and Apache HttpClient (both used widely in clojure codebases) that would need to be rebuilt in a new library then.

[felixbarny]

I feel like having a way to build these kind of quick integrations for other JVM languages/frameworks that don't necessarily bend well for instrumentation would be a good idea.

Thanks for your input. That seems like a very valid use case.

It might be possible if it targeted specific ring adapter (like jetty), but making a generic solution would be preferable.

Is there something like a ring interface? Then we could target the instrumentation to all implementations of that.

I also very much like your suggestion to add a addContext method. Having actual interfaces for all the context objects seems like a lot of maintenance and potential for breaking changes.

Some thoughts on the API design:

Adding the possibility to set String context values for Transactionss won't be enough. We would at least also need these methods for Spans and Transactions:

• addContext(String, String)
• addContext(String, boolean)
• addContext(String, Number)

Another question is what happens when providing an invalid context key. I wouldn't want to setting arbitrary key/value paris on context. Rather, the keys should be mapped to the corresponding object by the agent. For example, setting tx.addContext("request.url.pathname", "/hello"); should be translated to transactionImple.getContext().getRequest().getUrl().withPathname("/hello");. A related problem is when a valid key is added with an invalid type, like tx.addContext("request.url.pathname", false);

Throwing an exception would obviously not be a good idea. Options are:

• log a warning,
• return false, indicating that adding the context property was not successful
• A similar design to OpenTracing tags, providing constants for specific tags. I'll go into more detail in the next secion

Context constants Instead of having a signature like addContext(String, String), the signature would be addContext(StringContext, String). We would provide a Context class which contains constants for all valid context properties. That makes it impossible to set a context key which is invalid.

[mnylen]

Yes, you're right - the same would need to be supported for spans and for at least those data types.

Restricting the use to predefined constants - how about using enum for this?

Regarding there being a single Ring interface - unfortunately no. Ring just defines a generic abstraction of how HTTP requests and responses should be presented with Clojure data structures and how you can compose normal Clojure functions to alter the request and ultimately produce the response.

In Clojure code what you have is usually something like (a very simplistic example):

(require '[ring.adapter.jetty :refer [run-jetty]])

(defn handler [request]
  (if (= "/greet" (:uri request))
    {:status 200 :body "Hello, stranger!"}
    {:status 404 :body "Not found"}))

(run-jetty handler {:port 8080})

Nothing here is something you can target for instrumentation - there's no classes generated usually - it's just interpreted at runtime. You can't even add annotations to Clojure functions like you can with Java methods, as that would require generating a class. Mind, it's possible to generate a class and add annotations, but then you need to add some extra code on the application code.

One other way that this could work as a APM plugin would be to provide both plugin & library. The APM would replace a class provided by the library at runtime, that the application code would invoke to wrap their handler function with APM transaction. Like:

(defn handler [request]
  ...)

(def apmified-handler
  (ApmRingPlugin/wrapInTransaction handler))

(run-jetty apmified-handler {:port 8080})

In this case the ApmRingPlugin would be a Java class overridden by the plugin with a single static method wrapInTransaction that takes the handler function as a parameter, and returns a new handler function that starts the APM transaction and sets the context details. Like:

public class ApmRingPlugin {
  public static void wrapInTransaction(IFn handler) {
    return new AFn {
      @Override
      public Object invoke(Object request) {
        Transaction tx = createTransactionWithInternalAPI();
         // copy request data to tx
         Object response = handler.invoke(request);
         // copy response data to tx
         return response;
      }
    };
  }
}

But this seems like awfully complicated way of doing this, compared to doing it with just a simple clojure library that does the same wrapping, but in Clojure code using the public api.

[felixbarny]

This project might be interesting to you: https://github.com/Yleisradio/clojure-elastic-apm

[mnylen]

@felixbarny Hehe, I made that. It's why I requested this functionality - so I could have better support in that library. :)

[felixbarny]

Ahh, nice job and sorry for not noticing 😅

I have not forgotten about this issue btw. But another aspect of this is that it's likely that "context" will be deprecated in future versions of the Intake API. As of 7.0, there won't be a context in ES documents in order to to align with ECS. But maybe it's still fine that there is a concept of "context" in the public api.

[felixbarny]

Another challenge is how to make sure that all required fields are set. For example, only setting context.request.method without setting context.request.url.* will lead to the APM server dropping the event. APM Server errors, in turn, lead to the agent throttling requests to the server.

[jackshirazi]

Does OpenTelemetry Baggage cover the requirements of this feature request?