Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elasticsearch instrumentation #2550

Closed
5 of 7 tasks
AlexanderWert opened this issue Mar 28, 2022 · 4 comments
Closed
5 of 7 tasks

Elasticsearch instrumentation #2550

AlexanderWert opened this issue Mar 28, 2022 · 4 comments
Assignees
Labels
Milestone

Comments

@AlexanderWert
Copy link
Member

AlexanderWert commented Mar 28, 2022

This issue groups all the tasks that are required to allow proper usage of APM agent within Elasticsearch itself.

Relates to

@AlexanderWert AlexanderWert added the size:large Large (L) tasks label Mar 28, 2022
@AlexanderWert AlexanderWert added this to the 8.3 milestone Mar 28, 2022
@pugnascotia
Copy link

So far, the only change I've made to the agent is as follows:

diff --git a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java
index c61257d21..4bcf2ee57 100644
--- a/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java
+++ b/apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/logging/LoggerFactory.java
@@ -18,6 +18,9 @@
  */
 package co.elastic.apm.agent.sdk.logging;
 
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+
 public class LoggerFactory {
 
     private static volatile ILoggerFactory iLoggerFactory;
@@ -32,11 +35,16 @@ public class LoggerFactory {
      * @param name The name of the logger.
      * @return logger
      */
-    public static Logger getLogger(String name) {
+    public static Logger getLogger(final String name) {
         if (iLoggerFactory == null) {
             return NoopLogger.INSTANCE;
         }
-        return iLoggerFactory.getLogger(name);
+        return AccessController.doPrivileged(new PrivilegedAction<Logger> () {
+            @Override
+            public Logger run() {
+                return iLoggerFactory.getLogger(name);
+            }
+        });
     }
 
     /**

This may not ultimately be what we want to do, but I got me further.

The problem with java.lang.Throwable not being found was (I believe) due to the ES code lacking the accessSystemModules permission.

CC @ChrisHegarty

@felixbarny
Copy link
Member

I've noticed that the apm-agent module doesn't have all the sources of the agent, which makes debugging a challenge.

May not be entirely in scope of this issue but having the ability to pull in all sources to that the agent can be debugged within a project that it's integrated in seems like an important capability. That's not only in the context of Elasticsearch but arguably specifically important there.

@pugnascotia
Copy link

I tried running a 3 node cluster in docker-compose, and noticed that the service map doesn't break down the cluster, which I think would be useful if one was in a degraded state. I set a different service_node_name on each, which is reflected in the "JVMs" tab.

@SylvainJuge SylvainJuge moved this from Planned to In Progress in APM-Agents (OLD) Mar 31, 2022
@SylvainJuge
Copy link
Member

Regarding the spans that are created as transactions: all the parent-child relationships are created through the context propagator, as a consequence even the spans that are on the same node are considered to be remote.

We should add a distinction between:

  • local parent span, stored as SpanContext or Context, probably within ES ThreadContext as a transient object.
  • remote parent span, which are created from the context propagator.

Given the parent/child relationship for inherited ThreadContext isn't really straightforward, it would be more efficient to discuss this with someone familiar with the ES codebase.

Also, with OTel (and also our internal agent API), the "currently active span" is the one that is picked as parent when another child span is created. Ideally, in the Tracer implementation, onTraceStarted would be able to retrieve the parent span from TraceContext and use it. Because the ThreadContext inherited fields are just plain strings, we have to find a way to store and inherit a reference (or alternatively an ID if it's not directly possible) in order to preserve the link to the parent span in place of using a remote parent.

@AlexanderWert AlexanderWert removed this from the 8.3 milestone May 30, 2022
@AlexanderWert AlexanderWert modified the milestones: 8.4, 8.5 Jul 25, 2022
APM-Agents (OLD) automation moved this from In Progress to Done Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

No branches or pull requests

4 participants