Added additional details to Java integration #169
Conversation
To assist when utilizing frameworks such as Spring
|
|
docs/integration-java.mdx
Outdated
| @@ -43,18 +44,40 @@ Add pyroscope dependency: | |||
| ```kotlin | |||
| implementation("io.pyroscope:agent:${pyroscope_version}") | |||
| ``` | |||
| As .xml: | |||
There was a problem hiding this comment.
I do not think this is very useful, there are other build systems like sbt,ivy, etc, and we are not going to provide a snippet for each of them. The gradle snippet provides enough bits to be able to add it to other build systems. Also I beilieve intellij idea is able to convert it back and forth automatically when you paste a dependency to a build script.
I think there should be just one example and between gradle and maven I prefer gradle since its more concise.
There was a problem hiding this comment.
@korniltsev what do you think if we do a tabgroup and over time people can update docs accordingly:
There was a problem hiding this comment.
Looks good. This should do the job
nit: in your example tab with title "maven" contains gradle example
btw here is what maven central does (which we have a link to, just above the section)
There was a problem hiding this comment.
And what about other build systems? Should we add them as well?
There was a problem hiding this comment.
I think eventually we will have other helpful community members like @6fears7 to help us add those if they feel they are necessary :)
docs/integration-java.mdx
Outdated
| Then add the following code to your application: | ||
| ```java | ||
|
|
||
| import io.pyroscope.javaagent.PyroscopeAgent; |
There was a problem hiding this comment.
I do not think imports are that important. They make an example more verbose while not providing much value. Imports are also automatically added with modern smart IDE.
Although maybe we should still add imports for consistency(golang, python etc have import)
There was a problem hiding this comment.
yeah I agree it makes it a little verbose, but let's leave this in for consistency in this case.
docs/integration-java.mdx
Outdated
|
|
||
|
|
||
|
|
||
| @PostConstruct |
There was a problem hiding this comment.
Why spring? What about Vaadin, Spark, Grails, Play
Framework example should not be the default one. The default one should be just plain java.
Maybe we could add a spring notice somewhere down the page, like "Hey if you use spring, you can put this initialization in PostConstruct". At the same time I am not sure it should go to PostConstruct, I am not that familiar with spring but it looks like it will be invoked somewhere in the middle of bean graph initialization, and I believe the best moment to start profiling is before the whole initialization(somewhere in main or clinit)
There was a problem hiding this comment.
I'll default to your judgment. I do not primarily develop in Java and was going through setting up an example Spring Boot application for our Dev teams using Maven and had to spend some time working through what the docs implied vs declared and found these to be useful. I prefer exhaustive documentation that covers multiple popular frameworks, but that is a preference of mine obviously and not a standard
There was a problem hiding this comment.
I think this can be resolved with tabgroups see example here
I agree the default should be plain java, but I think with a tabgroup we can show the version that works with spring
There was a problem hiding this comment.
And what about other frameworks? Should we add them as well?
There was a problem hiding this comment.
I'll create an issue for it, but definitely not as part of this pr. I think if others want to add them we will appreciate their help similar to @6fears7
| @@ -0,0 +1,40 @@ | |||
| export const exampleGradle = `implementation("io.pyroscope:agent:\${pyroscope_version}")` | |||
There was a problem hiding this comment.
@petethepig how do we make this variable actually substitute in here?
There was a problem hiding this comment.
maybe out of scope for this pr though, happy to create an issue and address later if you think thats better
There was a problem hiding this comment.
LGTM -- your input on the docs is much appreciated @6fears7. We can now use this foundation to support other build systems and frameworks
Let us know if you have any other feedabck about either docs or the integration itself as you use it more!
To assist when utilizing frameworks such as Spring