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

Instrumentation for redisson 3.17.2 #6096

Merged
merged 4 commits into from
May 26, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ muzzle {
dependencies {
library("org.redisson:redisson:3.0.0")

// TODO (trask) split out instrumentation into two modules and support 3.17.2+
latestDepTestLibrary("org.redisson:redisson:3.17.1")
implementation(project(":instrumentation:redisson:redisson-common:javaagent"))

compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")

testImplementation(project(":instrumentation:redisson:redisson-common:testing"))

latestDepTestLibrary("org.redisson:redisson:3.17.1")
}

tasks.test {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson;
package io.opentelemetry.javaagent.instrumentation.redisson.v3_0;

import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.named;
Expand All @@ -12,6 +12,7 @@

import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.redisson.CompletableFutureWrapper;
import java.util.concurrent.CompletableFuture;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,20 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson;
package io.opentelemetry.javaagent.instrumentation.redisson.v3_0;

import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
import static io.opentelemetry.javaagent.instrumentation.redisson.RedissonSingletons.instrumenter;
import static io.opentelemetry.javaagent.instrumentation.redisson.v3_0.RedissonSingletons.instrumenter;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.named;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.redisson.EndOperationListener;
import io.opentelemetry.javaagent.instrumentation.redisson.PromiseWrapper;
import io.opentelemetry.javaagent.instrumentation.redisson.RedissonRequest;
import java.net.InetSocketAddress;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
Expand Down Expand Up @@ -57,7 +60,7 @@ public static void onEnter(
context = instrumenter().start(parentContext, request);
scope = context.makeCurrent();

promise.setEndOperationListener(new EndOperationListener<>(context, request));
promise.setEndOperationListener(new EndOperationListener<>(instrumenter(), context, request));
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson;
package io.opentelemetry.javaagent.instrumentation.redisson.v3_0;

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static java.util.Arrays.asList;

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import java.util.List;
import net.bytebuddy.matcher.ElementMatcher;

@AutoService(InstrumentationModule.class)
public class RedissonInstrumentationModule extends InstrumentationModule {
Expand All @@ -19,6 +21,11 @@ public RedissonInstrumentationModule() {
super("redisson", "redisson-3.0");
}

@Override
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassesNamed("org.redisson.misc.RPromise");
}

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(new RedisConnectionInstrumentation(), new RedisCommandDataInstrumentation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson;
package io.opentelemetry.javaagent.instrumentation.redisson.v3_0;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.instrumentation.redisson.EndOperationListener;
import io.opentelemetry.javaagent.instrumentation.redisson.PromiseWrapper;
import org.redisson.misc.RPromise;
import org.redisson.misc.RedissonPromise;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson.v3_0;

import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.javaagent.instrumentation.redisson.RedissonInstrumenterFactory;
import io.opentelemetry.javaagent.instrumentation.redisson.RedissonRequest;

public final class RedissonSingletons {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.redisson-3.0";

private static final Instrumenter<RedissonRequest, Void> INSTRUMENTER =
RedissonInstrumenterFactory.createInstrumenter(INSTRUMENTATION_NAME);

public static Instrumenter<RedissonRequest, Void> instrumenter() {
return INSTRUMENTER;
}

private RedissonSingletons() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

class RedissonAsyncClientTest extends AbstractRedissonAsyncClientTest {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

class RedissonClientTest extends AbstractRedissonClientTest {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
plugins {
id("otel.javaagent-instrumentation")
}

muzzle {
pass {
group.set("org.redisson")
module.set("redisson")
versions.set("[3.17.2,)")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add assertInverse and cross-test against the earlier instrumentation?

I'm wondering if the version split will be cleaner at 3.16.8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially used 3.16.8 and decided to go with 3.17.2 because testing for org.redisson.misc.RPromise seemed better than using an arbitrary class. assertInverse is problematic, apparently it is hard to distinguish 3.x from earlier versions. If I didn't mess anything up then 3.0.0 and 2.5.0 contain exactly the same classes, even all the inner class names match.
Our current approach of detecting version based on the presence or absence of some class resources is slightly flawed. It fails when a child class loader has a different version of the library than what the parent class loader has or if multiple versions of the same library are present in the same class loader.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially used 3.16.8 and decided to go with 3.17.2 because testing for org.redisson.misc.RPromise seemed better than using an arbitrary class

I almost find using RPromise more confusing because it was really dropped from our instrumentation in 3.16.8

how about breaking it on 3.17.0 (which would be a bonus to avoid breaking it on patch version)? looks like RFunction might be a good option for that detection: https://github.com/redisson/redisson/releases/tag/redisson-3.17.0

assertInverse is problematic, apparently it is hard to distinguish 3.x from earlier versions

we could add it to the 3.17 instrumentation module though?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, 3.17 is the most reasonable option. Thanks for tracking down RFunction.

}

dependencies {
library("org.redisson:redisson:3.17.2")

implementation(project(":instrumentation:redisson:redisson-common:javaagent"))

compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")

testImplementation(project(":instrumentation:redisson:redisson-common:testing"))
}

tasks.test {
systemProperty("testLatestDeps", findProperty("testLatestDeps") as Boolean)
usesService(gradle.sharedServices.registrations["testcontainersBuildService"].getService())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson.v3_17_2;

import static net.bytebuddy.matcher.ElementMatchers.isConstructor;
import static net.bytebuddy.matcher.ElementMatchers.namedOneOf;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.redisson.CompletableFutureWrapper;
import java.util.concurrent.CompletableFuture;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;

public class RedisCommandDataInstrumentation implements TypeInstrumentation {

@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return namedOneOf(
"org.redisson.client.protocol.CommandData", "org.redisson.client.protocol.CommandsData");
}

@Override
public void transform(TypeTransformer transformer) {
// since 3.16.8
transformer.applyAdviceToMethod(
isConstructor().and(takesArgument(0, CompletableFuture.class)),
this.getClass().getName() + "$WrapCompletableFutureAdvice");
}

@SuppressWarnings("unused")
public static class WrapCompletableFutureAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Argument(value = 0, readOnly = false) CompletableFuture<?> completableFuture) {
completableFuture = CompletableFutureWrapper.wrap(completableFuture);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson.v3_17_2;

import static io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge.currentContext;
import static io.opentelemetry.javaagent.instrumentation.redisson.v3_17_2.RedissonSingletons.instrumenter;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.named;

import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.redisson.EndOperationListener;
import io.opentelemetry.javaagent.instrumentation.redisson.PromiseWrapper;
import io.opentelemetry.javaagent.instrumentation.redisson.RedissonRequest;
import java.net.InetSocketAddress;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
import org.redisson.client.RedisConnection;

public class RedisConnectionInstrumentation implements TypeInstrumentation {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
return named("org.redisson.client.RedisConnection");
}

@Override
public void transform(TypeTransformer transformer) {
transformer.applyAdviceToMethod(
isMethod().and(named("send")), this.getClass().getName() + "$SendAdvice");
}

@SuppressWarnings("unused")
public static class SendAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.This RedisConnection connection,
@Advice.Argument(0) Object arg,
@Advice.Local("otelRequest") RedissonRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

Context parentContext = currentContext();
InetSocketAddress remoteAddress = (InetSocketAddress) connection.getChannel().remoteAddress();
request = RedissonRequest.create(remoteAddress, arg);
PromiseWrapper<?> promise = request.getPromiseWrapper();
if (promise == null) {
return;
}
if (!instrumenter().shouldStart(parentContext, request)) {
return;
}

context = instrumenter().start(parentContext, request);
scope = context.makeCurrent();

promise.setEndOperationListener(new EndOperationListener<>(instrumenter(), context, request));
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Thrown Throwable throwable,
@Advice.Local("otelRequest") RedissonRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

if (scope == null) {
return;
}
scope.close();

if (throwable != null) {
instrumenter().end(context, request, null, throwable);
}
// span ended in EndOperationListener
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.redisson.v3_17_2;

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static java.util.Arrays.asList;
import static net.bytebuddy.matcher.ElementMatchers.not;

import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import java.util.List;
import net.bytebuddy.matcher.ElementMatcher;

@AutoService(InstrumentationModule.class)
public class RedissonInstrumentationModule extends InstrumentationModule {

public RedissonInstrumentationModule() {
super("redisson", "redisson-3.17.2");
}

@Override
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return not(hasClassesNamed("org.redisson.misc.RPromise"));
}

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(new RedisConnectionInstrumentation(), new RedisCommandDataInstrumentation());
}
}
Loading