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

[issue-69] Redis instrumentation #176

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Expand Up @@ -33,3 +33,6 @@ hs_err_pid*
# Intellij Idea
*.iml
.idea/

# VS Code
.vscode/
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -15,6 +15,7 @@ It contains auto-configurations which instruments and trace following Spring Boo
* Mongo
* Zuul
* RxJava
* Redis
* Standard logging - logs are added to active span
* Spring Messaging - trace messages being sent through [Messaging Channels](https://docs.spring.io/spring-integration/reference/html/messaging-channels-section.html)
* RabbitMQ
Expand Down
77 changes: 77 additions & 0 deletions instrument-starters/opentracing-spring-cloud-redis-starter/pom.xml
@@ -0,0 +1,77 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--

Copyright 2017-2018 The OpenTracing Authors

Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
in compliance with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software distributed under the License
is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
or implied. See the License for the specific language governing permissions and limitations under
the License.

-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<parent>
<artifactId>opentracing-spring-cloud-parent</artifactId>
<groupId>io.opentracing.contrib</groupId>
<version>0.2.1-SNAPSHOT</version>
<relativePath>../../</relativePath>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>opentracing-spring-cloud-redis-starter</artifactId>

<properties>
<main.basedir>${project.basedir}/../..</main.basedir>
</properties>

<dependencies>
<dependency>
<groupId>io.opentracing.contrib</groupId>
<artifactId>opentracing-spring-tracer-configuration-starter</artifactId>
</dependency>
<dependency>
<groupId>io.opentracing.contrib</groupId>
<artifactId>opentracing-redis-spring</artifactId>
<version>${version.io.opentracing.contrib-opentracing-spring-redis}</version>
<exclusions>
<exclusion>
<artifactId>*</artifactId>
<groupId>*</groupId>
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-aop</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-data-redis</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

it has to be optional

Copy link
Author

Choose a reason for hiding this comment

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

I've made it optional, I'll push the change in a minute

Copy link
Contributor

Choose a reason for hiding this comment

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

RedisAutoConfiguration has to be changed to kick in only when redis classes are on classpath. Please also add test to NoDepsTest.java

Copy link
Author

@ddcprg ddcprg Sep 20, 2018

Choose a reason for hiding this comment

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

thanks, I'll have look

<optional>true</optional>
</dependency>

<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-test</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>ai.grakn</groupId>
<artifactId>redis-mock</artifactId>
<version>0.1.6</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>redis.clients</groupId>
<artifactId>jedis</artifactId>
<version>2.9.0</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
@@ -0,0 +1,71 @@
/**
* Copyright 2017-2018 The OpenTracing Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package io.opentracing.contrib.spring.cloud.redis;

import io.opentracing.contrib.redis.spring.connection.TracingRedisClusterConnection;
import io.opentracing.contrib.redis.spring.connection.TracingRedisConnection;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Pointcut;
import org.springframework.data.redis.connection.RedisClusterConnection;
import org.springframework.data.redis.connection.RedisConnection;
import org.springframework.data.redis.connection.RedisConnectionFactory;

/**
* Spring AOP Aspect wrapping Redis-related calls, delegating as much as possible to the official
* OpenTracing Java Redis framework integration.
*
* @author Daniel del Castillo
*/
@Aspect
public class RedisAspect {

@Pointcut("target(org.springframework.data.redis.connection.RedisConnectionFactory)")
public void connectionFactory() {}

@Pointcut("execution(org.springframework.data.redis.connection.RedisConnection *.getConnection(..))")
public void getConnection() {}

@Pointcut("execution(org.springframework.data.redis.connection.RedisClusterConnection *.getClusterConnection(..))")
public void getClusterConnection() {}


/**
* Intercepts calls to {@link RedisConnectionFactory#getConnection()} (and related), wrapping the
* outcome in a {@link TracingRedisConnection}
*
* @param pjp the intercepted join point
* @return a new {@link TracingRedisConnection} wrapping the result of the joint point
*/
@Around("getConnection() && connectionFactory()")
public Object aroundGetConnection(final ProceedingJoinPoint pjp) throws Throwable {
RedisConnection connection = (RedisConnection) pjp.proceed();
return new TracingRedisConnection(connection, false, null);
}

/**
* Intercepts calls to {@link RedisConnectionFactory#getClusterConnection()} (and related),
* wrapping the outcome in a {@link TracingRedisClusterConnection}
*
* @param pjp the intercepted join point
* @return a new {@link TracingRedisClusterConnection} wrapping the result of the joint point
*/
@Around("getClusterConnection() && connectionFactory()")
public Object aroundGetClusterConnection(final ProceedingJoinPoint pjp) throws Throwable {
RedisClusterConnection clusterConnection = (RedisClusterConnection) pjp.proceed();
return new TracingRedisClusterConnection(clusterConnection, false, null);
}

}
@@ -0,0 +1,40 @@
/**
* Copyright 2017-2018 The OpenTracing Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package io.opentracing.contrib.spring.cloud.redis;

import io.opentracing.contrib.spring.tracer.configuration.TracerRegisterAutoConfiguration;
import org.springframework.boot.autoconfigure.AutoConfigureAfter;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.redis.connection.RedisConnectionFactory;

/**
* Loads the integration with OpenTracing Redis if it's included in the classpath.
*
* @author Daniel del Castillo
*/
@Configuration
@AutoConfigureAfter(TracerRegisterAutoConfiguration.class)
@ConditionalOnBean(RedisConnectionFactory.class)
@ConditionalOnProperty(name = "opentracing.spring.cloud.redis.enabled", havingValue = "true", matchIfMissing = true)
public class RedisAutoConfiguration {

@Bean
public RedisAspect openTracingRedisAspect() {
return new RedisAspect();
}

}
@@ -0,0 +1,10 @@
{
"properties": [
{
"name": "opentracing.spring.cloud.redis.enabled",
"type": "java.lang.Boolean",
"description": "Enable Redis tracing.",
"defaultValue": true
}
]
}
@@ -0,0 +1,2 @@
org.springframework.boot.autoconfigure.EnableAutoConfiguration=\
io.opentracing.contrib.spring.cloud.redis.RedisAutoConfiguration
@@ -0,0 +1,122 @@
/**
* Copyright 2017-2018 The OpenTracing Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package io.opentracing.contrib.spring.cloud.redis;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

import ai.grakn.redismock.RedisServer;
import io.opentracing.Scope;
import io.opentracing.mock.MockSpan;
import io.opentracing.mock.MockTracer;
import io.opentracing.tag.Tags;
import io.opentracing.util.GlobalTracerTestUtil;
import java.util.Optional;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.redis.connection.RedisConnectionFactory;
import org.springframework.data.redis.connection.RedisStandaloneConfiguration;
import org.springframework.data.redis.connection.jedis.JedisConnectionFactory;
import org.springframework.data.redis.core.RedisTemplate;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;

/**
* @author Daniel del Castillo
*/
@SpringBootTest(classes = {IntegrationTest.IntegrationTestConfiguration.class})
@RunWith(SpringJUnit4ClassRunner.class)
public class IntegrationTest {

@Configuration
@EnableAutoConfiguration
public static class IntegrationTestConfiguration {

public @Bean MockTracer mockTracer() {
GlobalTracerTestUtil.resetGlobalTracer();
return new MockTracer();
}

public @Bean RedisConnectionFactory redisConnectionFactory() {
RedisStandaloneConfiguration config = new RedisStandaloneConfiguration();
config.setHostName(redis.getHost());
config.setPort(redis.getBindPort());
return new JedisConnectionFactory(config);
}

}

private static RedisServer redis;

@Autowired
MockTracer tracer;

@Autowired
RedisTemplate redisTemplate;

@BeforeClass
public static void setup() throws Exception {
redis = RedisServer.newRedisServer();
redis.start();
}

@AfterClass
public static void teardown() {
redis.stop();
}

@Before
public void init() {
tracer.reset();
}

@Test
public void commandCreatesNewSpan() {
redisTemplate.opsForValue().set(100L, "Some value here");
assertEquals(1, tracer.finishedSpans().size());
assertEquals("SET", tracer.finishedSpans().get(0).tags().get("command"));
}

@Test
public void spanJoinsActiveSpan() {
try (Scope ignored = tracer.buildSpan("parent").startActive(true)) {
redisTemplate.opsForList().leftPushAll("test-list", 1, 2, 3);
assertEquals(1, tracer.finishedSpans().size());
assertEquals("LPUSH", tracer.finishedSpans().get(0).tags().get("command"));
}

assertEquals(2, tracer.finishedSpans().size());
Optional<MockSpan> redisSpan = tracer.finishedSpans().stream()
.filter((s) -> "java-redis".equals(s.tags().get(Tags.COMPONENT.getKey()))).findFirst();

Optional<MockSpan> parentSpan =
tracer.finishedSpans().stream().filter((s) -> "parent".equals(s.operationName())).findFirst();

assertTrue(redisSpan.isPresent());
assertTrue(parentSpan.isPresent());

assertEquals(redisSpan.get().context().traceId(), parentSpan.get().context().traceId());
assertEquals(redisSpan.get().parentId(), parentSpan.get().context().spanId());
}

// Cluster operations can be tested once https://github.com/kstyrc/embedded-redis/issues/79 is fixed

}
4 changes: 4 additions & 0 deletions opentracing-spring-cloud-starter/pom.xml
Expand Up @@ -66,6 +66,10 @@
<groupId>${project.groupId}</groupId>
<artifactId>opentracing-spring-cloud-rxjava-starter</artifactId>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>opentracing-spring-cloud-redis-starter</artifactId>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>opentracing-spring-cloud-mongo-starter</artifactId>
Expand Down
Expand Up @@ -50,6 +50,21 @@ public void testNoRxJavaHooks() throws ClassNotFoundException {
this.getClass().getClassLoader().loadClass("rx.plugins.RxJavaHooks");
}

@Test(expected = ClassNotFoundException.class)
public void testNoRedisJavaHooks() throws ClassNotFoundException {
this.getClass().getClassLoader().loadClass("org.springframework.data.redis.core.RedisTemplate");
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please also add a check on some core redis class?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure how you mean. I could only depend on some java client class but that would restrict the integrations allowed with spring-data. IMO if someone depends on spring-data-redis is because they use Redis, that together with the autoconfig options should be sufficient to activate the aspect(?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Test if redis classes are not on classpath like: this.getClass().getClassLoader().loadClass("redis.clients.jedis.Client");

Copy link
Contributor

Choose a reason for hiding this comment

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

}

@Test(expected = ClassNotFoundException.class)
public void testNoJedisClient() throws ClassNotFoundException {
this.getClass().getClassLoader().loadClass("redis.clients.jedis.Client");
}

@Test(expected = ClassNotFoundException.class)
public void testNoLettuceClient() throws ClassNotFoundException {
this.getClass().getClassLoader().loadClass("io.lettuce.core.RedisClient");
}

@Test(expected = ClassNotFoundException.class)
public void testNoWebsocketMessageBroker() throws ClassNotFoundException {
this.getClass().getClassLoader().loadClass(
Expand Down