The `FailSafeDeclaredMethodsCompiler` does not check the visibility of method return types

co.elastic.apm.agent.bci.bytebuddy.FailSafeDeclaredMethodsCompiler#compile(net.bytebuddy.description.type.TypeDefinition, net.bytebuddy.description.type.TypeDescription)

@Override
    public MethodGraph.Linked compile(TypeDefinition typeDefinition, TypeDescription viewPoint) {
        LinkedHashMap<MethodDescription.SignatureToken, MethodGraph.Node> nodes = new LinkedHashMap<MethodDescription.SignatureToken, MethodGraph.Node>();
        for (MethodDescription methodDescription : typeDefinition.getDeclaredMethods()
            .filter(
                // ignores all methods which refer to unknown types
                failSafe(
                    isVirtual()
                        .and(not(isBridge()))
                        .and(isVisibleTo(viewPoint))
                        .and(hasParameters(whereNone(hasType(not(isVisibleTo(viewPoint))))))
                )
            )
        ) {
            nodes.put(methodDescription.asSignatureToken(), new MethodGraph.Node.Simple(methodDescription));
        }
        return new MethodGraph.Linked.Delegation(new MethodGraph.Simple(nodes), MethodGraph.Empty.INSTANCE, Collections.<TypeDescription, MethodGraph>emptyMap());
    }

There is no check for the visibility of the return type of methods within this method, such as .and(returns(isVisibleTo(viewPoint))), which leads to failures in methodDescription.asSignatureToken() when handling methods with return types that are not visible.

This issue commonly occurs when other javaagents inject methods into classes, and the return type is not visible to elastic-agent.

Should the Compiler safely skip these methods rather than causing a compile failure? I'm not sure if this is intentional by elastic or an oversight. Does anyone have insight into this? Thanks!

Do you have a specific issue that causes failure with the agent? We don't support multiple agents running in the same JVM, but hst we do try to play nicely with other agents where possible so if there is a specific combo that is stopping and we can easily adjust, we'll try

I integrated another javaagent, which inserts methods into classes. The return type of these methods is defined by this javaagent and injected into the bootstrap-classloader. This type can be loaded at runtime but is invisible to Elastic (i.e., loadClass works, but getResource cannot retrieve the resource).

Eventually, an exception is thrown during the processing by Elastic's FailsafeCompiler, causing the entire class to fail to be processed by Elastic. The exception is caught in net.bytebuddy.agent.builder.AgentBuilder.Listener#onError.

My thought is that Elastic shouldn't cause the entire class hook to fail just because it found one unresolvable method.Elastic should have the ability to safely skip these methods.

In principle I agree. In practice we won't fix this any time soon, it's too much of an edge case to prioritize