spring mvc3 - proper span name in case of sec fail
MODIFY
docs/contributing/running-tests.md
docs/contributing/running-tests.md
@@ -41,3 +41,7 @@ rule will kick in and do the following:
This works both for tasks named `test` and `latestDepTest`. But currently
does not work for other custom test tasks, such as those created by test sets
plugin.
+
+#### Executing single test
+
+Executing `./gradlew :instrumentation:<INSTRUMENTATION_NAME>:test --tests <GROOVY TEST FILE NAME>` will run only the selected test.

MODIFY
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/spring-webmvc-3.1-auto.gradle
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/spring-webmvc-3.1-auto.gradle
@@ -39,9 +39,10 @@ dependencies {
testImplementation group: 'org.spockframework', name: 'spock-spring', version: "$versions.spock"
- testImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '1.5.17.RELEASE'
- testImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.17.RELEASE'
- testImplementation group: 'org.springframework.boot', name: 'spring-boot-starter-security', version: '1.5.17.RELEASE'
+ testLibrary group: 'org.springframework.boot', name: 'spring-boot-starter-test', version: '1.5.17.RELEASE'
+ testLibrary group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.17.RELEASE'
+ testLibrary group: 'org.springframework.boot', name: 'spring-boot-starter-security', version: '1.5.17.RELEASE'
+
testImplementation group: 'org.springframework.security.oauth', name: 'spring-security-oauth2', version: '2.0.16.RELEASE'
// For spring security

MODIFY
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/boot/AppConfig.groovy
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/boot/AppConfig.groovy
package test.boot
-import org.apache.catalina.connector.Connector
import org.springframework.boot.autoconfigure.SpringBootApplication
-import org.springframework.boot.context.embedded.EmbeddedServletContainerFactory
-import org.springframework.boot.context.embedded.tomcat.TomcatConnectorCustomizer
-import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory
-import org.springframework.context.annotation.Bean
import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter
@SpringBootApplication
class AppConfig extends WebMvcConfigurerAdapter {
- @Bean
- EmbeddedServletContainerFactory servletContainerFactory() {
- def factory = new TomcatEmbeddedServletContainerFactory()
-
- factory.addConnectorCustomizers(
- new TomcatConnectorCustomizer() {
- @Override
- void customize(final Connector connector) {
- connector.setEnableLookups(true)
- }
- })
-
- return factory
- }
}

MODIFY
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/boot/SecurityConfig.groovy
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/boot/SecurityConfig.groovy
@@ -18,27 +18,55 @@ package test.boot
import org.springframework.context.annotation.Bean
import org.springframework.context.annotation.Configuration
+import org.springframework.core.annotation.Order
import org.springframework.security.config.annotation.web.builders.HttpSecurity
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter
@Configuration
@EnableWebSecurity
-class SecurityConfig extends WebSecurityConfigurerAdapter {
+class SecurityConfig {
+
+ @Bean
+ SavingAuthenticationProvider savingAuthenticationProvider() {
+ return new SavingAuthenticationProvider()
+ }
+
+ /**
+ * Following configuration is required for unauthorised call tests (form would redirect, we need 401)
+ */
+ @Configuration
+ @Order(1)
+ static class ApiWebSecurityConfigurationAdapter extends WebSecurityConfigurerAdapter {
- @Override
protected void configure(HttpSecurity http) throws Exception {
+ http
+ .csrf().disable()
+ .antMatcher("/basicsecured/**")
+ .authorizeRequests()
+ .antMatchers("/basicsecured/**").authenticated()
+ .and()
+ .httpBasic()
+ .and().authenticationProvider(applicationContext.getBean(SavingAuthenticationProvider));
+ }
+ }
+
+ /**
+ * Following configuration is required in order to get form login, needed by password tests
+ */
+ @Configuration
+ static class FormLoginWebSecurityConfigurerAdapter extends WebSecurityConfigurerAdapter {
+ @Override
+ protected void configure(HttpSecurity http) throws Exception {
http
+ .csrf().disable()
.authorizeRequests()
- .antMatchers("/secure/**").authenticated()
- .and().formLogin()
+ .antMatchers("/formsecured/**").authenticated()
+ .and()
+ .formLogin()
.and().authenticationProvider(applicationContext.getBean(SavingAuthenticationProvider))
- .csrf().disable()
}
-
- @Bean
- SavingAuthenticationProvider savingAuthenticationProvider() {
- return new SavingAuthenticationProvider()
}
}
+

MODIFY
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/boot/TestController.groovy
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/boot/TestController.groovy
@@ -37,6 +37,14 @@ import org.springframework.web.servlet.view.RedirectView
@Controller
class TestController {
+ @RequestMapping("/basicsecured/endpoint")
+ @ResponseBody
+ String secureEndpoint() {
+ HttpServerTest.controller(SUCCESS) {
+ SUCCESS.body
+ }
+ }
+
@RequestMapping("/success")
@ResponseBody
String success() {

MODIFY
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/FilteredAppConfig.groovy
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/FilteredAppConfig.groovy
@@ -33,11 +33,7 @@ import javax.servlet.ServletRequest
import javax.servlet.ServletResponse
import javax.servlet.http.HttpServletRequest
import javax.servlet.http.HttpServletResponse
-import org.apache.catalina.connector.Connector
import org.springframework.boot.autoconfigure.SpringBootApplication
-import org.springframework.boot.context.embedded.EmbeddedServletContainerFactory
-import org.springframework.boot.context.embedded.tomcat.TomcatConnectorCustomizer
-import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory
import org.springframework.context.annotation.Bean
import org.springframework.http.HttpInputMessage
import org.springframework.http.HttpOutputMessage
@@ -70,21 +66,6 @@ class FilteredAppConfig extends WebMvcConfigurerAdapter {
})
}
- @Bean
- EmbeddedServletContainerFactory servletContainerFactory() {
- def factory = new TomcatEmbeddedServletContainerFactory()
-
- factory.addConnectorCustomizers(
- new TomcatConnectorCustomizer() {
- @Override
- void customize(final Connector connector) {
- connector.setEnableLookups(true)
- }
- })
-
- return factory
- }
-
@Bean
HttpMessageConverter<Map<String, Object>> createPlainMapMessageConverter() {
return new AbstractHttpMessageConverter<Map<String, Object>>(MediaType.TEXT_PLAIN) {

MODIFY
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/ServletFilterTest.groovy
instrumentation/spring/spring-webmvc-3.1/spring-webmvc-3.1-auto/src/test/groovy/test/filter/ServletFilterTest.groovy
@@ -22,8 +22,8 @@ import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.PATH
import static io.opentelemetry.auto.test.base.HttpServerTest.ServerEndpoint.SUCCESS
import static io.opentelemetry.trace.Span.Kind.INTERNAL
import static io.opentelemetry.trace.Span.Kind.SERVER
-import static java.util.Collections.singletonMap
+import com.google.common.collect.ImmutableMap
import io.opentelemetry.auto.test.asserts.TraceAssert
import io.opentelemetry.auto.test.base.HttpServerTest
import io.opentelemetry.instrumentation.api.MoreAttributes
@@ -38,7 +38,7 @@ class ServletFilterTest extends HttpServerTest<ConfigurableApplicationContext> {
@Override
ConfigurableApplicationContext startServer(int port) {
def app = new SpringApplication(FilteredAppConfig, SecurityConfig)
- app.setDefaultProperties(singletonMap("server.port", port))
+ app.setDefaultProperties(ImmutableMap.of("server.port", port, "server.error.include-message", "always"))
def context = app.run()
return context
}

MODIFY
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java
javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcher.java
package io.opentelemetry.javaagent.tooling.matcher;
+import com.google.common.collect.Sets;
+import java.util.Set;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@@ -26,6 +28,8 @@ import net.bytebuddy.matcher.ElementMatcher;
* <p>This is separated from {@link GlobalIgnoresMatcher} to allow for better testing. The idea is
* that we should be able to remove this matcher from the agent and all tests should still pass.
* Moreover, no classes matched by this matcher should be modified during test run.
+ *
+ * <p>TODO: refactor/optimize this class (spring boot approach as a possible solution)
*/
public class AdditionalLibraryIgnoresMatcher<T extends TypeDescription>
extends ElementMatcher.Junction.AbstractBase<T> {
@@ -109,21 +113,7 @@ public class AdditionalLibraryIgnoresMatcher<T extends TypeDescription>
}
if (name.startsWith("org.springframework.boot.")) {
- // More runnables to deal with
- if (name.startsWith("org.springframework.boot.autoconfigure.BackgroundPreinitializer$")
- || name.startsWith("org.springframework.boot.autoconfigure.condition.OnClassCondition$")
- || name.startsWith("org.springframework.boot.web.embedded.netty.NettyWebServer$")
- || name.startsWith(
- "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer$")
- || name.equals(
- "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedWebappClassLoader")
- || name.equals(
- "org.springframework.boot.context.embedded.EmbeddedWebApplicationContext")
- || name.equals(
- "org.springframework.boot.context.embedded.AnnotationConfigEmbeddedWebApplicationContext")) {
- return false;
- }
- return true;
+ return !instrumentedSpringBootClasses(name);
}
if (name.startsWith("org.springframework.cglib.")) {
@@ -322,6 +312,31 @@ public class AdditionalLibraryIgnoresMatcher<T extends TypeDescription>
return false;
}
+ private static Set<String> INSTRUMENTED_SPRING_BOOT_CLASSES =
+ Sets.newHashSet(
+ "org.springframework.boot.autoconfigure.BackgroundPreinitializer$",
+ "org.springframework.boot.autoconfigure.condition.OnClassCondition$",
+ "org.springframework.boot.web.embedded.netty.NettyWebServer$",
+ "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainer$",
+ "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedWebappClassLoader",
+ "org.springframework.boot.context.embedded.EmbeddedWebApplicationContext",
+ "org.springframework.boot.context.embedded.AnnotationConfigEmbeddedWebApplicationContext",
+ // spring boot 2 classes
+ "org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext",
+ "org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext",
+ "org.springframework.boot.web.embedded.tomcat.TomcatWebServer$",
+ "org.springframework.boot.web.embedded.tomcat.TomcatEmbeddedWebappClassLoader");
+
+ private static String outerClassName(final String name) {
+ int separator = name.indexOf('$');
+ return (separator == -1 ? name : name.substring(0, separator + 1));
+ }
+
+ private static boolean instrumentedSpringBootClasses(final String name) {
+ String outerName = outerClassName(name);
+ return INSTRUMENTED_SPRING_BOOT_CLASSES.contains(outerName);
+ }
+
@Override
public String toString() {
return "additionalLibraryIgnoresMatcher()";

ADD
javaagent-tooling/src/test/groovy/io/opentelemetry/javaagent/tooling/matcher/AdditionalLibraryIgnoresMatcherTest.groovy
+/*
+ * Copyright The OpenTelemetry 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.opentelemetry.javaagent.tooling.matcher
+
+import net.bytebuddy.description.type.TypeDescription
+import spock.lang.Specification
+
+class AdditionalLibraryIgnoresMatcherTest extends Specification {
+
+ def underTest = new AdditionalLibraryIgnoresMatcher()
+
+ def "spring boot - match not instrumented class"() {
+
+ setup:
+ def type = Mock(TypeDescription)
+ type.getActualName() >> typeName
+
+ when:
+ def matches = underTest.matches(type)
+
+ then:
+ matches == true
+
+ where:
+ typeName << ["org.springframework.boot.NotInstrumentedClass",
+ "org.springframework.boot.embedded.tomcat.NotInstrumentedClass"]
+ }
+
+ def "spring boot - don't match instrumented class"() {
+
+ setup:
+ def type = Mock(TypeDescription)
+ type.getActualName() >> typeName
+
+ when:
+ def matches = underTest.matches(type)
+
+ then:
+ matches == false
+
+ where:
+ typeName << ["org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext",
+ "org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedWebappClassLoader"]
+ }
+
+ def "spring boot - don't match instrumented inner class"() {
+
+ setup:
+ def type = Mock(TypeDescription)
+ type.getActualName() >> typeName
+
+ when:
+ def matches = underTest.matches(type)
+
+ then:
+ matches == false
+
+ where:
+ typeName << ["org.springframework.boot.autoconfigure.BackgroundPreinitializer\$InnerClass1",
+ "org.springframework.boot.autoconfigure.condition.OnClassCondition\$ConditionMatch"]
+ }
+}

MODIFY
testing-common/src/main/groovy/io/opentelemetry/auto/test/InMemoryExporter.java
testing-common/src/main/groovy/io/opentelemetry/auto/test/InMemoryExporter.java
@@ -87,11 +87,12 @@ public class InMemoryExporter implements SpanProcessor {
public void onEnd(ReadableSpan readableSpan) {
SpanData sd = readableSpan.toSpanData();
log.debug(
- "<<< SPAN END: {} id={} traceid={} parent={}, attributes={}",
+ "<<< SPAN END: {} id={} traceid={} parent={}, library={}, attributes={}",
sd.getName(),
sd.getSpanId().toLowerBase16(),
sd.getTraceId().toLowerBase16(),
sd.getParentSpanId().toLowerBase16(),
+ sd.getInstrumentationLibraryInfo(),
printSpanAttributes(sd));
SpanData span = readableSpan.toSpanData();
synchronized (tracesLock) {

MODIFY
testing-common/src/main/groovy/io/opentelemetry/auto/test/base/HttpServerTest.groovy
testing-common/src/main/groovy/io/opentelemetry/auto/test/base/HttpServerTest.groovy
@@ -152,6 +152,7 @@ abstract class HttpServerTest<SERVER> extends AgentTestRunner {
PATH_PARAM("path/123/param", 200, "123"),
AUTH_REQUIRED("authRequired", 200, null),
LOGIN("login", 302, null),
+ AUTH_ERROR("basicsecured/endpoint", 401, null)
private final URI uriObj
private final String path

spring mvc3 - proper span name in case of sec fail (#718) (#1122)