-
Notifications
You must be signed in to change notification settings - Fork 147
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
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
12a283b
v6 module
jtduffy 2ed9017
Webflux module; name methdo tracer properly
jtduffy 6892f5e
Optimize annotation search; update tests and READMEs
jtduffy 7a25ab0
v6 module
jtduffy c73368c
Webflux module; name methdo tracer properly
jtduffy 5a2b76a
Optimize annotation search; update tests and READMEs
jtduffy 04dbcc5
Merge branch 'spring-controller-instr' of github.com:newrelic/newreli…
jtduffy 6dad195
Add config flag and fallback to point cut behavior
jtduffy a248a12
webflux controllers and tests
jtduffy 2339da0
Refactor webflux instrumentation so an additional module is not needed
jtduffy 73f8757
Normalize metric names for old and new transaction names
jtduffy c11cc51
Merge branch 'main' into spring-controller-instr-with-config
jtduffy 5d754c0
Remove webclient5 from settings.gradle
jtduffy e97c839
Addressed PR comments
jtduffy 91dc351
Refactor config option and tests
jtduffy d7bd6af
Disable functional test
jtduffy 3d39ef5
Revert config setting change
jtduffy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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`. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
81 changes: 81 additions & 0 deletions
81
.../main/java/com/nr/agent/instrumentation/AbstractHandlerMethodAdapter_Instrumentation.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
/* | ||
* | ||
* * 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); | ||
|
||
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(); | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.