opentelemetry-java-instrumentation icon indicating copy to clipboard operation
opentelemetry-java-instrumentation copied to clipboard

Trace methods of all classes under a package by `otel.instrumentation.methods.include=my.package.**`

Open Johnny850807 opened this issue 4 years ago • 11 comments

Is your feature request related to a problem? Please describe. I'd like to trace all the classes/methods under a package recursively using the otel.instrumentation.methods.include system property, without naming every class.

I think this is a really powerful feature if we can achieve such transparency for in-process tracing like this, as in many situations, we hope that our core domain's module is agnostic of how they are traced.

So we can no longer depend on the open-telemetry instrumentation API's @WithSpan.

Describe the solution you'd like

Given I have a Spring-Boot project as below

src
│   └── main
│       ├── java
│       │   └── com
│       │       └── example
│       │           └── demo
│       │               ├── DemoApplication.java
│       │               ├── SignIn.java
│       │               ├── SignInRequest.java
│       │               ├── SignUp.java
│       │               ├── User.java
│       │               ├── UserController.java
│       │               ├── UserPresenter.java
│       │               └── UserRepository.java

I want to set -Dotel.instrumentation.methods.include=com.example.demo or -Dotel.instrumentation.methods.include=com.example.demo.** to include all my application classes for tracing.

Could we achieve this?


This feature may produce many spans and is considered dangerous. However, I think we can emphasize its consequences to the users, but be able to support the full transparency if the user asks to.

If it's properly used then we can see a trace like: (In a Clean Architecture style)

[POST] /api/chessGames/13/chess
  └──PlayChessController.putChess
      ├── PutChessUseCase.putChess (Domain)
      | └── SpringBootChessGameRepository 
      |      ├── CrudRepository.findById 
      |      └── DataMapper.toEntity
      ├──  ChessGame.putChess (Domain)
      |      └── Board.setChess
      ├── SpringBootChessGameRepository 
      |      ├── DataMapper.toData
      |.     └── CrudRepository.save  
      │          └── INSERT INTO ...
      └── ChessGamePresenter.present

Alternatives

Consider that there may be so many spans produced, the alternative is that Include only the Spring bean's methods under a package*

Thanks for your time on my post.

Johnny850807 avatar Mar 15 '21 08:03 Johnny850807

It certainly can be added, but we have to think about corner cases here. Do you want ALL methods or only public ones? What about methods inherited from superclasses? What if one instrumented method directly calls another one, do you want 2 spans? This is quite dangerous feature that may result in millions of spans...

iNikem avatar Mar 15 '21 08:03 iNikem

It certainly can be added, but we have to think about corner cases here. Do you want ALL methods or only public ones? What about methods inherited from superclasses? What if one instrumented method directly calls another one, do you want 2 spans? This is quite dangerous feature that may result in millions of spans...

Do you want ALL methods or only public ones

Only the public methods are included.

What about methods inherited from superclasses?

Only the lowest method's execution is instrumented.

What if one instrumented method directly calls another one, do you want 2 spans?

Yes, in this case, I want 2 spans.

This may produce many spans. However, I think we can emphasize its consequences to the users, but be able to support the full transparency if the user asks to.

If it's properly used then we can see a trace like: (In a Clean Architecture style)

[POST] /api/chessGames/13/chess
  └──PlayChessController.putChess
      ├── PutChessUseCase.putChess (Domain)
      | └── SpringBootChessGameRepository 
      |      ├── CrudRepository.findById 
      |      └── DataMapper.toEntity
      ├──  ChessGame.putChess (Domain)
      |      └── Board.setChess
      ├── SpringBootChessGameRepository 
      |      ├── DataMapper.toData
      |.     └── CrudRepository.save  
      │          └── INSERT INTO ...
      └── ChessGamePresenter.present

This is helpful in case that we don't want our core domain module to depend on Open-telemetry's APIs.

Johnny850807 avatar Mar 15 '21 09:03 Johnny850807

If you don't want to depend on otel APIs you can always add your own @interface Traced annotation and configure the agent to add spans whenever it encounters it: -Dotel.instrumentation.external-annotations.include=my.app.Traced. The downside of this approach is that you have to annotate every single method, as we don't detect class usage.

mateuszrzeszutek avatar Mar 16 '21 08:03 mateuszrzeszutek

If you don't want to depend on otel APIs you can always add your own @interface Traced annotation and configure the agent to add spans whenever it encounters it: -Dotel.instrumentation.external-annotations.include=my.app.Traced. The downside of this approach is that you have to annotate every single method, as we don't detect class usage.

Good suggestion! Thanks for your info. As you said, I still need to annotate every single method, which does not suffice those who are really care about the domain's purity 😂.

Johnny850807 avatar Mar 16 '21 12:03 Johnny850807

Class level annotations aren't supported? -Dotel.instrumentation.external-annotations.include=org.springframework.stereotype.Service doesn't seem to do anything for example.

stnor avatar May 03 '21 17:05 stnor

I'd like to write my own aspect to handle my application's tracing to COMPLEMENT the otel javaagent, but it's unclear to me how to proceed after reading https://opentelemetry.io/docs/java/manual_instrumentation/

stnor avatar May 03 '21 18:05 stnor

I got some help from @anuraaga and solved it here: https://github.com/aws/aws-xray-sdk-java/issues/281#issuecomment-831698289

stnor avatar May 04 '21 08:05 stnor

Would be handy

delanym avatar Apr 26 '22 09:04 delanym

This would be very helpful. Temporarily turning on packages with regexes would be very useful to debug whenever a performance issue is not located in the currently included methods.

APM services such as AppDynamics only export these in-depth spans whenever the trace is problematic (slow or exception). I think this is a great strategy because it saves money (only a small percentage should be slow or error) and gives in-depth information for issues without having to manually instrument or manually configure specific methods/classes. I think what is considered "slow" and "error" can be configured. I think they also "crop" the spans to show only the relevant pieces involed in the slowness or the errors, that way 1000 spans won't be sent for each problematic trace. Further sampling could be done via the collector. This is much easier said than done, but it is an elegant solution that covers what 99% of users need.

Configuring specific methods or classes can be brittle, specially when the code that is instrumented is located in third party libraries that can change their class structure any time.

AndresPineros avatar Dec 26 '22 23:12 AndresPineros

profiling is generally my go-to when the performance issue is not located in the currently included methods.

the OpenTelemetry community kicked off Profiling work in 2022, with one of the goals being to correlate Profiling with Tracing, which would allow you to dig into a specific slow request and look at an execution profile for it

trask avatar Jan 03 '23 20:01 trask

Based on the information here (thanks) I created this repository specific to spring boot, with a working aspect-based instrumentation of all public methods inside @Component,@Service and @Repository: https://github.com/DGuhr/spring-otel-extended

~~currently, I am trying to build an agent extension to do this instead of the aspects, but for some reason it does not get picked up. If anyone could help me out, that'd be quite awesome. I thought I'd post it here so others have a working aspect-based solution (at least for spring).~~

The extension works now, so feel free to grab it and use it as a starting point for your use case.

DGuhr avatar Mar 03 '24 19:03 DGuhr