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

New Spring controller instrumentation modules #1675

Merged
merged 17 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 13 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
98 changes: 98 additions & 0 deletions instrumentation/spring-4.3.0/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
# spring-4.3.0 Instrumentation Module

This module provides instrumentation for Spring Controllers utilizing Spring Web-MVC v4.3.0 up to but not including v6.0.0.
(v6.0.0 instrumentation is provided by another module).

### Traditional Spring Controllers
The module will name transactions based on the controller mapping and HTTP method under the following scenarios:
- Single Spring controller class annotated with/without a class level `@RequestMapping` annotation and methods annotated
with `@RequestMapping`, `@GetMapping`, `@PostMapping`, `@PutMapping`, `@DeleteMapping` or `@PatchMapping`.
```java
@RestController
@RequestMapping("/root")
public class MyController {
@GetMapping("/doGet")
public String handleGet() {
//Do something
}
}
```

- A Spring controller class that implements an interface with/without an interface level `@RequestMapping` annotation and methods annotated
with `@RequestMapping`, `@GetMapping`, `@PostMapping`, `@PutMapping`, `@DeleteMapping` or `@PatchMapping`. In addition, the controller class
can also implement methods not on the interface with the same annotations.
```java
@RequestMapping("/root")
public interface MyControllerInterface {
@GetMapping("/doGet/{id}")
String get(@PathVariable String id);

@PostMapping("/doPost")
String post();
}

@RestController
public class MyController implements MyControllerInterface {
@Override
String get(@PathVariable String id) {
//Do something
}

@Override
String post() {
//Do something
}

//Method not defined in the interface
@DeleteMapping("/doDelete")
public String delete() {
//Do something
}
}
```

- A Spring controller class that extends another controller class with/without a class level `@RequestMapping` annotation and methods annotated
with `@RequestMapping`, `@GetMapping`, `@PostMapping`, `@PutMapping`, `@DeleteMapping` or `@PatchMapping`. In addition, the controller class
can also implement methods not on the parent controller with the same annotations.
```java
@RequestMapping("/root")
public abstract class MyCommonController {
@GetMapping("/doGet")
abstract public String doGet();
}

@RestController
public class MyController extends MyCommonController {
@Override
public String doGet() {
//Do something
}
}
```

- A Spring controller annotated with a custom annotation which itself is annotated with `@Controller` or `@RestController` and methods annotated
with `@RequestMapping`, `@GetMapping`, `@PostMapping`, `@PutMapping`, `@DeleteMapping` or `@PatchMapping`.
```java
@Retention(RetentionPolicy.RUNTIME)
@Target({ ElementType.TYPE})
@RestController
public @interface CustomRestControllerAnnotation {
//....
}

@CustomRestControllerAnnotation
public class TestControllerWithCustomAnnotation {
@GetMapping("/custom")
public String doGet() {
//Do something
}
}

```

The resulting transaction name will be the defined mapping route plus the HTTP method. For example: `root/doGet/{id} (GET)`.

### Other Controllers Invoked via DispatcherServlet

