grafana / pyroscope-java

pyroscope java integration
Apache License 2.0
83 stars 32 forks source link

feat: add stackDepthMax builtin opt #98

Closed tomershafir closed 1 year ago

tomershafir commented 1 year ago

@korniltsev I think you need to make a decision:

  1. Make pyroscope-java tightly coupled to ap, which makes sense because it's a great oss jvm profiler, and an alternative would be hard to find. Then, a single text param will make sense and be flexible.
  2. Make pyroscope-java a simple, loosely coupled abstraction, with profiler agnostic interface. In this case I would even suggest to remove APExtraArgs from the generic interface, and extend it when needed.

I would go with 2.

korniltsev commented 1 year ago

Oh i forgot we already have APExtraArgs.

I am leaning towards 1 as you can see.