For any other controllers invoked via the `DispatcherServlet` ([Actuator](https://docs.spring.io/spring-boot/docs/current/reference/html/actuator.html#actuator.enabling) endpoints, for example)
will be named based on the controller class name and the executed method. For example: `NonStandardController/myMethod`.
10 changes: 4 additions & 6 deletions instrumentation/spring-4.3.0/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@ plugins {

dependencies {
implementation(project(":agent-bridge"))
implementation("org.springframework:spring-context:4.3.0.RELEASE")
implementation("org.springframework:spring-web:4.3.0.RELEASE")
testImplementation("org.jetbrains.kotlin:kotlin-stdlib:1.8.21")
implementation("org.springframework:spring-webmvc:4.3.0.RELEASE")
implementation('jakarta.servlet:jakarta.servlet-api:4.0.4')
}

jar {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the weave-violation-filter?

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 was going to create another issue to discuss this later. I don't know if it has any applicability outside of the spring controller stuff or not.

Expand All @@ -16,9 +15,8 @@ jar {
}

verifyInstrumentation {
passesOnly 'org.springframework:spring-web:[4.3.0.RELEASE,)'

excludeRegex 'org.springframework:spring-web:.*(RC|SEC|M)[0-9]*$'
passesOnly 'org.springframework:spring-webmvc:[4.3.0.RELEASE,6.0.0)'
excludeRegex 'org.springframework:spring-webmvc:.*(RC|SEC|M)[0-9]*$'
}

site {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
*
* * Copyright 2023 New Relic Corporation. All rights reserved.
* * SPDX-License-Identifier: Apache-2.0
*
*/
package com.nr.agent.instrumentation;

import com.newrelic.agent.bridge.AgentBridge;
import com.newrelic.agent.bridge.Transaction;
import com.newrelic.api.agent.NewRelic;
import com.newrelic.api.agent.Trace;
import com.newrelic.api.agent.weaver.MatchType;
import com.newrelic.api.agent.weaver.Weave;
import com.newrelic.api.agent.weaver.Weaver;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.servlet.ModelAndView;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

@Weave(type = MatchType.BaseClass, originalName = "org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter")
public class AbstractHandlerMethodAdapter_Instrumentation {
@Trace
protected ModelAndView handleInternal(HttpServletRequest request,
HttpServletResponse response, HandlerMethod handlerMethod) throws Exception {
Transaction transaction = AgentBridge.getAgent().getTransaction(false);

if (transaction != null) {
Class<?> controllerClass = handlerMethod.getBeanType();
Method controllerMethod = handlerMethod.getMethod();

//If this setting is false, attempt to name transactions the way the legacy point cut
//named them
boolean isEnhancedNaming = NewRelic.getAgent().getConfig().getValue("class_transformer.enhanced_spring_transaction_naming", false);
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a constant in SpringControllerUtility.

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'll address this in all 3 modules.


String httpMethod = request.getMethod();
if (httpMethod != null) {
httpMethod = httpMethod.toUpperCase();
} else {
httpMethod = "Unknown";
}

//Optimization - If a class doesn't have @Controller/@RestController directly on the controller class
//the transaction is named in point cut style (when enhanced naming set to false)
if (!isEnhancedNaming && !SpringControllerUtility.doesClassContainControllerAnnotations(controllerClass, false)) {
SpringControllerUtility.assignTransactionNameFromControllerAndMethod(transaction, controllerClass, controllerMethod);
} else { //Normal flow to check for annotations based on enhanced naming config flag
String rootPath;
String methodPath;

//From this point, look for annotations on the class/method, respecting the config flag that controls if the
//annotation has to exist directly on the class/method or can be inherited.

//Handle typical controller methods with class and method annotations. Those annotations
//can come from implemented interfaces, extended controller classes or be on the controller class itself.
//Note that only RequestMapping mapping annotations can apply to a class (not Get/Post/etc)
rootPath = SpringControllerUtility.retrieveRootMappingPathFromController(controllerClass, isEnhancedNaming);

//Retrieve the mapping that applies to the target method
methodPath = SpringControllerUtility.retrieveMappingPathFromHandlerMethod(controllerMethod, httpMethod, isEnhancedNaming);

if (rootPath != null || methodPath != null) {
SpringControllerUtility.assignTransactionNameFromControllerAndMethodRoutes(transaction, httpMethod, rootPath, methodPath);
} else {
//Name based on class + method
SpringControllerUtility.assignTransactionNameFromControllerAndMethod(transaction, controllerClass, controllerMethod);
}
}
transaction.getTracedMethod().setMetricName("Spring", "Java",
SpringControllerUtility.getControllerClassAndMethodString(controllerClass, controllerMethod, true));
}

return Weaver.callOriginal();
}
}